History

Harden attachment materialization and static file path handling

- Materialize only valid image data URLs and restrict inferred file extensions to a safe whitelist - Preserve existing message attachments on upsert unless explicitly replaced or cleared - Serve static `/` via `index.html` and block traversal attempts in static file requests

Julius Marminge committed 8 months ago

Commit b5188e81 parent 992db167 files · +364 −15Browse files

Unified · Split

7 files
  1. apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts +111 −0
  2. apps/server/src/orchestration/Layers/ProjectionPipeline.ts +26 −5
  3. apps/server/src/persistence/Layers/ProjectionThreadMessages.test.ts +106 −0
  4. apps/server/src/persistence/Layers/ProjectionThreadMessages.ts +17 −5
  5. apps/server/src/wsServer.test.ts +62 −1
  6. apps/server/src/wsServer.ts +36 −3
  7. apps/web/src/store.ts +6 −1
apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts+111−0
@@ -247,6 +247,117 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => { · 246 unmodified lines
247247 }),
248248 );
249249
250+ it.effect("materializes only image data URLs and uses a safe extension whitelist", () =>
251+ Effect.gen(function* () {
252+ const projectionPipeline = yield* OrchestrationProjectionPipeline;
253+ const eventStore = yield* OrchestrationEventStore;
254+ const sql = yield* SqlClient.SqlClient;
255+ const now = new Date().toISOString();
256+ const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-projection-attachments-"));
257+
258+ const serverConfig = {
259+ mode: "web",
260+ port: 0,
261+ host: undefined,
262+ cwd: "/tmp/project-attachments",
263+ keybindingsConfigPath: path.join(stateDir, "keybindings.json"),
264+ stateDir,
265+ staticDir: undefined,
266+ devUrl: undefined,
267+ noBrowser: true,
268+ authToken: undefined,
269+ autoBootstrapProjectFromCwd: false,
270+ logWebSocketEvents: false,
271+ } satisfies ServerConfigShape;
272+
273+ yield* eventStore.append({
274+ type: "thread.message-sent",
275+ eventId: EventId.makeUnsafe("evt-attachments-safe"),
276+ aggregateKind: "thread",
277+ aggregateId: ThreadId.makeUnsafe("thread-attachments-safe"),
278+ occurredAt: now,
279+ commandId: CommandId.makeUnsafe("cmd-attachments-safe"),
280+ causationEventId: null,
281+ correlationId: CommandId.makeUnsafe("cmd-attachments-safe"),
282+ metadata: {},
283+ payload: {
284+ threadId: ThreadId.makeUnsafe("thread-attachments-safe"),
285+ messageId: MessageId.makeUnsafe("message-attachments-safe"),
286+ role: "user",
287+ text: "Inspect this",
288+ attachments: [
289+ {
290+ type: "image",
291+ name: "untrusted.exe",
292+ mimeType: "image/x-unknown",
293+ sizeBytes: 5,
294+ dataUrl: "data:image/x-unknown;base64,SGVsbG8=",
295+ },
296+ {
297+ type: "image",
298+ name: "not-image.png",
299+ mimeType: "image/png",
300+ sizeBytes: 5,
301+ dataUrl: "data:text/plain;base64,SGVsbG8=",
302+ },
303+ ],
304+ turnId: null,
305+ streaming: false,
306+ createdAt: now,
307+ updatedAt: now,
308+ },
309+ });
310+
311+ yield* projectionPipeline.bootstrap.pipe(Effect.provideService(ServerConfig, serverConfig));
312+
313+ const rows = yield* sql<{
314+ readonly attachmentsJson: string | null;
315+ }>`
316+ SELECT
317+ attachments_json AS "attachmentsJson"
318+ FROM projection_thread_messages
319+ WHERE message_id = 'message-attachments-safe'
320+ `;
321+ assert.equal(rows.length, 1);
322+ assert.deepEqual(JSON.parse(rows[0]?.attachmentsJson ?? "null"), [
323+ {
324+ type: "image",
325+ name: "untrusted.exe",
326+ mimeType: "image/x-unknown",
327+ sizeBytes: 5,
328+ dataUrl: "/attachments/thread-attachments-safe/message-attachments-safe/0.bin",
329+ },
330+ {
331+ type: "image",
332+ name: "not-image.png",
333+ mimeType: "image/png",
334+ sizeBytes: 5,
335+ dataUrl: "data:text/plain;base64,SGVsbG8=",
336+ },
337+ ]);
338+
339+ const firstAttachmentPath = path.join(
340+ stateDir,
341+ "attachments",
342+ "thread-attachments-safe",
343+ "message-attachments-safe",
344+ "0.bin",
345+ );
346+ assert.equal(fs.existsSync(firstAttachmentPath), true);
347+ assert.deepEqual(fs.readFileSync(firstAttachmentPath), Buffer.from("SGVsbG8=", "base64"));
348+
349+ const secondAttachmentPath = path.join(
350+ stateDir,
351+ "attachments",
352+ "thread-attachments-safe",
353+ "message-attachments-safe",
354+ "1.png",
355+ );
356+ assert.equal(fs.existsSync(secondAttachmentPath), false);
357+ fs.rmSync(stateDir, { recursive: true, force: true });
358+ }),
359+ );
360+
250361 it.effect("resumes from projector last_applied_sequence without replaying older events", () =>
251362 Effect.gen(function* () {
252363 const projectionPipeline = yield* OrchestrationProjectionPipeline;
apps/server/src/orchestration/Layers/ProjectionPipeline.ts+26−5
@@ -70,6 +70,17 @@ const IMAGE_EXTENSION_BY_MIME_TYPE: Record<string, string> = { · 69 unmodified lines
7070 "image/tiff": ".tiff",
7171 "image/heic": ".heic",
7272 };
73+const SAFE_IMAGE_FILE_EXTENSIONS = new Set([
74+ ".jpg",
75+ ".jpeg",
76+ ".png",
77+ ".gif",
78+ ".webp",
79+ ".bmp",
80+ ".tiff",
81+ ".svg",
82+ ".ico",
83+]);
7384
7485 function parseBase64DataUrl(
7586 dataUrl: string,
@@ -87,14 +98,18 @@ function parseBase64DataUrl( · 11 unmodified lines
8798 }
8899
89100 function inferImageExtension(attachment: Extract<ChatAttachment, { type: "image" }>): string {
90− const fromMime = IMAGE_EXTENSION_BY_MIME_TYPE[attachment.mimeType.toLowerCase()];
91− if (fromMime) {
92− return fromMime;
101+ const normalizedMimeType = attachment.mimeType.trim().toLowerCase();
102+ if (normalizedMimeType.startsWith("image/")) {
103+ const fromMime = IMAGE_EXTENSION_BY_MIME_TYPE[normalizedMimeType];
104+ if (fromMime) {
105+ return fromMime;
106+ }
93107 }
108+
94109 const fromName = attachment.name.includes(".")
95110 ? `.${attachment.name.split(".").slice(-1)[0]?.toLowerCase() ?? ""}`
96111 : "";
97− if (/^\.[a-z0-9]{1,12}$/.test(fromName)) {
112+ if (/^\.[a-z0-9]{1,12}$/.test(fromName) && SAFE_IMAGE_FILE_EXTENSIONS.has(fromName)) {
98113 return fromName;
99114 }
100115 return ".bin";
@@ -132,13 +147,19 @@ const materializeAttachmentsForProjection = Effect.fn(function* (input: { · 31 unmodified lines
132147 if (!parsed) {
133148 return attachment;
134149 }
150+ if (!parsed.mimeType.startsWith("image/")) {
151+ return attachment;
152+ }
135153
136154 const bytes = Buffer.from(parsed.base64, "base64");
137155 if (bytes.byteLength === 0) {
138156 return attachment;
139157 }
140158
141− const fileName = `${index}${inferImageExtension(attachment)}`;
159+ const fileName = `${index}${inferImageExtension({
160+ ...attachment,
161+ mimeType: parsed.mimeType,
162+ })}`;
142163 const relativePath = `${threadSegment}/${messageSegment}/${fileName}`;
143164 const absolutePath = path.join(attachmentsRootDir, threadSegment, messageSegment, fileName);
144165 yield* fileSystem.makeDirectory(path.dirname(absolutePath), { recursive: true });
apps/server/src/persistence/Layers/ProjectionThreadMessages.test.ts+106−0
@@ -0,0 +1,106 @@
1+import { MessageId, ThreadId } from "@t3tools/contracts";
2+import { assert, it } from "@effect/vitest";
3+import { Effect, Layer } from "effect";
4+
5+import { ProjectionThreadMessageRepository } from "../Services/ProjectionThreadMessages.ts";
6+import { ProjectionThreadMessageRepositoryLive } from "./ProjectionThreadMessages.ts";
7+import { SqlitePersistenceMemory } from "./Sqlite.ts";
8+
9+const layer = it.layer(
10+ ProjectionThreadMessageRepositoryLive.pipe(Layer.provideMerge(SqlitePersistenceMemory)),
11+);
12+
13+layer("ProjectionThreadMessageRepository", (it) => {
14+ it.effect("preserves existing attachments when upsert omits attachments", () =>
15+ Effect.gen(function* () {
16+ const repository = yield* ProjectionThreadMessageRepository;
17+ const threadId = ThreadId.makeUnsafe("thread-preserve-attachments");
18+ const messageId = MessageId.makeUnsafe("message-preserve-attachments");
19+ const createdAt = "2026-02-28T19:00:00.000Z";
20+ const updatedAt = "2026-02-28T19:00:01.000Z";
21+ const persistedAttachments = [
22+ {
23+ type: "image" as const,
24+ name: "example.png",
25+ mimeType: "image/png",
26+ sizeBytes: 5,
27+ dataUrl: "/attachments/thread-preserve-attachments/message-preserve-attachments/0.png",
28+ },
29+ ];
30+
31+ yield* repository.upsert({
32+ messageId,
33+ threadId,
34+ turnId: null,
35+ role: "user",
36+ text: "initial",
37+ attachments: persistedAttachments,
38+ isStreaming: false,
39+ createdAt,
40+ updatedAt,
41+ });
42+
43+ yield* repository.upsert({
44+ messageId,
45+ threadId,
46+ turnId: null,
47+ role: "user",
48+ text: "updated",
49+ isStreaming: false,
50+ createdAt,
51+ updatedAt: "2026-02-28T19:00:02.000Z",
52+ });
53+
54+ const rows = yield* repository.listByThreadId({ threadId });
55+ assert.equal(rows.length, 1);
56+ assert.equal(rows[0]?.text, "updated");
57+ assert.deepEqual(rows[0]?.attachments, persistedAttachments);
58+ }),
59+ );
60+
61+ it.effect("allows explicit attachment clearing with an empty array", () =>
62+ Effect.gen(function* () {
63+ const repository = yield* ProjectionThreadMessageRepository;
64+ const threadId = ThreadId.makeUnsafe("thread-clear-attachments");
65+ const messageId = MessageId.makeUnsafe("message-clear-attachments");
66+ const createdAt = "2026-02-28T19:10:00.000Z";
67+
68+ yield* repository.upsert({
69+ messageId,
70+ threadId,
71+ turnId: null,
72+ role: "assistant",
73+ text: "with attachment",
74+ attachments: [
75+ {
76+ type: "image",
77+ name: "example.png",
78+ mimeType: "image/png",
79+ sizeBytes: 5,
80+ dataUrl: "/attachments/thread-clear-attachments/message-clear-attachments/0.png",
81+ },
82+ ],
83+ isStreaming: false,
84+ createdAt,
85+ updatedAt: "2026-02-28T19:10:01.000Z",
86+ });
87+
88+ yield* repository.upsert({
89+ messageId,
90+ threadId,
91+ turnId: null,
92+ role: "assistant",
93+ text: "cleared",
94+ attachments: [],
95+ isStreaming: false,
96+ createdAt,
97+ updatedAt: "2026-02-28T19:10:02.000Z",
98+ });
99+
100+ const rows = yield* repository.listByThreadId({ threadId });
101+ assert.equal(rows.length, 1);
102+ assert.equal(rows[0]?.text, "cleared");
103+ assert.deepEqual(rows[0]?.attachments, []);
104+ }),
105+ );
106+});
apps/server/src/persistence/Layers/ProjectionThreadMessages.ts+17−5
@@ -23,8 +23,9 @@ const makeProjectionThreadMessageRepository = Effect.gen(function* () { · 22 unmodified lines
2323
2424 const upsertProjectionThreadMessageRow = SqlSchema.void({
2525 Request: ProjectionThreadMessage,
26− execute: (row) =>
27− sql`
26+ execute: (row) => {
27+ const nextAttachmentsJson = row.attachments !== undefined ? JSON.stringify(row.attachments) : null;
28+ return sql`
2829 INSERT INTO projection_thread_messages (
2930 message_id,
3031 thread_id,
@@ -42,7 +43,14 @@ const makeProjectionThreadMessageRepository = Effect.gen(function* () { · 11 unmodified lines
4243 ${row.turnId},
4344 ${row.role},
4445 ${row.text},
45− ${row.attachments !== undefined ? JSON.stringify(row.attachments) : null},
46+ COALESCE(
47+ ${nextAttachmentsJson},
48+ (
49+ SELECT attachments_json
50+ FROM projection_thread_messages
51+ WHERE message_id = ${row.messageId}
52+ )
53+ ),
4654 ${row.isStreaming ? 1 : 0},
4755 ${row.createdAt},
4856 ${row.updatedAt}
@@ -53,11 +61,15 @@ const makeProjectionThreadMessageRepository = Effect.gen(function* () { · 4 unmodified lines
5361 turn_id = excluded.turn_id,
5462 role = excluded.role,
5563 text = excluded.text,
56− attachments_json = excluded.attachments_json,
64+ attachments_json = COALESCE(
65+ excluded.attachments_json,
66+ projection_thread_messages.attachments_json
67+ ),
5768 is_streaming = excluded.is_streaming,
5869 created_at = excluded.created_at,
5970 updated_at = excluded.updated_at
60− `,
71+ `;
72+ },
6173 });
6274
6375 const listProjectionThreadMessageRows = SqlSchema.findAll({
apps/server/src/wsServer.test.ts+62−1
@@ -296,6 +296,36 @@ async function waitForPush( · 295 unmodified lines
296296 return take(maxMessages);
297297 }
298298
299+async function requestPath(
300+ port: number,
301+ requestPath: string,
302+): Promise<{ statusCode: number; body: string }> {
303+ return new Promise((resolve, reject) => {
304+ const req = Http.request(
305+ {
306+ hostname: "127.0.0.1",
307+ port,
308+ path: requestPath,
309+ method: "GET",
310+ },
311+ (res) => {
312+ const chunks: Buffer[] = [];
313+ res.on("data", (chunk) => {
314+ chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
315+ });
316+ res.on("end", () => {
317+ resolve({
318+ statusCode: res.statusCode ?? 0,
319+ body: Buffer.concat(chunks).toString("utf8"),
320+ });
321+ });
322+ },
323+ );
324+ req.once("error", reject);
325+ req.end();
326+ });
327+}
328+
299329 function compileKeybindings(bindings: KeybindingsConfig): ResolvedKeybindingsConfig {
300330 const resolved: Array<ResolvedKeybindingsConfig[number]> = [];
301331 for (const binding of bindings) {
@@ -331,6 +361,7 @@ describe("WebSocket Server", () => { · 29 unmodified lines
331361 devUrl?: string;
332362 authToken?: string;
333363 stateDir?: string;
364+ staticDir?: string;
334365 providerLayer?: Layer.Layer<ProviderService, never>;
335366 open?: OpenShape;
336367 gitManager?: GitManagerShape;
@@ -354,7 +385,7 @@ describe("WebSocket Server", () => { · 17 unmodified lines
354385 cwd: options.cwd ?? "/test/project",
355386 keybindingsConfigPath: path.join(stateDir, "keybindings.json"),
356387 stateDir,
357− staticDir: undefined,
388+ staticDir: options.staticDir,
358389 devUrl: options.devUrl ? new URL(options.devUrl) : undefined,
359390 noBrowser: true,
360391 authToken: options.authToken,
@@ -456,6 +487,36 @@ describe("WebSocket Server", () => { · 95 unmodified lines
456487 expect(bytes).toEqual(Buffer.from("hello-attachment"));
457488 });
458489
490+ it("serves static index for root path", async () => {
491+ const stateDir = makeTempDir("t3code-state-static-root-");
492+ const staticDir = makeTempDir("t3code-static-root-");
493+ fs.writeFileSync(path.join(staticDir, "index.html"), "<h1>static-root</h1>", "utf8");
494+
495+ server = await createTestServer({ cwd: "/test/project", stateDir, staticDir });
496+ const addr = server.address();
497+ const port = typeof addr === "object" && addr !== null ? addr.port : 0;
498+ expect(port).toBeGreaterThan(0);
499+
500+ const response = await fetch(`http://127.0.0.1:${port}/`);
501+ expect(response.status).toBe(200);
502+ expect(await response.text()).toContain("static-root");
503+ });
504+
505+ it("rejects static path traversal attempts", async () => {
506+ const stateDir = makeTempDir("t3code-state-static-traversal-");
507+ const staticDir = makeTempDir("t3code-static-traversal-");
508+ fs.writeFileSync(path.join(staticDir, "index.html"), "<h1>safe</h1>", "utf8");
509+
510+ server = await createTestServer({ cwd: "/test/project", stateDir, staticDir });
511+ const addr = server.address();
512+ const port = typeof addr === "object" && addr !== null ? addr.port : 0;
513+ expect(port).toBeGreaterThan(0);
514+
515+ const response = await requestPath(port, "/..%2f..%2fetc/passwd");
516+ expect(response.statusCode).toBe(400);
517+ expect(response.body).toBe("Invalid static file path");
518+ });
519+
459520 it("bootstraps the cwd project on startup when enabled", async () => {
460521 server = await createTestServer({
461522 cwd: "/test/bootstrap-workspace",
apps/server/src/wsServer.ts+36−3
@@ -315,17 +315,50 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< · 314 unmodified lines
315315 return;
316316 }
317317
318− let filePath = path.join(staticDir, url.pathname);
318+ const staticRoot = path.resolve(staticDir);
319+ const staticRequestPath = url.pathname === "/" ? "/index.html" : url.pathname;
320+ const rawStaticRelativePath = staticRequestPath.replace(/^[/\\]+/, "");
321+ const hasRawLeadingParentSegment = rawStaticRelativePath.startsWith("..");
322+ const staticRelativePath = path
323+ .normalize(rawStaticRelativePath)
324+ .replace(/^[/\\]+/, "");
325+ const hasPathTraversalSegment = staticRelativePath.startsWith("..");
326+ if (
327+ staticRelativePath.length === 0 ||
328+ hasRawLeadingParentSegment ||
329+ hasPathTraversalSegment ||
330+ staticRelativePath.includes("\0")
331+ ) {
332+ respond(400, { "Content-Type": "text/plain" }, "Invalid static file path");
333+ return;
334+ }
335+
336+ const isWithinStaticRoot = (candidate: string) =>
337+ candidate === staticRoot ||
338+ candidate.startsWith(
339+ staticRoot.endsWith(path.sep) ? staticRoot : `${staticRoot}${path.sep}`,
340+ );
341+
342+ let filePath = path.resolve(staticRoot, staticRelativePath);
343+ if (!isWithinStaticRoot(filePath)) {
344+ respond(400, { "Content-Type": "text/plain" }, "Invalid static file path");
345+ return;
346+ }
347+
319348 const ext = path.extname(filePath);
320349 if (!ext) {
321− filePath = path.join(filePath, "index.html");
350+ filePath = path.resolve(filePath, "index.html");
351+ if (!isWithinStaticRoot(filePath)) {
352+ respond(400, { "Content-Type": "text/plain" }, "Invalid static file path");
353+ return;
354+ }
322355 }
323356
324357 const fileInfo = yield* fileSystem
325358 .stat(filePath)
326359 .pipe(Effect.catch(() => Effect.succeed(null)));
327360 if (!fileInfo || fileInfo.type !== "File") {
328− const indexPath = path.join(staticDir, "index.html");
361+ const indexPath = path.resolve(staticRoot, "index.html");
329362 const indexData = yield* fileSystem
330363 .readFile(indexPath)
331364 .pipe(Effect.catch(() => Effect.succeed(null)));
apps/web/src/store.ts+6−1
@@ -210,7 +210,12 @@ function resolveWsHttpOrigin(): string { · 209 unmodified lines
210210
211211 try {
212212 const wsUrl = new URL(wsCandidate);
213− const protocol = wsUrl.protocol === "wss:" ? "https:" : "http:";
213+ const protocol =
214+ wsUrl.protocol === "wss:"
215+ ? "https:"
216+ : wsUrl.protocol === "ws:"
217+ ? "http:"
218+ : wsUrl.protocol;
214219 return `${protocol}//${wsUrl.host}`;
215220 } catch {
216221 return window.location.origin;