mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 22:24:16 +01:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
9b58daba6e
commit
2e26147639
4 files changed
+72
-10
No files matched your search
@@ -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(),
|
||||
});
|
||||
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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],
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user