fix(web): show private repository media in pull request tabs#11706

🔴 Critical assets/AssetAccess.ts:791

resolveAsset accepts any claims.url as github-media without verifying that its repository matches claims.cwd, so an authenticated client can use a cwd with access to private repositories to proxy content from a different repository. Resolve the cwd's configured remote and require the URL's owner/repository to match it, rejecting URLs that cannot be associated with that repository.

Not taking this one, and the reason is in the URL shape. A dropped screenshot is github.com/user-attachments/assets/<uuid> — an opaque id with no owner or repository in it, and no API that maps one back to its repository. Requiring the URL to match the remote at cwd would reject exactly the case this PR exists for, while passing only the raw.githubusercontent.com minority.

What bounds the reach instead:

• the two hosts in githubMediaFetchUrl, GET only, and a response restricted to image/video/audio with nosniff;
• the token never leaving GitHub — the redirect is followed here with redirect: "manual" and the credential dropped on the hop;
• the bytes going back to the caller that asked, over its own authenticated connection.

That caller already holds an authenticated session to this environment, which runs agent commands in the workspace. A GET against two GitHub hosts is strictly narrower than what it can already do, so binding the URL to the repository buys no confidentiality it does not already have.

🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | 🏗️ Heavy lift

IDOR

Reachability: External
Exploitability: Moderate
CWE: CWE-639 — Authorization Bypass Through User-Controlled Key (IDOR)

Bind GitHub media to the repository authorized by cwd.

assetsCreateUrl signs any allowed GitHub media URL with caller-supplied cwd. The asset route then fetches that URL with the GitHub token obtained for cwd. No code compares the URL repository with the repository represented by cwd. A caller with AuthOrchestrationReadScope can request private media from another repository accessible to the server credential.

Bind the URL to the repository derived from a validated cwd before signing. Reject the resource when that repository context is unavailable or does not match. The token lookup using cwd alone is not sufficient.

Not taking this one, and the reason is in the URL shape. A dropped screenshot is github.com/user-attachments/assets/<uuid> — an opaque id with no owner or repository in it, and no API that maps one back to its repository. Requiring the URL to match the remote at cwd would reject exactly the case this PR exists for, while passing only the raw.githubusercontent.com minority.

What bounds the reach instead:

• the two hosts in githubMediaFetchUrl, GET only, and a response restricted to image/video/audio with nosniff;
• the token never leaving GitHub — the redirect is followed here with redirect: "manual" and the credential dropped on the hop;
• the bytes going back to the caller that asked, over its own authenticated connection.

That caller already holds an authenticated session to this environment, which runs agent commands in the workspace. A GET against two GitHub hosts is strictly narrower than what it can already do, so binding the URL to the repository buys no confidentiality it does not already have.

@maria-rcks, the repository-match recommendation does not apply to github.com/user-attachments/assets/<uuid> URLs. Those URLs have no repository identifier. I withdraw that specific remediation.

The authorization concern remains. WS_METHODS.assetsCreateUrl requires only orchestration:read. A session with only that scope is supported by the authentication code. It cannot call dispatchCommand, terminal RPCs, or other orchestration:operate methods.

The host allowlist, GET-only requests, MIME checks, and redirect credential handling reduce request and token-exposure risk. They do not restrict which private GitHub media object the server credential can read. A caller that knows a valid private media URL can use this RPC to have the server fetch its bytes.

A repository binding is not feasible for opaque attachment URLs. The remaining design decision is whether orchestration:read intentionally grants access to all GitHub media visible to the environment’s gh credential. If it is intentional, this thread can be closed as an accepted privilege boundary. Otherwise, use a stronger caller capability or bind issuance to trusted pull-request markdown provenance rather than URL-to-repository matching.

🐇

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the authored GitHub URL for media previews.

ChatMarkdownAssetImage uses the signed URL for src but does not forward imageSource.uri as originalUrl. Expanded images therefore omit the Open original link, and failed videos omit the original target from MediaVideoPlayer.

Add originalUrl to ChatMarkdownAssetImage, pass imageSource.uri, and forward it to ChatMarkdownImage and ChatMarkdownVideo. Do not add it to actionsSource; MediaActionSource does not define or consume originalUrl.

✅ Addressed in commit 7810e5a

Fixed in 7810e5a. ChatMarkdownAssetImage now takes originalUrl and forwards it to ChatMarkdownImage and ChatMarkdownVideo; the call site passes imageSource.uri. It is not on actionsSource — it builds a mediaUrlReference there instead, which is what MediaActionSource consumes. The authored className and sanitized imageProps are forwarded now too, so id and align survive.

@maria-rcks, confirmed. The current code passes the authored URL and forwards it to both image and video rendering. actionsSource remains unchanged.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle malformed percent escapes before decoding the filename.

A URL such as https://raw.githubusercontent.com/org/repo/% passes githubMediaFetchUrl, but Line 45 throws URIError. This makes asset issuance fail instead of rejecting the media URL or using the fallback filename.

✅ Addressed in commit 7810e5a

Fixed in 7810e5a, with the encoded segment kept rather than discarded: a name GitHub serves but decodeURIComponent rejects still identifies the file, so it is only used undecoded.

@maria-rcks, thanks. Keeping the encoded segment is correct. It preserves the filename when decodeURIComponent rejects malformed percent escapes and avoids URIError.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

🟡 Medium src/githubMedia.ts:25

canonicalUrl drops url.hash, so GitHub media URLs such as #t=30 or SVG view fragments are rendered without their client-side behavior. Preserve the fragment separately and append it only to the client-facing signed URL, not to the server fetch URL.

Fixed in 47854df, on the client side rather than in the URL. The fragment never belonged in the fetch URL or the signed claim — the server has no use for it — so the image branch passes it as srcFragment, which ChatMarkdownAssetImage already appends to the signed URL for workspace media.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the canonical raw URL when signed asset resolution fails.

githubMediaFetchUrl recognizes github.com/.../blob/... URLs but converts them to raw-content URLs. Both fallbacks retain the authored blob URL, so public media becomes unavailable when signing is unsupported or fails.

• apps/web/src/components/ChatMarkdown.tsx#L3165-L3165: pass githubMediaFetchUrl(imageSource.uri) ?? imageSource.uri as fallbackSrc.
• apps/web/src/components/pullRequest/PullRequestMarkdown.tsx#L39-L39: use githubMediaFetchUrl(url) ?? url for the failure fallback.

✅ Addressed in commit 47854df

Right, and fixed in 47854df. Both fallbacks now use the canonical URL: the image branch passes githubMediaFetchUrl(...) and the video component takes it as fetchUrl, keeping the authored link only for originalUrl.

@maria-rcks, confirmed. The image path now uses the canonical URL for both the signed github-media resource and fallbackSrc. The video path also keeps the authored URL only as originalUrl.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

🟡 Medium components/ChatMarkdown.tsx:1644

When signing fails, the fallback image displays but cannot be expanded: actionsSource.asset remains attached, so ExpandedImageDialog ignores item.src and resolves the failed asset to null. Omit asset when fallbackSrc is being used after failure so expansion uses the direct fallback URL.

No longer relevant as of f83a9360244681e91293a34866b48898b8e4c4ee

src/assets.ts:300

AssetGitHubMediaUrlValidationError retains the entire resource, including an arbitrary URL/query string, in a structured error that may cross RPC/logging boundaries. Error attributes must not expose signed URLs or query strings. Store only bounded diagnostics such as the normalized protocol/hostname (and a safe resource kind), while keeping the caller-visible message static; update construction sites accordingly.

A self-contained diff is not feasible because the error schema and its construction sites must change together.

No longer relevant as of 3963b50d313440a68eeecbc7188ba3edfc97b60b

Removed the resource payload from this validation error. Its wire encoding now contains no rejected URL or query string. The existing asset-access test covers a URL with a private token and checks the encoded error. All 32 focused tests and the server typecheck pass at 3da793bfe0.