Skip to content

Add Comment::META_DID guard for comment cleanup paths #70

@kraftbj

Description

@kraftbj

Follow-up from PR 63 (round-4 approval): the post-side DID-provenance guard added in PR 63 (Post::META_DID + Document::META_DID + the atmosphere_did_mismatch check in Publisher::delete_post) defends the disconnect → reconnect-to-different-DID flow for posts. The symmetric defense is missing for comments.

Threat

Site is connected to DID A. Author publishes a comment under DID A. The Comment transformer mints a TID and writes it to comment meta. Operator disconnects, reconnects to DID B. Later, the comment is unapproved or the parent post is permanently deleted. Publisher::delete_comment() or Publisher::delete_comment_by_tid() issues applyWrites#delete against DID B's repo for a TID that lives (if it lives) on DID A's repo. AT Proto returns 200 idempotently. Local meta clears. The reply on DID A is permanently orphaned.

This is the same threat model the round-2 work blocked for Publisher::delete_post, just on a path that wasn't widened.

Affected code

  • includes/transformer/class-comment.phpComment::get_rkey() (around line 138-144) persists Comment::META_TID but no Comment::META_DID.
  • includes/class-publisher.php:1449Publisher::delete_comment( \WP_Comment $comment ) has no DID-mismatch check.
  • includes/class-publisher.php:1494Publisher::delete_comment_by_tid( string $tid ) has no DID-mismatch check.

Suggested shape

Mirror the Post/Document pattern from PR 63:

  1. Add Comment::META_DID = '_atmosphere_bsky_comment_did'.
  2. In Comment::get_rkey(), write META_DID = \Atmosphere\get_did() unconditionally on every call, BEFORE the META_TID write (matching the order Post and Document use so a partial failure leaves "DID set, no TID" rather than "TID set, no DID").
  3. In Publisher::delete_comment(), read Comment::META_DID and compare against get_did(). Skip with WP_Error( 'atmosphere_did_mismatch', ... ) when origin DID is non-empty and differs from current. Preserve local meta on bail for manual remediation.
  4. For Publisher::delete_comment_by_tid() — the path called from atmosphere_delete_comment_record cron after a comment row is already gone — the comment meta is unavailable at fire time. Either: (a) capture META_DID in the comment's on_before_delete handler and pass it through the cron event args alongside the TID, or (b) accept that this path is best-effort and document.
  5. Add the new meta key to uninstall.php and clear_all_record_meta() equivalents.
  6. Backfill Comment::META_DID = get_did() once on upgrade for comments with META_TID but no META_DID — analogous concern to the legacy-post backfill noted in the P3 polish list for PR 63.
  7. Tests: atmosphere_did_mismatch happy path, legacy-fallback (no META_DID present), and ambient retry-after-reconnect-to-original-DID.

Priority

P2 — bounded by the specific account-rotation sequence and the fact that the orphaned record is on a repo the site no longer authenticates to. Not exploitable on its own; pairs naturally with the delete_post_by_tids follow-up.

Related

  • Closed by PR 63 for the post path. This is the comment-shaped sibling.
  • See also the delete_post_by_tids DID-guard follow-up (sibling issue).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions