diff --git a/hub/src/feishu/fileDelivery.ts b/hub/src/feishu/fileDelivery.ts index d48b19d..1c0665b 100644 --- a/hub/src/feishu/fileDelivery.ts +++ b/hub/src/feishu/fileDelivery.ts @@ -1,4 +1,4 @@ -import { extname, isAbsolute, join } from "node:path"; +import { isAbsolute, join, relative, resolve, sep } from "node:path"; import { WorkspaceFileBoundaryError, readWorkspaceFileNoFollow, @@ -10,39 +10,21 @@ export interface DeliverableFile { readonly data: Buffer; } -const DELIVERABLE_EXTENSIONS = new Set([ - ".csv", - ".doc", - ".docx", - ".gif", - ".jpeg", - ".jpg", - ".md", - ".pdf", - ".png", - ".ppt", - ".pptx", - ".svg", - ".txt", - ".typ", - ".xls", - ".xlsx", - ".zip", -]); +const DELIVERABLE_CPH_DIRECTORY = "inbox"; export async function resolveDeliverableFile( requestedPath: string, workspaceRoot: string, workspaceDir: string, + maxBytes?: number, ): Promise { const token = requestedPath.trim(); - if (token === "" || !DELIVERABLE_EXTENSIONS.has(extname(token).toLowerCase())) { - return null; - } + if (token === "") return null; for (const candidate of candidatePaths(token)) { + assertNotPlatformRuntimePath(candidate, workspaceDir); try { - return await readWorkspaceFileNoFollow(workspaceRoot, workspaceDir, candidate); + return await readWorkspaceFileNoFollow(workspaceRoot, workspaceDir, candidate, maxBytes); } catch (error) { if (!(error instanceof WorkspaceFileBoundaryError) || error.reason !== "not_found") { throw error; @@ -54,6 +36,20 @@ export async function resolveDeliverableFile( return null; } +function assertNotPlatformRuntimePath(candidate: string, workspaceDir: string): void { + const workspace = resolve(workspaceDir); + const target = isAbsolute(candidate) ? resolve(candidate) : resolve(workspace, candidate); + const rel = relative(workspace, target); + const components = rel.split(sep); + if (components[0] === ".cph" && components[1] !== DELIVERABLE_CPH_DIRECTORY) { + throw new WorkspaceFileBoundaryError( + `platform runtime files cannot be delivered: ${candidate}`, + candidate, + "boundary", + ); + } +} + function candidatePaths(token: string): string[] { if (isAbsolute(token)) return [token]; const candidates = [token]; diff --git a/hub/src/feishu/fileDeliveryTool.ts b/hub/src/feishu/fileDeliveryTool.ts index 1fc4672..795bfa3 100644 --- a/hub/src/feishu/fileDeliveryTool.ts +++ b/hub/src/feishu/fileDeliveryTool.ts @@ -15,6 +15,7 @@ export interface FileDeliveryToolOptions { readonly runId: string; readonly workspaceRoot?: string | undefined; readonly workspaceDir: string; + readonly maxFileBytes?: number | undefined; readonly sendOptions?: SendMessageOptions | undefined; readonly approvalManager: ApprovalManager; readonly onDelivered?: (path: string) => void; @@ -29,7 +30,7 @@ export function createFileDeliveryMcpServer(options: FileDeliveryToolOptions): M tools.push( tool( "send_file", - "Upload an existing file from the current project workspace to the current Feishu chat. The path must point to a concrete no-symlink file, for example build/student.pdf or README.md.", + "Upload any existing regular file from the current project workspace to the current Feishu chat. The path must point to a concrete no-symlink file and must not be inside platform runtime directories.", { path: z.string().describe("Workspace-relative path, or an absolute path physically inside the current workspace."), name: z.string().optional().describe("Optional display filename. Defaults to the file's basename."), @@ -46,12 +47,18 @@ export function createFileDeliveryMcpServer(options: FileDeliveryToolOptions): M content: [{ type: "text", text: "File delivery is unavailable because workspace isolation is not configured." }], }; } + if (options.maxFileBytes === undefined) { + throw new Error("Agent file delivery requires a configured maximum file size"); + } let file; try { - file = await resolveDeliverableFile(args.path, workspaceRoot, options.workspaceDir); + file = await resolveDeliverableFile(args.path, workspaceRoot, options.workspaceDir, options.maxFileBytes); } catch (error) { const boundaryViolation = error instanceof WorkspaceFileBoundaryError && error.reason === "boundary"; - const log = boundaryViolation ? options.rt.logger.warn.bind(options.rt.logger) : options.rt.logger.error.bind(options.rt.logger); + const limitViolation = error instanceof WorkspaceFileBoundaryError && error.reason === "limit"; + const log = boundaryViolation || limitViolation + ? options.rt.logger.warn.bind(options.rt.logger) + : options.rt.logger.error.bind(options.rt.logger); log( { runId: options.runId, @@ -61,12 +68,20 @@ export function createFileDeliveryMcpServer(options: FileDeliveryToolOptions): M }, boundaryViolation ? "Agent file delivery refused by workspace boundary" - : "Agent file delivery failed during workspace file access", + : limitViolation + ? "Agent file delivery refused by file size limit" + : "Agent file delivery failed during workspace file access", ); + if (limitViolation) { + return { + isError: true, + content: [{ type: "text", text: `File exceeds the configured delivery limit: ${args.path}` }], + }; + } if (!boundaryViolation) throw error; return { isError: true, - content: [{ type: "text", text: "File path is outside the current workspace or uses a symlink." }], + content: [{ type: "text", text: "File path is outside the current workspace, belongs to platform runtime, or uses a symlink." }], }; } if (file === null) { diff --git a/hub/src/feishu/trigger.ts b/hub/src/feishu/trigger.ts index 40fd477..f35c462 100644 --- a/hub/src/feishu/trigger.ts +++ b/hub/src/feishu/trigger.ts @@ -444,6 +444,7 @@ export function makeTriggerHandler(deps: TriggerDeps): TriggerHandler { runId: run.id, workspaceRoot: projectWorkspaceRoot, workspaceDir: project.workspaceDir, + maxFileBytes: deps.resourceLimits?.maxBytesPerFile, sendOptions, approvalManager, tools: cphHubMcpToolsForRole(roleTools), diff --git a/hub/src/security/workspaceFiles.ts b/hub/src/security/workspaceFiles.ts index 20fcaff..479c912 100644 --- a/hub/src/security/workspaceFiles.ts +++ b/hub/src/security/workspaceFiles.ts @@ -37,6 +37,7 @@ export async function readWorkspaceFileNoFollow( workspaceRoot: string, workspaceDir: string, requestedPath: string, + maxBytes?: number, ): Promise { const workspace = await canonicalWorkspace(workspaceRoot, workspaceDir); const components = fileComponents(workspace, requestedPath); @@ -53,8 +54,15 @@ export async function readWorkspaceFileNoFollow( if (!metadata.isFile()) { throw new WorkspaceFileBoundaryError(`deliverable is not a regular file: ${requestedPath}`, requestedPath); } + if (maxBytes !== undefined && metadata.size > maxBytes) { + throw new WorkspaceFileBoundaryError( + `workspace file exceeds ${maxBytes} bytes: ${requestedPath}`, + requestedPath, + "limit", + ); + } await assertNameStillReferences(parent, name, metadata.dev, metadata.ino, requestedPath); - const data = await file.readFile(); + const data = await readFileSnapshot(file, maxBytes, requestedPath); result = { path: join(workspace, ...components), name, data }; } catch (error) { failure = boundaryError(error, requestedPath, "cannot read workspace file without following symlinks"); @@ -67,6 +75,33 @@ export async function readWorkspaceFileNoFollow( return result!; } +async function readFileSnapshot( + file: FileHandle, + maxBytes: number | undefined, + requestedPath: string, +): Promise { + if (maxBytes === undefined) return file.readFile(); + const chunks: Buffer[] = []; + let total = 0; + let position = 0; + while (true) { + const remainingWithSentinel = maxBytes - total + 1; + const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, remainingWithSentinel)); + const { bytesRead } = await file.read(chunk, 0, chunk.length, position); + if (bytesRead === 0) return Buffer.concat(chunks, total); + total += bytesRead; + if (total > maxBytes) { + throw new WorkspaceFileBoundaryError( + `workspace file exceeds ${maxBytes} bytes: ${requestedPath}`, + requestedPath, + "limit", + ); + } + chunks.push(chunk.subarray(0, bytesRead)); + position += bytesRead; + } +} + /** * Create one inbound file exclusively below the workspace and stream into its * already-open descriptor. Linux uses /proc/self/fd-relative traversal so a diff --git a/hub/test/unit/file-delivery.test.ts b/hub/test/unit/file-delivery.test.ts index db5a5a0..7dd7fbb 100644 --- a/hub/test/unit/file-delivery.test.ts +++ b/hub/test/unit/file-delivery.test.ts @@ -34,6 +34,65 @@ describe("file delivery path resolution", () => { } }); + itOnLinux("accepts arbitrary extensions and extensionless workspace files", async () => { + const root = await makeRepo(); + try { + const workspace = join(root, "examples", "TH-141"); + await writeFile(join(workspace, "lesson.json"), "{\"ok\":true}\n"); + await writeFile(join(workspace, "Makefile"), "all:\n\t@true\n"); + + await expect(resolveDeliverableFile("lesson.json", join(root, "examples"), workspace)) + .resolves.toMatchObject({ name: "lesson.json" }); + await expect(resolveDeliverableFile("Makefile", join(root, "examples"), workspace)) + .resolves.toMatchObject({ name: "Makefile" }); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + itOnLinux("allows the Feishu inbox but refuses other platform runtime files", async () => { + const root = await makeRepo(); + try { + const workspace = join(root, "examples", "TH-141"); + await mkdir(join(workspace, ".cph", "agent-runtime"), { recursive: true }); + await mkdir(join(workspace, ".cph", "inbox"), { recursive: true }); + await writeFile(join(workspace, ".cph", "agent-runtime", "session.jsonl"), "internal\n"); + await writeFile(join(workspace, ".cph", "denials.log"), "internal\n"); + await writeFile(join(workspace, ".cph", "inbox", "source.bin"), "source\n"); + + await expect( + resolveDeliverableFile(".cph/inbox/source.bin", join(root, "examples"), workspace), + ).resolves.toMatchObject({ name: "source.bin" }); + + await expect( + resolveDeliverableFile(".cph/agent-runtime/session.jsonl", join(root, "examples"), workspace), + ).rejects.toMatchObject({ reason: "boundary" }); + await expect( + resolveDeliverableFile(".cph/denials.log", join(root, "examples"), workspace), + ).rejects.toMatchObject({ reason: "boundary" }); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + + itOnLinux("refuses a file larger than the configured delivery limit", async () => { + const root = await makeRepo(); + try { + const workspace = join(root, "examples", "TH-141"); + await writeFile(join(workspace, "exact.bin"), Buffer.alloc(10)); + await writeFile(join(workspace, "artifact.bin"), Buffer.alloc(11)); + + await expect( + resolveDeliverableFile("exact.bin", join(root, "examples"), workspace, 10), + ).resolves.toMatchObject({ name: "exact.bin", data: Buffer.alloc(10) }); + await expect( + resolveDeliverableFile("artifact.bin", join(root, "examples"), workspace, 10), + ).rejects.toMatchObject({ reason: "limit" }); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + itOnLinux("rejects a deliverable symlink even when its target exists", async () => { const root = await makeRepo(); try { @@ -50,7 +109,7 @@ describe("file delivery path resolution", () => { } }); - it("does not infer files from natural-language prompts", async () => { + itOnLinux("does not infer files from natural-language prompts", async () => { const root = await makeRepo(); try { const workspace = join(root, "examples", "TH-141");