Extend soft delete to draft, pending, and private post statuses (#2860)

Author: pfefferle

.claude/agents/security-audit.md

@@ -12,29 +12,57 @@ You are a security auditor for the WordPress ActivityPub plugin. You check for v
 
 Past CVEs and security fixes inform what patterns to watch for. The full list is tracked on [WPScan](https://wpscan.com/plugin/activitypub/). Check this list periodically to stay current on newly disclosed vulnerabilities and update the entries below accordingly.
 
-1. **Unauthenticated REST API access** (CVE, fixed 1.0.6) — endpoints accessible without auth
-2. **Post title/content disclosure** (CVE, fixed 1.0.0) — low-privilege users accessing unpublished content
-3. **Stored XSS** (CVE, fixed 1.0.0/1.0.1) — contributor+ injecting scripts
-4. **Content negotiation leak** (PR #3045, 2026) — non-public posts served via ActivityPub Accept header
+1. **Unauthenticated REST API access** (WPScan `5fb58642-61ba-447c-80ac-68d3777486d7`, fixed 1.0.6, 2024) — endpoints accessible without auth.
+2. **Post title/content disclosure** (WPScan `daa4d93a-…` / `541bbe4c-…`, fixed 1.0.0, 2023) — low-privilege users accessing unpublished content.
+3. **Stored XSS** (WPScan `58a63507-…` / `c15a6032-…`, fixed 1.0.0/1.0.1, 2023) — contributor+ injecting scripts.
+4. **Unauthenticated drafts/scheduled/pending posts disclosure via content negotiation** (WPScan `50f68395-72fc-4f99-8e6d-6aa90cc640b5`, fixed 8.0.2, PR #3045, 2026) — non-public posts served via ActivityPub `Accept` header.
 5. **Per-post REST routes leaking non-public posts** (2026) — `/posts/{id}/reactions`, `/posts/{id}/replies`, `/posts/{id}/likes`, `/posts/{id}/shares`, and `/comments/{id}/remote-reply` returned reaction / reply / like / share / remote-reply metadata for private, draft, password-protected, and local-only posts — including posts that had been federated earlier and were then made non-public. Root cause: routes loaded the post via `get_post()` and only bailed on "post doesn't exist", rather than gating on current public visibility. The canonical content-exposure predicate is now `is_post_publicly_queryable()`; `is_post_disabled()` is the pipeline gate only and must not be used as a content-exposure check (its lifecycle escape hatch intentionally lets previously-federated posts through so Delete / Create activities can fire).
 6. **Ownership-check divergence cluster** (2026) — multiple endpoints re-implemented owner determination instead of using the canonical `verify_owner()` / `maybe_verify_owner()` gate (`includes/rest/trait-verification.php`), and diverged in ways that leaked data: (a) the Outbox controller treated the unscoped `current_user_can('activitypub')` capability as ownership of *every* user's outbox, skipping the public-only visibility filter; (b) the SSE outbox stream's permission callback was correctly owner-gated but the *query* behind it filtered by the shared actor-**type** meta (`_activitypub_activity_actor` = `'user'`) instead of the requesting actor, so a legitimately-opened stream emitted every user's outbox items. Lesson: access-control on the endpoint and data-scoping of the query are **two separate checks** — verify both.
 7. **Inbound-activity actor↔object binding gaps** (2026) — Update / Undo / Accept handlers and remote-record upserts (`Remote_Actors`, `Remote_Posts`, `Interactions::update_comment`, `Inbox::undo`) mutated or deleted local records resolved purely by the attacker-supplied `object.id` / GUID, without confirming the activity's `actor` owns that object — unlike the Delete handler which requires `object_to_uri($activity['object']) === $activity['actor']`. A valid signature only binds the key to `actor`, never to `object.id`, so signing does not close this gap.
 8. **HTTP-signature keyId parser divergence** (2026) — `verify_key_id()` (the only key-host ↔ actor-host binding) extracted keyId with a quote-*required* regex and returned `true` (fail-open) on no match, while the RFC-9421 verifier parsed keyId with quotes *optional* — so an unquoted `keyid` validated cryptographically but skipped the host-equality binding.
+9. **Public-to-password-protected transition leak** (fix/password-protected-post-leak, 2026) — a previously federated public post that was changed to password-protected continued to expose its current content through (a) the scheduler emitting a new `Update` activity with the now-protected text instead of a `Delete`, (b) `generate_post_summary()` reading `post_excerpt` / `post_content` raw with no password gate, so `summary`, `summaryMap`, and `preview.content` all leaked on every fresh transformer run, and (c) the content-negotiation router using `is_post_disabled()` as the gate, whose lifecycle escape hatch let the protected post's representation be served to any unauthenticated `Accept: application/activity+json` request. Notably, the outbox stores the *serialized* activity at scheduling time, so any leak in the transformer is *frozen* into outbox snapshots and continues being served from `/wp-json/activitypub/1.0/actors/{id}/outbox` until the snapshot is purged. **The audit pattern from this finding:** check every transition that can flip a federated post out of "publicly queryable" *without* changing its `post_status` — password applied, AP visibility meta flipped to `local`/`private`, post type losing `activitypub` support, etc. Each transition needs (i) a scheduler path that emits `Delete`, not `Update`, (ii) every content-derivation helper to refuse to read raw fields, and (iii) the content-negotiation surface to refuse to render.
 
 ## Audit Scope
 
 Run ALL checks below unless the user specifies a subset. Each check should read the relevant source files and trace the code path.
 
 ### 1. Content Negotiation & Post Visibility
 
-Files: `includes/class-router.php`, `includes/class-query.php`, `includes/functions-post.php`
+Files: `includes/class-router.php`, `includes/class-query.php`, `includes/functions-post.php`, `includes/scheduler/class-post.php`, `includes/transformer/`, `includes/collection/class-outbox.php`
 
-- Verify `is_activitypub_request()` cannot be abused to bypass access controls (check `?activitypub` query param path)
-- Verify `is_post_publicly_queryable()` blocks all non-public statuses (draft, pending, future, trash, private) on content-exposure surfaces
-- Check that password-protected posts are not served via ActivityPub
-- Verify attachments (`inherit` status) are only served when the parent post is published
-- Check that the transformer strips content/summary/attachments for non-published posts
-- Verify `pre_handle_404` and `template_include` hooks respect post visibility
+**ALWAYS check both visibility AND password on every content-exposure surface.** Skipping either one is the canonical leak vector. The plugin has three independent kinds of "not publicly readable":
+
+| Trigger | Where it lives | Detected by |
+|---|---|---|
+| `post_status` not `publish` (draft, pending, private, trash, future) | Core `posts` table | `is_post_publicly_queryable()` |
+| `activitypub_content_visibility` meta = `local` or `private` | Post meta | `get_content_visibility()` (also in `is_post_publicly_queryable()`) |
+| `post_password` non-empty | Core `posts` table | `! empty( $post->post_password )` (also in `is_post_publicly_queryable()`) |
+
+The third one is the one that is easiest to forget — it's a single column on the WP_Post object, not surfaced through any AP-specific meta, and `get_content_visibility()` does NOT see it. **Any check that only goes through `get_content_visibility()` is incomplete.** Either use `is_post_publicly_queryable()` (which folds in all three) or add an explicit `! empty( $post->post_password )` clause alongside the visibility check.
+
+- Verify `is_activitypub_request()` cannot be abused to bypass access controls (check `?activitypub` query param path).
+- Verify `is_post_publicly_queryable()` blocks all non-public statuses (draft, pending, future, trash, private) on content-exposure surfaces.
+- **Verify password-protected posts are blocked on every content-exposure surface — not just the REST controllers, but also `template_include` / content-negotiation rendering, transformer-derived helpers (`generate_post_summary()`, etc.), and any block server-side render callbacks.**
+- Verify attachments (`inherit` status) are only served when the parent post is published *and* not password-protected — `is_post_publicly_queryable()` recurses into the parent, so direct callers of that function are safe; manual parent-status checks are not.
+- Check that the transformer strips content / summary / attachments for non-published posts AND for password-protected posts. The `[ap_content]` shortcode is the only path that historically checked `post_password_required()`; helpers like `generate_post_summary()`, `get_name()`, `get_preview()`, and any new derivation must add their own gate.
+- Verify `pre_handle_404` and `template_include` hooks respect post visibility AND password.
+
+**Federation lifecycle transitions — the scheduler must downgrade to Delete on ANY transition out of "publicly queryable":**
+
+The scheduler in `includes/scheduler/class-post.php` decides between `Create` / `Update` / `Delete` based on `post_status` transitions. But a federated post can also leave "publicly queryable" *without* changing its `post_status` — and each of those silent transitions is its own leak class:
+
+| Transition | Old check covered it? | What should happen |
+|---|---|---|
+| `publish` → `trash` | Yes (`new_status` switch) | Delete |
+| `publish` → `private` (status) | Yes — `is_post_publicly_queryable()` returns false | Delete |
+| visibility meta → `local` / `private` | Yes (since #3045) | Delete |
+| `post_password` applied | **No (was the public→password leak)** | Delete |
+| post type loses `activitypub` support | Partial — flag if missing | Delete |
+| activitypub `supports` removed at runtime via filter | Partial — flag if missing | Delete |
+
+For each of these, confirm the scheduler's "downgrade to Delete" branch fires. The canonical check is `ACTIVITYPUB_OBJECT_STATE_FEDERATED === $object_status && ! is_post_publicly_queryable( $post )` (or an explicit OR of the password and visibility clauses) — anything narrower will miss at least one transition.
+
+**Outbox snapshots freeze the leak.** `add_to_outbox()` serializes the activity at scheduling time and stores it as an `ap_outbox` post. Once a leaked Update is in the outbox, the public `/outbox` endpoint will continue serving the snapshot even after the underlying post is fixed. When auditing a fix, verify the outbox does not retain pre-fix leaked activities — and when reporting a finding, treat the outbox as a separate attack surface, not just a downstream consumer.
 
 - **Know which gate to use — `is_post_disabled()` vs `is_post_publicly_queryable()`.** The plugin has two post-visibility predicates with different jobs, and using the wrong one leaks data.
   - `is_post_disabled()` is the **pipeline gate**: schedulers, transformers, and outbox dispatch use it to decide whether a post participates in federation processing at all. It intentionally returns `false` for posts in a federation lifecycle transition (`federated` → now private, or `deleted` → now restored) so Delete/Create activities can still fire.

.github/changelog/add-fediverse-deletion-warning

@@ -0,0 +1,4 @@
+Significance: minor
+Type: added
+
+Warn in the editor before making a post that's already shared on the Fediverse a draft, private, or password-protected, since followers' copies will be removed.

.github/changelog/add-soft-delete-draft-pending-private

@@ -0,0 +1,4 @@
+Significance: major
+Type: changed
+
+Federated posts moved to draft, pending, private, trash, or password-protected now send a Delete to followers (previously sent a placeholder "editing" Update or were silent).