diff --git a/apps/server/src/git/Layers/CodexTextGeneration.test.ts b/apps/server/src/git/Layers/CodexTextGeneration.test.ts index 2ebde70e3f6bf80f67f1467af86daf0cb065b9b4..36be39f8df5ae95641fcfa5677cd8d25a79444ba 100644 --- a/apps/server/src/git/Layers/CodexTextGeneration.test.ts +++ b/apps/server/src/git/Layers/CodexTextGeneration.test.ts @@ -349,6 +349,15 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => { }, ], }) + .pipe( + Effect.tap(() => + fs.stat(imagePath).pipe( + Effect.map((fileInfo) => { + expect(fileInfo.type).toBe("File"); + }), + ), + ), + ) .pipe( Effect.ensuring( fs.remove(imagePath).pipe(Effect.catch(() => Effect.void)), @@ -360,6 +369,54 @@ it.layer(CodexTextGenerationTestLayer)("CodexTextGenerationLive", (it) => { ), ); + it.effect("ignores absolute non-route attachment paths for codex image inputs", () => + withFakeCodexEnv( + { + output: JSON.stringify({ + branch: "fix/ui-regression", + }), + requireImage: true, + }, + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const tempDir = yield* fs.makeTempDirectoryScoped({ + prefix: "t3code-codex-attachment-bypass-", + }); + const absoluteImagePath = path.join(tempDir, "outside.png"); + yield* fs.writeFile(absoluteImagePath, Buffer.from("hello")); + + const textGeneration = yield* TextGeneration; + const result = yield* textGeneration + .generateBranchName({ + cwd: process.cwd(), + message: "Fix layout bug from screenshot.", + attachments: [ + { + type: "image", + name: "outside.png", + mimeType: "image/png", + sizeBytes: 5, + dataUrl: absoluteImagePath, + }, + ], + }) + .pipe( + Effect.match({ + onFailure: (error) => ({ _tag: "Left" as const, left: error }), + onSuccess: (value) => ({ _tag: "Right" as const, right: value }), + }), + ); + + expect(result._tag).toBe("Left"); + if (result._tag === "Left") { + expect(result.left).toBeInstanceOf(TextGenerationError); + expect(result.left.message).toContain("missing --image input"); + } + }), + ), + ); + it.effect( "fails with typed TextGenerationError when codex returns wrong branch payload shape", () => diff --git a/apps/server/src/git/Layers/CodexTextGeneration.ts b/apps/server/src/git/Layers/CodexTextGeneration.ts index a5a161d84e1843915d9b9e737bfab0e50df02a31..93416ea3dc0417cf9de0be15357db811355fbb6b 100644 --- a/apps/server/src/git/Layers/CodexTextGeneration.ts +++ b/apps/server/src/git/Layers/CodexTextGeneration.ts @@ -107,6 +107,11 @@ const makeCodexTextGeneration = Effect.gen(function* () { const commandSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; const serverConfig = yield* Effect.service(ServerConfig); + type MaterializedImageAttachments = { + readonly imagePaths: ReadonlyArray; + readonly tempImagePaths: ReadonlyArray; + }; + const readStreamAsString = ( operation: string, stream: Stream.Stream, @@ -176,10 +181,12 @@ const makeCodexTextGeneration = Effect.gen(function* () { const materializeImageAttachments = ( operation: "generateCommitMessage" | "generatePrContent" | "generateBranchName", attachments: BranchNameGenerationInput["attachments"], - ): Effect.Effect, TextGenerationError> => - Effect.gen(function* () { + ): Effect.Effect => { + const tempImagePaths: string[] = []; + + return Effect.gen(function* () { if (!attachments || attachments.length === 0) { - return []; + return { imagePaths: [], tempImagePaths }; } const imagePaths: string[] = []; @@ -206,6 +213,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { extension, ); imagePaths.push(imagePath); + tempImagePaths.push(imagePath); continue; } @@ -213,20 +221,26 @@ const makeCodexTextGeneration = Effect.gen(function* () { stateDir: serverConfig.stateDir, dataUrl: attachment.dataUrl, }); - const localPath = persistedPath ?? attachment.dataUrl; - if (!path.isAbsolute(localPath)) { + if (!persistedPath || !path.isAbsolute(persistedPath)) { continue; } const fileInfo = yield* fileSystem - .stat(localPath) + .stat(persistedPath) .pipe(Effect.catch(() => Effect.succeed(null))); if (!fileInfo || fileInfo.type !== "File") { continue; } - imagePaths.push(localPath); + imagePaths.push(persistedPath); } - return imagePaths; - }); + return { imagePaths, tempImagePaths }; + }).pipe( + Effect.tapError(() => + Effect.all(tempImagePaths.map((filePath) => safeUnlink(filePath)), { + concurrency: "unbounded", + }).pipe(Effect.asVoid), + ), + ); + }; const runCodexJson = ({ operation, @@ -234,12 +248,14 @@ const makeCodexTextGeneration = Effect.gen(function* () { prompt, outputSchemaJson, imagePaths = [], + cleanupPaths = [], }: { operation: "generateCommitMessage" | "generatePrContent" | "generateBranchName"; cwd: string; prompt: string; outputSchemaJson: S; imagePaths?: ReadonlyArray; + cleanupPaths?: ReadonlyArray; }): Effect.Effect => Effect.gen(function* () { const schemaPath = yield* writeTempFile( @@ -313,7 +329,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { }); const cleanup = Effect.all( - [schemaPath, outputPath, ...imagePaths].map((filePath) => safeUnlink(filePath)), + [schemaPath, outputPath, ...cleanupPaths].map((filePath) => safeUnlink(filePath)), { concurrency: "unbounded", }, @@ -438,7 +454,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { const generateBranchName: TextGenerationShape["generateBranchName"] = (input) => { return Effect.gen(function* () { - const imagePaths = yield* materializeImageAttachments( + const { imagePaths, tempImagePaths } = yield* materializeImageAttachments( "generateBranchName", input.attachments, ); @@ -476,6 +492,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { branch: Schema.String, }), imagePaths, + cleanupPaths: tempImagePaths, }); return { diff --git a/apps/server/src/imageMime.test.ts b/apps/server/src/imageMime.test.ts index a420e1f6b8bcf5d2e20f34a76f948ba5ce3ee0fb..ceec282794ebe258403e6dac6cf37dbd7cd22488 100644 --- a/apps/server/src/imageMime.test.ts +++ b/apps/server/src/imageMime.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { parseBase64DataUrl } from "./imageMime.ts"; +import { inferImageExtension, parseBase64DataUrl } from "./imageMime.ts"; describe("imageMime", () => { it("parses base64 data URL with mime type", () => { @@ -24,4 +24,15 @@ describe("imageMime", () => { it("rejects missing mime type", () => { expect(parseBase64DataUrl("data:;base64,SGVsbG8=")).toBeNull(); }); + + it("parses base64 data URL with spaces in payload", () => { + expect(parseBase64DataUrl("data:image/png;base64,SGVs bG8=\n")).toEqual({ + mimeType: "image/png", + base64: "SGVsbG8=", + }); + }); + + it("does not read inherited keys from mime extension map", () => { + expect(inferImageExtension({ mimeType: "constructor" })).toBe(".bin"); + }); }); diff --git a/apps/server/src/imageMime.ts b/apps/server/src/imageMime.ts index 9ecf7615a6eed2fc8e88b17d7daab6023ff40df7..814abbb32c1c483b595a2a0c8e82e3d03718f0db 100644 --- a/apps/server/src/imageMime.ts +++ b/apps/server/src/imageMime.ts @@ -32,7 +32,7 @@ export const SAFE_IMAGE_FILE_EXTENSIONS = new Set([ export function parseBase64DataUrl( dataUrl: string, ): { readonly mimeType: string; readonly base64: string } | null { - const match = /^data:([^,]+),([a-z0-9+/=\r\n]+)$/i.exec(dataUrl.trim()); + const match = /^data:([^,]+),([a-z0-9+/=\r\n ]+)$/i.exec(dataUrl.trim()); if (!match) return null; const headerParts = (match[1] ?? "") @@ -55,7 +55,10 @@ export function parseBase64DataUrl( } export function inferImageExtension(input: { mimeType: string; fileName?: string }): string { - const fromMime = IMAGE_EXTENSION_BY_MIME_TYPE[input.mimeType.toLowerCase()]; + const key = input.mimeType.toLowerCase(); + const fromMime = Object.hasOwn(IMAGE_EXTENSION_BY_MIME_TYPE, key) + ? IMAGE_EXTENSION_BY_MIME_TYPE[key] + : undefined; if (fromMime) { return fromMime; } diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts index a2f24607c2bf545d0fe89f4e013fd451c1402bac..18ce1c648c59a6380d56886cc8e8da8ac050bcc7 100644 --- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts +++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts @@ -1,5 +1,3 @@ -import fs from "node:fs"; - import { type ChatAttachment, CommandId, @@ -14,7 +12,7 @@ import { type ProviderThreadId, type TurnId, } from "@t3tools/contracts"; -import { Cache, Cause, Duration, Effect, Layer, Option, Queue, Stream } from "effect"; +import { Cache, Cause, Duration, Effect, FileSystem, Layer, Option, Queue, Stream } from "effect"; import { resolveThreadWorkspaceCwd } from "../../checkpointing/Utils.ts"; import { @@ -111,6 +109,7 @@ const make = Effect.gen(function* () { const providerService = yield* ProviderService; const git = yield* GitCore; const textGeneration = yield* TextGeneration; + const fileSystem = yield* FileSystem.FileSystem; const serverConfig = yield* Effect.service(ServerConfig); const handledTurnStartKeys = yield* Cache.make({ capacity: HANDLED_TURN_START_KEY_MAX, @@ -333,14 +332,10 @@ const make = Effect.gen(function* () { return attachment; } - const isFile = yield* Effect.sync(() => { - try { - return fs.statSync(resolvedMaterializedPath).isFile(); - } catch { - return false; - } - }); - if (!isFile) { + const fileInfo = yield* fileSystem + .stat(resolvedMaterializedPath) + .pipe(Effect.catch(() => Effect.succeed(null))); + if (!fileInfo || fileInfo.type !== "File") { return attachment; } return {