From 2e261476393f24765f4ae575a9b827fb3f16d4f7 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 24 Apr 2026 11:23:06 -0500 Subject: [PATCH] Fix file viewer QA defects: null-byte 500 + OpenFilePrompt :line parsing (PAP-1964) - Reject ASCII control characters in workspaceFileResourceQuerySchema and translate that ZodError to the existing 422 + {code: "invalid_path"} shape in the route layer. The old service-level null-byte check still fires, but now we reject at the boundary before audit logging tries to write the raw path into Postgres JSONB (the true source of the 500). - Parse path:line[:col] in OpenFilePrompt.handleSubmit via parseWorkspaceFileRef, matching what inline WorkspaceFileLinks and URL deep-links already do, and widen onSubmit/handlePromptSubmit to carry line/column through to viewer.open. - Add a regression test for the control-character rejection. Fix a pre-existing bug in file-resources.test.ts (missing await + wrong field names on the embedded-postgres helper) so the DB-backed tests actually run instead of silently skipping. Co-Authored-By: Paperclip --- .../src/validators/workspace-file-resource.ts | 8 ++++- server/src/__tests__/file-resources.test.ts | 36 ++++++++++++++++--- server/src/routes/file-resources.ts | 14 +++++++- ui/src/components/FileViewerSheet.tsx | 24 ++++++++++--- 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/packages/shared/src/validators/workspace-file-resource.ts b/packages/shared/src/validators/workspace-file-resource.ts index f7a9a029cb9..f525c16ac4f 100644 --- a/packages/shared/src/validators/workspace-file-resource.ts +++ b/packages/shared/src/validators/workspace-file-resource.ts @@ -17,7 +17,13 @@ export const workspaceFileRefSchema = z.object({ }); export const workspaceFileResourceQuerySchema = z.object({ - path: z.string().min(1), + path: z + .string() + .min(1) + .refine((value) => !/[\x00-\x1f\x7f]/.test(value), { + message: "Workspace file path contains an invalid character", + params: { code: "invalid_path" }, + }), workspace: workspaceFileSelectorSchema.optional(), }); diff --git a/server/src/__tests__/file-resources.test.ts b/server/src/__tests__/file-resources.test.ts index 89e10320eb3..dd40b18340f 100644 --- a/server/src/__tests__/file-resources.test.ts +++ b/server/src/__tests__/file-resources.test.ts @@ -5,7 +5,7 @@ import os from "node:os"; import path from "node:path"; import request from "supertest"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -import { activityLog, companies, executionWorkspaces, goals, issues, projects, projectWorkspaces, type Db } from "@paperclipai/db"; +import { activityLog, companies, createDb, executionWorkspaces, goals, issues, projects, projectWorkspaces, type Db } from "@paperclipai/db"; import { eq } from "drizzle-orm"; import { errorHandler } from "../middleware/index.js"; import { createFileResourceLimiter, fileResourceRoutes, type WorkspaceFileResourceService } from "../routes/file-resources.js"; @@ -18,7 +18,7 @@ import { startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; -const embeddedPostgresSupport = getEmbeddedPostgresTestSupport(); +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; type TestGraph = { @@ -157,11 +157,11 @@ describeEmbeddedPostgres("workspace file resources", () => { beforeAll(async () => { tempDb = await startEmbeddedPostgresTestDatabase("paperclip-file-resources-"); - db = tempDb.db; + db = createDb(tempDb.connectionString); }, 60_000); afterAll(async () => { - await tempDb?.stop(); + await tempDb?.cleanup(); }); it("resolves and reads a project file without exposing absolute paths", async () => { @@ -206,6 +206,34 @@ describeEmbeddedPostgres("workspace file resources", () => { expect(resolved.capabilities.preview).toBe(true); }); + it("rejects control characters in the path without crashing the audit log", async () => { + const { projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + + const app = createApp(db, { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }); + + const nullByte = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/content?workspace=project&path=foo%00bar.ts`); + expect(nullByte.status).toBe(422); + expect(nullByte.body?.details?.code).toBe("invalid_path"); + + const resolveNullByte = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/resolve?workspace=project&path=foo%00bar.ts`); + expect(resolveNullByte.status).toBe(422); + expect(resolveNullByte.body?.details?.code).toBe("invalid_path"); + + const otherControl = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/content?workspace=project&path=a%0Bb.ts`); + expect(otherControl.status).toBe(422); + expect(otherControl.body?.details?.code).toBe("invalid_path"); + }); + it("rejects traversal, encoded traversal, backslash traversal, and double-encoding without double-decoding", async () => { const { projectRoot, executionRoot } = await makeWorkspace(); const graph = await seedGraph(db, { projectRoot, executionRoot }); diff --git a/server/src/routes/file-resources.ts b/server/src/routes/file-resources.ts index 574d189d55e..f691de5fe38 100644 --- a/server/src/routes/file-resources.ts +++ b/server/src/routes/file-resources.ts @@ -1,4 +1,5 @@ import { Router } from "express"; +import { ZodError } from "zod"; import type { Db } from "@paperclipai/db"; import { workspaceFileResourceQuerySchema, @@ -63,7 +64,18 @@ function limiterKey(companyId: string, actorId: string, issueId: string) { } function readQuery(query: unknown) { - const parsed = workspaceFileResourceQuerySchema.parse(query); + let parsed; + try { + parsed = workspaceFileResourceQuerySchema.parse(query); + } catch (error) { + if (error instanceof ZodError) { + const refinement = error.errors.find( + (issue) => (issue as { params?: { code?: string } }).params?.code === "invalid_path", + ); + if (refinement) throw unprocessable(refinement.message, { code: "invalid_path" }); + } + throw error; + } return { path: parsed.path, workspace: parsed.workspace ?? "auto", diff --git a/ui/src/components/FileViewerSheet.tsx b/ui/src/components/FileViewerSheet.tsx index 4fa160a5433..0faea263148 100644 --- a/ui/src/components/FileViewerSheet.tsx +++ b/ui/src/components/FileViewerSheet.tsx @@ -37,6 +37,7 @@ import { useRequiredFileViewer, type FileViewerUrlState, } from "@/context/FileViewerContext"; +import { parseWorkspaceFileRef } from "@/lib/workspace-file-parser"; import type { ResolvedWorkspaceResource, WorkspaceFileContent, @@ -193,7 +194,12 @@ function FileViewerStateView({ } interface OpenFilePromptProps { - onSubmit: (path: string, workspace: WorkspaceFileSelector) => void; + onSubmit: ( + path: string, + workspace: WorkspaceFileSelector, + line: number | null, + column: number | null, + ) => void; } function OpenFilePrompt({ onSubmit }: OpenFilePromptProps) { @@ -210,7 +216,12 @@ function OpenFilePrompt({ onSubmit }: OpenFilePromptProps) { event.preventDefault(); const trimmed = value.trim(); if (!trimmed) return; - onSubmit(trimmed, workspace); + const parsed = parseWorkspaceFileRef(trimmed); + if (parsed) { + onSubmit(parsed.path, workspace, parsed.line, parsed.column); + return; + } + onSubmit(trimmed, workspace, null, null); }; return ( @@ -489,8 +500,13 @@ export function FileViewerSheet({ ); const handlePromptSubmit = useCallback( - (path: string, workspace: WorkspaceFileSelector) => { - viewer.open({ path, line: null, column: null, workspace }); + ( + path: string, + workspace: WorkspaceFileSelector, + line: number | null, + column: number | null, + ) => { + viewer.open({ path, line, column, workspace }); }, [viewer], );