From 914502eba39b115f07221c9cc0739db9195ac9ba Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 7 Jun 2026 01:05:34 +0000 Subject: [PATCH] Audit workspace file route limit denials Co-Authored-By: Paperclip --- server/src/__tests__/file-resources.test.ts | 184 +++++++++++++++++- server/src/routes/file-resources.ts | 56 +++++- .../src/services/workspace-file-resources.ts | 2 +- 3 files changed, 232 insertions(+), 10 deletions(-) diff --git a/server/src/__tests__/file-resources.test.ts b/server/src/__tests__/file-resources.test.ts index bd1913ec0d2..5e354e65705 100644 --- a/server/src/__tests__/file-resources.test.ts +++ b/server/src/__tests__/file-resources.test.ts @@ -569,6 +569,164 @@ describeEmbeddedPostgres("workspace file resources", () => { expect(JSON.stringify(rows.map((row) => row.details))).not.toContain(projectRoot); }); + it("logs successful resolves and resolve/content limiter denials", async () => { + const { projectRoot, executionRoot } = await makeWorkspace(); + const graph = await seedGraph(db, { projectRoot, executionRoot }); + await fs.writeFile(path.join(projectRoot, "README.md"), "# Visible\n", "utf8"); + + const app = createApp( + db, + { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }, + { + limiter: createFileResourceLimiter({ maxConcurrent: 1, maxRequests: 100, windowMs: 60_000 }), + }, + ); + + const resolved = await request(app) + .get(`/api/issues/${graph.issueId}/file-resources/resolve`) + .query({ workspace: "project", path: "README.md" }); + expect(resolved.status).toBe(200); + + const rowsAfterResolve = await db.select().from(activityLog).where(eq(activityLog.entityId, graph.issueId)); + const resolveRead = rowsAfterResolve.find((row) => row.action === "issue.file_resource_resolve"); + expect(resolveRead?.details).toMatchObject({ + outcome: "success", + workspaceKind: "project_workspace", + displayPath: "README.md", + }); + expect(JSON.stringify(resolveRead?.details)).not.toContain(projectRoot); + + let releaseSlowResolve: (() => void) | null = null; + let slowResolveStarted: (() => void) | null = null; + const slowResolve = new Promise((resolve) => { + releaseSlowResolve = resolve; + }); + const resolveStarted = new Promise((resolve) => { + slowResolveStarted = resolve; + }); + const resolveLimitedService: WorkspaceFileResourceService = { + getIssue: vi.fn(async () => ({ companyId: graph.companyId })), + list: vi.fn(async () => { + throw new Error("not used"); + }), + resolve: vi.fn(async () => { + slowResolveStarted?.(); + await slowResolve; + return { + kind: "file", + provider: "local_fs", + title: "README.md", + displayPath: "README.md", + workspaceLabel: "Workspace", + workspaceKind: "project_workspace", + workspaceId: "11111111-1111-4111-8111-111111111111", + previewKind: "text", + capabilities: { preview: true, download: false, listChildren: false }, + }; + }), + readContent: vi.fn(async () => { + throw new Error("not used"); + }), + }; + const resolveLimitedApp = createApp( + db, + { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }, + { + service: resolveLimitedService, + limiter: createFileResourceLimiter({ maxConcurrent: 1, maxRequests: 100, windowMs: 60_000 }), + }, + ); + const firstResolve = request(resolveLimitedApp) + .get(`/api/issues/${graph.issueId}/file-resources/resolve`) + .query({ path: "README.md" }); + const firstResolveResponse = firstResolve.then((res) => res); + await resolveStarted; + const secondResolve = await request(resolveLimitedApp) + .get(`/api/issues/${graph.issueId}/file-resources/resolve`) + .query({ path: "README.md" }); + expect(secondResolve.status).toBe(429); + releaseSlowResolve?.(); + expect((await firstResolveResponse).status).toBe(200); + + let releaseSlowContent: (() => void) | null = null; + let slowContentStarted: (() => void) | null = null; + const slowContent = new Promise((resolve) => { + releaseSlowContent = resolve; + }); + const contentStarted = new Promise((resolve) => { + slowContentStarted = resolve; + }); + const contentLimitedService: WorkspaceFileResourceService = { + getIssue: vi.fn(async () => ({ companyId: graph.companyId })), + list: vi.fn(async () => { + throw new Error("not used"); + }), + resolve: vi.fn(async () => { + throw new Error("not used"); + }), + readContent: vi.fn(async () => { + slowContentStarted?.(); + await slowContent; + return { + resource: { + kind: "file", + provider: "local_fs", + title: "README.md", + displayPath: "README.md", + workspaceLabel: "Workspace", + workspaceKind: "project_workspace", + workspaceId: "11111111-1111-4111-8111-111111111111", + previewKind: "text", + capabilities: { preview: true, download: false, listChildren: false }, + }, + content: { encoding: "utf8", data: "# Visible\n" }, + }; + }), + }; + const contentLimitedApp = createApp( + db, + { + type: "board", + userId: "board-user", + companyIds: [graph.companyId], + source: "session", + isInstanceAdmin: false, + }, + { + service: contentLimitedService, + limiter: createFileResourceLimiter({ maxConcurrent: 1, maxRequests: 100, windowMs: 60_000 }), + }, + ); + const firstContent = request(contentLimitedApp) + .get(`/api/issues/${graph.issueId}/file-resources/content`) + .query({ path: "README.md" }); + const firstContentResponse = firstContent.then((res) => res); + await contentStarted; + const secondContent = await request(contentLimitedApp) + .get(`/api/issues/${graph.issueId}/file-resources/content`) + .query({ path: "README.md" }); + expect(secondContent.status).toBe(429); + releaseSlowContent?.(); + expect((await firstContentResponse).status).toBe(200); + + const rowsAfterLimits = await db.select().from(activityLog).where(eq(activityLog.entityId, graph.issueId)); + expect(rowsAfterLimits.some((row) => row.action === "issue.file_resource_resolve_denied")).toBe(true); + expect(rowsAfterLimits.some((row) => row.action === "issue.file_resource_content_denied")).toBe(true); + expect(JSON.stringify(rowsAfterLimits.map((row) => row.details))).not.toContain(projectRoot); + }); + it("uses tighter list-specific rate and concurrency limits", async () => { const { projectRoot, executionRoot } = await makeWorkspace(); const graph = await seedGraph(db, { projectRoot, executionRoot }); @@ -639,8 +797,26 @@ describeEmbeddedPostgres("workspace file resources", () => { }); }); -describe("file resource route guards", () => { +describeEmbeddedPostgres("file resource route guards", () => { + let tempDb: Awaited> | null = null; + let db: Db; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-file-resource-guards-"); + db = createDb(tempDb.connectionString); + }, 60_000); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + it("enforces bounded rate and concurrency limits", async () => { + const companyId = crypto.randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Rate limit company", + issuePrefix: "RAT", + }); let releaseSlowRead: (() => void) | null = null; let slowReadStarted: (() => void) | null = null; const slowRead = new Promise((resolve) => { @@ -650,7 +826,7 @@ describe("file resource route guards", () => { slowReadStarted = resolve; }); const service: WorkspaceFileResourceService = { - getIssue: vi.fn(async () => ({ companyId: "company-1" })), + getIssue: vi.fn(async () => ({ companyId })), list: vi.fn(async () => { throw new Error("not used"); }), @@ -678,13 +854,13 @@ describe("file resource route guards", () => { req.actor = { type: "board", userId: "board-user", - companyIds: ["company-1"], + companyIds: [companyId], source: "session", isInstanceAdmin: false, }; next(); }); - app.use("/api", fileResourceRoutes({} as Db, { + app.use("/api", fileResourceRoutes(db, { service, limiter: createFileResourceLimiter({ maxConcurrent: 1, maxRequests: 2, windowMs: 60_000 }), })); diff --git a/server/src/routes/file-resources.ts b/server/src/routes/file-resources.ts index cee1ad9385a..5b9d7a450cc 100644 --- a/server/src/routes/file-resources.ts +++ b/server/src/routes/file-resources.ts @@ -218,12 +218,13 @@ export function fileResourceRoutes(db: Db, opts: { issueId: string; displayPath: string; error: unknown; + action?: "issue.file_resource_content_denied" | "issue.file_resource_resolve_denied"; }) { await logActivity(db, { companyId: input.companyId, actorType: input.actor.actorType, actorId: input.actor.actorId, - action: "issue.file_resource_content_denied", + action: input.action ?? "issue.file_resource_content_denied", entityType: "issue", entityId: input.issueId, agentId: input.actor.agentId, @@ -364,9 +365,41 @@ export function fileResourceRoutes(db: Db, opts: { assertCompanyAccess(req, issue.companyId); const actor = getActorInfo(req); const query = readQuery(req.query); - const release = limiter.acquire(limiterKey(issue.companyId, actor.actorId, req.params.issueId)); + let release: (() => void) | null = null; + try { + release = limiter.acquire(limiterKey(issue.companyId, actor.actorId, req.params.issueId)); + } catch (error) { + await logDeniedAttempt({ + companyId: issue.companyId, + actor, + issueId: req.params.issueId, + displayPath: query.path, + error, + action: "issue.file_resource_resolve_denied", + }); + throw error; + } try { const result = await svc.resolve(req.params.issueId, query); + await logActivity(db, { + companyId: issue.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + action: "issue.file_resource_resolve", + entityType: "issue", + entityId: req.params.issueId, + agentId: actor.agentId, + runId: actor.runId, + details: activityDetails({ + outcome: "success", + workspaceKind: result.workspaceKind, + workspaceId: result.workspaceId, + displayPath: result.displayPath, + byteSize: result.byteSize ?? null, + contentType: result.contentType ?? null, + denialReason: result.denialReason ?? null, + }), + }); res.json(result); } catch (error) { await logDeniedAttempt({ @@ -375,10 +408,11 @@ export function fileResourceRoutes(db: Db, opts: { issueId: req.params.issueId, displayPath: query.path, error, + action: "issue.file_resource_resolve_denied", }); throw error; } finally { - release(); + release?.(); } }); @@ -388,7 +422,19 @@ export function fileResourceRoutes(db: Db, opts: { assertCompanyAccess(req, issue.companyId); const actor = getActorInfo(req); const query = readQuery(req.query); - const release = limiter.acquire(limiterKey(issue.companyId, actor.actorId, req.params.issueId)); + let release: (() => void) | null = null; + try { + release = limiter.acquire(limiterKey(issue.companyId, actor.actorId, req.params.issueId)); + } catch (error) { + await logDeniedAttempt({ + companyId: issue.companyId, + actor, + issueId: req.params.issueId, + displayPath: query.path, + error, + }); + throw error; + } try { let result: WorkspaceFileContent | null = null; try { @@ -427,7 +473,7 @@ export function fileResourceRoutes(db: Db, opts: { res.set("X-Content-Type-Options", "nosniff"); res.json(result); } finally { - release(); + release?.(); } }); diff --git a/server/src/services/workspace-file-resources.ts b/server/src/services/workspace-file-resources.ts index 1b151482e44..d1a209b2463 100644 --- a/server/src/services/workspace-file-resources.ts +++ b/server/src/services/workspace-file-resources.ts @@ -615,7 +615,7 @@ async function listChangedWorkspaceFiles(input: { return { items, scannedCount, - truncated: hitScanCap || paths.length > scannedCount || items.length >= input.limit, + truncated: hitScanCap || paths.length > scannedCount || items.length > input.limit, }; }