fix(tui): address PR #3720 review feedback on markdown image rendering#3721
Merged
Conversation
Reject file:// and sandbox:// URIs in LoadMarkdownReference to prevent prompt-injected markdown from reading local files. Clear failed image sources from loadingImages so a later SetMessage can retry them. Fix code-block line-shift comparison to use the pre-replacement line number. Assisted-By: Claude
Member
Author
rumpl
approved these changes
Jul 17, 2026
aheritier
added
area/security
aheritier
added a commit
that referenced
this pull request
Jul 18, 2026
aheritier
added a commit
that referenced
this pull request
Jul 18, 2026
aheritier
added a commit
that referenced
this pull request
Jul 18, 2026
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
4 participants
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.
Three code-review findings left on PR #3720 (#3720) were not addressed before merge. This commit fixes all three.
LoadMarkdownReferenceinpkg/tui/image/image.gonow rejectsfile://andsandbox://URI schemes before attempting a fetch. LLM-generated markdown could otherwise craft an image reference that causes the agent to read arbitrary local files on behalf of the user — a confused-deputy attack. Bare relative paths (used for agent-generated images) are unaffected.In
pkg/tui/components/message/message.go, failed image fetches are now removed fromloadingImagesrather than left in place. Because the loaded message carries only the refs that succeeded, leaving failures in the set caused them to be treated as already-in-flight on the nextSetMessagecall, permanently suppressing any retry. The fix lets transient failures recover on a subsequent render cycle. The same file also fixesreplaceMarkdownImagePlaceholders, which was computing code-block line shifts against the post-replacement line number instead of the original one; any code block appearing after an image placeholder ended up shifted by the wrong amount, breaking click-to-copy on those blocks.Each fix is covered by a new unit test (
TestLoadMarkdownReferenceRejectsLocalSchemes,TestFailedMarkdownImageLoadCanRetry,TestReplaceMarkdownImagePlaceholdersShiftsCodeBlocksByOriginalLine). Build, tests, and linter all pass.