Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions apps/mobile/src/features/threads/ThreadFeed.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 (
<ThreadMarkdownImageView
uri={assetUrl._tag === "Success" ? assetUrl.url : null}
sourceKey={props.path}
unavailable={assetUrl._tag === "Failure"}
uri={assetUrl._tag === "Success" ? assetUrl.url : fallbackUri}
sourceKey={props.sourceKey}
unavailable={assetUrl._tag === "Failure" && fallbackUri === null}
alt={props.alt}
onPressImage={props.onPressImage}
/>
Expand Down Expand Up @@ -1624,8 +1626,13 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) {
return (
<ThreadMarkdownImage
environmentId={props.environmentId}
threadId={props.threadId}
path={imageSource.path}
resource={
imageSource._tag === "GitHubAttachment"
? { _tag: "github-attachment", url: imageSource.url }
: { _tag: "workspace-file", threadId: props.threadId, path: imageSource.path }
}
sourceKey={imageSource._tag === "GitHubAttachment" ? imageSource.url : imageSource.path}
{...(imageSource._tag === "GitHubAttachment" ? { fallbackUri: imageSource.url } : {})}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium threads/ThreadFeed.tsx:1635

When the signed URL issued for a GitHub attachment returns a load error, the image is rendered as Image unavailable even though fallbackUri is loadable. fallbackUri is only selected when useAssetUrlState fails before loading; update ThreadMarkdownImageView to retry fallbackUri from onError before marking the image unavailable, matching the web renderer.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1635:

When the signed URL issued for a GitHub attachment returns a load error, the image is rendered as `Image unavailable` even though `fallbackUri` is loadable. `fallbackUri` is only selected when `useAssetUrlState` fails before loading; update `ThreadMarkdownImageView` to retry `fallbackUri` from `onError` before marking the image unavailable, matching the web renderer.

alt={image.alt}
onPressImage={(uri) => setExpandedImage({ uri })}
/>
Expand Down
17 changes: 17 additions & 0 deletions apps/server/src/assets/AssetAccess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
);
});
40 changes: 35 additions & 5 deletions apps/server/src/assets/AssetAccess.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { AssetResource } from "@t3tools/contracts";
import {
AssetAttachmentNotFoundError,
AssetRemoteUrlValidationError,
isGitHubUserAttachmentUrl,
AssetPreviewTypeValidationError,
AssetProjectFaviconInspectionError,
AssetProjectFaviconNotFoundError,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -94,14 +96,22 @@ 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;

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 {
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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({
Expand Down
154 changes: 154 additions & 0 deletions apps/server/src/assets/GitHubAttachmentProxy.test.ts
Original file line number Diff line number Diff line change
@@ -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<ReadonlyArray<string>>;
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<RecordedRequest>;
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<ReadonlyArray<string>>;
readonly requests?: Array<RecordedRequest>;
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<ReadonlyArray<string>> = [];
const requests: Array<RecordedRequest> = [];
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<RecordedRequest> = [];
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({}));
}),
);
});
Loading
Loading