From dfd500984ed08954113627424fb29a78c10182a8 Mon Sep 17 00:00:00 2001 From: maria-rcks <254055478+maria-rcks@users.noreply.github.com> Date: Mon, 14 Sep 2026 02:39:32 -0300 Subject: [PATCH 01/11] fix(web): show private repository media in pull request tabs GitHub serves an upload embedded in a private repository's pull request only to a request that carries a credential for it, and the renderer carries no GitHub session, so every screenshot and video in those descriptions drew "Image unavailable" or "Video unavailable". Pull request markdown now loads GitHub-hosted media through a signed asset URL, and the server fetches it with the repository's `gh` credential, following GitHub's redirect to the signed object without carrying the token off GitHub. Range requests pass through, so videos still stream and seek. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/assets/AssetAccess.test.ts | 44 +++++++++++++-- apps/server/src/assets/AssetAccess.ts | 51 +++++++++++++++--- apps/server/src/assets/GitHubMediaFetch.ts | Bin 0 -> 5174 bytes apps/server/src/http.ts | 9 ++++ apps/server/src/server.test.ts | 2 + apps/server/src/server.ts | 4 +- apps/server/src/ws.ts | 2 + apps/web/src/components/ChatMarkdown.tsx | 43 +++++++++++++-- .../pullRequest/PullRequestMarkdown.tsx | 45 +++++++++++++++- packages/contracts/src/assets.ts | 19 +++++++ packages/shared/package.json | 4 ++ packages/shared/src/githubMedia.ts | 47 ++++++++++++++++ 12 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 apps/server/src/assets/GitHubMediaFetch.ts create mode 100644 packages/shared/src/githubMedia.ts diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 225910e38426..dac09df60bed 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -228,7 +228,7 @@ describe("AssetAccess", () => { suffix.slice(0, separator), suffix.slice(separator + 1), ); - if (!asset) throw new Error("Expected a resolved media file"); + if (asset?.kind !== "file") throw new Error("Expected a resolved media file"); yield* fs.rename(filePath, savedPath); yield* fs.symlink(secretPath, filePath); @@ -391,7 +391,7 @@ describe("AssetAccess", () => { const name = suffix.slice(separator + 1); yield* fs.writeFileString(filePath, "in-place edit"); const edited = yield* resolveAsset(token, name); - if (!edited) throw new Error("Expected the edited media file"); + if (edited?.kind !== "file") throw new Error("Expected the edited media file"); const editedResponse = HttpServerResponse.toWeb(yield* assetFileResponse(edited)); expect(yield* Effect.promise(() => editedResponse.text())).toBe("in-place edit"); @@ -407,7 +407,7 @@ describe("AssetAccess", () => { renewedSuffix.slice(0, renewedSeparator), renewedSuffix.slice(renewedSeparator + 1), ); - if (!renewedAsset) throw new Error("Expected the replacement media file"); + if (renewedAsset?.kind !== "file") throw new Error("Expected the replacement media file"); const renewedResponse = HttpServerResponse.toWeb(yield* assetFileResponse(renewedAsset)); expect(yield* Effect.promise(() => renewedResponse.text())).toBe("replacement"); yield* fs.remove(filePath); @@ -1062,4 +1062,42 @@ describe("AssetAccess", () => { expect(error.cause).toBe(resolutionCause); }).pipe(Effect.provide(testLayer)), ); + + it.effect("serves GitHub-hosted pull request media through the repository's credential", () => + Effect.gen(function* () { + const resolve = (relativeUrl: string) => { + const suffix = relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separator = suffix.indexOf("/"); + return resolveAsset(suffix.slice(0, separator), suffix.slice(separator + 1)); + }; + const issue = (url: string) => + issueAssetUrl({ resource: { _tag: "github-media", cwd: "/repo", url } }); + + const attachment = yield* issue( + "https://github.com/user-attachments/assets/1a1842fb-6383-492f-873c-57aa0033fa6c", + ); + expect(attachment.relativeUrl.endsWith("/1a1842fb-6383-492f-873c-57aa0033fa6c")).toBe(true); + expect(yield* resolve(attachment.relativeUrl)).toEqual({ + kind: "github-media", + url: "https://github.com/user-attachments/assets/1a1842fb-6383-492f-873c-57aa0033fa6c", + cwd: "/repo", + }); + + // A `blob` link addresses the page; only the raw host answers a credential with bytes. + const committed = yield* issue("https://github.com/owner/repo/blob/main/docs/shot.png"); + expect(yield* resolve(committed.relativeUrl)).toMatchObject({ + url: "https://raw.githubusercontent.com/owner/repo/main/docs/shot.png", + }); + + for (const url of [ + "https://example.com/shot.png", + "http://github.com/user-attachments/assets/1a1842fb", + "https://github.com/owner/repo/pull/1", + ]) { + expect((yield* issue(url).pipe(Effect.flip))._tag).toBe( + "AssetGitHubMediaUrlValidationError", + ); + } + }).pipe(Effect.provide(testLayer)), + ); }); diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index 700aeacb19a5..19d522050eff 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -1,6 +1,7 @@ import type { AssetResource } from "@t3tools/contracts"; import { AssetAttachmentNotFoundError, + AssetGitHubMediaUrlValidationError, AssetPreviewTypeValidationError, AssetProjectFaviconInspectionError, AssetProjectFaviconNotFoundError, @@ -27,6 +28,7 @@ import { readImageDimensions, type ImageDimensions, } from "@t3tools/shared/imageDimensions"; +import { githubMediaFetchUrl, githubMediaFileName } from "@t3tools/shared/githubMedia"; import { PROJECT_FAVICON_FALLBACK_MARKER } from "@t3tools/shared/projectFavicon"; import * as Clock from "effect/Clock"; import * as Crypto from "effect/Crypto"; @@ -135,6 +137,14 @@ const AssetClaimsSchema = Schema.Union([ app: ToolActivityNativeAppReference, expiresAt: Schema.Number, }), + Schema.Struct({ + version: Schema.Literal(1), + kind: Schema.Literal("github-media"), + /** Already narrowed to a GitHub media host at mint time; the signature is what keeps it there. */ + url: Schema.String, + cwd: Schema.String, + expiresAt: Schema.Number, + }), ]); type AssetClaims = typeof AssetClaimsSchema.Type; @@ -142,14 +152,20 @@ const AssetClaimsJson = Schema.fromJsonString(AssetClaimsSchema); const decodeAssetClaims = Schema.decodeUnknownOption(AssetClaimsJson); const encodeAssetClaims = Schema.encodeSync(AssetClaimsJson); -export type ResolvedAsset = { - readonly kind: "file"; - readonly path: string; - readonly download?: boolean; - readonly fileName?: string; - readonly mimeType?: string; - readonly file?: OpenMediaFile; -}; +export type ResolvedAsset = + | { + readonly kind: "file"; + readonly path: string; + readonly download?: boolean; + readonly fileName?: string; + readonly mimeType?: string; + readonly file?: OpenMediaFile; + } + | { + readonly kind: "github-media"; + readonly url: string; + readonly cwd: string; + }; function decodeClaims(encodedPayload: string): AssetClaims | null { try { @@ -657,6 +673,21 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i fileName = "native-app-icon.png"; break; } + case "github-media": { + const fetchUrl = githubMediaFetchUrl(input.resource.url); + if (fetchUrl === null) { + return yield* new AssetGitHubMediaUrlValidationError({ resource: input.resource }); + } + claims = { + version: 1, + kind: "github-media", + url: fetchUrl, + cwd: input.resource.cwd, + expiresAt, + }; + fileName = githubMediaFileName(fetchUrl); + break; + } } const secretStore = yield* ServerSecretStore.ServerSecretStore; @@ -757,6 +788,10 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( : null; } + if (claims.kind === "github-media") { + return { kind: "github-media", url: claims.url, cwd: claims.cwd } satisfies ResolvedAsset; + } + if (claims.kind === "native-app-icon") { const nativeAppIconResolver = yield* NativeAppIconResolver.NativeAppIconResolver; const iconPath = yield* nativeAppIconResolver.resolve(claims.app); diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts new file mode 100644 index 0000000000000000000000000000000000000000..523033b7b14004f9f6bba9edb9716a1f4efc1eb0 GIT binary patch literal 5174 zcma)AVRIX|5$$LFiZ#cRlB#r4;-nv_$gM5YRwj|0QuMSl9*+dwfjmUV1AD-UW@Y~O zzQqFMQMQw4#^xQcSS)tmzFnM|FKpxKfo8^+?d^pwjCySE>QHW9=5UcA7=~YN0!;w^Ql-@g!~Syt=LQQHh7v=PX>)02q1@_weB%qoT*~*o;)Vp80ZjEbhr-D$n)8%g#=uv{y5Es+9AG zb6c3H(FKOOmaz}_;Pw|Z=k!a?poMP8%oW_DY6Kl#;}P+sgL@B1tGpyTrDbXGu(WLj zIbZ5#X`CjvP|KQbX5~%B>L;ZJitcnx#$g3;gDFxiAQpyiqJl+ZY6M}X5ItKXsu}?M z=qXoKbr4K3o>i0@y8(0@U)}Hy03!!HqEw*FAg#>3c5Fj}WZ3Tv#%lzw)=Rojiyr|9 zQ_l{-MF2)&>uN>H52&2HWuZ}Vn|}?Nq4*ivbPiKL0Bc0-02lgdGquk z=*ysC{g?E@%hornNn~rggzU<|wfjP*S>84c@HjDZePOD~xUF=~9xSk6>(6YaeIGl* z5kzK6{V4O~^px7V&{M+%^3690+>#CvQ`V3H3qR85bTGiVecRMYENrvgUL3czS3C;} z%r;Lfd~`&YU20 zsaCQBAX!-LgmG>gyL3pEK#tgrNhN~P)WBhMb~w(~d^rwgi8B3B=dIWMhwzF89QIB8 zm=*X>H8VE^@o)|??*xVXaCpdptx-eqCd+u>Lt3cod*F)RoL|2hy?pm(NWC7dWpX_b z3l?Uf`*EL$-nm-z`?mE!<$z9~lRRe57kJ8$J@fuxkhX|xY%{8yzG}fuy6E@Ax?V3z zZ9ozX&xGIT6i_+dguVt9CoBcAkhYmK|I_sRbO$t@;vo1(Y32Nr6@{)eJlYunOkIDF zF}dL9URAAr5=>g-+>#HdXHx%9xA{I-cTr6c?kGg*Wb8AA1m>{I1F>KTAV)>(uqfg> zjh^iBaN^0c3YA`)Kg|j zNseKd?No9Gk!|2skTaKks3lauu$e*@YS|&|?`@+YAjHa;FHz@e%i3>ShZJ{?P$594 z$gJR?F8fZI_T-R|7{|joWWw*!aOf&5W(_PT7!y_a-(uy@E@+I8{E*-2ly_)Q@|_O9tfXLZ9{$5TG|D49iPxic>LQbeVei) zVO(0#Vdprh2hFyory(edCoDIv*zgs+$$#Bdq4^DELC!qLy43O|S)|dqL~X-=tTfBA z7|g^mJPLN70Oq$V58%QrTpZEYp~&DtnOO;6OJ8`vjiDq+@ODkQ&e0J8q?KC1pTfbscm4td=Ni>n@Zv86sR3aAa>OTq{7%YbDVU2)+4q|diXxAH-B-%cANip#ct}_V@ zO}sHACuC`k*->ikMue+c>rhjl9Us%DPaqHpF}ONRfH=RGnI}mLi!oF%wbQUCtZ)w! zYH`vf+cH%mgA$s#mqN$YPg=o$VQP?hdlmXCZjpG&u1CaQJhR@R!f;t~u`lH!jXX$~ zlUrM?XkpPTBJiGDMParS??7;kURYxv@H#1@)9Uos>@?Jf*D zfqK>&S2DV=wJP`qw&@0x_vm2eK!sep6#V+VaGm}bZnlzRQrkn*H;%DD9{nEOePP9C zR2Mp2hS#P&?2uolW9>AJQs$!zW0rk|8|l-wV*^hL5Rh&!(c#C4a8nVpj&HRE(lB1+ zv)s<%aokC4iwp6tn|YZ8!*L}+*W|-i%^&ne6Rzg{2kC~w;klbV0B*VY?TO2HXLK_G zX5xHM;Zik)Mho@jB7jgltQmZPG-VMtPkgoMUf6c7d?_(RJaiAxv+LRStmT6Mo||Ej z+UbP{ys4Nc(ohn_BpDJBq^!eiWZ$pU!@l?hKseZHj%`Tr3EPph?eal7QA~6u@!gLV znVv8F3IPxr;jU(vSGaXU0-#j!|3mr#9}XlE+m`AN3Nptc5JV*Q&n|N@RcE;Q`V-`2 zykMa8u4vu@Sln%+A>~USfW?sFJ2YQ34iPK<7Q$2>`+2_c9|jagPT$Ugd zW9kZh%?VBMaf+jQ+I#xY$Nx_85c>dydc_e(&E!((JcUq=W2DQxY?K1SBp z<196_3lpxX=xc*BzY=|^%5g_|LCNkX6E=WV)-l1Sj$!K5-ai%$6;#S3 IV~PL$A4`s;>Hq)$ literal 0 HcmV?d00001 diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 4d2dac735425..d5f070fda20a 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -30,6 +30,7 @@ import { OtlpTracer } from "effect/unstable/observability"; import * as ServerConfig from "./config.ts"; import { ASSET_ROUTE_PREFIX, resolveAsset } from "./assets/AssetAccess.ts"; +import { githubMediaResponse } from "./assets/GitHubMediaFetch.ts"; import { statMediaFile, streamMediaFile, type OpenMediaFile } from "./assets/MediaFile.ts"; import { ATTACHMENT_UPLOAD_ROUTE_PREFIX, @@ -389,6 +390,14 @@ export const assetRouteLayer = HttpRouter.add( if (!asset) { return HttpServerResponse.text("Not Found", { status: 404 }); } + if (asset.kind === "github-media") { + return yield* githubMediaResponse(asset, request.headers).pipe( + Effect.tapError((cause) => + Effect.logWarning("Failed to fetch GitHub media.", { url: asset.url, cause }), + ), + Effect.orElseSucceed(() => HttpServerResponse.empty({ status: 502 })), + ); + } return yield* assetFileResponse( asset, request.method === "GET" ? request.headers.range : undefined, diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 3eb40a1b9906..951b83ef6f1f 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -166,6 +166,7 @@ import * as VcsDriver from "./vcs/VcsDriver.ts"; import * as VcsStatusBroadcaster from "./vcs/VcsStatusBroadcaster.ts"; import * as VcsDriverRegistry from "./vcs/VcsDriverRegistry.ts"; import * as VcsProvisioningService from "./vcs/VcsProvisioningService.ts"; +import * as GitHubCli from "./sourceControl/GitHubCli.ts"; import * as VcsProcess from "./vcs/VcsProcess.ts"; import * as GitWorkflowService from "./git/GitWorkflowService.ts"; import * as ReviewService from "./review/ReviewService.ts"; @@ -1196,6 +1197,7 @@ const buildAppUnderTest = (options?: { Layer.provideMerge(ServerSecretStore.layer), Layer.provide(workspaceAndProjectServicesLayer), Layer.provideMerge(FetchHttpClient.layer), + Layer.provide(GitHubCli.layer), Layer.provide(VcsProcess.layer), Layer.provide(layerConfig), ); diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 189d62dd8362..25fdf17e78a9 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -477,8 +477,10 @@ const RuntimeCoreDependenciesLive = ReactorLayerLive.pipe( // Core Services Layer.provideMerge(ServerSettingsLayerLive), Layer.provideMerge(CheckpointingLayerLive), + // `GitHubCli` is the registry's own instance, exposed because the asset route fetches + // GitHub-hosted pull request media with the repository's credential. Layer.provideMerge( - Layer.mergeAll(SourceControlProviderRegistryLayerLive, PullRequestServiceLive), + Layer.mergeAll(SourceControlProviderRegistryLayerLive, PullRequestServiceLive, GitHubCli.layer), ), Layer.provideMerge(GitLayerLive), Layer.provideMerge(VcsLayerLive), diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 6ddd8c821c9f..37b8c3b8dcef 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -2797,6 +2797,8 @@ const makeWsRpcLayer = ( if ( input.resource._tag === "attachment" || input.resource._tag === "native-app-icon" || + // GitHub media names the repository it authenticates through itself. + input.resource._tag === "github-media" || (input.resource._tag === "media-file" && path.isAbsolute(input.resource.path)) ) { return yield* issueAssetUrl({ resource: input.resource }); diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index b0209754b0d0..8f4e3df7b845 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -34,6 +34,7 @@ import type { ThreadPullRequestKey, } from "@t3tools/contracts"; import { faviconUrlForOrigin } from "@t3tools/shared/favicon"; +import { githubMediaFetchUrl } from "@t3tools/shared/githubMedia"; import { isAtomCommandInterrupted, squashAtomCommandFailure, @@ -220,6 +221,9 @@ interface ChatMarkdownProps { extraRemarkPlugins?: NonNullable; /** Renders a `t3-context://` link as a chip; without it the link shows its label as text. */ renderContextReference?: ((reference: ChatMarkdownContextReference) => ReactNode) | undefined; + /** Loads GitHub-hosted media through `cwd`'s GitHub credential, which a private repository's + uploads need; without it those images and videos load unauthenticated and 404. */ + githubMedia?: boolean | undefined; /** Levels added to each markdown heading in the accessibility tree so the text nests under the heading that introduces it, such as a chat message's author. Rendered tags and their styling are unchanged. */ @@ -1573,7 +1577,7 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props readonly environmentId: EnvironmentId; readonly resource: Extract< AssetResource, - { readonly _tag: "attachment" | "workspace-file" | "media-file" } + { readonly _tag: "attachment" | "workspace-file" | "media-file" | "github-media" } >; readonly kind?: "image" | "video"; readonly alt: string; @@ -1617,7 +1621,7 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props src, asset: { environmentId: props.environmentId, resource }, ...(reference ? { reference } : {}), - ...(relativePath && resource._tag !== "attachment" + ...(relativePath && (resource._tag === "media-file" || resource._tag === "workspace-file") ? { onOpenFile: () => useRightPanelStore @@ -2226,6 +2230,7 @@ function useChatMarkdownState({ onImageExpand, renderContextReference, headingLevelOffset = 0, + githubMedia = false, }: ChatMarkdownProps) { const { resolvedTheme } = useTheme(); const [localMediaPreview, setLocalMediaPreview] = useState(null); @@ -2623,6 +2628,7 @@ function useChatMarkdownState({ environmentId, expandMedia, fileLinkChip, + githubMedia, renderContextReference, headingLevelOffset, imageBaseDir, @@ -2652,6 +2658,7 @@ function useChatMarkdownState({ environmentId, expandMedia, fileLinkChip, + githubMedia, renderContextReference, headingLevelOffset, imageBaseDir, @@ -3081,9 +3088,15 @@ const CHAT_MARKDOWN_COMPONENTS = { ); }, img: function MarkdownImage({ node, title, src, alt, ...props }) { - const { expandMedia, cwd, imageBaseDir, threadRef, renderContextReference } = use( - ChatMarkdownRendererContext, - ); + const { + expandMedia, + cwd, + environmentId, + githubMedia, + imageBaseDir, + threadRef, + renderContextReference, + } = use(ChatMarkdownRendererContext); const imageExpand = use(MarkdownLinkContext) ? undefined : expandMedia; const contextReference = typeof src === "string" ? parseComposerContextHref(src) : null; if (contextReference) { @@ -3109,6 +3122,26 @@ const CHAT_MARKDOWN_COMPONENTS = { const authoredSizeStyle = authoredImageSizeStyle(width, height); const imageSource = classifyMarkdownImageSource(classifiedSrc, imageBaseDir ?? cwd); const kind = mediaKindFromPath(classifiedSrc) ?? "image"; + if ( + githubMedia && + cwd !== undefined && + environmentId !== null && + imageSource._tag === "Direct" && + githubMediaFetchUrl(imageSource.uri) !== null + ) { + return ( + + ); + } if (imageSource._tag === "Direct") { const mediaSrc = resolveProtocolRelativeMediaUrl(imageSource.uri); const originalUrl = diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 9a4b63044ad2..97189e30f2f5 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -1,8 +1,10 @@ import { ExternalLinkIcon, PaperclipIcon } from "lucide-react"; -import type { EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; +import { githubMediaFetchUrl } from "@t3tools/shared/githubMedia"; +import type { AssetResource, EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; import { createContext, useContext, useMemo } from "react"; import type { Options as ReactMarkdownOptions } from "react-markdown"; +import { useAssetUrlRefresh, useAssetUrlState } from "~/assets/assetUrls"; import { cn } from "~/lib/utils"; import { PULL_REQUESTS_PANEL_REF } from "~/rightPanelStore"; @@ -15,6 +17,36 @@ export const PullRequestMarkdownContext = createContext<{ threadRef: ScopedThreadRef | null; } | null>(null); +/** + * A video GitHub hosts for the repository. It plays through a signed asset URL the server + * fetches with the repository's GitHub credential, which is what a private repository's + * uploads need; the URL is re-signed on retry, so a stale one recovers without a reload. + */ +function PullRequestGitHubVideo({ + environmentId, + cwd, + url, +}: { + environmentId: EnvironmentId; + cwd: string; + url: string; +}) { + const resource = useMemo(() => ({ _tag: "github-media", cwd, url }), [cwd, url]); + const assetUrl = useAssetUrlState(environmentId, resource); + const refreshAssetUrl = useAssetUrlRefresh(environmentId, resource); + return ( + + ); +} + /** Renders PR uploads inline, with retry and an original link when video playback fails. */ export function PullRequestMarkdown({ text, @@ -57,6 +89,17 @@ export function PullRequestMarkdown({ pullRequestPanelRef={resolvedThreadRef ?? PULL_REQUESTS_PANEL_REF} environmentId={environmentId} extraRemarkPlugins={extraRemarkPlugins} + githubMedia + /> + ); + } + if (segment.media === "video" && githubMediaFetchUrl(segment.url) !== null) { + return ( + ); } diff --git a/packages/contracts/src/assets.ts b/packages/contracts/src/assets.ts index 6447aab22585..63fb095b787e 100644 --- a/packages/contracts/src/assets.ts +++ b/packages/contracts/src/assets.ts @@ -50,6 +50,13 @@ export const AssetResource = Schema.Union([ Schema.TaggedStruct("native-app-icon", { app: ToolActivityNativeAppReference, }), + // An upload a pull request body points at on GitHub. A private repository serves these only + // to a request that carries a credential, which the client has none of, so the server fetches + // them with the `gh` credential the repository at `cwd` authenticates with. + Schema.TaggedStruct("github-media", { + cwd: TrimmedNonEmptyString.check(Schema.isMaxLength(ASSET_PATH_MAX_LENGTH)), + url: TrimmedNonEmptyString.check(Schema.isMaxLength(2048)), + }), ]); export type AssetResource = typeof AssetResource.Type; @@ -285,6 +292,17 @@ export class AssetSigningKeyLoadError extends Schema.TaggedError()( + "AssetGitHubMediaUrlValidationError", + { + resource: AssetResource, + }, +) { + override get message(): string { + return "Only media hosted by GitHub can be fetched with a GitHub credential."; + } +} + export const AssetAccessError = Schema.Union([ AssetWorkspaceContextNotFoundError, AssetWorkspaceContextResolutionError, @@ -298,6 +316,7 @@ export const AssetAccessError = Schema.Union([ AssetProjectFaviconResolutionError, AssetProjectFaviconInspectionError, AssetProjectFaviconNotFoundError, + AssetGitHubMediaUrlValidationError, AssetSigningKeyLoadError, ]); export type AssetAccessError = typeof AssetAccessError.Type; diff --git a/packages/shared/package.json b/packages/shared/package.json index d30c26c2de79..ccf1a2f1fa3e 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -39,6 +39,10 @@ "types": "./src/git.ts", "import": "./src/git.ts" }, + "./githubMedia": { + "types": "./src/githubMedia.ts", + "import": "./src/githubMedia.ts" + }, "./sourceControl": { "types": "./src/sourceControl.ts", "import": "./src/sourceControl.ts" diff --git a/packages/shared/src/githubMedia.ts b/packages/shared/src/githubMedia.ts new file mode 100644 index 000000000000..a9331c5cd52e --- /dev/null +++ b/packages/shared/src/githubMedia.ts @@ -0,0 +1,47 @@ +/** + * Media a pull request body points at on GitHub's own hosts. In a private repository GitHub + * answers an unauthenticated request for one with 404 — a screenshot dropped into a description + * becomes `github.com/user-attachments/assets/`, and a file committed alongside the code + * becomes a `raw.githubusercontent.com` path — so the renderer, which carries no GitHub session, + * draws a broken image where the reviewer expects the evidence. The server can fetch these with + * the repository's `gh` credential and hand the bytes back. + * + * Only hosts where a credential is what decides the answer belong here. `objects.githubusercontent.com` + * and the `private-user-images` links GitHub's own HTML carries are already signed and load on their + * own, and a token does nothing for them once that signature expires. + */ + +const RAW_HOST = "raw.githubusercontent.com"; +const ATTACHMENT_PATH_PATTERN = /^\/user-attachments\/assets\/[\w-]+$/u; +/** `blob` and `raw` both address file bytes; `raw` is the one the token is honoured on. */ +const REPOSITORY_FILE_PATTERN = /^\/([^/]+)\/([^/]+)\/(?:raw|blob)\/(.+)$/u; + +/** + * The URL to fetch with a GitHub credential for `source`, or null when the source is not + * GitHub-hosted media — those keep loading directly, exactly as they do today. + */ +export function githubMediaFetchUrl(source: string): string | null { + let url: URL; + try { + url = new URL(source); + } catch { + return null; + } + if (url.protocol !== "https:") return null; + const host = url.hostname.toLowerCase(); + if (host === RAW_HOST) return url.toString(); + if (host !== "github.com" && host !== "www.github.com") return null; + if (ATTACHMENT_PATH_PATTERN.test(url.pathname)) return `https://github.com${url.pathname}`; + const repositoryFile = REPOSITORY_FILE_PATTERN.exec(url.pathname); + // `?raw=true` is how the web UI spells "the bytes, not the page"; the raw host needs no query. + return repositoryFile + ? `https://${RAW_HOST}/${repositoryFile[1]}/${repositoryFile[2]}/${repositoryFile[3]}` + : null; +} + +/** Last path segment, for the signed URL's display name and the download filename. */ +export function githubMediaFileName(fetchUrl: string): string { + const segment = new URL(fetchUrl).pathname.split("/").pop() ?? ""; + const name = decodeURIComponent(segment).replace(/[\\/\0]/gu, ""); + return name.length > 0 ? name : "github-media"; +} From 7810e5a4ea1c090c9e721985b434c1fb39681880 Mon Sep 17 00:00:00 2001 From: maria-rcks <254055478+maria-rcks@users.noreply.github.com> Date: Mon, 14 Sep 2026 03:23:22 -0300 Subject: [PATCH 02/11] fix(server): harden the GitHub media proxy and fall back to the direct URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the private repository media fix: the cache key held a literal NUL byte, which made the new module a binary blob to git; a name no `decodeURIComponent` accepts killed asset issuance; and the proxied bytes carried GitHub's content type without the `nosniff` and SVG policy every other asset gets. The proxy now serves only pictures and recordings, naming them from the file when the raw host says `application/octet-stream`, forwards an upstream refusal instead of turning it into 502, and follows GitHub's redirect itself so the credential provably stops at GitHub. A signed URL that cannot be minted — an older server, or one with no route to GitHub — now falls back to the direct link instead of a dead placeholder, so public media keeps working exactly as it did. Authored attributes, the original link, and the legacy attachment and Git LFS hosts are handled too. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/assets/AssetAccess.test.ts | 14 +++++++ apps/server/src/assets/GitHubMediaFetch.ts | Bin 5174 -> 6688 bytes apps/web/src/components/ChatMarkdown.tsx | 37 +++++++++++++++--- .../pullRequest/PullRequestMarkdown.tsx | 6 ++- packages/shared/src/githubMedia.ts | 35 ++++++++++++++--- 5 files changed, 78 insertions(+), 14 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index dac09df60bed..59d2cdfd2d7b 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -1089,10 +1089,24 @@ describe("AssetAccess", () => { url: "https://raw.githubusercontent.com/owner/repo/main/docs/shot.png", }); + // The pre-`user-attachments` form, Git LFS bytes, and a name no `decodeURIComponent` + // accepts all arrive from real bodies. + const legacy = yield* issue("https://github.com/owner/repo/assets/45952064/1a1842fb"); + expect(yield* resolve(legacy.relativeUrl)).toMatchObject({ + url: "https://github.com/owner/repo/assets/45952064/1a1842fb", + }); + const lfs = yield* issue("https://media.githubusercontent.com/media/owner/repo/main/a.mp4"); + expect(yield* resolve(lfs.relativeUrl)).toMatchObject({ + url: "https://media.githubusercontent.com/media/owner/repo/main/a.mp4", + }); + const awkward = yield* issue("https://raw.githubusercontent.com/o/r/main/100%.png"); + expect(awkward.relativeUrl.endsWith("/100%25.png")).toBe(true); + for (const url of [ "https://example.com/shot.png", "http://github.com/user-attachments/assets/1a1842fb", "https://github.com/owner/repo/pull/1", + "https://github.com/owner/repo/blob/main/", ]) { expect((yield* issue(url).pipe(Effect.flip))._tag).toBe( "AssetGitHubMediaUrlValidationError", diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index 523033b7b14004f9f6bba9edb9716a1f4efc1eb0..f36efeb80b6829ab255170e0d0f17e64a5aad115 100644 GIT binary patch delta 1824 zcmZ`(L2nyH6jrFCTXrHTqd23WKgobvRm-`-tC<98n!6W2qI!sLJa z)ted?7Z+hwhN0|p(E-y%P#ADUIM79fc~FcF6}@2sxPXJUMLEa=*U;}W1BZyQ&H{}* zL;+I3Lv9`=Ee{rp)4mj%NY*x<*4Fp88=tIgH|ot2Y~$w%YO^M8a|0DPK2KT#Ct@OD zp#vwU|4zLc5v)O+gdttZq5z{71{z}Sn?#`oCIWirOBGNJ+Jl-Hn651cLyeZGkOnlW z9tNXU$wYxvyu-yLNn%&quWxQN8yn62=FV1Qf2-CUHXGX;M6viHS1s{~b?{}62Pj{% zB;eA0Ufg}&zw^QThcCRP!rsz+k(b9cKY8}>0vKU9S$t9VI!i}UNTL*r#1-hKCCMO= zeiD%~CD4pijb+IF;UMcR(;5vZnHYo)@7bcEFV&%rnU5AB4_Z?1anziEDkZU)&Crs; z0DLBF6I3EV`-;Z~Sch?|4RuPNHRFG%fBK|Rf4aTa+}YpSTwkm2*fIiavm`VHt$bJz zlyspC+6+P*og_lDHWs)D`REnt(~?Kd=@-}jnVRiNZBAhRn2R_up5G5nzkIX)*4#8? z9}kbvry_Iooc?}&CH>?2&GhEXt<;@)FRjk(rr*pgrN7KP7y_KQ`Ed}y`Cs=8C+0Ec z8reX2v65D^C*%O`WA4%%c;{y1CV$zgu3OF!M?Mav&kQGrS5DoxKbnGrPWJ$O8W<9d zvoKE+!TqiklV)Zb7Eix^=dU-?zh}R9jtYce=vu)HVl2qm!f1!=E~FN`-FDJHZ+yEq zGDH$UN@8I0ghio7)9zc-=>p&wQQfmzi79n@pH886g~yLs}{u$*znE!b|1s z>NTn+{bBC*5I(NJ{pDq-((ekCD3y+h5qjD%ljyV7c98|B#*ZgiGfoJCcy8JhuNgrI zk@axWTKQ~91@>r_*bOnvRs<0y69>RTGEpfrx?=qizV!)&tVJ^nHjF$hqlglwK&Nr*brm~)|81&4;(!dvt)${76d#qF{>UN3gL zc!pMCZj#K%#nlT02^UDGoZaVM2aUzer9a`{Wr U=u3MjHFKZcn#g7iYdPERe;xQ*p8x;= delta 324 zcmZ2rvQ1;6{^^PTB{r)v3Nx}M=jRodOy0;Ly!j1N8RKScws(wz$>k}v3@X)`c?G2< zdKvk}C7ZQ4oY}baQY%uEOG{EUxF$F78BK2E6PkRCPki!yKC#K)`64F=@oP@5<+l^8 z)zslq00Ssvv$B96qi#`ZL4I*&Nq$kKda*)6dPagma#3nZYF&O9nRz*xd8yV4X_+~xd5O8Hwn{41lM^K5m}`|1CfkY5o4i~y siA7IOPh;{radAPQ>D3BQlR_#BQmrPh6_*fEs8z61sD}6nWF-g#01pjjI{*Lx diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 8f4e3df7b845..58184d682d91 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1588,6 +1588,16 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props /** Caps the box height in rem while keeping the image's ratio; 30 by default. */ readonly maxHeightRem?: number | undefined; readonly style?: CSSProperties | undefined; + readonly className?: string | undefined; + /** Sanitized authored attributes (`id`, `align`, …) that fragment links and layout rely on. */ + readonly imageProps?: + | Omit, "src" | "alt" | "className" | "style"> + | undefined; + /** Where the media also lives on the web, for the failure state's escape hatch. */ + readonly originalUrl?: string | undefined; + /** Loaded instead of the failure state when no URL can be signed, such as against a server + too old to know this resource. Only safe when the client can reach it directly. */ + readonly fallbackSrc?: string | undefined; readonly workspaceRoot?: string | undefined; readonly onImageExpand?: ((preview: ExpandedImagePreview) => void) | undefined; }) { @@ -1600,9 +1610,15 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props : resource._tag === "workspace-file" && props.workspaceRoot ? `${props.workspaceRoot.replace(/[\\/]+$/, "")}/${resource.path}` : undefined; - const reference = path ? mediaFileReference(path, props.workspaceRoot) : undefined; - const relativePath = reference?.relativePath; - const src = assetUrl._tag === "Success" ? assetUrl.url + (props.srcFragment ?? "") : null; + const reference = path + ? mediaFileReference(path, props.workspaceRoot) + : props.originalUrl + ? mediaUrlReference(props.originalUrl) + : undefined; + const relativePath = reference?.kind === "file" ? reference.relativePath : undefined; + const fallbackSrc = assetUrl._tag === "Failure" ? props.fallbackSrc : undefined; + const src = + assetUrl._tag === "Success" ? assetUrl.url + (props.srcFragment ?? "") : (fallbackSrc ?? null); // The server reads the pixel size from the file header, so the slot can be // the image's final box instead of a 16:9 guess. An authored size wins; a // caller's height cap shrinks the box while keeping the ratio. @@ -1638,9 +1654,10 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props return ( ); @@ -3137,7 +3156,13 @@ const CHAT_MARKDOWN_COMPONENTS = { kind={kind} copyMarkdown={copyMarkdown} standalone={standalone} + className={className} style={authoredSizeStyle} + imageProps={imageProps} + originalUrl={imageSource.uri} + // A server too old to sign this resource, or one with no route to GitHub, still leaves + // the public half of these working exactly as it did before. + fallbackSrc={imageSource.uri} onImageExpand={imageExpand} /> ); diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 97189e30f2f5..1354b76d2578 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -34,10 +34,12 @@ function PullRequestGitHubVideo({ const resource = useMemo(() => ({ _tag: "github-media", cwd, url }), [cwd, url]); const assetUrl = useAssetUrlState(environmentId, resource); const refreshAssetUrl = useAssetUrlRefresh(environmentId, resource); + // A server too old to sign this resource, or one with no route to GitHub, still leaves a + // public repository's video playing exactly as it did before. + const src = assetUrl._tag === "Success" ? assetUrl.url : assetUrl._tag === "Failure" ? url : null; return ( `, and a file committed alongside the code * becomes a `raw.githubusercontent.com` path — so the renderer, which carries no GitHub session, * draws a broken image where the reviewer expects the evidence. The server can fetch these with - * the repository's `gh` credential and hand the bytes back. + * the `gh` credential and hand the bytes back. * * Only hosts where a credential is what decides the answer belong here. `objects.githubusercontent.com` * and the `private-user-images` links GitHub's own HTML carries are already signed and load on their @@ -12,9 +12,18 @@ */ const RAW_HOST = "raw.githubusercontent.com"; +/** Git LFS pointers resolve here, which is where an LFS-tracked screenshot's bytes live. */ +const LFS_HOST = "media.githubusercontent.com"; const ATTACHMENT_PATH_PATTERN = /^\/user-attachments\/assets\/[\w-]+$/u; +/** What GitHub wrote into a body before `user-attachments`; older descriptions still carry it. */ +const LEGACY_ATTACHMENT_PATH_PATTERN = /^\/[^/]+\/[^/]+\/assets\/\d+\/[\w-]+$/u; /** `blob` and `raw` both address file bytes; `raw` is the one the token is honoured on. */ -const REPOSITORY_FILE_PATTERN = /^\/([^/]+)\/([^/]+)\/(?:raw|blob)\/(.+)$/u; +const REPOSITORY_FILE_PATTERN = /^\/([^/]+)\/([^/]+)\/(?:raw|blob)\/(.*[^/])$/u; + +/** Port, userinfo, and fragment say nothing about which bytes GitHub will serve. */ +function canonicalUrl(host: string, url: URL): string { + return `https://${host}${url.pathname}${url.search}`; +} /** * The URL to fetch with a GitHub credential for `source`, or null when the source is not @@ -29,9 +38,14 @@ export function githubMediaFetchUrl(source: string): string | null { } if (url.protocol !== "https:") return null; const host = url.hostname.toLowerCase(); - if (host === RAW_HOST) return url.toString(); + if (host === RAW_HOST || host === LFS_HOST) return canonicalUrl(host, url); if (host !== "github.com" && host !== "www.github.com") return null; - if (ATTACHMENT_PATH_PATTERN.test(url.pathname)) return `https://github.com${url.pathname}`; + if ( + ATTACHMENT_PATH_PATTERN.test(url.pathname) || + LEGACY_ATTACHMENT_PATH_PATTERN.test(url.pathname) + ) { + return `https://github.com${url.pathname}`; + } const repositoryFile = REPOSITORY_FILE_PATTERN.exec(url.pathname); // `?raw=true` is how the web UI spells "the bytes, not the page"; the raw host needs no query. return repositoryFile @@ -39,9 +53,18 @@ export function githubMediaFetchUrl(source: string): string | null { : null; } -/** Last path segment, for the signed URL's display name and the download filename. */ +/** + * Last path segment, for the signed URL's display name. A percent sequence GitHub accepts but + * `decodeURIComponent` rejects is left encoded rather than failing the whole asset. + */ export function githubMediaFileName(fetchUrl: string): string { const segment = new URL(fetchUrl).pathname.split("/").pop() ?? ""; - const name = decodeURIComponent(segment).replace(/[\\/\0]/gu, ""); + let decoded: string; + try { + decoded = decodeURIComponent(segment); + } catch { + decoded = segment; + } + const name = decoded.replace(/[\p{Cc}\\/]/gu, ""); return name.length > 0 ? name : "github-media"; } From 47854dfc5746e1339f631f8843d6507f7af58b1f Mon Sep 17 00:00:00 2001 From: maria-rcks <254055478+maria-rcks@users.noreply.github.com> Date: Mon, 14 Sep 2026 03:50:01 -0300 Subject: [PATCH 03/11] fix(server): answer a refused GitHub media fetch without stalling the client An upstream 404 or 416 carried GitHub's `content-length` on a response with no body behind it, so the browser held the connection until it gave up instead of reading the status. The refusal now carries only this server's own headers, and an exhausted redirect chain answers 502 rather than streaming a redirect page as media. The credential is decided by the target host rather than by the hop count, so it cannot ride to an object store even if the client ever stopped following redirects itself; a hop off https is refused. The token cache is keyed by host alone, which is what `gh` stores it under. A pull request body keeps its own boxes rather than the workspace media frame, and a `blob` link falls back to the raw URL it would have been fetched from. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/assets/GitHubMediaFetch.ts | 42 ++++++++++++++----- apps/web/src/components/ChatMarkdown.tsx | 26 ++++++++---- .../pullRequest/PullRequestMarkdown.tsx | 16 +++++-- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index f36efeb80b68..432bab729a82 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -13,6 +13,16 @@ import { import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +/** Hosts the credential is for. A redirect off them is answered with its own signature instead. */ +const GITHUB_HOST_PATTERN = /^(?:[\w-]+\.)*github(?:usercontent)?\.com$/iu; +const isGitHubHost = (url: string) => { + try { + return GITHUB_HOST_PATTERN.test(new URL(url).hostname); + } catch { + return false; + } +}; + /** GitHub answers an asset request with a 302 to a signed object URL that needs no credential. */ const MAX_REDIRECTS = 3; /** Following the redirect here, rather than in `fetch`, is what keeps the token on GitHub. */ @@ -47,7 +57,9 @@ const githubToken = Effect.fn("GitHubMediaFetch.githubToken")(function* (input: readonly cwd: string; readonly host: string; }) { - const key = `${input.host} ${input.cwd}`; + // `gh` stores a token per host, not per repository, so the directory it runs in is not part + // of the answer and must not fragment the cache a client could otherwise churn. + const key = input.host; const now = yield* Clock.currentTimeMillis; const cached = tokenCache.get(key); if (cached !== undefined && now - cached.at < TOKEN_CACHE_TTL_MS) return cached.token; @@ -85,8 +97,12 @@ const fetchFollowingRedirects = Effect.fn("GitHubMediaFetch.fetchFollowingRedire ) { const httpClient = HttpClient.withScope(yield* HttpClient.HttpClient); let target = url; - let authorization = token === null ? null : `Bearer ${Redacted.value(token)}`; for (let hop = 0; ; hop += 1) { + // The credential rides only on a request to GitHub itself. A redirect leads to a signed + // object URL that authorizes on its own, and the store it lives in has no business seeing + // a token — deciding that from the target, not from the hop count, is what makes it so. + const authorization = + token !== null && isGitHubHost(target) ? `Bearer ${Redacted.value(token)}` : null; const response: HttpClientResponse.HttpClientResponse = yield* httpClient .execute( HttpClientRequest.get(target).pipe( @@ -100,11 +116,12 @@ const fetchFollowingRedirects = Effect.fn("GitHubMediaFetch.fetchFollowingRedire ) .pipe(Effect.provideService(FetchHttpClient.RequestInit, MANUAL_REDIRECT)); const location = response.headers.location; - if (response.status < 300 || response.status >= 400 || !location || hop >= MAX_REDIRECTS) { - return response; - } - target = new URL(location, target).toString(); - authorization = null; + if (response.status < 300 || response.status >= 400) return response; + // A chain this long is not GitHub answering with bytes, and its body is not the media. + if (!location || hop >= MAX_REDIRECTS) return null; + const next = new URL(location, target); + if (next.protocol !== "https:") return null; + target = next.toString(); } }); @@ -129,17 +146,20 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon "cache-control": "private, no-store", "x-content-type-options": "nosniff", }; - for (const name of FORWARDED_RESPONSE_HEADERS) { - const value = response.headers[name]; - if (value !== undefined) headers[name] = value; - } + if (response === null) return HttpServerResponse.empty({ status: 502, headers }); // An upstream refusal is the client's answer, not this server's fault; only a broken hop is. + // It carries none of the upstream entity headers: a `content-length` with no body behind it + // holds the connection open until the browser gives up on it. if (response.status >= 400) { return HttpServerResponse.empty({ status: response.status >= 500 ? 502 : response.status, headers, }); } + for (const name of FORWARDED_RESPONSE_HEADERS) { + const value = response.headers[name]; + if (value !== undefined) headers[name] = value; + } // Only pictures and recordings leave this origin, and never on GitHub's word alone: the raw // host labels every committed binary `application/octet-stream`, so the name decides those. const upstreamType = headers["content-type"]?.split(";", 1)[0]?.trim().toLowerCase() ?? ""; diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 58184d682d91..a3a7a0ef1a62 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1595,6 +1595,8 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props | undefined; /** Where the media also lives on the web, for the failure state's escape hatch. */ readonly originalUrl?: string | undefined; + /** The workspace media frame, on by default; off for media that keeps the author's own box. */ + readonly framed?: boolean | undefined; /** Loaded instead of the failure state when no URL can be signed, such as against a server too old to know this resource. Only safe when the client can reach it directly. */ readonly fallbackSrc?: string | undefined; @@ -1674,7 +1676,10 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props alt={props.alt} copyMarkdown={props.copyMarkdown} standalone={props.standalone ?? true} - className={cn(CHAT_MARKDOWN_WORKSPACE_IMAGE_CLASS_NAME, props.className)} + className={cn( + props.framed === false ? undefined : CHAT_MARKDOWN_WORKSPACE_IMAGE_CLASS_NAME, + props.className, + )} style={style} imageProps={props.imageProps} actionsSource={actionsSource} @@ -3141,17 +3146,20 @@ const CHAT_MARKDOWN_COMPONENTS = { const authoredSizeStyle = authoredImageSizeStyle(width, height); const imageSource = classifyMarkdownImageSource(classifiedSrc, imageBaseDir ?? cwd); const kind = mediaKindFromPath(classifiedSrc) ?? "image"; + const directUri = imageSource._tag === "Direct" ? imageSource.uri : null; + const githubMediaUrl = + directUri === null ? null : githubMediaFetchUrl(resolveProtocolRelativeMediaUrl(directUri)); if ( githubMedia && cwd !== undefined && environmentId !== null && - imageSource._tag === "Direct" && - githubMediaFetchUrl(imageSource.uri) !== null + directUri !== null && + githubMediaUrl !== null ) { return ( ); diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index 1354b76d2578..a89117f87601 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -26,17 +26,25 @@ function PullRequestGitHubVideo({ environmentId, cwd, url, + fetchUrl, }: { environmentId: EnvironmentId; cwd: string; + /** What the body authored, which is what "Open original" should reach. */ url: string; + /** The canonical GitHub media URL: a `blob` link addresses the page, not the bytes. */ + fetchUrl: string; }) { - const resource = useMemo(() => ({ _tag: "github-media", cwd, url }), [cwd, url]); + const resource = useMemo( + () => ({ _tag: "github-media", cwd, url: fetchUrl }), + [cwd, fetchUrl], + ); const assetUrl = useAssetUrlState(environmentId, resource); const refreshAssetUrl = useAssetUrlRefresh(environmentId, resource); // A server too old to sign this resource, or one with no route to GitHub, still leaves a // public repository's video playing exactly as it did before. - const src = assetUrl._tag === "Success" ? assetUrl.url : assetUrl._tag === "Failure" ? url : null; + const src = + assetUrl._tag === "Success" ? assetUrl.url : assetUrl._tag === "Failure" ? fetchUrl : null; return ( ); } - if (segment.media === "video" && githubMediaFetchUrl(segment.url) !== null) { + const githubMediaUrl = segment.media === "video" ? githubMediaFetchUrl(segment.url) : null; + if (githubMediaUrl !== null) { return ( ); } From 6b1d1ba422c5d9bb7149cc49090383fbd84577b1 Mon Sep 17 00:00:00 2001 From: maria-rcks <254055478+maria-rcks@users.noreply.github.com> Date: Mon, 14 Sep 2026 04:10:03 -0300 Subject: [PATCH 04/11] perf(server): let a client keep GitHub media for as long as its signed URL lives An upload GitHub hosts never changes under its URL, so the only thing a cached copy must not outlive is the signed URL that granted it. The proxy now answers with that URL's own remaining life instead of `no-store`, which is what a body full of screenshots costs on every remount, and what a video seek costs on every range request. The credential is now attached only on the four hosts it is for, rather than on anything under a GitHub domain; the absence of a credential is cached too, so an unauthenticated machine stops spawning `gh` per request; and the refusal paths carry this server's own headers. Co-Authored-By: Claude Opus 5 (1M context) --- apps/server/src/assets/AssetAccess.test.ts | 2 + apps/server/src/assets/AssetAccess.ts | 10 ++++- apps/server/src/assets/GitHubMediaFetch.ts | 43 +++++++++++++++------- apps/server/src/http.ts | 7 +++- apps/web/src/components/ChatMarkdown.tsx | 8 +++- 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 59d2cdfd2d7b..e2294040f382 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -1081,6 +1081,8 @@ describe("AssetAccess", () => { kind: "github-media", url: "https://github.com/user-attachments/assets/1a1842fb-6383-492f-873c-57aa0033fa6c", cwd: "/repo", + // The signed URL's own expiry, which is how long a client may keep the bytes. + expiresAt: attachment.expiresAt, }); // A `blob` link addresses the page; only the raw host answers a credential with bytes. diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index 19d522050eff..90218ae42bae 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -165,6 +165,9 @@ export type ResolvedAsset = readonly kind: "github-media"; readonly url: string; readonly cwd: string; + /** When the signed URL that granted this stops working, which bounds how long a client + may keep the bytes it fetched with it. */ + readonly expiresAt: number; }; function decodeClaims(encodedPayload: string): AssetClaims | null { @@ -789,7 +792,12 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( } if (claims.kind === "github-media") { - return { kind: "github-media", url: claims.url, cwd: claims.cwd } satisfies ResolvedAsset; + return { + kind: "github-media", + url: claims.url, + cwd: claims.cwd, + expiresAt: claims.expiresAt, + } satisfies ResolvedAsset; } if (claims.kind === "native-app-icon") { diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index 432bab729a82..feb3450cadf6 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -13,11 +13,20 @@ import { import * as GitHubCli from "../sourceControl/GitHubCli.ts"; -/** Hosts the credential is for. A redirect off them is answered with its own signature instead. */ -const GITHUB_HOST_PATTERN = /^(?:[\w-]+\.)*github(?:usercontent)?\.com$/iu; -const isGitHubHost = (url: string) => { +/** + * Exactly the hosts the credential is for. Everything a redirect leads to — the presigned + * object stores GitHub hands assets off to — authorizes with its own signature, and some of + * them reject a request that also carries a bearer token. + */ +const CREDENTIALED_HOSTS = new Set([ + "github.com", + "www.github.com", + "raw.githubusercontent.com", + "media.githubusercontent.com", +]); +const isCredentialedHost = (url: string) => { try { - return GITHUB_HOST_PATTERN.test(new URL(url).hostname); + return CREDENTIALED_HOSTS.has(new URL(url).hostname.toLowerCase()); } catch { return false; } @@ -51,14 +60,18 @@ const SVG_CONTENT_SECURITY_POLICY = "default-src 'none'; style-src 'unsafe-inlin * The token is what `gh auth token` would print again on the next call, and it is held no longer * than a signed asset URL lives. */ -const tokenCache = new Map(); +const tokenCache = new Map< + string, + { readonly at: number; readonly token: Redacted.Redacted | null } +>(); const githubToken = Effect.fn("GitHubMediaFetch.githubToken")(function* (input: { readonly cwd: string; readonly host: string; }) { // `gh` stores a token per host, not per repository, so the directory it runs in is not part - // of the answer and must not fragment the cache a client could otherwise churn. + // of the answer and must not fragment the cache a client could otherwise churn. This route + // pins no credential; if it ever does, the pin belongs in this key. const key = input.host; const now = yield* Clock.currentTimeMillis; const cached = tokenCache.get(key); @@ -76,11 +89,12 @@ const githubToken = Effect.fn("GitHubMediaFetch.githubToken")(function* (input: Effect.map((output) => output.stdout.trim()), Effect.orElseSucceed(() => ""), ); - if (token.length === 0) return null; if (tokenCache.size >= TOKEN_CACHE_MAX_ENTRIES) { tokenCache.delete(tokenCache.keys().next().value!); } - const redacted = Redacted.make(token); + // The absence of a credential is cached too, or an unauthenticated machine spawns `gh` again + // for every image and every video range request. + const redacted = token.length === 0 ? null : Redacted.make(token); tokenCache.set(key, { at: now, token: redacted }); return redacted; }); @@ -102,7 +116,7 @@ const fetchFollowingRedirects = Effect.fn("GitHubMediaFetch.fetchFollowingRedire // object URL that authorizes on its own, and the store it lives in has no business seeing // a token — deciding that from the target, not from the hop count, is what makes it so. const authorization = - token !== null && isGitHubHost(target) ? `Bearer ${Redacted.value(token)}` : null; + token !== null && isCredentialedHost(target) ? `Bearer ${Redacted.value(token)}` : null; const response: HttpClientResponse.HttpClientResponse = yield* httpClient .execute( HttpClientRequest.get(target).pipe( @@ -130,7 +144,7 @@ const fetchFollowingRedirects = Effect.fn("GitHubMediaFetch.fetchFollowingRedire * only thing that distinguishes a readable private attachment from a 404. */ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaResponse")(function* ( - asset: { readonly url: string; readonly cwd: string }, + asset: { readonly url: string; readonly cwd: string; readonly expiresAt: number }, requestHeaders: Record, ) { // Both media hosts are served by github.com's account, which is the host `gh` stores it under. @@ -141,9 +155,12 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon if (value !== undefined) forwarded[name] = value; } const response = yield* fetchFollowingRedirects(asset.url, forwarded, token); + // An upload GitHub hosts never changes under its URL, so the only thing a cached copy must + // not outlive is the signed URL that granted it — which is the same bound the URL itself has. + const remainingSeconds = Math.floor((asset.expiresAt - (yield* Clock.currentTimeMillis)) / 1000); const headers: Record = { - // The signed asset URL is the grant; a cached copy must not outlive it. - "cache-control": "private, no-store", + "cache-control": + remainingSeconds > 0 ? `private, max-age=${remainingSeconds}` : "private, no-store", "x-content-type-options": "nosniff", }; if (response === null) return HttpServerResponse.empty({ status: 502, headers }); @@ -167,7 +184,7 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon ? upstreamType : (Mime.getType(githubMediaFileName(asset.url))?.toLowerCase() ?? ""); if (!MEDIA_CONTENT_TYPE_PATTERN.test(contentType)) { - return HttpServerResponse.empty({ status: 415 }); + return HttpServerResponse.empty({ status: 415, headers }); } headers["content-type"] = contentType; if (contentType === SVG_CONTENT_TYPE) { diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index d5f070fda20a..bdd7ab7b299c 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -395,7 +395,12 @@ export const assetRouteLayer = HttpRouter.add( Effect.tapError((cause) => Effect.logWarning("Failed to fetch GitHub media.", { url: asset.url, cause }), ), - Effect.orElseSucceed(() => HttpServerResponse.empty({ status: 502 })), + Effect.orElseSucceed(() => + HttpServerResponse.empty({ + status: 502, + headers: { "cache-control": "private, no-store", "x-content-type-options": "nosniff" }, + }), + ), ); } return yield* assetFileResponse( diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index a3a7a0ef1a62..772542d0c6b7 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1620,7 +1620,11 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props const relativePath = reference?.kind === "file" ? reference.relativePath : undefined; const fallbackSrc = assetUrl._tag === "Failure" ? props.fallbackSrc : undefined; const src = - assetUrl._tag === "Success" ? assetUrl.url + (props.srcFragment ?? "") : (fallbackSrc ?? null); + assetUrl._tag === "Success" + ? assetUrl.url + (props.srcFragment ?? "") + : fallbackSrc === undefined + ? null + : fallbackSrc + (props.srcFragment ?? ""); // The server reads the pixel size from the file header, so the slot can be // the image's final box instead of a 16:9 guess. An authored size wins; a // caller's height cap shrinks the box while keeping the ratio. @@ -3168,7 +3172,7 @@ const CHAT_MARKDOWN_COMPONENTS = { style={authoredSizeStyle} imageProps={imageProps} srcFragment={markdownImageSourceFragment(classifiedSrc)} - originalUrl={directUri} + originalUrl={resolveProtocolRelativeMediaUrl(directUri)} // A pull request body draws its own boxes; keep the author's, not the workspace frame. framed={false} // A server too old to sign this resource, or one with no route to GitHub, still leaves From f83a9360244681e91293a34866b48898b8e4c4ee Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 02:16:26 +0000 Subject: [PATCH 05/11] fix(media): preserve fallback expansion and rejection headers --- apps/server/src/assets/GitHubMediaFetch.ts | 10 +++++----- apps/web/src/components/ChatMarkdown.tsx | 4 +++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index feb3450cadf6..776b0930290c 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -173,19 +173,19 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon headers, }); } - for (const name of FORWARDED_RESPONSE_HEADERS) { - const value = response.headers[name]; - if (value !== undefined) headers[name] = value; - } // Only pictures and recordings leave this origin, and never on GitHub's word alone: the raw // host labels every committed binary `application/octet-stream`, so the name decides those. - const upstreamType = headers["content-type"]?.split(";", 1)[0]?.trim().toLowerCase() ?? ""; + const upstreamType = response.headers["content-type"]?.split(";", 1)[0]?.trim().toLowerCase() ?? ""; const contentType = MEDIA_CONTENT_TYPE_PATTERN.test(upstreamType) ? upstreamType : (Mime.getType(githubMediaFileName(asset.url))?.toLowerCase() ?? ""); if (!MEDIA_CONTENT_TYPE_PATTERN.test(contentType)) { return HttpServerResponse.empty({ status: 415, headers }); } + for (const name of FORWARDED_RESPONSE_HEADERS) { + const value = response.headers[name]; + if (value !== undefined) headers[name] = value; + } headers["content-type"] = contentType; if (contentType === SVG_CONTENT_TYPE) { headers["content-security-policy"] = SVG_CONTENT_SECURITY_POLICY; diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 772542d0c6b7..4e80768b315a 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1641,7 +1641,9 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props kind: props.kind ?? "image", name: props.alt || (props.kind ?? "image"), src, - asset: { environmentId: props.environmentId, resource }, + ...(fallbackSrc === undefined + ? { asset: { environmentId: props.environmentId, resource } } + : {}), ...(reference ? { reference } : {}), ...(relativePath && (resource._tag === "media-file" || resource._tag === "workspace-file") ? { From 64cc05bf17d7ad9b5ea8c6fede1e4a470eb9490e Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 03:12:59 +0000 Subject: [PATCH 06/11] style(server): format github media content type parsing --- apps/server/src/assets/GitHubMediaFetch.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index 776b0930290c..f263188481ef 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -175,7 +175,8 @@ export const githubMediaResponse = Effect.fn("GitHubMediaFetch.githubMediaRespon } // Only pictures and recordings leave this origin, and never on GitHub's word alone: the raw // host labels every committed binary `application/octet-stream`, so the name decides those. - const upstreamType = response.headers["content-type"]?.split(";", 1)[0]?.trim().toLowerCase() ?? ""; + const upstreamType = + response.headers["content-type"]?.split(";", 1)[0]?.trim().toLowerCase() ?? ""; const contentType = MEDIA_CONTENT_TYPE_PATTERN.test(upstreamType) ? upstreamType : (Mime.getType(githubMediaFileName(asset.url))?.toLowerCase() ?? ""); From ae1c7429cecb8f2156568e790d0cf64096150706 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 03:30:53 +0000 Subject: [PATCH 07/11] fix(server): keep media test layers within pipe limit --- apps/server/src/server.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 951b83ef6f1f..854432cb2e99 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -1197,8 +1197,7 @@ const buildAppUnderTest = (options?: { Layer.provideMerge(ServerSecretStore.layer), Layer.provide(workspaceAndProjectServicesLayer), Layer.provideMerge(FetchHttpClient.layer), - Layer.provide(GitHubCli.layer), - Layer.provide(VcsProcess.layer), + Layer.provide(GitHubCli.layer.pipe(Layer.provideMerge(VcsProcess.layer))), Layer.provide(layerConfig), ); From 3963b50d313440a68eeecbc7188ba3edfc97b60b Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 03:44:48 +0000 Subject: [PATCH 08/11] fix(assets): omit rejected media URLs from errors --- apps/server/src/assets/AssetAccess.test.ts | 7 ++++--- apps/server/src/assets/AssetAccess.ts | 2 +- packages/contracts/src/assets.ts | 4 +--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index e2294040f382..8874dff712ff 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -1106,13 +1106,14 @@ describe("AssetAccess", () => { for (const url of [ "https://example.com/shot.png", + "https://example.com/shot.png?token=private-media-token", "http://github.com/user-attachments/assets/1a1842fb", "https://github.com/owner/repo/pull/1", "https://github.com/owner/repo/blob/main/", ]) { - expect((yield* issue(url).pipe(Effect.flip))._tag).toBe( - "AssetGitHubMediaUrlValidationError", - ); + const error = yield* issue(url).pipe(Effect.flip); + expect(error._tag).toBe("AssetGitHubMediaUrlValidationError"); + expect(JSON.stringify(error)).not.toContain(url); } }).pipe(Effect.provide(testLayer)), ); diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index 90218ae42bae..e1f6633fbeca 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -679,7 +679,7 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i case "github-media": { const fetchUrl = githubMediaFetchUrl(input.resource.url); if (fetchUrl === null) { - return yield* new AssetGitHubMediaUrlValidationError({ resource: input.resource }); + return yield* new AssetGitHubMediaUrlValidationError({}); } claims = { version: 1, diff --git a/packages/contracts/src/assets.ts b/packages/contracts/src/assets.ts index 63fb095b787e..cd5d1757301a 100644 --- a/packages/contracts/src/assets.ts +++ b/packages/contracts/src/assets.ts @@ -294,9 +294,7 @@ export class AssetSigningKeyLoadError extends Schema.TaggedError()( "AssetGitHubMediaUrlValidationError", - { - resource: AssetResource, - }, + {}, ) { override get message(): string { return "Only media hosted by GitHub can be fetched with a GitHub credential."; From 3da793bfe097ac5bcdc772f781e55891f09bd98d Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 03:47:05 +0000 Subject: [PATCH 09/11] test(assets): encode media errors through effect schema --- apps/server/src/assets/AssetAccess.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 8874dff712ff..94cf55fa8f28 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -2,7 +2,7 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import * as NodeHttpPlatform from "@effect/platform-node/NodeHttpPlatform"; import * as NodeFSP from "node:fs/promises"; -import { AssetPreviewTypeValidationError, ThreadId } from "@t3tools/contracts"; +import { AssetAccessError, AssetPreviewTypeValidationError, ThreadId } from "@t3tools/contracts"; import { PROJECT_FAVICON_FALLBACK_MARKER } from "@t3tools/shared/projectFavicon"; import { describe, expect, it } from "@effect/vitest"; import * as Crypto from "effect/Crypto"; @@ -11,6 +11,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; +import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; import { HttpServerResponse } from "effect/unstable/http"; import { vi } from "vite-plus/test"; @@ -1113,7 +1114,8 @@ describe("AssetAccess", () => { ]) { const error = yield* issue(url).pipe(Effect.flip); expect(error._tag).toBe("AssetGitHubMediaUrlValidationError"); - expect(JSON.stringify(error)).not.toContain(url); + const encoded = yield* Schema.encodeEffect(Schema.fromJsonString(AssetAccessError))(error); + expect(encoded).not.toContain(url); } }).pipe(Effect.provide(testLayer)), ); From f8e9e22394ea3aaf7ce448a6f8bb44c79c6a2b90 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 20:51:41 +0000 Subject: [PATCH 10/11] fix(assets): retry private media credentials after login --- apps/server/src/assets/AssetAccess.test.ts | 51 +++++++++++++++++++++- apps/server/src/assets/GitHubMediaFetch.ts | 11 ++--- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 94cf55fa8f28..71a2c769ef4e 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -13,7 +13,8 @@ import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import * as TestClock from "effect/testing/TestClock"; -import { HttpServerResponse } from "effect/unstable/http"; +import { HttpClient, HttpClientResponse, HttpServerResponse } from "effect/unstable/http"; +import { ChildProcessSpawner } from "effect/unstable/process"; import { vi } from "vite-plus/test"; import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; @@ -26,6 +27,8 @@ import { ASSET_ROUTE_PREFIX, issueAssetUrl, resolveAsset } from "./AssetAccess.t import * as NativeAppIconResolver from "./NativeAppIconResolver.ts"; import { openMediaFile } from "./MediaFile.ts"; import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import { githubMediaResponse } from "./GitHubMediaFetch.ts"; vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); @@ -48,6 +51,52 @@ const testLayer = Layer.mergeAll( ).pipe(Layer.provideMerge(NodeServices.layer)); describe("AssetAccess", () => { + it.effect("loads private media immediately after login and reuses the found credential", () => { + let lookups = 0; + const authorizations: Array = []; + return Effect.gen(function* () { + const asset = { + url: "https://raw.githubusercontent.com/owner/repo/main/shot.png", + cwd: "/repo", + expiresAt: Number.MAX_SAFE_INTEGER, + }; + expect((yield* githubMediaResponse(asset, {})).status).toBe(404); + expect((yield* githubMediaResponse(asset, {})).status).toBe(200); + expect((yield* githubMediaResponse(asset, {})).status).toBe(200); + expect(lookups).toBe(2); + expect(authorizations).toEqual([undefined, "Bearer signed-in", "Bearer signed-in"]); + }).pipe( + Effect.provide( + Layer.mock(GitHubCli.GitHubCli)({ + execute: () => + Effect.sync(() => ({ + exitCode: ChildProcessSpawner.ExitCode(0), + stdout: ++lookups === 1 ? "" : "signed-in", + stderr: "", + stdoutTruncated: false, + stderrTruncated: false, + })), + }), + ), + Effect.provideService( + HttpClient.HttpClient, + HttpClient.make((request) => { + authorizations.push(request.headers.authorization); + return Effect.succeed( + HttpClientResponse.fromWeb( + request, + new Response(null, { + status: request.headers.authorization ? 200 : 404, + headers: { "content-type": "image/png" }, + }), + ), + ); + }), + ), + Effect.scoped, + ); + }); + it.effect("issues exact URLs for media and browser documents outside the workspace", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/assets/GitHubMediaFetch.ts b/apps/server/src/assets/GitHubMediaFetch.ts index f263188481ef..fc2bb8c3e6de 100644 --- a/apps/server/src/assets/GitHubMediaFetch.ts +++ b/apps/server/src/assets/GitHubMediaFetch.ts @@ -60,10 +60,7 @@ const SVG_CONTENT_SECURITY_POLICY = "default-src 'none'; style-src 'unsafe-inlin * The token is what `gh auth token` would print again on the next call, and it is held no longer * than a signed asset URL lives. */ -const tokenCache = new Map< - string, - { readonly at: number; readonly token: Redacted.Redacted | null } ->(); +const tokenCache = new Map(); const githubToken = Effect.fn("GitHubMediaFetch.githubToken")(function* (input: { readonly cwd: string; @@ -89,12 +86,12 @@ const githubToken = Effect.fn("GitHubMediaFetch.githubToken")(function* (input: Effect.map((output) => output.stdout.trim()), Effect.orElseSucceed(() => ""), ); + // A login or recovered CLI failure must take effect on the next media request. + if (token.length === 0) return null; if (tokenCache.size >= TOKEN_CACHE_MAX_ENTRIES) { tokenCache.delete(tokenCache.keys().next().value!); } - // The absence of a credential is cached too, or an unauthenticated machine spawns `gh` again - // for every image and every video range request. - const redacted = token.length === 0 ? null : Redacted.make(token); + const redacted = Redacted.make(token); tokenCache.set(key, { at: now, token: redacted }); return redacted; }); From 8e3f71f4d85b6d3d0112603cae4955667855ec97 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Wed, 16 Sep 2026 20:57:18 +0000 Subject: [PATCH 11/11] fix(web): preserve private pull request video timestamps --- apps/web/src/components/pullRequest/PullRequestMarkdown.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx index a89117f87601..6ffb54569ef3 100644 --- a/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx +++ b/apps/web/src/components/pullRequest/PullRequestMarkdown.tsx @@ -1,4 +1,5 @@ import { ExternalLinkIcon, PaperclipIcon } from "lucide-react"; +import { markdownImageSourceFragment } from "@t3tools/client-runtime/markdown-images"; import { githubMediaFetchUrl } from "@t3tools/shared/githubMedia"; import type { AssetResource, EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; import { createContext, useContext, useMemo } from "react"; @@ -47,7 +48,7 @@ function PullRequestGitHubVideo({ assetUrl._tag === "Success" ? assetUrl.url : assetUrl._tag === "Failure" ? fetchUrl : null; return (