authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-03-01 10:31:43-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-03-01 20:06:21-08:00
log70c9147c7fa86ada23c55fa17b3a63f6a832f5f4
tree527e9c4750dddece094f02165ce15e3992c671b4
parentc0054cb56f1439c7bfda7771f919bdb7f948bab7

Harden attachment ID resolution and missing-file handling

- Sanitize thread-derived attachment ID segments to safe, bounded tokens - Resolve attachment IDs only when a matching file exists on disk - Return 404 (not 400) for missing attachment IDs in the attachment route - Add attachmentStore tests for ID sanitization and extension-based path lookup

5 files changed, 90 insertions(+), 27 deletions(-)

apps/server/src/attachmentStore.test.ts created+56
...@@ -0,0 +1,56 @@
1import fs from "node:fs";
2import os from "node:os";
3import path from "node:path";
4
5import { describe, expect, it } from "vitest";
6
7import { createAttachmentId, resolveAttachmentPathById } from "./attachmentStore.ts";
8
9describe("attachmentStore", () => {
10 it("sanitizes thread ids when creating attachment ids", () => {
11 const attachmentId = createAttachmentId("thread.folder/unsafe space");
12 expect(attachmentId).toBeTruthy();
13 if (!attachmentId) {
14 return;
15 }
16
17 const [threadSegment] = attachmentId.split("-", 1);
18 expect(threadSegment).toBeTruthy();
19 expect(threadSegment).toMatch(/^[a-z0-9_-]+$/i);
20 expect(threadSegment).not.toContain(".");
21 expect(threadSegment).not.toContain("%");
22 expect(threadSegment).not.toContain("/");
23 });
24
25 it("resolves attachment path by id using the extension that exists on disk", () => {
26 const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3code-attachment-store-"));
27 try {
28 const attachmentId = "thread-1-attachment";
29 const attachmentsDir = path.join(stateDir, "attachments");
30 fs.mkdirSync(attachmentsDir, { recursive: true });
31 const pngPath = path.join(attachmentsDir, `${attachmentId}.png`);
32 fs.writeFileSync(pngPath, Buffer.from("hello"));
33
34 const resolved = resolveAttachmentPathById({
35 stateDir,
36 attachmentId,
37 });
38 expect(resolved).toBe(pngPath);
39 } finally {
40 fs.rmSync(stateDir, { recursive: true, force: true });
41 }
42 });
43
44 it("returns null when no attachment file exists for the id", () => {
45 const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3code-attachment-store-"));
46 try {
47 const resolved = resolveAttachmentPathById({
48 stateDir,
49 attachmentId: "thread-1-missing",
50 });
51 expect(resolved).toBeNull();
52 } finally {
53 fs.rmSync(stateDir, { recursive: true, force: true });
54 }
55 });
56});
apps/server/src/attachmentStore.ts+11-11
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1import { randomUUID } from "node:crypto";1import { randomUUID } from "node:crypto";
2import { existsSync } from "node:fs";
23
3import type { ChatAttachment } from "@t3tools/contracts";4import type { ChatAttachment } from "@t3tools/contracts";
45
...@@ -10,17 +11,17 @@ import {...@@ -10,17 +11,17 @@ import {
10import { inferImageExtension, SAFE_IMAGE_FILE_EXTENSIONS } from "./imageMime.ts";11import { inferImageExtension, SAFE_IMAGE_FILE_EXTENSIONS } from "./imageMime.ts";
1112
12const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"];13const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"];
14const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80;
1315
14function toSafeThreadAttachmentSegment(threadId: string): string | null {16function toSafeThreadAttachmentSegment(threadId: string): string | null {
15 const segment = encodeURIComponent(threadId);17 const segment = threadId
16 if (18 .trim()
17 segment.length === 0 ||19 .replace(/[^a-z0-9_-]+/gi, "-")
18 segment === "." ||20 .replace(/-+/g, "-")
19 segment === ".." ||21 .replace(/^[-_]+|[-_]+$/g, "")
20 segment.includes("/") ||22 .slice(0, ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS)
21 segment.includes("\\") ||23 .replace(/[-_]+$/g, "");
22 segment.includes("\0")24 if (segment.length === 0) {
23 ) {
24 return null;25 return null;
25 }26 }
26 return segment;27 return segment;
...@@ -73,7 +74,7 @@ export function resolveAttachmentPathById(input: {...@@ -73,7 +74,7 @@ export function resolveAttachmentPathById(input: {
73 stateDir: input.stateDir,74 stateDir: input.stateDir,
74 relativePath: `${normalizedId}${extension}`,75 relativePath: `${normalizedId}${extension}`,
75 });76 });
76 if (maybePath) {77 if (maybePath && existsSync(maybePath)) {
77 return maybePath;78 return maybePath;
78 }79 }
79 }80 }
...@@ -92,4 +93,3 @@ export function parseAttachmentIdFromRelativePath(relativePath: string): string...@@ -92,4 +93,3 @@ export function parseAttachmentIdFromRelativePath(relativePath: string): string
92 const id = normalized.slice(0, extensionIndex);93 const id = normalized.slice(0, extensionIndex);
93 return id.length > 0 && !id.includes(".") ? id : null;94 return id.length > 0 && !id.includes(".") ? id : null;
94}95}
95
apps/server/src/git/Layers/GitCore.ts+1-1
...@@ -1167,7 +1167,7 @@ const makeGitCore = Effect.gen(function* () {...@@ -1167,7 +1167,7 @@ const makeGitCore = Effect.gen(function* () {
1167 });1167 });
11681168
1169 // Refresh upstream refs in the background so checkout remains responsive.1169 // Refresh upstream refs in the background so checkout remains responsive.
1170 yield* Effect.forkScoped(1170 yield* Effect.forkDetach(
1171 refreshCheckedOutBranchUpstream(input.cwd).pipe(Effect.catch(() => Effect.void)),1171 refreshCheckedOutBranchUpstream(input.cwd).pipe(Effect.catch(() => Effect.void)),
1172 );1172 );
1173 });1173 });
apps/server/src/provider/Layers/CodexAdapter.ts+6-4
...@@ -489,10 +489,12 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) =>...@@ -489,10 +489,12 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) =>
489 attachment,489 attachment,
490 });490 });
491 if (!attachmentPath) {491 if (!attachmentPath) {
492 return yield* toRequestError(492 return yield* Effect.fail(
493 input.sessionId,493 toRequestError(
494 "turn/start",494 input.sessionId,
495 new Error(`Invalid attachment id '${attachment.id}'.`),495 "turn/start",
496 new Error(`Invalid attachment id '${attachment.id}'.`),
497 ),
496 );498 );
497 }499 }
498 const bytes = yield* fileSystem.readFile(attachmentPath).pipe(500 const bytes = yield* fileSystem.readFile(attachmentPath).pipe(
apps/server/src/wsServer.ts+16-11
...@@ -348,18 +348,23 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return<...@@ -348,18 +348,23 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return<
348 return;348 return;
349 }349 }
350350
351 const filePath =351 const isIdLookup =
352 !normalizedRelativePath.includes("/") && !normalizedRelativePath.includes(".")352 !normalizedRelativePath.includes("/") && !normalizedRelativePath.includes(".");
353 ? resolveAttachmentPathById({353 const filePath = isIdLookup
354 stateDir: serverConfig.stateDir,354 ? resolveAttachmentPathById({
355 attachmentId: normalizedRelativePath,355 stateDir: serverConfig.stateDir,
356 })356 attachmentId: normalizedRelativePath,
357 : resolveAttachmentRelativePath({357 })
358 stateDir: serverConfig.stateDir,358 : resolveAttachmentRelativePath({
359 relativePath: normalizedRelativePath,359 stateDir: serverConfig.stateDir,
360 });360 relativePath: normalizedRelativePath,
361 });
361 if (!filePath) {362 if (!filePath) {
362 respond(400, { "Content-Type": "text/plain" }, "Invalid attachment path");363 respond(
364 isIdLookup ? 404 : 400,
365 { "Content-Type": "text/plain" },
366 isIdLookup ? "Not Found" : "Invalid attachment path",
367 );
363 return;368 return;
364 }369 }
365370