Automattic / Automattic/wordpress-atmosphere
Document-only delete guard is bypassed when META_DID is refreshed by a pageview after reconnect
- 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
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