feat(desktop): show sent images above user messages - #99397
Conversation
|
😎 Merged successfully - details. |
Images attached in the composer reach the thread only as <file path> tags in the message text, so a sent image drew as a file chip. Lift absolute-path image tags out of the text as attachments, show them above the bubble at their own aspect ratio, and drop the attachment summary text once the image itself is drawn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G62zuZBdg8iZLQoGmQ7Vkn
90d06c3 to
d82f70b
Compare
adamleithp
added
stamphog
|
React Doctor found 5 issues in 3 files · 5 warnings. 5 warnings
Reviewed by React Doctor for commit |
![parameterai[bot]](https://avatars.githubusercontent.com/in/2665233?s=60&v=4){kind=small}
🤖 CI report✅ Trunk lane — non-backend lane (
|
| File | Comment lines | Added lines |
|---|---|---|
products/desktop/packages/core/src/sessions/promptContent.ts |
3 | 62 |
products/desktop/packages/core/src/editor/cloud-prompt.ts |
1 | 17 |
products/desktop/packages/shared/src/path.ts |
1 | 17 |
products/desktop/packages/workspace-server/src/services/os/os.ts |
1 | 25 |
This check does not block merging. It updates on every push and clears when the share drops.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
HostHog preview —
|
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved.
Contained UI/display change in desktop's own session rendering pipeline, authored by a team member, with new unit tests for the core extraction logic and no risky-territory surface (no auth, billing, migrations, public API, CI, or deps).
- Author wrote 0% of the modified lines and has 17 merged PRs in these paths (familiarity MODERATE).
- 7 of the 7 changed files are governed by
products/desktop/AGENT_APPROVALS.md.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 132L, 6F substantive, 159L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (159L, 7F, single-area, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ d82f70b · reviewed head d82f70b |
Message text is not trusted, so an image tag pointing anywhere on disk must not trigger a local file read. Lift a tag only when its path is a file the composer saved under the clipboard attachment folder, and share that folder name between the workspace server and core. Also keep a stable empty attachments default so the memoized user message rows do not recompute on every render. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G62zuZBdg8iZLQoGmQ7Vkn
A new stamphog review started for this PR — the fresh verdict replaces this approval.
|
React Doctor: the two |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
This touches security-sensitive surface (reading local files based on message content) and carries an unresolved substantive security finding from two independent automated reviewers (parameterai and greptile-apps) about a peer-agent-writable message body potentially triggering disclosure of local image files; while the diff does add an isClipboardAttachmentPath restriction, whether that fully closes the gap the reviewers describe needs security-domain judgment, not my own read of the diff.
- Author wrote 0% of the modified lines and has 24 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot].
- 10 of the 10 changed files are governed by
products/desktop/AGENT_APPROVALS.md. - Unresolved inline security comment from @parameterai on products/desktop/packages/core/src/sessions/promptContent.ts flags that attacker-controlled (peer-agent) message text can trigger automatic local image disclosure.
- greptile-apps raised the same 'Arbitrary local image reads' concern in both an inline comment and a discussion 'Security Review' comment; neither appears formally resolved on the current head.
- The new isClipboardAttachmentPath check narrows the path pattern that can be lifted, but confirming it fully closes the disclosure path described by both reviewers requires security review, not automated approval.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 171L, 9F substantive, 206L/10F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (206L, 10F, single-area, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 8b5e0b8 · reviewed head 8b5e0b8 |
A folder named like the clipboard attachment folder can sit anywhere, and a symlink inside the real one can point anywhere. The workspace server now reads a clipboard-shaped path only when it resolves inside the real clipboard temp folder. The shared path check also rejects network share paths. The attachment summary line is stripped only when every file it names is shown, so message text cannot hide an unrelated line. The attachment rules move into one core function, and the mention check, tag regex, and whitespace helper are shared instead of copied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G62zuZBdg8iZLQoGmQ7Vkn
There was a problem hiding this comment.
Approved.
This touches local file-read from untrusted message text (risky territory), but the author is on the owning team, which counts as assurance, and the diff itself adds a host-side realpath containment check plus tests that directly close the symlink/look-alike-folder read flagged by the earlier reviewer comments — those comments target a stale commit that predates this fix.
- Author wrote 0% of the modified lines and has 24 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from greptile-apps[bot].
- 14 of the 14 changed files are governed by
products/desktop/AGENT_APPROVALS.md.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 249L, 12F substantive, 349L/14F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (349L, 14F, single-area, feat) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 6294d06 · reviewed head 6294d06 |
2 participants
Problem
attachment-…/clipboard.png), not the image.<file path>tag in the message text, and the thread draws that tag as a chip.Changes
resolveMessageAttachmentslifts an image<file>tag out of the text only when the path is a file the composer saved (<tmp>/posthog-code-clipboard/attachment-*/<name>).file://ids, so they dedupe against theresource_linkblocks that task creation builds.splitUserMessagecallsresolveMessageAttachments, so the optimistic row, the stored row, and cloud rows share one path.px-0).How did you test this code?
promptContent.test.tscases: composer image tags are lifted, and other, network share, and non-image paths stay inline.os.test.tscases:readFileAsDataUrlrejects a look-alike folder and a symlink that resolves outside the clipboard folder.👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
/writing-tests,/writing-pr-descriptions,/reviewing-with-coderabbit,/thermo-nuclear-code-quality-review,/ship-it-pr.pathToFileUriids.🤖 Generated with Claude Code
https://claude.ai/code/session_01G62zuZBdg8iZLQoGmQ7Vkn