Display images attached to Fediverse comments

[pfefferle]

Fixes #1852

Proposed changes:

• Display image attachments in incoming Fediverse comments, including image-only replies accepted through both inbox routes and updated attachments.
• Reuse the existing attachment parser, image processing, activitypub/image block, and lazy Media cache. Comment images share their parent post's cache directory; there is no separate comment cache or initialization.
• Preserve image descriptions through WordPress comment sanitization, including less-than signs and literal entities. Strip untrusted block syntax before adding our own blocks; render only leaf emoji blocks and, for received Fediverse comments, image blocks. Ordinary WordPress comments cannot reconstruct images from block attributes.
• Clean up cached images when their parent post is deleted, including ordinary WordPress posts. Deleting or editing an individual comment does not clear the shared cache.
• Skip malformed attachment references and non-absolute or non-HTTP(S) attachment URLs, using the same absolute-URL requirement as the existing cache.
• Keep this first step image-only; audio and video attachments are not included.

Other information:

• Shared URI resolution is reviewed separately in Handle malformed ActivityPub object references #3855. Merge that prerequisite first; its changes remain on this branch to preserve the malformed-reference guards and will drop out of this diff once merged into trunk.

• The unrelated OAuth rate-limit test fix has moved to Avoid false OAuth rate-limit test failures at minute rollover #3856 and is removed from this branch.

• Have you written new tests for your changes, if applicable?

Testing instructions:

• Publish a federated WordPress post and reply from Mastodon with an attached image and image description. Approve the comment if moderation requires it.
• View the comment and verify the image and description appear. Its image URL should point into /uploads/activitypub/posts/{parent-post-id}/ after caching; refreshing should reuse the cached file.
• Repeat with an image-only reply that omits content, then edit a reply to replace or remove its attachment. Verify the displayed comment follows the update. Use image descriptions containing width < height, quotes, ampersands, and literal <; verify they survive creation and updates.
• Delete the parent WordPress post permanently and verify its cached image directory is removed.
• Run npm run env-test (3,503 tests, 8,592 assertions, 25 skipped). Coverage includes both inbox routes rejecting malformed and relative image URLs, shared URI resolution, attachment normalization, comment creation and updates, received-comment rendering, ordinary-comment restrictions, nested-block rejection, and shared-cache cleanup. PHPStan (composer analyze -- --no-progress --error-format=github) and PHP coding standards pass on the changed files.

Changelog entry

• Automatically create a changelog entry from the details below.

Changelog Entry Details

Significance

• Patch
• Minor
• Major

Type

• Added - for new features
• Changed - for changes in existing functionality
• Deprecated - for soon-to-be removed features
• Removed - for now removed features
• Fixed - for any bug fixes
• Security - in case of vulnerabilities

Message

Display images attached to Fediverse comments, with local caching and image descriptions.

The changelog entry is already committed in .github/changelog/fix-comment-images.

[Copilot]

Copilot review overview

🟡 Changes recommended

Attachment classification and unconditional URL entity decoding can produce broken or incorrect image resources.

Review effort: Balanced
Findings: 2

Open (2)

• Avoid HTML-decoding block attributes from ActivityPub JSON URLs · New
• Classify attachments by object type when mediaType is missing · New

What changed in this PR

Adds image attachments to incoming Fediverse comments using existing block rendering and media caching.

Changes:

• Parses, sanitizes, and renders comment image attachments.
• Shares parent-post media caching and cleanup.
• Adds coverage for rendering, updates, sanitization, and deletion.

File	Description
includes/​functions-media.php	Preserves image alt text in block attributes.
includes/​collection/​class-remote-posts.php	Exposes and extends attachment parsing.
includes/​collection/​class-interactions.php	Adds images to created and updated comments.
includes/​class-comment.php	Supplies parent-post context while rendering comments.
includes/​class-blocks.php	Reconstructs sanitized images and uses block context.
includes/​cache/​class-media.php	Cleans caches for ordinary parent posts.
tests/​phpunit/​tests/​includes/​collection/​class-test-interactions.php	Tests comment image lifecycle and sanitization.
tests/​phpunit/​tests/​includes/​class-test-functions-media.php	Tests safe block attributes and media filtering.
tests/​phpunit/​tests/​includes/​class-test-comment.php	Tests comment block context and filters.
tests/​phpunit/​tests/​includes/​cache/​class-test-media.php	Tests regular-post cache cleanup.
.github/​changelog/​fix-comment-images	Adds the user-facing changelog entry.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Unresolved image-handling issues and changes to untrusted-content rendering require human security review.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Classify attachments by object type when mediaType is missing
• Avoid HTML-decoding block attributes from ActivityPub JSON URLs

Previously missed (2)

In code that hasn't changed since last review

Prevent remote image tracking URLs in ordinary WordPress comments

includes/​class-comment.php:74

comment_text also handles ordinary WordPress comments, which do not pass through the remote-content sanitizer. A commenter can submit <!-- wp:activitypub/image {"url":"https://example.com/tracker.jpg","alt":""} /-->; KSES preserves the block attributes, and the new image callback reconstructs an image despite comment HTML restrictions. If caching is disabled or fails, readers load the remote tracking URL. Require a supplied comment with was_received() before rendering image blocks, while preserving emoji rendering. Update the rendering tests to use received-comment metadata and add an ordinary-comment submission regression test.

Preserve linked image attachments during comment creation and updates

includes/​collection/​class-interactions.php:528

Both comment creation here and updates at line 164 can omit an image attachment when the reply also contains its URL in text or a link. process_remote_images() skips URLs found anywhere in the content (includes/functions-media.php:171–174), even when sanitization has removed the linked image. Generate attachment blocks with process_remote_images( '', ... ) and append them to the sanitized content at both call sites. Add creation and update tests where the reply links to its attachment.

[pfefferle]

Addressed both "Previously missed" findings in 3457443. Image-block rendering now requires a supplied comment recognized by Comment::was_received(); emoji rendering is unchanged. Comment creation and updates append attachment blocks independently of sanitized text, so linked attachment URLs no longer suppress images. Regression coverage includes ordinary comment submission with unavailable caching, missing comment context, emoji compatibility, and plain URL / linked / linked-image attachments on creation and update. Full PHPUnit suite passes (3,464 tests, 25 skipped); PHPStan, PHPCS, and code review pass.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Changes to sanitization and dynamic rendering of untrusted content warrant final human security review.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Prefix wp_delete_post() with a leading backslash

tests/​phpunit/​tests/​includes/​cache/​class-test-media.php:100

Prefix this call with \wp_delete_post() to follow the namespaced WordPress-function convention in AGENTS.md:30. The current call works through PHP's global fallback, so this is a consistency fix rather than a behavior change.

[pfefferle]

Fixed the previously missed namespace-convention finding in 9cd8dad: the media cache cleanup test now calls \wp_delete_post(). This is a test-only consistency change with no behavior change. The focused cleanup regression test and PHPCS both pass.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Security-sensitive changes to remote-content sanitization and block rendering need final human validation.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Add coverage for single-attachment normalization

tests/​phpunit/​tests/​includes/​collection/​class-test-remote-posts.php:41

Every case here supplies attachment as a list, so the new single-attachment normalization in Remote_Posts::extract_attachments() is not covered. Existing single-object tests exercise the separate image fallback instead. Run these cases with a single associative array and a stdClass as well, and assert the URL, description, and type survive normalization. This would catch regressions that silently drop valid comment images.

[pfefferle]

Addressed the summary-only finding Add coverage for single-attachment normalization in e630293. The existing classifier test now runs each of seven media/type cases as a list, a single associative attachment, and a stdClass attachment (21 combinations). It asserts the complete normalized result, including URL, description, and type. No production changes or new helpers. Focused attachment/image-fallback tests pass: 7 tests / 27 assertions; PHPCS passes.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Validation and description preservation need fixes, and the sanitization changes warrant final human review.

Review effort: Balanced
Findings: 1

Open (1)

• Allow image-only replies through inbox validation · New

Previously missed (1)

In code that hasn't changed since last review

Preserve less-than signs in serialized image alt text

includes/​functions-media.php:63

get_attribute() returns decoded text, but WordPress sanitizes block attribute strings as HTML when saving comments. A description such as width < height can therefore lose everything from the less-than sign onward before the image is rebuilt. HTML-encode alt before serialization, as with url; the renderer already escapes it with esc_attr(), which preserves existing entities. Update the block-generation expectations and add a comment save/render regression containing a literal <.

[pfefferle]

Whole-PR review and feedback assessment for d0892ee:

The earlier tests bypassed inbox validation and did not follow descriptions through comment persistence. That was too narrow. This review traced ingestion, attachment normalization, sanitization, block generation, saving/updating, rendering, and shared-cache cleanup, with an independent review of the full diff.

• Missing-content validation: valid, fixed by reusing attachment extraction while retaining required IDs and previous validation vetoes. Both inbox routes now have regressions. I assess the impact as P2 functionality, not a demonstrated security vulnerability.
• Alt-text preservation: valid, fixed by encoding decoded text before block serialization. The wider review also found literal entity spellings were lost earlier during attachment-to-HTML conversion; that step now encodes JSON-derived descriptions before the existing esc_attr() call. The regression starts with attachment.name and follows conversion, comment creation/update, and rendering. Existing HTML stripping remains intact.
• Rendering boundaries: received-comment metadata is still required for image rendering; ordinary comments cannot reconstruct tracking images from block attributes. Only allowed leaf image/emoji blocks render. The nested-block regression now supplies a received comment, exercising the image-permitted branch instead of trivially rejecting every image.
• KISS: no new helper classes, cache lifecycle, or generic media support. Removed duplicated image-fallback normalization; the common normalization pass already handles it.

Verification: full PHPUnit suite passed (3,474 tests, 8,519 assertions, 25 skipped); final PR-focused suite passed (607 tests, 1,594 assertions, one skipped). PHPStan and coding standards passed. A selective-run authentication failure was traced to a pre-existing test setup issue: an earlier page query leaked into REQUEST_URI and thus into the signed request-target. The signature test now supplies its actual inbox URI for dispatch and restores server state; production authentication is unchanged.

No additional blocker found in the whole-PR review. The description and testing instructions have been updated.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Security-sensitive sanitization, rendering, and caching changes need human verification, and attachment URL validation issues remain unresolved.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Shared URI resolver crashes on malformed image URLs · New
• Relative image URLs bypass validation and caching · New

Resolved since last review (1)

• Allow image-only replies through inbox validation

[Copilot]

Copilot review overview

🔵 Needs a closer look

Explicitly malformed attachment types are classified as images and pass image-only Create validation.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Shared URI resolver crashes on malformed image URLs
• Relative image URLs bypass validation and caching

Previously missed (1)

In code that hasn't changed since last review

Reject explicitly invalid attachment types instead of defaulting to image

includes/​collection/​class-remote-posts.php:441

When mediaType is absent, an explicitly non-string type falls back to image. A reply with no content and attachment: {"type": 42, "url": "https://example.com/photo.jpg"} therefore passes the new image-only Create validation and gets an image block, even though its attachment type is invalid. Keep the image default for omitted types only; classify explicit invalid types as unsupported. Add regression cases for the extractor and Create validator.

[pfefferle]

Fixed the PHP 8.3 CI failure in b61be19 with four test-only lines. The registration test assumed its two requests shared a calendar-minute rate-limit bucket. The limiter deliberately resets that bucket at the next minute, so a second registration returning 201 can be correct across a rollover. The failed job places this test between 14:50:58.982 and 14:51:04.774 UTC, consistent with that race. The test now uses the same minute-rollover guard already present in the Rate_Limit tests, removing its allowance filter before skipping if the minute changed. Its original 201 / remaining=0 / 429 assertions still apply when both requests stay in one window. No production rate-limit behavior changed.

Verification: 19 focused OAuth/rate-limit tests and 74 assertions passed; the full suite passed again (3,503 tests, 8,592 assertions, 25 skipped); coding standards pass. Fresh CI is triggered by the push.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Security-sensitive sanitization and block-rendering changes need final human validation of end-to-end behavior.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🟡 Changes recommended

Remote-image handling still has unresolved security and description-preservation issues.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Reject unsafe image URLs when caching fails · New
• Preserve comparison characters in attachment descriptions · New

[Copilot]

Copilot review overview

🔵 Needs a closer look

Security-sensitive rendering and caching require human verification, with an unresolved validation-performance concern.

Review effort: Balanced
Findings: 1

Open (1)

• Bound attachment URL validation to prevent synchronous DNS delays · New

Resolved since last review (2)

• Reject unsafe image URLs when caching fails
• Preserve comparison characters in attachment descriptions