From 4ea53f9eb5a24de10db3dfda18456132987c4af8 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 4 Nov 2026 12:30:26 -0800 Subject: [PATCH] fix(server): resolve issue identifiers in file-resources routes (#16352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents and the board read task workspace files through the `/api/issues/:issueId/file-resources/*` routes > - Clients can identify a task by its UUID or by its human identifier, for example `ABC-230` > - The issues router changes identifiers to UUIDs, but the file-resources router does not > - Thus an identifier goes into a `uuid` comparison, Postgres rejects the query, and the route returns 500 > - This pull request resolves identifiers in the file-resources router and returns 404 for malformed ids > - The benefit is that identifier links work, and bad ids give a correct 404 and no server error ## Linked Issues or Issue Description **What happened?** A request to `GET /api/issues/ABC-230/file-resources/list` returned 500. The server log showed `invalid input syntax for type uuid` from `workspaceFileResourceService.getIssue`. Sentry recorded this error on hosted staging. **Steps to reproduce** 1. Create a task that has an identifier, for example `ABC-1`. 2. Send `GET /api/issues/ABC-1/file-resources/list?workspace=project`. 3. The server returns 500. The same request with the task UUID returns 200. **Expected behavior** The route accepts the identifier, as the other `/api/issues/:id` routes do. An unknown or malformed id returns 404. ## What Changed - Added `resolveIssueId` to `WorkspaceFileResourceService`. It changes a task identifier to the task UUID. It does not check visibility. `getIssue` still does the privacy-scoped read. - Added a `router.param("issueId")` handler to the file-resources router. It calls `resolveIssueId`, the same as the issues router does for its routes. - `getIssue` now returns 404 before the query when the id is not a UUID. This stops the Postgres cast error for all callers. - Added a regression test for identifier, lowercase identifier, private task, unknown identifier, and malformed id on the `content`, `list`, and `availability` routes. - Updated the test service fakes with the new method. ## Verification - `pnpm vitest run src/__tests__/file-resources.test.ts src/__tests__/file-resources-git-scan-load.test.ts` in `server/`: 55 passed. - Without the source change, the new test fails with `expected 200 "OK", got 500 "Internal Server Error"`. This reproduces the production error. - I did not run a full local `tsc` because the sandbox did not have sufficient memory. CI runs the typecheck. ## Risks - Low risk. The change adds one indexed lookup on `issues.identifier` (unique index) only when the path value has the identifier format. - The identifier lookup does not check visibility. `getIssue` then applies the privacy-scoped read, so a private or unknown task gives the same 404 as before. The route does not show if a task exists. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, test runs). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: Paperclip --- .../file-resources-git-scan-load.test.ts | 3 ++ server/src/__tests__/file-resources.test.ts | 54 +++++++++++++++++++ server/src/routes/file-resources.ts | 17 ++++++ .../src/services/workspace-file-resources.ts | 13 +++++ 4 files changed, 87 insertions(+) diff --git a/server/src/__tests__/file-resources-git-scan-load.test.ts b/server/src/__tests__/file-resources-git-scan-load.test.ts index e3dec44ac9e..f476f31cfc3 100644 --- a/server/src/__tests__/file-resources-git-scan-load.test.ts +++ b/server/src/__tests__/file-resources-git-scan-load.test.ts @@ -119,6 +119,7 @@ describe("workspace Git scan route load regression", () => { }); const roots = [path.join(tempRoot, "repository-a"), path.join(tempRoot, "repository-b")]; const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId })), list: vi.fn(async (issueId, input, opts) => { const numericId = Number(issueId.slice("issue-".length)); @@ -201,6 +202,7 @@ describe("workspace Git scan route load regression", () => { [WORKSPACE_GIT_SCAN_ERROR_CODES.timeout, 504], ] as const)("returns stable retryable %s responses", async (code, status) => { const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId })), list: vi.fn(async () => { throw new WorkspaceGitScanError(code, "Changed files temporarily unavailable"); @@ -236,6 +238,7 @@ describe("workspace Git scan route load regression", () => { }), }); const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId })), list: vi.fn(async (_issueId, input, opts) => { await scheduler.run({ diff --git a/server/src/__tests__/file-resources.test.ts b/server/src/__tests__/file-resources.test.ts index 18b5c6a610e..3aedd70b5aa 100644 --- a/server/src/__tests__/file-resources.test.ts +++ b/server/src/__tests__/file-resources.test.ts @@ -240,6 +240,54 @@ describeEmbeddedPostgres("workspace file resources", () => { await request(app).get(`/api/issues/${graph.issueId}/file-resources/content`).query({ projectId: graph.targetProjectId, workspaceId: graph.targetProjectWorkspaceId, path: "secret.txt" }).expect(404); }); + it("accepts issue identifiers and returns 404 for unknown or malformed issue ids", async () => { + const { projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + await fs.writeFile(path.join(projectRoot, "notes.txt"), "IDENTIFIER_OK", "utf8"); + const [company] = await db.select().from(companies).where(eq(companies.id, graph.companyId)); + const identifier = `${company!.issuePrefix}-1`; + await db.update(issues).set({ issueNumber: 1, identifier }).where(eq(issues.id, graph.issueId)); + const app = createApp(db, { type: "board", userId: "board-user", companyIds: [graph.companyId], source: "session", isInstanceAdmin: false }); + + for (const ref of [identifier, identifier.toLowerCase()]) { + const content = await request(app).get(`/api/issues/${ref}/file-resources/content`).query({ workspace: "project", path: "notes.txt" }).expect(200); + expect(content.body.content.data).toBe("IDENTIFIER_OK"); + await request(app).get(`/api/issues/${ref}/file-resources/list`).query({ workspace: "project" }).expect(200); + } + + await db.update(issues).set({ visibility: "private", responsibleUserId: "owner" }).where(eq(issues.id, graph.issueId)); + await request(app).get(`/api/issues/${identifier}/file-resources/content`).query({ workspace: "project", path: "notes.txt" }).expect(404); + + for (const ref of [`${company!.issuePrefix}-999999`, "not-a-uuid"]) { + await request(app).get(`/api/issues/${ref}/file-resources/content`).query({ workspace: "project", path: "notes.txt" }).expect(404); + await request(app).get(`/api/issues/${ref}/file-resources/list`).query({ workspace: "project" }).expect(404); + await request(app).post(`/api/issues/${ref}/file-resources/availability`).send({ queries: [{ workspace: "project", path: "notes.txt" }] }).expect(404); + } + }); + + it("does not record the issue UUID behind an identifier when an agent is denied", async () => { + const { projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + const [otherCompany] = await db.select().from(companies).where(eq(companies.id, graph.otherCompanyId)); + const otherIssueId = crypto.randomUUID(); + const identifier = `${otherCompany!.issuePrefix}-7`; + await db.insert(issues).values({ id: otherIssueId, companyId: graph.otherCompanyId, title: "Other company issue", issueNumber: 7, identifier }); + const agentId = crypto.randomUUID(); + await db.insert(agents).values({ id: agentId, companyId: graph.companyId, name: "Identifier probe agent", role: "engineer", adapterType: "process", adapterConfig: {} }); + const agentApp = createApp(db, { type: "agent", agentId, companyId: graph.companyId, source: "agent_key" }); + + await request(agentApp).get(`/api/issues/${identifier}/file-resources/content`).query({ workspace: "project", path: "notes.txt" }).expect(403); + await request(agentApp).get(`/api/issues/${identifier}/file-resources/resolve`).query({ workspace: "project", path: "notes.txt" }).expect(403); + await request(agentApp).get(`/api/issues/${identifier}/file-resources/list`).query({ workspace: "project" }).expect(403); + await request(agentApp).post(`/api/issues/${identifier}/file-resources/availability`).send({ queries: [{ workspace: "project", path: "notes.txt" }] }).expect(403); + + const denials = await db.select().from(activityLog).where(eq(activityLog.companyId, graph.companyId)); + const fileDenials = denials.filter((row) => row.action.startsWith("issue.file_resource_") && row.action.endsWith("_denied")); + expect(fileDenials).toHaveLength(4); + expect(fileDenials.every((row) => row.entityId === identifier)).toBe(true); + expect(JSON.stringify(denials)).not.toContain(otherIssueId); + }); + it("resolves and reads a project file without exposing absolute paths", async () => { const { root, projectRoot, executionRoot } = await makeWorkspace(); const graph = await seedGraph(db, { projectRoot, executionRoot }); @@ -1483,6 +1531,7 @@ describeEmbeddedPostgres("workspace file resources", () => { slowResolveStarted = resolve; }); const resolveLimitedService: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId: graph.companyId })), availability: vi.fn(async () => { throw new Error("not used"); @@ -1547,6 +1596,7 @@ describeEmbeddedPostgres("workspace file resources", () => { slowContentStarted = resolve; }); const contentLimitedService: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId: graph.companyId })), availability: vi.fn(async () => { throw new Error("not used"); @@ -1623,6 +1673,7 @@ describeEmbeddedPostgres("workspace file resources", () => { slowDownloadStarted = resolve; }); const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId: graph.companyId })), availability: vi.fn(async () => { throw new Error("not used"); @@ -1712,6 +1763,7 @@ describeEmbeddedPostgres("workspace file resources", () => { slowListStarted = resolve; }); const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId: graph.companyId })), availability: vi.fn(async () => { throw new Error("not used"); @@ -1993,6 +2045,7 @@ describeEmbeddedPostgres("file resource route guards", () => { slowReadStarted = resolve; }); const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId })), availability: vi.fn(async () => { throw new Error("not used"); @@ -2066,6 +2119,7 @@ describeEmbeddedPostgres("file resource route guards", () => { slowAvailabilityStarted = resolve; }); const service: WorkspaceFileResourceService = { + resolveIssueId: vi.fn(async (issueId: string) => issueId), getIssue: vi.fn(async () => ({ companyId })), availability: vi.fn(async (): Promise => { slowAvailabilityStarted?.(); diff --git a/server/src/routes/file-resources.ts b/server/src/routes/file-resources.ts index 6844c15d14b..fd1ac7e7f7b 100644 --- a/server/src/routes/file-resources.ts +++ b/server/src/routes/file-resources.ts @@ -25,6 +25,7 @@ import { } from "../services/workspace-git-operation-scheduler.js"; export type WorkspaceFileResourceService = { + resolveIssueId(rawId: string): Promise; getIssue(issueId: string): Promise<{ companyId: string }>; availability( issueId: string, @@ -363,6 +364,22 @@ export function fileResourceRoutes(db: Db, opts: { const listLimiter = opts.listLimiter ?? createFileResourceListLimiter(); const availabilityLimiter = opts.availabilityLimiter ?? createFileResourceAvailabilityLimiter(); + // Resolve issue identifiers (e.g. "PAP-39") to UUIDs, matching the issues router. + // Only board actors can use these routes. Other actors keep the raw value so a + // denied request's activity row never records the UUID behind a guessed identifier. + router.param("issueId", async (req, _res, next, rawId) => { + if (req.actor.type !== "board") { + next(); + return; + } + try { + req.params.issueId = await serviceFor(req).resolveIssueId(String(rawId)); + next(); + } catch (err) { + next(err); + } + }); + async function logAvailabilityAttempt(input: { companyId: string; actor: ReturnType; diff --git a/server/src/services/workspace-file-resources.ts b/server/src/services/workspace-file-resources.ts index 7c0852cf836..55ab59c881d 100644 --- a/server/src/services/workspace-file-resources.ts +++ b/server/src/services/workspace-file-resources.ts @@ -5,6 +5,7 @@ import type { Readable } from "node:stream"; import { and, desc, eq, inArray, isNull } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { executionWorkspaces, issues, projects, projectWorkspaces } from "@paperclipai/db"; +import { normalizeIssueIdentifier } from "@paperclipai/shared"; import type { NormalizedWorkspaceFileAvailabilityQuery, ResolvedWorkspaceResource, @@ -39,6 +40,7 @@ const MAX_RELATIVE_PATH_BYTES = 4096; const TEXT_SNIFF_BYTES = 4096; const MAX_LIST_DEPTH = 20; const GIT_STATUS_MAX_BUFFER_BYTES = 1024 * 1024; +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; const LOCAL_PROJECT_WORKSPACE_SOURCE_TYPES = new Set(["local_path", "non_git_path", "git_repo", "git_worktree"]); export interface WorkspaceFileScanContext { @@ -1143,7 +1145,17 @@ export function workspaceFileResourceService(db: Db, actor?: AuthorizationActor, } return visible; } + // Visibility is not checked here; getIssue applies the privacy-scoped read. + async function resolveIssueId(rawId: string): Promise { + const identifier = normalizeIssueIdentifier(rawId); + if (!identifier) return rawId; + const [issue] = await db.select({ id: issues.id }).from(issues).where(eq(issues.identifier, identifier)).limit(1); + return issue?.id ?? rawId; + } + async function getIssue(issueId: string): Promise { + // A non-UUID value would make Postgres reject the uuid comparison with a 500. + if (!UUID_RE.test(issueId)) throw notFound("Issue not found"); const [issue] = await db.select().from(issues).where(and(eq(issues.id, issueId), actor ? await issueReadSqlCondition(db, actor) : undefined)).limit(1); if (!issue) throw notFound("Issue not found"); return issue; @@ -1955,6 +1967,7 @@ export function workspaceFileResourceService(db: Db, actor?: AuthorizationActor, } return { + resolveIssueId, getIssue, availability, list,