Extend soft delete to draft, pending, and private post statuses#2860
Merged
Conversation
pfefferle
changed the title
Member
Author
|
Blocked by: mastodon/mastodon#37730 |
A federated post moved to draft, pending, or private status now emits a Delete activity instead of an Update carrying the "(This post is being modified)" placeholder (draft/pending) or being silent (private). The soft-delete pattern lines up with FEP-4f05 and matches the existing behavior for visibility=local/private and post_status=trash. The placeholder branches in get_summary() / get_content() are kept @deprecated for the preview path and any direct transformer callers, but no longer fire from the scheduler.
Handles the soft-delete re-publish race: a post is moved to draft (Delete scheduled), then re-published before the Delete has fired. The pending Delete must be invalidated so we do not send both a Delete and a Create for the same object. delete_superseded_items() previously only invalidated same-type pending items for non-Delete activities. It now also clears any pending Delete when a Create or Update is added.
pfefferle
force-pushed
the
add/soft-delete-draft-pending
branch
from
731842f to
1718215
Compare
Adds entry #6 to the Known Vulnerability History documenting the public-to-password-protected transition leak (scheduler, summary helper, content-negotiation router, outbox snapshot) and cross- references WPScan IDs on entries #1–#4 so future audits have stable references. Rewrites the Content Negotiation & Post Visibility section to flag that `get_content_visibility()` does NOT see `post_password` — any check that only routes through visibility meta is incomplete. Adds a federation-lifecycle-transition matrix so audits cover every silent move out of "publicly queryable" (password applied, visibility flipped, post type support removed) and an outbox-snapshot warning so future audits treat the stored activity as its own attack surface.
Collapse the draft/pending/private/trash/default switch arms into a single Delete branch. A federated post moved to any non-publish status — including a custom or third-party post status that lands in default — now emits Delete so federated copies are torn down. Without the default fallthrough, a custom non-public status would slip through silently.
Introduces a single private helper on the Post transformer that fires when either trigger applies: password-protected post (federation output is per-instance, never per-request — must not use post_password_required which respects a per-request cookie and would leak through outbox snapshots) or non-publish status outside preview mode. Wires the four content-derivation methods through it: get_attachment, get_summary, get_content, and get_preview each become a one-line "if is_redacted return null". The four duplicate inline 'publish' !== get_post_status checks are removed, and the placeholder strings that broadcast "(This post is being modified)" to followers are gone — the property is omitted from the activity entirely instead.
The factory uses is_post_disabled() as its gate, which intentionally lets a federated-then-hidden post through the federation pipeline so a Delete activity can fire. That lifecycle escape hatch must not leak into front-end rendering during the window between status change and Delete delivery — otherwise an unauthenticated AP request can still fetch the post's id/name/url/image/attributedTo even after it has been moved to draft, pending, or private. Gate render_activitypub_template() on is_post_publicly_queryable() for the queried WP_Post. Exclude ap_outbox so outbox items keep resolving through their own visibility logic, and skip non-WP_Post queried objects (actors, terms, comments) which have their own predicates. Dataprovider test asserts publish->draft/pending/private all stop rendering the AP JSON template.
Two corrections to delete_superseded_items() after the soft-delete
expansion:
1. Restrict pending-Delete cancellation to Create and Update. The
earlier widening dropped pending Delete on every non-Delete
activity, so an unrelated event for the same object (Add/Remove
from a sticky transition, Like, Undo) would silently keep the
remote copy alive. Now only a republish (Create) or republish-edit
(Update) cancels the queued Delete; other types fall back to the
original same-type-only invalidation.
2. When a Delete is queued, also wipe already-sent ('publish') outbox
items for the same object, not just pending ones. A redelivery
retry against an inbox that previously failed could otherwise
resurrect the very content we are tearing down.
New tests assert: Add/Remove/Like/Undo leave a pending Delete intact,
and a publish-status Create is wiped when Delete supersedes the object.
Replace the visibility-meta-only checks in the soft-deleted no-op and the federated-to-non-public downgrade with is_post_publicly_queryable(). The helper already encodes all four "not for unauthenticated public consumption" reasons (status, AP visibility, post-type support, post_password) in one place, so the scheduler picks them all up without duplicating the conditions. Adds the resurrection path: a soft-deleted post that is back in a publicly queryable state emits Create, not Update. Remote followers either dropped the original Create on the Delete fan-out (so they need to learn about the post again) or had it cancelled before fanning out (the outbox supersession invalidates the pending Delete, and Create is the correct re-introduction).
delete_superseded_items() leaned on get_posts()'s default limit of five rows. When Delete is queued and the new status_filter='any' behavior would normally wipe the object's entire outbox history, only the newest five sent or pending items were actually deleted — older Create / Update snapshots stayed available for redelivery retries to resurrect. Add 'numberposts' => -1 so every matching outbox row is processed, and a regression test that queues seven sent Creates before adding a Delete and asserts all seven get wiped.
Covers the full back-and-forth matrix: - publish -> draft/pending/private -> publish (Delete cancelled, Create queued) - publish + password -> unlock (Delete cancelled, Create queued) - publish -> draft (Delete sent) -> publish (fresh Create, sent Delete preserved) - repeated saves in soft-deleted state (no oscillation, state stays deleted) - three publish/draft/publish cycles (settles to a single pending Create) - publish + password (Delete emitted, not Update) - publish -> custom non-public status (default switch arm fires) Transformer-side: dataprovider asserts to_object() omits content, summary, summaryMap, contentMap, preview, and attachments for redacted posts. The password row exercises the gate even with a valid wp-postpass cookie (filtered to return false from post_password_required); the non-publish rows skip themselves when an earlier suite test has defined ACTIVITYPUB_PREVIEW (PHP cannot undef a constant — the underlying behavior is exercised by the scheduler tests anyway).
The previous version of the default case unconditionally emitted Delete for any federated post whose new status fell through (i.e. anything that wasn't publish). That caught attachments with post_status='inherit' when their parent post was still public — wp_after_insert_post fires for attachment edits too, so triage() was running on an attachment whose "inherit" status hit default and scheduling a spurious Delete. Tighten the guard to also require ! is_post_publicly_queryable(). For the canonical non-public statuses (draft/pending/private/trash) the helper still returns false, so behavior is unchanged. For inherit-status attachments under a public parent the helper recurses into the parent and returns true, so no Delete is scheduled. Regression assertion added to test_transition_attachment_status: after an attachment Update, the outbox must not also contain a Delete.
The federation semantics for federated posts moved to draft, pending, private, trash, or password-protected genuinely change with this PR: previously a Delete was only sent for trash and a placeholder "(post being modified)" Update was sent for draft/pending; private and password-protected were silent. After this PR all of those transitions emit Delete. Existing followers will see drafts disappear from their feeds where they used to see the placeholder. Reword the entry as "changed" and lift significance to major so the behavior shift surfaces in update notices.
Update was an over-permissive cancel signal: external callers — the `wp activitypub post update <post_id>` CLI command, third-party plugins or filter hooks that go through add_to_outbox() — can queue an Update for a soft-deleted object that is still hidden, because is_post_disabled() deliberately lets soft-deleted posts through the transform pipeline. With the previous rule, that Update would silently flip the object state back to federated while the post stayed draft/private/locked. Only Create cancels a pending Delete now. The scheduler's resurrection branch already rewrites Update → Create when a soft-deleted post comes back to a publicly queryable state, so the legitimate re-publish path still cancels Delete. Renamed test_update_supersedes_pending_delete to test_update_does_not_supersede_pending_delete and flipped the assertion to match the new contract.
Posts created before activitypub_last_post_with_permalink_as_id still use their permalink as the ActivityPub ID (the post-ID URL is only used past that threshold). If a slug change accompanies the soft-delete transition — for example moving from publish to draft and editing the post_name in the same wp_update_post call — the Delete would target the new-slug URL while remote servers cached the original. The Delete would silently miss and the cached object would remain live. Trash already mitigates this via the wp_trash_post hook setting _activitypub_canonical_url before the status flips. Extend the same mechanism to the scheduler-driven path: the triage() function now populates the meta from $post_before whenever it queues a Delete, and clears it on the resurrection path so a re-publish under the current permalink works as expected. The transformer's get_url() switch is restructured so any non-publish post prefers the cached canonical URL when present, with the sample-permalink fallback for drafts that were never federated. Regression test exercises the legacy case: create a post, mark it as legacy via the option, change slug + status in one update, assert the emitted Delete carries the original-slug URL as its object.
…fan-out" This reverts commit a274715.
The trash path caches the pre-transition URL via the wp_trash_post hook, but the scheduler-driven soft deletes (draft / pending / private / password) do not. For posts at or below activitypub_last_post_with_permalink_as_id (those still using their permalink as the AP ID), a slug change combined with a soft-delete transition will emit a Delete that targets the new permalink while remote servers cached the original. We accept the limitation rather than ship the scheduler-side canonical-URL persistence — it adds meta writes on every soft-delete for every site even though only legacy posts on long-running installs are affected, and the workaround is to do the slug change as a separate save. Note added to Post::get_id() docblock alongside the existing legacy behavior description so anyone tracing why old posts use permalinks finds the caveat in the same place.
get_name(), get_image(), and get_icon() bypassed the existing is_redacted() gate, so a same-save edit that changed the title or featured image alongside a soft-delete transition would have those new values appear on any transformer surface that doesn't go through the Outbox::get_activity() Delete-collapse filter — direct transformer calls from debug tools, REST endpoints, etc. The federation-facing surfaces (dispatcher, /outbox, SSE) already collapse the Delete object to its URI via Delete::outbox_activity, so this is purely a defense-in-depth measure for non-federation readers. Adds the same one-line gate already used by get_content / get_summary / get_preview / get_attachment. Test assertions extended for name / nameMap / image / icon.
to_object() conditionally sets sensitive / summary / summaryMap / dcterms based on activitypub_content_warning meta. That override bypassed is_redacted(), so a redacted post with a content warning would still surface the warning via dcterms (and would re-populate the summary that get_summary() had just nulled out). Wrap the override in an early-return-after-parent if is_redacted() fires so the underlying summary/summaryMap stay omitted. Test extended to set activitypub_content_warning meta on the probe post and assert sensitive + dcterms are both empty alongside the existing field checks.
…t-pending # Conflicts: # .claude/agents/security-audit.md
The publish/Create/Update path is unchanged; only previously-federated posts moving to a non-public status change behavior, so this is a minor change rather than a major version bump.
This is a backward-incompatible behavior change: federated posts moved to draft/pending/private previously persisted on followers' servers as a masked Update, but now receive a Delete that removes the remote copy and its engagement. A re-publish creates a new object rather than restoring the original, so an existing draft/private round-trip workflow loses federated replies, boosts, and likes. That data-losing change to the federation contract warrants a major bump.
A redacted (password-protected or non-public) post previously transformed the full post and relied on each getter to null its own output. That is fail-open: getters for tags, @-mentions, location, and in-reply-to were not gated, so a Delete for a hidden post still serialized those fields and was delivered to actors mentioned only in the now-hidden content, and the public outbox re-served the metadata. Represent a redacted post as a Tombstone instead. A Tombstone is content-free by type, so no body-derived field can leak — including one added to the transformer later. Move the 'a Delete always fans out' policy to Dispatcher::should_send_to_followers() so the Tombstone needs no audience to be delivered, which keeps the delete fan-out working while no recipient is ever derived from hidden content.
The redaction gate only checked status and password, but triage() decides to emit a Delete on the broader is_post_publicly_queryable() predicate. So a federated post made non-public via the activitypub_content_visibility meta (local/private) or by losing post-type support stayed publish and its Delete serialized the full body/attachments instead of a Tombstone, leaking hidden content and addressing actors mentioned only in it. Redact exactly when the post is not publicly queryable, the same predicate the scheduler uses, so the two never disagree. This also drops the ACTIVITYPUB_PREVIEW coupling (is_post_publicly_queryable already treats a preview request's draft as queryable). Attachments inherit their parent's visibility instead, since they have no public status of their own. Fixes two tests the Tombstone change had broken (a draft and an inherit attachment) and adds data-provider coverage for the full transition matrix: initial-hidden does not federate, federated->hidden emits a Tombstone Delete, and hidden->public re-emits a Create.
triage() evaluated is_post_publicly_queryable() four times for the same post. Compute it once into a local and reuse it, which also collapses the already-soft-deleted guard to a single line. No behavior change.
…mbstones Broadcasting every Delete to all known remote inboxes leaked private and direct activities (C2S, Outbox::undo on a private outbox item): the Delete reached servers that never received the original, exposing the object URI and the deletion event. Address a soft-deleted post's Tombstone to the public collection instead, and drop the unconditional Delete branch in should_send_to_followers(). A post only reaches the soft-delete path after being federated, and only public and quiet-public posts federate, so its teardown was always public; private and direct activities keep their original non-public audience and are no longer broadcast. Because the Tombstone now passes the standard is_activity_public() gate, this also works with an older dispatcher, so the transformer no longer depends on a matching dispatcher being deployed. Also keep the permalink on the Tombstone so Delete::maybe_bury() registers it; without it a soft-deleted post on a site that uses the post-ID ActivityPub ID would miss the tombstone registry when requested by URL.
…tachment visibility Reject an explicit Update for an already-deleted object in add_to_outbox() with a WP_Error. Such an Update only federates a Tombstone and, worse, reset the object state to federated, so a later re-publish skipped the Create that cancels the still-pending Delete — the queued Delete could then tear down a post that is public again. Re-publishing (which emits a Create) remains the supported way to bring a deleted object back. The post and comment update CLI commands now surface the WP_Error instead of reporting a queued Update. Drop the Attachment transformer's redaction override: it only inherited a parent's visibility, so a standalone attachment made non-public still serialized its media. Inheriting is_post_publicly_queryable() from the base transformer resolves the attachment's own visibility, password, and post-type support as well as parent inheritance.
When a federated post is about to be hidden — set to draft/pending/private, password-protected, or marked do-not-federate — show a warning notice in the editor explaining that followers' copies will be removed and may not be restorable on some platforms. The warning is driven by a pure, unit-tested helper and surfaced through the native editor notices store.
github-actions
Bot
added
[Block] Post settings
[Feature] CLI
[Focus] Editor
…ss to authorized previews The preview template now refuses to render for logged-out visitors, and the content-negotiation tombstone branch only defers to a publicly queryable post during an authorized ?preview=true request. A normal ActivityPub fetch of a tombstoned URL always returns the Tombstone, even if the slug was later reused by a fresh public post.
…t rendering - Assert the Fediverse Preview template refuses logged-out visitors. - Cover the CLI update commands surfacing an outbox WP_Error as a halt (adds minimal, guarded WP-CLI stubs so the command classes load under PHPUnit). - Assert an authorized preview of an unpublished post renders real content, not a Tombstone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes:
Extends the soft delete functionality (FEP-4f05) to also send Delete activities when a federated post's status is changed to "draft", "pending", or "private". Previously, changing a post to draft would send an Update activity with placeholder content "(This post is being modified)". Now it sends a proper Delete activity and returns a Tombstone, consistent with other soft delete scenarios.
Changes
is_post_disabled()to handle draft/pending/private statuses for federated postsinvalidate_existing_items()to invalidate pending Delete activities when Create/Update is added (prevents sending Delete after re-publish)Soft Delete Behavior
When a federated post is changed to draft/pending/private:
Re-publish Handling
When a soft-deleted post is re-published:
This follows the same pattern as visibility changes to local/private from #2824.
Known limitations
activitypub_last_post_with_permalink_as_idmigration use their permalink as the ActivityPub ID. If such a legacy post has its slug changed in the same save as a soft-delete transition (e.g. publish → draft with a newpost_name), the Delete targets the new permalink while remote servers cached the original, so the remote copy is not torn down. The trash path mitigates this by caching the pre-transition URL in_activitypub_canonical_urlvia thewp_trash_posthook; draft/pending/private/password transitions do not. Narrow blast radius (legacy posts only, and only when slug and status change together) and documented inPost::get_id(). Workaround: on a pre-ID-migration site, avoid editing the slug in the same save as the visibility change.Other information:
Testing instructions:
Soft Delete
activitypub_statusmeta is set to "deleted"Re-publishing (before Delete is sent)
Re-publishing (after Delete is sent)
Changelog entry
Changelog entry has been added manually.