[pfefferle]

Fixes #1852

Proposed changes:

• Add an option "Show images attached to replies" in the ActivityPub settings. It is off by default, because WordPress itself does not show images in comments and this changes how comments look.
• With the option on, images attached to incoming Fediverse replies are shown below the comment text, including image-only replies and updated attachments. Up to three images per comment, the limit is filterable with activitypub_comment_image_limit. Audio and video are not included.
• With the option off, no image blocks are stored and image-only replies are rejected like before. Images of comments that came in while the option was on are not shown or downloaded either, they show their description as text instead. The stored blocks are not deleted, so the images come back when the option is turned on again. The same applies when remote caching is turned off, so visitors never load images from the remote host.
• Only replies can be image-only. Other posts without text are still rejected, also with the option on.
• I reuse the existing attachment parser, the activitypub/image block and the lazy Media cache. Comment images are cached per comment in /uploads/activitypub/comments/{comment-id}/. They are removed from the server when the comment is deleted, edited, trashed, marked as spam or unapproved, also when caching is turned off. A restored comment downloads them again on the next view.
• Images only show for approved comments. Pending, trashed and spam comments show only their text, also in the moderation screens, so their images are never downloaded.
• I removed the unused comment parts of Media (CONTEXT_COMMENT, BASE_DIR_COMMENTS, get_storage_paths_for_context() and invalidate_comment()). They were reserved since 5.6.0 but never cached anything. Comment images are handled by Comment_Media now.
• Image descriptions survive the comment sanitization. Only our own leaf emoji and image blocks are rendered, and image blocks only for received Fediverse comments. Before, every activitypub/* block in a comment was rendered.
• Attachment URLs must be absolute HTTP(S) URLs on a public host. The inbox validation runs before the signature check, so it only checks for an image attachment. The host check runs when the comment is stored, and an image-only reply without a valid image is not stored.

One thing to keep in mind: on sites that auto-approve comments, remote images become visible without moderation once the option is on.

Comment::render_blocks() now gets the comment as second argument, so the comment_text filter is registered with accepted_args 2. The priority is still 5, so remove_filter() keeps working.

Testing instructions:

• Enable "Show images attached to replies" under Settings → ActivityPub.
• Reply to a federated post from Mastodon with an image and an image description. Check that the image and description show up, and that the image URL points to /uploads/activitypub/comments/{comment-id}/ after caching.
• Repeat with an image-only reply, and edit a reply to replace or remove its image.
• Disable the option. The comments from before show only their text, and a new reply with an image shows only the text too.
• Trash, spam or delete the comment and check that its cached image directory is removed.
• Delete the parent post permanently and check that the image directories of its comments are removed.

[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.

[jeherve]

Thanks for taking a look at this one. I like it, that should be a nice improvement!

Claude highlighted a few things worth fixing before we merge:

1. Images never show when remote caching is off

Comment images are only shown from the local cache, which makes sense; visitors shouldn't load anything from the remote host. But when caching is off (ACTIVITYPUB_DISABLE_REMOTE_CACHE, activitypub_remote_cache_enabled or activitypub_cache_comment_media_enabled returning false), Comment_Media::init() never runs and every image renders as "Image: …" text. Nothing in the admin says so. The help text still says images "are shown below the comment text", and image-only replies are still accepted and stored with only that placeholder.

I'm wondering if we could:

• Disable the checkbox, or add a note under it, when Cache::is_enabled() && Comment_Media::is_enabled() is false.
• Reject image-only replies in Create::validate_object() in that case, since they can only ever show a placeholder.

I wouldn't stop storing image blocks on regular replies though; keeping them is what lets the images come back once caching is turned on again.

The Photon snippet turns caching off too, so it has the same problem. Maybe we should add comment_media to its context.

2. Image downloads during page views

Comment images are downloaded while the post renders: up to 3 per comment, each with the 15-second timeout from download_url(). Failed downloads aren't remembered, so every uncached page view tries again. On a site that auto-approves comments, a reply whose images point at a slow host would make every uncached view of that post wait up to 45 seconds. Avatars and emoji work the same way, but they're small and shared between comments.

I can think of two options: remember failed URLs for a little while (a transient keyed on the URL hash), or download the images in a cron event when the comment is approved and show the description until they're cached. The first one is probably the smaller change.

Let me know what you think!

[pfefferle]

Thanks @jeherve! I changed it a bit differently in 351f770:

1. Images without the cache

I decided to show the images from the remote host when they are not cached, the same way we already do it for avatars and emoji. The feature is opt-in, so if someone doesn't want images from other servers, they can just keep it disabled. The help text now says that uncached images are loaded from the server of the person who replied.

So image-only replies always have something to show and I didn't change the validation for them. Image-only quotes are now accepted too.

The Photon snippet was a good hint. It now sends everything except audio and video through the CDN, so comment_media is covered. While I was at it, I found out that the snippet registered the filter without accepted_args, so it threw a fatal error on every media URL since #2999. That is fixed too (with its own changelog entry).

2. Downloads during page views

I went with the transient. If a download fails, the URL is not tried again for an hour (per cache type and URL). This only applies to the lazy caching during page views, so calling cache() directly (like Avatar::save()) still downloads. It helps avatars and emoji as well.

The cron version would be nicer, but it's a bigger change, so I would do that separately if we still need it.

I also restored the Media comment methods and constants that the PR had removed, as deprecated wrappers around Comment_Media, because they shipped in 8.0.0.

[jeherve]

I think this should be good to merge now! 🚢