Extend soft delete to draft, pending, and private post statuses#2860
Open
pfefferle wants to merge 18 commits into
Open
Extend soft delete to draft, pending, and private post statuses#2860pfefferle wants to merge 18 commits into
pfefferle wants to merge 18 commits into
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.
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.
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.