mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 12:33:05 +01:00
fix(server): resolve issue identifiers in file-resources routes (#16352)
## 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 <noreply@anthropic.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
4 files changed
+87
No files matched your search
@@ -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({
|
||||
|
||||
@@ -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<WorkspaceFileAvailabilityResponse> => {
|
||||
slowAvailabilityStarted?.();
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
} from "../services/workspace-git-operation-scheduler.js";
|
||||
|
||||
export type WorkspaceFileResourceService = {
|
||||
resolveIssueId(rawId: string): Promise<string>;
|
||||
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<typeof getActorInfo>;
|
||||
|
||||
@@ -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<string> {
|
||||
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<IssueRow> {
|
||||
// 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,
|
||||
|
||||
Reference in new issue
Block a user