Automattic / Automattic/wordpress-atmosphere

Document-only delete guard is bypassed when META_DID is refreshed by a pageview after reconnect

Open
#217 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
PHP
Stars
52
Forks
3
Avg merge
2d 1h
Merged PRs (30d)
22

Description

Surfaced while reviewing [#216](). This is a pre-existing hole in the wrong-repo-delete guard from [#63](), specific to document-only posts, so I filed it separately rather than folding it into [#216]().

## The problem

For a document-only post (Bluesky cross-post disabled) the post has no `Post::META_DID`, so the delete guard rests entirely on `Document::META_DID` (`doc_origin_did`). But that value gets rewritten to the currently-connected account by an ordinary front-end pageview:

`Atmosphere::output_document_link()` runs on `wp_head` for every singular view of a publishable post and calls `( new Document( $post ) )->get_rkey()`, which refreshes `Document::META_DID` to `get_did()` on the first pageload after a DID change.

So:

1. Publish a document-only post under account A (`Document::META_DID = A`, `META_URI = at://A/...`).
2. Disconnect, reconnect to account B.
3. Anyone views the post. `output_document_link` → `Document::get_rkey()` rewrites `META_DID` to B.
4. Permanently delete (or trash) the post. The guard sees `doc_origin_did == current == B`, so `record_is_foreign()` is false and the delete proceeds. `applyWrites#delete` fires against B where the record never existed, no-ops idempotently, local meta clears, and the original document is **orphaned on A with no mismatch logged**.

Unlike the comment cascade (Automattic/wordpress-atmosphere#215), there is no parallel guarded cleanup path for documents, so there is no breadcrumb at all.

## Root cause

`META_DID` (for documents) tracks "the last account we touched this record under," not "the account the live record actually lives on," because `get_rkey()` refreshes it on a read path. After step 3 the row is internally contradictory: `META_URI` still embeds A while `META_DID` says B.

The Bluesky post and comment paths do not have this problem: `Post::get_rkey()` and `Comment::get_rkey()` are only called from publish paths, never from a `wp_head`/read path, so their provenance reflects the last actual publish.

## Fix directions (pick one)

* Write DID provenance only on a successful publish (in the Publisher, after `applyWrites`), not in `get_rkey()`. Then `output_document_link()` mints the TID without touching provenance.
* Or derive the origin DID from `Document::META_URI` (which embeds the DID of the repo the record was actually written to) instead of a separately-maintained `META_DID`, at least in the guard.

Either removes the read-path refresh as the guard's weak point.

Contributor guide

Open the contributing guide

Research direction

Start with Atmosphere::output_document_link(), Document::get_rkey(), and the wrong-repo delete guard, then trace how Document::META_DID and META_URI are written through publishing and applyWrites#delete. Choose a fix direction that preserves the original repository provenance across a reconnect and verify that a pageview cannot make deletion target the new account; confirm the documented delete scenario no longer orphans the original document.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.