diff --git a/apps/mobile/src/features/threads/ThreadFeed.tsx b/apps/mobile/src/features/threads/ThreadFeed.tsx index 60b397802ccc..397d4c083add 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,25 +324,27 @@ 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. + * `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 threadId: ThreadId; - readonly path: string; + readonly resource: AssetResource; + readonly sourceKey: string; readonly alt: string | null; + readonly fallbackUri?: string; 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); + const fallbackUri = assetUrl._tag === "Failure" ? (props.fallbackUri ?? null) : null; return ( @@ -1624,8 +1626,13 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) { 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.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({})); + }), + ); +}); 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..ee8327a2c1db 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), @@ -683,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), 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..5605c4780aa2 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,23 +1065,21 @@ 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. + * `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.threadRef.environmentId, { - _tag: "workspace-file", - threadId: props.threadRef.threadId, - path: props.path, - }); - const [failedUrl, setFailedUrl] = useState(null); + const assetUrl = useAssetUrlState(props.environmentId, props.resource); + 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} setFailedUrls((failed) => (failed.includes(src) ? failed : [...failed, src]))} + /> + ); +}); + +/** 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} setFailedUrl(assetUrl.url)} + onError={() => setFailed(true)} /> ); }); @@ -2161,11 +2191,29 @@ function ChatMarkdown({ /> ); } + if (imageSource._tag === "GitHubAttachment") { + if (environmentId === null) { + // No environment to proxy through; public uploads still load directly. + return ; + } + return ( + + ); + } if (imageSource._tag === "WorkspaceFile" && threadRef) { return ( - ); @@ -2211,6 +2259,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;