From 1da4829ea237c5995f05ed74354b34fcf1bda328 Mon Sep 17 00:00:00 2001 From: Xanacas <53140620+Xanacas@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:08:20 +0200 Subject: [PATCH 1/5] fix(pull-requests): render private GitHub images in PR bodies GitHub serves user-attachments uploads only to an authenticated viewer, so images in PR bodies rendered as broken placeholders in the app. Clients now route those URLs through the signed asset route; the server resolves GitHub's redirect with the gh CLI token and answers 302, so bytes flow from signed storage straight to the browser and the token never leaves the server. Co-Authored-By: Claude Fable 5 --- .../src/features/threads/ThreadFeed.tsx | 24 +++---- apps/server/src/assets/AssetAccess.test.ts | 17 +++++ apps/server/src/assets/AssetAccess.ts | 40 +++++++++-- .../src/assets/GitHubAttachmentProxy.ts | 72 +++++++++++++++++++ apps/server/src/http.ts | 18 +++++ apps/server/src/server.ts | 8 +++ apps/server/src/ws.ts | 5 +- apps/web/src/components/ChatMarkdown.tsx | 35 +++++---- .../pullRequest/pullRequestMarkdown.logic.ts | 7 +- .../client-runtime/src/markdownImages.test.ts | 5 ++ packages/client-runtime/src/markdownImages.ts | 8 ++- packages/contracts/src/assets.test.ts | 21 +++++- packages/contracts/src/assets.ts | 23 ++++++ 13 files changed, 248 insertions(+), 35 deletions(-) create mode 100644 apps/server/src/assets/GitHubAttachmentProxy.ts diff --git a/apps/mobile/src/features/threads/ThreadFeed.tsx b/apps/mobile/src/features/threads/ThreadFeed.tsx index 60b397802ccc..8eba0f9beac2 100644 --- a/apps/mobile/src/features/threads/ThreadFeed.tsx +++ b/apps/mobile/src/features/threads/ThreadFeed.tsx @@ -1,7 +1,7 @@ import * as Haptics from "expo-haptics"; import { KeyboardAwareLegendList } from "@legendapp/list/keyboard"; import { type LegendListRef } from "@legendapp/list/react-native"; -import type { EnvironmentId, MessageId, ThreadId, TurnId } from "@t3tools/contracts"; +import type { AssetResource, EnvironmentId, MessageId, ThreadId, TurnId } from "@t3tools/contracts"; import { classifyMarkdownImageSource } from "@t3tools/client-runtime/markdown-images"; import { CHAT_LIST_ANCHOR_OFFSET, resolveChatListAnchoredEndSpace } from "@t3tools/shared/chatList"; import { formatElapsed } from "@t3tools/shared/orchestrationTiming"; @@ -324,24 +324,20 @@ function ThreadMarkdownImageRequest(props: { ); } -/** Markdown image whose src is a workspace file — loads through a signed asset URL. */ +/** Markdown image whose bytes load through a signed asset URL from the environment. */ function ThreadMarkdownImage(props: { readonly environmentId: EnvironmentId; - readonly threadId: ThreadId; - readonly path: string; + readonly resource: AssetResource; + readonly sourceKey: string; readonly alt: string | null; readonly onPressImage: (uri: string) => void; }) { - const assetUrl = useAssetUrlState(props.environmentId, { - _tag: "workspace-file", - threadId: props.threadId, - path: props.path, - }); + const assetUrl = useAssetUrlState(props.environmentId, props.resource); return ( setExpandedImage({ uri })} /> diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index aa47a78238bb..059462bc8661 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -442,4 +442,21 @@ describe("AssetAccess", () => { expect(error.cause).toBe(resolutionCause); }).pipe(Effect.provide(testLayer)), ); + + it.effect("issues GitHub attachment URLs only for allowlisted uploads", () => + Effect.gen(function* () { + const url = "https://github.com/user-attachments/assets/4dcab2ba-0674-4d3b-a3a7-3546601b1550"; + const result = yield* issueAssetUrl({ resource: { _tag: "github-attachment", url } }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const [token, fileName] = suffix.split("/") as [string, string]; + + expect(yield* resolveAsset(token, fileName)).toEqual({ kind: "github-attachment", url }); + expect(yield* resolveAsset(`${token}tampered`, fileName)).toBeNull(); + + const error = yield* issueAssetUrl({ + resource: { _tag: "github-attachment", url: "https://evil.example.com/assets/abc" }, + }).pipe(Effect.flip); + expect(error._tag).toBe("AssetRemoteUrlValidationError"); + }).pipe(Effect.provide(testLayer)), + ); }); diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index 232a41e5a9c8..bcd142dd601f 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -1,6 +1,8 @@ import type { AssetResource } from "@t3tools/contracts"; import { AssetAttachmentNotFoundError, + AssetRemoteUrlValidationError, + isGitHubUserAttachmentUrl, AssetPreviewTypeValidationError, AssetProjectFaviconInspectionError, AssetProjectFaviconNotFoundError, @@ -46,7 +48,7 @@ export const ASSET_ROUTE_PREFIX = "/api/assets"; const SIGNING_SECRET_NAME = "asset-access-signing-key"; const ASSET_TOKEN_TTL_MS = 60 * 60 * 1000; -const PROJECT_FAVICON_TOKEN_BUCKET_MS = 30 * 60 * 1000; +const STABLE_URL_TOKEN_BUCKET_MS = 30 * 60 * 1000; const PROJECT_FAVICON_VERSION_PREFIX = "v"; const PREVIEW_ASSET_EXTENSIONS = new Set([ ...WORKSPACE_BROWSER_PREVIEW_EXTENSIONS, @@ -94,6 +96,12 @@ const AssetClaimsSchema = Schema.Union([ filePath: Schema.String, expiresAt: Schema.Number, }), + Schema.Struct({ + version: Schema.Literal(1), + kind: Schema.Literal("github-attachment"), + url: Schema.String, + expiresAt: Schema.Number, + }), ]); type AssetClaims = typeof AssetClaimsSchema.Type; @@ -101,7 +109,9 @@ const AssetClaimsJson = Schema.fromJsonString(AssetClaimsSchema); const decodeAssetClaims = Schema.decodeUnknownOption(AssetClaimsJson); const encodeAssetClaims = Schema.encodeSync(AssetClaimsJson); -export type ResolvedAsset = { readonly kind: "file"; readonly path: string }; +export type ResolvedAsset = + | { readonly kind: "file"; readonly path: string } + | { readonly kind: "github-attachment"; readonly url: string }; function decodeClaims(encodedPayload: string): AssetClaims | null { try { @@ -275,6 +285,15 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i fileName = path.basename(resolved.relativePath); break; } + case "github-attachment": { + const url = input.resource.url; + if (!isGitHubUserAttachmentUrl(url)) { + return yield* new AssetRemoteUrlValidationError({ resource: input.resource }); + } + claims = { version: 1, kind: "github-attachment", url, expiresAt }; + fileName = path.basename(url); + break; + } case "attachment": { const config = yield* ServerConfig.ServerConfig; const attachmentPath = resolveAttachmentPathById({ @@ -411,11 +430,15 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i }), ), ); - if (claims.kind === "project-favicon" || claims.kind === "project-favicon-external") { + // Bucketed expiry keeps repeat issuances byte-identical, so browser caches get hits. + if ( + claims.kind === "project-favicon" || + claims.kind === "project-favicon-external" || + claims.kind === "github-attachment" + ) { const issuedAt = yield* Clock.currentTimeMillis; expiresAt = - (Math.floor(issuedAt / PROJECT_FAVICON_TOKEN_BUCKET_MS) + 2) * - PROJECT_FAVICON_TOKEN_BUCKET_MS; + (Math.floor(issuedAt / STABLE_URL_TOKEN_BUCKET_MS) + 2) * STABLE_URL_TOKEN_BUCKET_MS; claims = { ...claims, expiresAt }; } const encodedPayload = base64UrlEncode(encodeAssetClaims(claims)); @@ -445,6 +468,13 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( const claims = decodeClaims(encodedPayload); if (!claims || claims.expiresAt <= (yield* Clock.currentTimeMillis)) return null; + if (claims.kind === "github-attachment") { + // Re-checked so a stale claim can never point the proxy at an arbitrary host. + return isGitHubUserAttachmentUrl(claims.url) + ? ({ kind: "github-attachment", url: claims.url } satisfies ResolvedAsset) + : null; + } + if (claims.kind === "attachment") { const config = yield* ServerConfig.ServerConfig; const attachmentPath = resolveAttachmentPathById({ diff --git a/apps/server/src/assets/GitHubAttachmentProxy.ts b/apps/server/src/assets/GitHubAttachmentProxy.ts new file mode 100644 index 000000000000..2683b3137532 --- /dev/null +++ b/apps/server/src/assets/GitHubAttachmentProxy.ts @@ -0,0 +1,72 @@ +import * as Context from "effect/Context"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http"; + +import * as ServerConfig from "../config.ts"; +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; + +/** `gh auth token` spawns a process; one cached read serves a whole page of images. */ +const TOKEN_CACHE_TTL = "5 minutes"; + +/** + * Resolves GitHub `user-attachments` uploads, which GitHub serves only to an authenticated + * viewer, into their signed-storage redirect targets using the developer's `gh` credentials. + * Only the redirect is resolved here — the bytes never pass through the server, and the token + * never leaves it. Without a token the request still goes out: public-repository uploads + * redirect anonymously. + */ +export class GitHubAttachmentProxy extends Context.Service< + GitHubAttachmentProxy, + { + /** `null` on any failure; the route answers 404 and the client shows its fallback. */ + readonly resolveAttachmentLocation: (url: string) => Effect.Effect; + } +>()("t3/assets/GitHubAttachmentProxy") {} + +export const make = Effect.gen(function* () { + const gitHubCli = yield* GitHubCli.GitHubCli; + const httpClient = yield* HttpClient.HttpClient; + const config = yield* ServerConfig.ServerConfig; + + const readToken = yield* Effect.cachedWithTTL( + gitHubCli + .execute({ + cwd: config.stateDir, + args: ["auth", "token", "--hostname", "github.com"], + }) + .pipe( + Effect.map((output) => { + const trimmed = output.stdout.trim(); + return trimmed.length > 0 ? trimmed : null; + }), + Effect.orElseSucceed(() => null), + ), + TOKEN_CACHE_TTL, + ); + + const resolveAttachmentLocation: GitHubAttachmentProxy["Service"]["resolveAttachmentLocation"] = ( + url, + ) => + Effect.gen(function* () { + const token = yield* readToken; + const request = HttpClientRequest.get(url, { + headers: token === null ? {} : { authorization: `token ${token}` }, + }); + const response = yield* httpClient + .execute(request) + .pipe(Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" })); + if (response.status < 300 || response.status >= 400) return null; + const location = response.headers["location"]; + return location !== undefined && location.startsWith("https://") ? location : null; + }).pipe( + Effect.tapError((cause) => + Effect.logWarning("Failed to resolve a GitHub attachment.", { url, cause }), + ), + Effect.orElseSucceed(() => null), + ); + + return { resolveAttachmentLocation } satisfies GitHubAttachmentProxy["Service"]; +}); + +export const layer = Layer.effect(GitHubAttachmentProxy, make); diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index c3104e7bc420..ad0531725df0 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -28,6 +28,7 @@ import { OtlpTracer } from "effect/unstable/observability"; import * as ServerConfig from "./config.ts"; import { ASSET_ROUTE_PREFIX, resolveAsset } from "./assets/AssetAccess.ts"; +import * as GitHubAttachmentProxy from "./assets/GitHubAttachmentProxy.ts"; import { ATTACHMENT_UPLOAD_ROUTE_PREFIX, storeAttachmentUpload, @@ -226,6 +227,23 @@ export const assetRouteLayer = HttpRouter.add( if (!asset) { return HttpServerResponse.text("Not Found", { status: 404 }); } + if (asset.kind === "github-attachment") { + // serviceOption because route requirements defer to HttpRouter.serve and a hard + // dependency leaks into every serve site; a server without the proxy answers 404. + const proxy = yield* Effect.serviceOption(GitHubAttachmentProxy.GitHubAttachmentProxy); + const location = Option.isSome(proxy) + ? yield* proxy.value.resolveAttachmentLocation(asset.url) + : null; + if (location === null) { + return HttpServerResponse.text("Not Found", { status: 404 }); + } + return HttpServerResponse.redirect(location, { + status: 302, + // Below the 300-second expiry of GitHub's signed storage URL, so a cached redirect + // never lands on an expired one. + headers: { "cache-control": "private, max-age=240" }, + }); + } return yield* HttpServerResponse.file(asset.path, { status: 200, headers: assetResponseHeaders(asset.path), diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index d5bebe3d5000..cf4f2d0d36e0 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -8,6 +8,7 @@ import { FetchHttpClient, HttpRouter, HttpServer } from "effect/unstable/http"; import * as HttpApiBuilder from "effect/unstable/httpapi/HttpApiBuilder"; import * as BackgroundPolicy from "./background/BackgroundPolicy.ts"; +import * as GitHubAttachmentProxy from "./assets/GitHubAttachmentProxy.ts"; import * as HostPowerMonitor from "./background/HostPowerMonitor.ts"; import * as ServerConfig from "./config.ts"; import { @@ -444,6 +445,12 @@ const commandReadinessLayer = HttpRouter.middleware( { global: true }, ); +const GitHubAttachmentProxyLive = GitHubAttachmentProxy.layer.pipe( + Layer.provide(GitHubCli.layer), + Layer.provide(VcsProcess.layer), + Layer.provide(FetchHttpClient.layer), +); + const PullRequestServiceLive = PullRequestService.layer.pipe( // One registry entry per supported host; the service only knows the registry. Layer.provide(PullRequestProviderRegistry.layer), @@ -473,6 +480,7 @@ export const makeRoutesLayer = Layer.mergeAll( // Both transports consume the same service instance, so caches single-flight across clients // and mutations observed on WebSocket invalidate patches subsequently read over HTTP. Layer.provide(PullRequestServiceLive), + Layer.provide(GitHubAttachmentProxyLive), Layer.provide(PreviewAutomationBroker.layer), Layer.provide(ServerSelfUpdate.layer), Layer.provide(commandReadinessLayer), diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 226c82cdb1ac..f6de7d07b62f 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -2017,7 +2017,10 @@ const makeWsRpcLayer = ( observeRpcEffect( WS_METHODS.assetsCreateUrl, Effect.gen(function* () { - if (input.resource._tag === "attachment") { + if ( + input.resource._tag === "attachment" || + input.resource._tag === "github-attachment" + ) { return yield* issueAssetUrl({ resource: input.resource }); } if (input.resource._tag === "project-favicon") { diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index dd21a4e1bf40..0ea592cf9e6e 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -14,6 +14,7 @@ import { WrapTextIcon, } from "lucide-react"; import type { + AssetResource, EnvironmentId, ScopedThreadRef, ServerProviderSkill, @@ -1064,17 +1065,13 @@ function ChatMarkdownImageFallback(props: { readonly alt: string }) { ); } -/** Markdown images whose src is a workspace file path load through a signed asset URL. */ -const ChatMarkdownWorkspaceImage = memo(function ChatMarkdownWorkspaceImage(props: { - readonly threadRef: ScopedThreadRef; - readonly path: string; +/** Markdown images whose bytes load through a signed asset URL from the environment. */ +const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props: { + readonly environmentId: EnvironmentId; + readonly resource: AssetResource; readonly alt: string; }) { - const assetUrl = useAssetUrlState(props.threadRef.environmentId, { - _tag: "workspace-file", - threadId: props.threadRef.threadId, - path: props.path, - }); + const assetUrl = useAssetUrlState(props.environmentId, props.resource); const [failedUrl, setFailedUrl] = useState(null); if (assetUrl._tag === "Failure" || (assetUrl._tag === "Success" && failedUrl === assetUrl.url)) { @@ -2161,11 +2158,24 @@ function ChatMarkdown({ /> ); } + if (imageSource._tag === "GitHubAttachment" && environmentId !== null) { + return ( + + ); + } if (imageSource._tag === "WorkspaceFile" && threadRef) { return ( - ); @@ -2211,6 +2221,7 @@ function ChatMarkdown({ canUseShellActions, cwd, diffThemeName, + environmentId, fileLinkParentSuffixByPath, inlineCodeFileLinkMetaByText, isStreaming, diff --git a/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts b/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts index e322b595a182..a02cca81c9cf 100644 --- a/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestMarkdown.logic.ts @@ -1,3 +1,5 @@ +import { isGitHubUserAttachmentUrl } from "@t3tools/contracts"; + /** `id` is positional on purpose: the same attachment can be embedded twice in one body. */ export type PullRequestBodySegment = | { readonly id: string; readonly kind: "markdown"; readonly text: string } @@ -24,8 +26,6 @@ const VIDEO_TAG_MAX_LINES = 8; const INDENTED_CODE_PATTERN = /^(?: {4}|\t)/u; const BARE_URL_PATTERN = /^?$/u; const VIDEO_EXTENSION_PATTERN = /\.(?:mp4|webm|mov|m4v|ogv)(?:$|[?#])/iu; -/** A dropped video becomes a bare asset link; a dropped image becomes an `` tag. */ -const GITHUB_ASSET_PATTERN = /^https:\/\/github\.com\/user-attachments\/assets\/[\w-]+$/iu; const VIDEO_TAG_SRC_PATTERN = /<(?:video|source)\b[^>]*\bsrc\s*=\s*["']([^"']+)["']/iu; /** Only a tag that owns its line is an embed; inline, it is prose the renderer should keep. */ const STANDALONE_VIDEO_TAG_PATTERN = /^\s*` tag. + if (VIDEO_EXTENSION_PATTERN.test(url) || isGitHubUserAttachmentUrl(url)) { return { url, media: "video" }; } return null; diff --git a/packages/client-runtime/src/markdownImages.test.ts b/packages/client-runtime/src/markdownImages.test.ts index a4160c3da4c1..a6806833fcb3 100644 --- a/packages/client-runtime/src/markdownImages.test.ts +++ b/packages/client-runtime/src/markdownImages.test.ts @@ -16,6 +16,11 @@ describe("classifyMarkdownImageSource", () => { }); }); + it("routes GitHub user-attachment uploads through the signed asset proxy", () => { + const url = "https://github.com/user-attachments/assets/4dcab2ba-0674-4d3b-a3a7-3546601b1550"; + expect(classifyMarkdownImageSource(url, null)).toEqual({ _tag: "GitHubAttachment", url }); + }); + it.each([ ["images/result.png", "/workspace/project", "/workspace/project/images/result.png"], ["./images/result.png", "/workspace/project", "/workspace/project/./images/result.png"], diff --git a/packages/client-runtime/src/markdownImages.ts b/packages/client-runtime/src/markdownImages.ts index 404f828390ac..1acd9e9fe721 100644 --- a/packages/client-runtime/src/markdownImages.ts +++ b/packages/client-runtime/src/markdownImages.ts @@ -1,9 +1,13 @@ +import { isGitHubUserAttachmentUrl } from "@t3tools/contracts"; + const DIRECT_IMAGE_SOURCE_PATTERN = /^(?:https?:|data:|blob:|\/\/)/i; const URI_SCHEME_PATTERN = /^[A-Za-z][A-Za-z0-9+.-]*:/; const WINDOWS_DRIVE_PATH_PATTERN = /^[A-Za-z]:[\\/]/; export type MarkdownImageSource = | { readonly _tag: "Direct"; readonly uri: string } + /** GitHub serves these only to an authenticated viewer; load through the signed asset proxy. */ + | { readonly _tag: "GitHubAttachment"; readonly url: string } | { readonly _tag: "WorkspaceFile"; readonly path: string } | { readonly _tag: "Blocked" }; @@ -76,7 +80,9 @@ export function classifyMarkdownImageSource( return { _tag: "Blocked" }; } if (DIRECT_IMAGE_SOURCE_PATTERN.test(source)) { - return { _tag: "Direct", uri: source }; + return isGitHubUserAttachmentUrl(source) + ? { _tag: "GitHubAttachment", url: source } + : { _tag: "Direct", uri: source }; } if (/^file:/i.test(source)) { diff --git a/packages/contracts/src/assets.test.ts b/packages/contracts/src/assets.test.ts index ce4214d300da..11273b73e01b 100644 --- a/packages/contracts/src/assets.test.ts +++ b/packages/contracts/src/assets.test.ts @@ -1,7 +1,7 @@ import * as Schema from "effect/Schema"; import { describe, expect, it } from "vite-plus/test"; -import { AttachmentCreateUploadUrlInput } from "./assets.ts"; +import { AttachmentCreateUploadUrlInput, isGitHubUserAttachmentUrl } from "./assets.ts"; import { PROVIDER_SEND_TURN_MAX_IMAGE_BYTES } from "./orchestration.ts"; const isUploadInput = Schema.is(AttachmentCreateUploadUrlInput); @@ -28,3 +28,22 @@ describe("AttachmentCreateUploadUrlInput", () => { ).toBe(false); }); }); + +describe("isGitHubUserAttachmentUrl", () => { + it("accepts exactly GitHub user-attachment asset URLs", () => { + expect( + isGitHubUserAttachmentUrl( + "https://github.com/user-attachments/assets/4dcab2ba-0674-4d3b-a3a7-3546601b1550", + ), + ).toBe(true); + }); + + it.each([ + "https://github.com.evil.example.com/user-attachments/assets/a", + "https://github.com/user-attachments/assets/a/../../login", + "https://github.com/user-attachments/assets/a?next=b", + "https://github.com/owner/repo/blob/main/a.png", + ])("rejects %s", (url) => { + expect(isGitHubUserAttachmentUrl(url)).toBe(false); + }); +}); diff --git a/packages/contracts/src/assets.ts b/packages/contracts/src/assets.ts index bfc2c9472aaa..ecb513363523 100644 --- a/packages/contracts/src/assets.ts +++ b/packages/contracts/src/assets.ts @@ -9,6 +9,14 @@ import { const ASSET_PATH_MAX_LENGTH = 1024; +const GITHUB_USER_ATTACHMENT_URL_PATTERN = + /^https:\/\/github\.com\/user-attachments\/assets\/[\w-]+$/iu; + +/** GitHub serves these only to an authenticated viewer; load them through the signed asset proxy. */ +export function isGitHubUserAttachmentUrl(value: string): boolean { + return GITHUB_USER_ATTACHMENT_URL_PATTERN.test(value.trim()); +} + export const AssetResource = Schema.Union([ Schema.TaggedStruct("workspace-file", { threadId: ThreadId, @@ -23,6 +31,9 @@ export const AssetResource = Schema.Union([ // project projection before it issues the signed URL. path: Schema.optional(ProjectFaviconPath), }), + Schema.TaggedStruct("github-attachment", { + url: TrimmedNonEmptyString.check(Schema.isMaxLength(2048)), + }), ]); export type AssetResource = typeof AssetResource.Type; @@ -226,6 +237,17 @@ export class AssetSigningKeyLoadError extends Schema.TaggedErrorClass()( + "AssetRemoteUrlValidationError", + { + resource: AssetResource, + }, +) { + override get message(): string { + return "Only GitHub attachment URLs can be proxied."; + } +} + export const AssetAccessError = Schema.Union([ AssetWorkspaceContextNotFoundError, AssetWorkspaceContextResolutionError, @@ -240,5 +262,6 @@ export const AssetAccessError = Schema.Union([ AssetProjectFaviconInspectionError, AssetProjectFaviconNotFoundError, AssetSigningKeyLoadError, + AssetRemoteUrlValidationError, ]); export type AssetAccessError = typeof AssetAccessError.Type; From 1a1253576089526c235b2a60ebd0b2af2cfad5d3 Mon Sep 17 00:00:00 2001 From: Xanacas <53140620+Xanacas@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:46:49 +0200 Subject: [PATCH 2/5] fix(server): provide the attachment proxy where route fibers resolve it Route handlers resolve services at serve time, so providing the proxy to makeRoutesLayer never reached the request fiber and every proxied image answered 404. Caught in an integrated web pass; unit tests could not see it. Co-Authored-By: Claude Fable 5 --- apps/server/src/server.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index cf4f2d0d36e0..ee8327a2c1db 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -480,7 +480,6 @@ export const makeRoutesLayer = Layer.mergeAll( // Both transports consume the same service instance, so caches single-flight across clients // and mutations observed on WebSocket invalidate patches subsequently read over HTTP. Layer.provide(PullRequestServiceLive), - Layer.provide(GitHubAttachmentProxyLive), Layer.provide(PreviewAutomationBroker.layer), Layer.provide(ServerSelfUpdate.layer), Layer.provide(commandReadinessLayer), @@ -691,6 +690,9 @@ export const makeServerLayer = Layer.unwrap( return serverApplicationLayer.pipe( Layer.provideMerge(runtimeServicesLive), + // Route handlers resolve services at serve time, so the proxy must sit in the runtime + // context here — providing it to makeRoutesLayer never reaches the request fiber. + Layer.provide(GitHubAttachmentProxyLive), Layer.provide(activationLayer), Layer.provideMerge(serverRelayBrokerTracingLayer), Layer.provideMerge(HttpServerLive), From bd78fb85fae0a1f3212ab321a0846c29c1aedc8f Mon Sep 17 00:00:00 2001 From: Xanacas <53140620+Xanacas@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:56:15 +0200 Subject: [PATCH 3/5] test(server): cover GitHubAttachmentProxy redirect resolution Stub GitHubCli and HttpClient layers cover the token header and its cached read, the anonymous fallback when gh has no token, and the null answers for non-redirect responses and non-https locations. Co-Authored-By: Claude Fable 5 --- .../src/assets/GitHubAttachmentProxy.test.ts | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 apps/server/src/assets/GitHubAttachmentProxy.test.ts diff --git a/apps/server/src/assets/GitHubAttachmentProxy.test.ts b/apps/server/src/assets/GitHubAttachmentProxy.test.ts new file mode 100644 index 000000000000..acb70a239b19 --- /dev/null +++ b/apps/server/src/assets/GitHubAttachmentProxy.test.ts @@ -0,0 +1,154 @@ +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { describe, expect, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as HttpClient from "effect/unstable/http/HttpClient"; +import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; + +import { ExitCode } from "effect/unstable/process/ChildProcessSpawner"; + +import * as ServerConfig from "../config.ts"; +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import * as GitHubAttachmentProxy from "./GitHubAttachmentProxy.ts"; + +const ATTACHMENT_URL = "https://github.com/user-attachments/assets/4dcab2ba"; +const STORAGE_URL = "https://objects.example.test/signed/4dcab2ba"; + +const notImplemented = () => Effect.die("not implemented in this test"); + +const makeGitHubCliLayer = (input: { + readonly calls: Array>; + readonly fail?: boolean; +}) => + Layer.succeed( + GitHubCli.GitHubCli, + GitHubCli.GitHubCli.of({ + execute: ({ args }) => { + input.calls.push(args); + if (input.fail) { + return Effect.fail( + new GitHubCli.GitHubCliUnavailableError({ + command: "gh", + cwd: "/tmp", + cause: "gh is not installed", + }), + ); + } + return Effect.succeed({ + exitCode: ExitCode(0), + stdout: "gh-token-1\n", + stderr: "", + stdoutTruncated: false, + stderrTruncated: false, + }); + }, + listOpenPullRequests: notImplemented, + getPullRequest: notImplemented, + getRepositoryCloneUrls: notImplemented, + createRepository: notImplemented, + createPullRequest: notImplemented, + getDefaultBranch: notImplemented, + checkoutPullRequest: notImplemented, + }), + ); + +type RecordedRequest = { readonly url: string; readonly authorization: string | undefined }; + +const makeHttpClientLayer = (input: { + readonly requests: Array; + readonly status?: number; + readonly location?: string; +}) => + Layer.succeed( + HttpClient.HttpClient, + HttpClient.make((request) => + Effect.sync(() => { + input.requests.push({ url: request.url, authorization: request.headers["authorization"] }); + return HttpClientResponse.fromWeb( + request, + new Response(null, { + status: input.status ?? 302, + headers: input.location === undefined ? {} : { location: input.location }, + }), + ); + }), + ), + ); + +const provideProxy = (input: { + readonly calls?: Array>; + readonly requests?: Array; + readonly cliFails?: boolean; + readonly status?: number; + readonly location?: string; +}) => + Effect.provide( + GitHubAttachmentProxy.layer.pipe( + Layer.provide( + makeGitHubCliLayer({ + calls: input.calls ?? [], + ...(input.cliFails !== undefined ? { fail: input.cliFails } : {}), + }), + ), + Layer.provide( + makeHttpClientLayer({ + requests: input.requests ?? [], + ...(input.status !== undefined ? { status: input.status } : {}), + ...(input.location !== undefined ? { location: input.location } : {}), + }), + ), + Layer.provide(ServerConfig.ServerConfig.layerTest(process.cwd(), { prefix: "t3-gh-proxy-" })), + Layer.provideMerge(NodeServices.layer), + ), + ); + +describe("GitHubAttachmentProxy", () => { + it.effect("resolves the redirect with the gh token and caches the token read", () => + Effect.gen(function* () { + const calls: Array> = []; + const requests: Array = []; + yield* Effect.gen(function* () { + const proxy = yield* GitHubAttachmentProxy.GitHubAttachmentProxy; + expect(yield* proxy.resolveAttachmentLocation(ATTACHMENT_URL)).toBe(STORAGE_URL); + expect(yield* proxy.resolveAttachmentLocation(ATTACHMENT_URL)).toBe(STORAGE_URL); + }).pipe(provideProxy({ calls, requests, location: STORAGE_URL })); + + expect(requests.map((request) => request.authorization)).toEqual([ + "token gh-token-1", + "token gh-token-1", + ]); + expect(calls).toEqual([["auth", "token", "--hostname", "github.com"]]); + }), + ); + + it.effect("still resolves anonymously when no gh token is available", () => + Effect.gen(function* () { + const requests: Array = []; + yield* Effect.gen(function* () { + const proxy = yield* GitHubAttachmentProxy.GitHubAttachmentProxy; + expect(yield* proxy.resolveAttachmentLocation(ATTACHMENT_URL)).toBe(STORAGE_URL); + }).pipe(provideProxy({ requests, cliFails: true, location: STORAGE_URL })); + + expect(requests.map((request) => request.authorization)).toEqual([undefined]); + }), + ); + + it.effect("answers null for non-redirect responses and non-https locations", () => + Effect.gen(function* () { + yield* Effect.gen(function* () { + const proxy = yield* GitHubAttachmentProxy.GitHubAttachmentProxy; + expect(yield* proxy.resolveAttachmentLocation(ATTACHMENT_URL)).toBeNull(); + }).pipe(provideProxy({ status: 200, location: STORAGE_URL })); + + yield* Effect.gen(function* () { + const proxy = yield* GitHubAttachmentProxy.GitHubAttachmentProxy; + expect(yield* proxy.resolveAttachmentLocation(ATTACHMENT_URL)).toBeNull(); + }).pipe(provideProxy({ location: "http://plain.example.test/asset" })); + + yield* Effect.gen(function* () { + const proxy = yield* GitHubAttachmentProxy.GitHubAttachmentProxy; + expect(yield* proxy.resolveAttachmentLocation(ATTACHMENT_URL)).toBeNull(); + }).pipe(provideProxy({})); + }), + ); +}); From fbba68ecb6d2cc3779896064f957aefd9995f6e9 Mon Sep 17 00:00:00 2001 From: Xanacas <53140620+Xanacas@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:58:23 +0200 Subject: [PATCH 4/5] fix(clients): fall back to the direct URL when the signed proxy is unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An older server rejects the github-attachment resource during version skew, and a markdown render without an environment has no proxy at all. Both now fall back to loading the GitHub URL directly — public uploads keep working, private ones degrade to the unavailable chip instead of regressing. Co-Authored-By: Claude Fable 5 --- .../src/features/threads/ThreadFeed.tsx | 13 ++++-- apps/web/src/components/ChatMarkdown.tsx | 43 +++++++++++++++---- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/apps/mobile/src/features/threads/ThreadFeed.tsx b/apps/mobile/src/features/threads/ThreadFeed.tsx index 8eba0f9beac2..397d4c083add 100644 --- a/apps/mobile/src/features/threads/ThreadFeed.tsx +++ b/apps/mobile/src/features/threads/ThreadFeed.tsx @@ -324,21 +324,27 @@ function ThreadMarkdownImageRequest(props: { ); } -/** Markdown image whose bytes load through a signed asset URL from the environment. */ +/** + * Markdown image whose bytes load through a signed asset URL from the environment. + * `fallbackUri` is tried directly when the signed URL cannot be issued — an older server + * rejects the `github-attachment` resource, and public uploads still load direct. + */ function ThreadMarkdownImage(props: { readonly environmentId: EnvironmentId; readonly resource: AssetResource; readonly sourceKey: string; readonly alt: string | null; + readonly fallbackUri?: string; readonly onPressImage: (uri: string) => void; }) { const assetUrl = useAssetUrlState(props.environmentId, props.resource); + const fallbackUri = assetUrl._tag === "Failure" ? (props.fallbackUri ?? null) : null; return ( @@ -1626,6 +1632,7 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) { : { _tag: "workspace-file", threadId: props.threadId, path: imageSource.path } } sourceKey={imageSource._tag === "GitHubAttachment" ? imageSource.url : imageSource.path} + {...(imageSource._tag === "GitHubAttachment" ? { fallbackUri: imageSource.url } : {})} alt={image.alt} onPressImage={(uri) => setExpandedImage({ uri })} /> diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 0ea592cf9e6e..8f057c46293c 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1065,19 +1065,21 @@ function ChatMarkdownImageFallback(props: { readonly alt: string }) { ); } -/** Markdown images whose bytes load through a signed asset URL from the environment. */ +/** + * Markdown images whose bytes load through a signed asset URL from the environment. + * `fallbackUrl` is tried directly when the signed URL cannot be issued or fails to load — an + * older server rejects the `github-attachment` resource, and public uploads still load direct. + */ const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props: { readonly environmentId: EnvironmentId; readonly resource: AssetResource; readonly alt: string; + readonly fallbackUrl?: string; }) { const assetUrl = useAssetUrlState(props.environmentId, props.resource); - const [failedUrl, setFailedUrl] = useState(null); + const [failedUrls, setFailedUrls] = useState>([]); - if (assetUrl._tag === "Failure" || (assetUrl._tag === "Success" && failedUrl === assetUrl.url)) { - return ; - } - if (assetUrl._tag !== "Success") { + if (assetUrl._tag === "Loading") { return ( ); } + const signedUrl = assetUrl._tag === "Success" ? assetUrl.url : null; + const src = + signedUrl !== null && !failedUrls.includes(signedUrl) + ? signedUrl + : props.fallbackUrl !== undefined && !failedUrls.includes(props.fallbackUrl) + ? props.fallbackUrl + : null; + if (src === null) { + return ; + } return ( {props.alt} setFailedUrl(assetUrl.url)} + onError={() => setFailedUrls((failed) => (failed.includes(src) ? failed : [...failed, src]))} /> ); }); @@ -2158,12 +2170,25 @@ function ChatMarkdown({ /> ); } - if (imageSource._tag === "GitHubAttachment" && environmentId !== null) { + if (imageSource._tag === "GitHubAttachment") { + if (environmentId === null) { + // No environment to proxy through; public uploads still load directly. + return ( + {altText} + ); + } return ( ); } From 42a2295524a7f8a3b6b11d128bde141ca1eb6cb8 Mon Sep 17 00:00:00 2001 From: Xanacas <53140620+Xanacas@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:07:46 +0200 Subject: [PATCH 5/5] fix(web): keep the standard image fallback on the direct attachment path The no-environment branch rendered a raw img with inline geometry and the browser's broken-image glyph on failure; it now shares the block layout and the unavailable chip of the proxied path. Co-Authored-By: Claude Fable 5 --- apps/web/src/components/ChatMarkdown.tsx | 31 +++++++++++++++++------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 8f057c46293c..5605c4780aa2 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -1110,6 +1110,27 @@ const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props: { ); }); +/** GitHub attachment with no environment to proxy through: direct load, standard fallback. */ +const ChatMarkdownDirectAttachmentImage = memo(function ChatMarkdownDirectAttachmentImage(props: { + readonly url: string; + readonly alt: string; +}) { + const [failed, setFailed] = useState(false); + if (failed) { + return ; + } + return ( + {props.alt} setFailed(true)} + /> + ); +}); + function leadingExternalLinkTextLength(text: string): number { const protocol = /^(?:https?:\/\/)/i.exec(text)?.[0]; if (protocol) return protocol.length; @@ -2173,15 +2194,7 @@ function ChatMarkdown({ if (imageSource._tag === "GitHubAttachment") { if (environmentId === null) { // No environment to proxy through; public uploads still load directly. - return ( - {altText} - ); + return ; } return (