diff --git a/server/src/__tests__/activity-routes.test.ts b/server/src/__tests__/activity-routes.test.ts index 45862b0551..97a23fb288 100644 --- a/server/src/__tests__/activity-routes.test.ts +++ b/server/src/__tests__/activity-routes.test.ts @@ -177,7 +177,7 @@ describe.sequential("activity routes", () => { expect(mockActivityService.create).not.toHaveBeenCalled(); }); - it("requires company access before listing issues for another company's run", async () => { + it("returns 200 [] (not 404) when listing issues for another company's run, preserving API contract and the cross-tenant oracle", async () => { mockHeartbeatService.getRun.mockResolvedValue({ id: "run-2", companyId: "company-2", @@ -186,7 +186,19 @@ describe.sequential("activity routes", () => { const app = await createApp(); const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/run-2/issues")); - expect(res.status).toBe(403); + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); + }); + + it("returns 200 [] (not 404) for a non-existent heartbeat run, matching the cross-tenant response", async () => { + mockHeartbeatService.getRun.mockResolvedValue(null); + + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/missing-run/issues")); + + expect(res.status).toBe(200); + expect(res.body).toEqual([]); expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts index 738d2ebf20..256024e328 100644 --- a/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts +++ b/server/src/__tests__/agent-cross-tenant-authz-routes.test.ts @@ -144,6 +144,28 @@ vi.mock("../routes/authz.js", async () => { } } + function hasCompanyAccess(req: Express.Request, expectedCompanyId: string): boolean { + if (req.actor.type === "none") return false; + if (req.actor.type === "agent") return req.actor.companyId === expectedCompanyId; + if (req.actor.source === "local_implicit") return true; + return (req.actor.companyIds ?? []).includes(expectedCompanyId); + } + + async function getAccessibleResource( + req: Express.Request, + res: { status(code: number): { json(body: unknown): unknown } }, + resource: T | null | undefined | Promise, + notFoundMessage: string, + ): Promise { + const resolved = await resource; + if (!resolved || !hasCompanyAccess(req, resolved.companyId)) { + res.status(404).json({ error: notFoundMessage }); + return null; + } + assertCompanyAccess(req, resolved.companyId); + return resolved; + } + function assertInstanceAdmin(req: Express.Request) { assertBoard(req); if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return; @@ -173,7 +195,9 @@ vi.mock("../routes/authz.js", async () => { assertBoard, assertCompanyAccess, assertInstanceAdmin, + getAccessibleResource, getActorInfo, + hasCompanyAccess, }; }); @@ -368,8 +392,8 @@ describe.sequential("agent cross-tenant route authorization", () => { const app = await createApp(crossTenantActor); const res = await deniedCase.request(app); - expect(res.status, `${deniedCase.label}: ${JSON.stringify(res.body)}`).toBe(403); - expect(res.body.error).toContain("User does not have access to this company"); + expect(res.status, `${deniedCase.label}: ${JSON.stringify(res.body)}`).toBe(404); + expect(res.body.error).toBe("Agent not found"); expect(mockAgentService.getById).toHaveBeenCalledWith(agentId); for (const mock of deniedCase.untouched) { expect(mock).not.toHaveBeenCalled(); diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 1822baee2d..83bcaeadd2 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -1641,8 +1641,10 @@ describe.sequential("agent permission routes", () => { .patch(`/api/agents/${agentId}/permissions`) .send({ canCreateAgents: true, canAssignTasks: true })); - expect(res.status).toBe(403); - expect(res.body.error).toContain("another company"); + // Cross-tenant requests return 404 (not 403) so the status code cannot be + // used as an existence oracle for other tenants' agent ids. + expect(res.status).toBe(404); + expect(res.body.error).toBe("Agent not found"); expect(mockAgentService.updatePermissions).not.toHaveBeenCalled(); expect(mockAccessService.setPrincipalPermission).not.toHaveBeenCalled(); }); @@ -1803,7 +1805,8 @@ describe.sequential("agent permission routes", () => { const res = await requestApp(app, (baseUrl) => request(baseUrl).post("/api/heartbeat-runs/run-1/cancel").send({})); - expect(res.status).toBe(403); + expect(res.status).toBe(404); + expect(res.body.error).toBe("Heartbeat run not found"); expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); }); }); diff --git a/server/src/__tests__/approval-routes-idempotency.test.ts b/server/src/__tests__/approval-routes-idempotency.test.ts index 317299044c..afce78bf3e 100644 --- a/server/src/__tests__/approval-routes-idempotency.test.ts +++ b/server/src/__tests__/approval-routes-idempotency.test.ts @@ -215,7 +215,8 @@ describe("approval routes idempotent retries", () => { .post("/api/approvals/approval-2/approve") .send({}); - expect(res.status).toBe(403); + expect(res.status).toBe(404); + expect(res.body.error).toBe("Approval not found"); expect(mockApprovalService.approve).not.toHaveBeenCalled(); }); @@ -232,7 +233,8 @@ describe("approval routes idempotent retries", () => { .post("/api/approvals/approval-3/request-revision") .send({ decisionNote: "Need changes" }); - expect(res.status).toBe(403); + expect(res.status).toBe(404); + expect(res.body.error).toBe("Approval not found"); expect(mockApprovalService.requestRevision).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/authz-company-access.test.ts b/server/src/__tests__/authz-company-access.test.ts index d389d168a3..c6a5934e31 100644 --- a/server/src/__tests__/authz-company-access.test.ts +++ b/server/src/__tests__/authz-company-access.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { HttpError } from "../errors.js"; -import { assertBoardOrgAccess, assertCompanyAccess, hasBoardOrgAccess } from "../routes/authz.js"; +import { assertBoardOrgAccess, assertCompanyAccess, hasBoardOrgAccess, hasCompanyAccess } from "../routes/authz.js"; function makeReq(input: { method?: string; @@ -194,6 +194,78 @@ describe("assertCompanyAccess", () => { }); }); +describe("hasCompanyAccess", () => { + it("allows members of the company", () => { + const req = makeReq({ + actor: { + type: "board", + userId: "user-1", + source: "session", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole: "viewer", status: "active" }], + }, + }); + + expect(hasCompanyAccess(req, "company-1")).toBe(true); + }); + + it("denies users from other companies", () => { + const req = makeReq({ + actor: { + type: "board", + userId: "user-1", + source: "session", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", membershipRole: "operator", status: "active" }], + }, + }); + + expect(hasCompanyAccess(req, "company-2")).toBe(false); + }); + + it("denies signed-in instance admins without explicit company access, matching assertCompanyAccess", () => { + const req = makeReq({ + actor: { + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: [], + memberships: [], + }, + }); + + expect(hasCompanyAccess(req, "company-1")).toBe(false); + expect(() => assertCompanyAccess(req, "company-1")).toThrow("User does not have access to this company"); + }); + + it("allows local trusted board access without explicit membership", () => { + const req = makeReq({ + actor: { + type: "board", + userId: "local-board", + source: "local_implicit", + isInstanceAdmin: true, + }, + }); + + expect(hasCompanyAccess(req, "company-1")).toBe(true); + }); + + it("scopes agent actors to their own company", () => { + const agent = { type: "agent", agentId: "agent-1", companyId: "company-1" } as const; + + expect(hasCompanyAccess(makeReq({ actor: agent }), "company-1")).toBe(true); + expect(hasCompanyAccess(makeReq({ actor: agent }), "company-2")).toBe(false); + }); + + it("denies unauthenticated actors", () => { + const req = makeReq({ actor: { type: "none" } }); + + expect(hasCompanyAccess(req, "company-1")).toBe(false); + }); +}); + describe("assertBoardOrgAccess", () => { it("allows signed-in board users with active company access", () => { const req = makeReq({ diff --git a/server/src/__tests__/authz-existence-oracle-guard.test.ts b/server/src/__tests__/authz-existence-oracle-guard.test.ts new file mode 100644 index 0000000000..a6d1f61648 --- /dev/null +++ b/server/src/__tests__/authz-existence-oracle-guard.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, it } from "vitest"; +import { readFileSync, readdirSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** + * Static guard against the cross-tenant existence oracle. + * + * Route handlers that look a resource up by id and then call + * `assertCompanyAccess(req, resource.companyId)` leak resource existence + * across tenants: missing ids return 404 while cross-tenant ids return 403, + * letting any authenticated user enumerate other tenants' ids. The required + * pattern (documented on `hasCompanyAccess` in routes/authz.ts) folds the + * access check into the existence check: + * + * const issue = await svc.getById(id); + * if (!issue || !hasCompanyAccess(req, issue.companyId)) { + * res.status(404).json({ error: "Issue not found" }); + * return; + * } + * assertCompanyAccess(req, issue.companyId); // write paths only + * + * This test scans every route file and fails when it finds an + * `assertCompanyAccess(req, .companyId)` call that is not preceded + * by a `hasCompanyAccess(req, .companyId)` gate. Sites where the + * companyId comes from request input (params/body) rather than a looked-up + * resource carry no oracle and belong on the allowlist below. + */ + +const ROUTES_DIR = join(dirname(fileURLToPath(import.meta.url)), "..", "routes"); + +// ":" call sites where the companyId is request input, not a +// discovered resource — no existence oracle to close. Add new entries only +// when the value cannot reveal whether a cross-tenant resource exists. +const REQUEST_INPUT_ALLOWLIST = new Set([ + "companies.ts:target", // import target chosen by the caller (req.body.target) + "plugins.ts:runContext", // run context supplied in the request body +]); + +const GATE_LOOKBACK_LINES = 12; + +function findUngatedSites() { + const ungated: string[] = []; + for (const file of readdirSync(ROUTES_DIR).filter((name) => name.endsWith(".ts"))) { + if (file === "authz.ts") continue; + const lines = readFileSync(join(ROUTES_DIR, file), "utf8").split("\n"); + lines.forEach((line, index) => { + const match = line.match(/assertCompanyAccess\(req,\s*(\w+)\.companyId\)/); + if (!match) return; + const variable = match[1]; + const lookback = lines + .slice(Math.max(0, index - GATE_LOOKBACK_LINES), index) + .join("\n"); + if (new RegExp(`hasCompanyAccess\\(req,\\s*${variable}\\.companyId\\)`).test(lookback)) return; + if (REQUEST_INPUT_ALLOWLIST.has(`${file}:${variable}`)) return; + ungated.push(`${file}:${index + 1} (${variable}.companyId)`); + }); + } + return ungated; +} + +describe("cross-tenant existence oracle guard", () => { + it("requires a hasCompanyAccess gate before assertCompanyAccess on looked-up resources", () => { + const ungated = findUngatedSites(); + expect( + ungated, + "assertCompanyAccess on a looked-up resource without a hasCompanyAccess 404 gate " + + "reintroduces the cross-tenant existence oracle (403 vs 404). " + + "Apply the two-step pattern documented on hasCompanyAccess in routes/authz.ts, " + + "or — only if the companyId is request input — add the site to REQUEST_INPUT_ALLOWLIST. " + + `Offending sites: ${ungated.join(", ")}`, + ).toEqual([]); + }); + + it("keeps the allowlist free of stale entries", () => { + const stale: string[] = []; + for (const entry of REQUEST_INPUT_ALLOWLIST) { + const [file, variable] = entry.split(":"); + const source = readFileSync(join(ROUTES_DIR, file!), "utf8"); + if (!new RegExp(`assertCompanyAccess\\(req,\\s*${variable}\\.companyId\\)`).test(source)) { + stale.push(entry); + } + } + expect(stale, `Allowlist entries no longer present in the code: ${stale.join(", ")}`).toEqual([]); + }); +}); diff --git a/server/src/__tests__/costs-service.test.ts b/server/src/__tests__/costs-service.test.ts index 3ce190c093..25cccb8094 100644 --- a/server/src/__tests__/costs-service.test.ts +++ b/server/src/__tests__/costs-service.test.ts @@ -311,7 +311,8 @@ describe("cost routes", () => { .patch("/api/agents/agent-1/budgets") .send({ budgetMonthlyCents: 2500 }); - expect(res.status).toBe(403); + expect(res.status).toBe(404); + expect(res.body.error).toBe("Agent not found"); expect(mockAgentService.update).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/document-annotation-routes.test.ts b/server/src/__tests__/document-annotation-routes.test.ts index 450d7fbfdd..b488476ab2 100644 --- a/server/src/__tests__/document-annotation-routes.test.ts +++ b/server/src/__tests__/document-annotation-routes.test.ts @@ -309,10 +309,10 @@ describe("document annotation routes", () => { expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); - it("rejects agent cross-company annotation reads", async () => { + it("rejects agent cross-company annotation reads with a uniform 404", async () => { await request(await createApp("agent", otherCompanyId)) .get(`/api/issues/${issueId}/documents/plan/annotations`) - .expect(403); + .expect(404); }); it("adds annotation comments without waking the assignee and resolves threads", async () => { diff --git a/server/src/__tests__/external-object-routes.test.ts b/server/src/__tests__/external-object-routes.test.ts index 96a6d30e3f..14b142b7df 100644 --- a/server/src/__tests__/external-object-routes.test.ts +++ b/server/src/__tests__/external-object-routes.test.ts @@ -194,7 +194,9 @@ describe("external object routes", () => { const res = await request(app).get(`/api/issues/${issueId}/external-object-summary`); - expect(res.status).toBe(403); + // Uniform 404 so cross-tenant ids are indistinguishable from missing ones. + expect(res.status).toBe(404); + expect(res.body.error).toBe("Issue not found"); expect(mockExternalObjectsService.getIssueSummary).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/file-resources.test.ts b/server/src/__tests__/file-resources.test.ts index 277ada7a15..c912109f10 100644 --- a/server/src/__tests__/file-resources.test.ts +++ b/server/src/__tests__/file-resources.test.ts @@ -1078,11 +1078,11 @@ describeEmbeddedPostgres("workspace file resources", () => { }); expect((await request(agentApp).get(`/api/issues/${graph.issueId}/file-resources/resolve`).query({ path: "README.md" })).status).toBe(403); - expect((await request(boardApp).get(`/api/issues/${graph.issueId}/file-resources/resolve`).query({ path: "README.md" })).status).toBe(403); + expect((await request(boardApp).get(`/api/issues/${graph.issueId}/file-resources/resolve`).query({ path: "README.md" })).status).toBe(404); expect((await request(agentApp).get(`/api/issues/${graph.issueId}/file-resources/content`).query({ path: "README.md" })).status).toBe(403); - expect((await request(boardApp).get(`/api/issues/${graph.issueId}/file-resources/content`).query({ path: "README.md" })).status).toBe(403); + expect((await request(boardApp).get(`/api/issues/${graph.issueId}/file-resources/content`).query({ path: "README.md" })).status).toBe(404); expect((await request(agentApp).get(`/api/issues/${graph.issueId}/file-resources/list`)).status).toBe(403); - expect((await request(boardApp).get(`/api/issues/${graph.issueId}/file-resources/list`)).status).toBe(403); + expect((await request(boardApp).get(`/api/issues/${graph.issueId}/file-resources/list`)).status).toBe(404); const rows = await db.select().from(activityLog).where(eq(activityLog.entityId, graph.issueId)); const listDenials = rows.filter((row) => row.action === "issue.file_resource_list_denied"); diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 821a5c37f0..1a5dd41613 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -921,7 +921,10 @@ describe("agent issue mutation checkout ownership", () => { .post(`/api/issues/${issueId}/comments`) .send({ body: "Wrong company." }); - expect(res.status, JSON.stringify(res.body)).toBe(403); + // Cross-tenant requests return 404 (not 403) so the response is + // indistinguishable from a nonexistent issue — no existence oracle. + expect(res.status, JSON.stringify(res.body)).toBe(404); + expect(res.body.error).toBe("Issue not found"); expect(mockAccessService.decide).not.toHaveBeenCalledWith(expect.objectContaining({ action: "issue:comment" })); expect(mockIssueService.addComment).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/issue-attachment-routes.test.ts b/server/src/__tests__/issue-attachment-routes.test.ts index 42a9423392..648f2b4fc9 100644 --- a/server/src/__tests__/issue-attachment-routes.test.ts +++ b/server/src/__tests__/issue-attachment-routes.test.ts @@ -519,7 +519,10 @@ describe("issue attachment routes", () => { const app = await createApp(storage, { companyIds: ["company-2"], source: "session" }); const res = await request(app).get("/api/attachments/attachment-1/content"); - expect(res.status).toBe(403); + // Cross-tenant reads return 404 (not 403) so the status code cannot be + // used as an existence oracle for other tenants' attachment ids. + expect(res.status).toBe(404); + expect(res.body.error).toBe("Attachment not found"); expect(storage.getObject).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/issue-blocker-diagnostics-routes.test.ts b/server/src/__tests__/issue-blocker-diagnostics-routes.test.ts index b9c53105c9..e488f0266e 100644 --- a/server/src/__tests__/issue-blocker-diagnostics-routes.test.ts +++ b/server/src/__tests__/issue-blocker-diagnostics-routes.test.ts @@ -344,7 +344,9 @@ describeEmbeddedPostgres("issue blocker diagnostics route", () => { const res = await request(createApp(db, agentActor(companyB, agentB, runB!.id))) .get(`/api/issues/${issueA.id}/diagnostics/blockers`); - expect(res.status, JSON.stringify(res.body)).toBe(403); + // Uniform 404 so cross-tenant ids are indistinguishable from missing ones. + expect(res.status, JSON.stringify(res.body)).toBe(404); + expect(res.body.error).toBe("Issue not found"); }); it("caps blocker output and withholds readiness when truncated", async () => { diff --git a/server/src/__tests__/issue-recovery-actions.test.ts b/server/src/__tests__/issue-recovery-actions.test.ts index 54a740c1cf..136c9efb92 100644 --- a/server/src/__tests__/issue-recovery-actions.test.ts +++ b/server/src/__tests__/issue-recovery-actions.test.ts @@ -1264,7 +1264,7 @@ describeEmbeddedPostgres("issue recovery actions", () => { outcome: "restored", sourceIssueStatus: "done", }) - .expect(403); + .expect(404); const [actionRow] = await db .select() diff --git a/server/src/__tests__/issue-scheduled-retry-routes.test.ts b/server/src/__tests__/issue-scheduled-retry-routes.test.ts index 1c43ea7f83..2828e9ff00 100644 --- a/server/src/__tests__/issue-scheduled-retry-routes.test.ts +++ b/server/src/__tests__/issue-scheduled-retry-routes.test.ts @@ -348,14 +348,14 @@ describeEmbeddedPostgres("issue scheduled retry routes", () => { expect(res.status).toBe(403); }); - it("enforces company scoping for retry-now", async () => { + it("enforces company scoping for retry-now with a uniform 404", async () => { const { issueId } = await seedIssueWithRetry(); const res = await request(createApp(boardActor(randomUUID()))) .post(`/api/issues/${issueId}/scheduled-retry/retry-now`) .send({}); - expect(res.status).toBe(403); + expect(res.status).toBe(404); }); it("suppresses retry-now when the issue is under a budget hard-stop", async () => { diff --git a/server/src/__tests__/issue-stale-execution-lock-routes.test.ts b/server/src/__tests__/issue-stale-execution-lock-routes.test.ts index 944948ec03..6f1a8ce4d7 100644 --- a/server/src/__tests__/issue-stale-execution-lock-routes.test.ts +++ b/server/src/__tests__/issue-stale-execution-lock-routes.test.ts @@ -379,7 +379,7 @@ describeEmbeddedPostgres("stale issue execution lock routes", () => { source: "session", })) .post(`/api/issues/${issueId}/admin/force-release`) - .expect(403); + .expect(404); const res = await request(createApp(boardActor(companyId))) .post(`/api/issues/${issueId}/admin/force-release?clearAssignee=true`) diff --git a/server/src/__tests__/issue-subtree-diagnostics-routes.test.ts b/server/src/__tests__/issue-subtree-diagnostics-routes.test.ts index 7c77ce03ec..626662a1f4 100644 --- a/server/src/__tests__/issue-subtree-diagnostics-routes.test.ts +++ b/server/src/__tests__/issue-subtree-diagnostics-routes.test.ts @@ -424,6 +424,8 @@ describeEmbeddedPostgres("issue subtree diagnostics route", () => { const res = await request(createApp(db, agentActor(companyB, agentB, runB!.id))) .get(`/api/issues/${issueA.id}/diagnostics/subtree`); - expect(res.status, JSON.stringify(res.body)).toBe(403); + // Uniform 404 so cross-tenant ids are indistinguishable from missing ones. + expect(res.status, JSON.stringify(res.body)).toBe(404); + expect(res.body.error).toBe("Issue not found"); }); }); diff --git a/server/src/__tests__/issue-tree-control-routes.test.ts b/server/src/__tests__/issue-tree-control-routes.test.ts index c71a354b25..31df97bb7e 100644 --- a/server/src/__tests__/issue-tree-control-routes.test.ts +++ b/server/src/__tests__/issue-tree-control-routes.test.ts @@ -64,7 +64,7 @@ describe("issue tree control routes", () => { mockHeartbeatService.wakeup.mockResolvedValue(null); }); - it("rejects cross-company preview requests before calling the preview service", async () => { + it("rejects cross-company preview requests with a uniform 404 before calling the preview service", async () => { const app = await createApp({ type: "board", userId: "user-1", @@ -77,7 +77,7 @@ describe("issue tree control routes", () => { .post("/api/issues/11111111-1111-4111-8111-111111111111/tree-control/preview") .send({ mode: "pause" }); - expect(res.status).toBe(403); + expect(res.status).toBe(404); expect(mockTreeControlService.preview).not.toHaveBeenCalled(); expect(mockLogActivity).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/issue-wake-diagnostics-routes.test.ts b/server/src/__tests__/issue-wake-diagnostics-routes.test.ts index e361a51239..e07093fdf0 100644 --- a/server/src/__tests__/issue-wake-diagnostics-routes.test.ts +++ b/server/src/__tests__/issue-wake-diagnostics-routes.test.ts @@ -445,7 +445,9 @@ describeEmbeddedPostgres("issue wake diagnostics route", () => { const res = await request(createApp(db, agentActor(companyB, agentB, runB!.id))) .get(`/api/issues/${issueA.id}/diagnostics/wakes`); - expect(res.status, JSON.stringify(res.body)).toBe(403); + // Uniform 404 so cross-tenant ids are indistinguishable from missing ones. + expect(res.status, JSON.stringify(res.body)).toBe(404); + expect(res.body.error).toBe("Issue not found"); }); it("projects activity records and wake failures without raw blobs", async () => { diff --git a/server/src/__tests__/issue-watchdogs-routes.test.ts b/server/src/__tests__/issue-watchdogs-routes.test.ts index c3139b147b..3d2922f492 100644 --- a/server/src/__tests__/issue-watchdogs-routes.test.ts +++ b/server/src/__tests__/issue-watchdogs-routes.test.ts @@ -607,7 +607,9 @@ describeEmbeddedPostgres("issue watchdog routes", () => { const foreignIssue = await request(app) .put(`/api/issues/${otherIssueId}/watchdog`) .send({ agentId: otherAgentId }); - expect(foreignIssue.status, JSON.stringify(foreignIssue.body)).toBe(403); + // Uniform 404 so cross-tenant ids are indistinguishable from missing ones. + expect(foreignIssue.status, JSON.stringify(foreignIssue.body)).toBe(404); + expect(foreignIssue.body.error).toBe("Issue not found"); const foreignAgent = await request(app) .put(`/api/issues/${issueId}/watchdog`) diff --git a/server/src/__tests__/permissions-upgrade-boundary-routes.test.ts b/server/src/__tests__/permissions-upgrade-boundary-routes.test.ts index 2d53659115..6dffd0e907 100644 --- a/server/src/__tests__/permissions-upgrade-boundary-routes.test.ts +++ b/server/src/__tests__/permissions-upgrade-boundary-routes.test.ts @@ -284,8 +284,10 @@ describeEmbeddedPostgres("permissions upgrade visibility and route boundaries", const res = await request(await createApp(db, agentActor(sourceCompany.id, sourceAgent.id))) .get(`/api/issues/${issue.id}`); - expect(res.status).toBe(403); - expect(res.body.error).toContain("Agent key cannot access another company"); + // Cross-tenant reads return 404 (not 403) so the response is + // indistinguishable from a nonexistent issue — no existence oracle. + expect(res.status).toBe(404); + expect(res.body.error).toBe("Issue not found"); }); it("allows same-company route assignment after upgrade but keeps private target assignment grant constrained", async () => { diff --git a/server/src/__tests__/routine-document-annotation-routes.test.ts b/server/src/__tests__/routine-document-annotation-routes.test.ts index 66d791f290..9f2bdc8360 100644 --- a/server/src/__tests__/routine-document-annotation-routes.test.ts +++ b/server/src/__tests__/routine-document-annotation-routes.test.ts @@ -295,8 +295,10 @@ describe("routine description annotation routes", () => { }); it("rejects agent cross-company routine annotation reads", async () => { + // Cross-tenant requests return 404 (not 403) so the status code cannot be + // used as an existence oracle for other tenants' routine ids. await request(await createApp("agent", otherCompanyId)) .get(`/api/routines/${routineId}/description/annotations`) - .expect(403); + .expect(404); }); }); diff --git a/server/src/__tests__/routines-routes.test.ts b/server/src/__tests__/routines-routes.test.ts index 3aa806f409..15263c9b1f 100644 --- a/server/src/__tests__/routines-routes.test.ts +++ b/server/src/__tests__/routines-routes.test.ts @@ -432,10 +432,33 @@ describe("routine routes", () => { const res = await request(app).get(`/api/routines/${routineId}/revisions`); - expect(res.status).toBe(403); + expect(res.status).toBe(404); expect(mockRoutineService.listRevisions).not.toHaveBeenCalled(); }); + it("returns an identical 404 body for missing and cross-tenant routine triggers", async () => { + const crossTenantApp = await createApp({ + type: "board", + userId: "board-user", + source: "session", + isInstanceAdmin: false, + companyIds: ["99999999-9999-4999-8999-999999999999"], + }); + const crossTenant = await request(crossTenantApp) + .patch(`/api/routine-triggers/${trigger.id}`) + .send({ kind: "cron", config: { expression: "0 9 * * *" } }); + + mockRoutineService.getTrigger.mockResolvedValue(null); + const missing = await request(crossTenantApp) + .patch(`/api/routine-triggers/${trigger.id}`) + .send({ kind: "cron", config: { expression: "0 9 * * *" } }); + + expect(crossTenant.status).toBe(404); + expect(missing.status).toBe(404); + expect(crossTenant.body).toEqual(missing.body); + expect(mockRoutineService.updateTrigger).not.toHaveBeenCalled(); + }); + it("requires an assigned agent for routine revision history access", async () => { const app = await createApp({ type: "agent", diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index 8c984d48b6..aa6832ba11 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -34,6 +34,8 @@ const mockSecretService = vi.hoisted(() => ({ removeCurrentUserSecretValue: vi.fn(), previewRemoteImport: vi.fn(), importRemoteSecrets: vi.fn(), + listBindingReferences: vi.fn(), + listAccessEvents: vi.fn(), })); const mockLogActivity = vi.hoisted(() => vi.fn()); @@ -839,6 +841,112 @@ describe("secret routes", () => { expect(JSON.stringify(mockLogActivity.mock.calls)).not.toContain("shared/repointed"); }); + it("returns 404 for cross-tenant GET /secrets/:id/usage without leaking existence", async () => { + mockSecretService.getById.mockResolvedValue({ + id: "44444444-4444-4444-8444-444444444444", + companyId: "company-2", + name: "Other tenant secret", + key: "other-secret", + provider: "aws_secrets_manager", + managedMode: "paperclip_managed", + }); + + const crossTenantApp = createApp({ + type: "board", + userId: "mallory", + source: "session", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + isInstanceAdmin: false, + }); + + const res = await request(crossTenantApp).get( + "/api/secrets/44444444-4444-4444-8444-444444444444/usage", + ); + + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: "Secret not found" }); + expect(mockSecretService.listBindingReferences).not.toHaveBeenCalled(); + }); + + it("returns 404 for missing GET /secrets/:id/usage with identical response shape", async () => { + mockSecretService.getById.mockResolvedValue(null); + + const res = await request(createApp()).get( + "/api/secrets/55555555-5555-4555-8555-555555555555/usage", + ); + + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: "Secret not found" }); + expect(mockSecretService.listBindingReferences).not.toHaveBeenCalled(); + }); + + it("returns 404 for cross-tenant GET /secrets/:id/access-events without leaking existence", async () => { + mockSecretService.getById.mockResolvedValue({ + id: "66666666-6666-4666-8666-666666666666", + companyId: "company-2", + name: "Other tenant secret", + key: "other-secret", + provider: "aws_secrets_manager", + managedMode: "paperclip_managed", + }); + + const crossTenantApp = createApp({ + type: "board", + userId: "mallory", + source: "session", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "admin" }], + isInstanceAdmin: false, + }); + + const res = await request(crossTenantApp).get( + "/api/secrets/66666666-6666-4666-8666-666666666666/access-events", + ); + + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: "Secret not found" }); + expect(mockSecretService.listAccessEvents).not.toHaveBeenCalled(); + }); + + it("returns 404 for missing GET /secrets/:id/access-events with identical response shape", async () => { + mockSecretService.getById.mockResolvedValue(null); + + const res = await request(createApp()).get( + "/api/secrets/77777777-7777-4777-8777-777777777777/access-events", + ); + + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: "Secret not found" }); + expect(mockSecretService.listAccessEvents).not.toHaveBeenCalled(); + }); + + it("returns usage bindings for in-tenant GET /secrets/:id/usage", async () => { + mockSecretService.getById.mockResolvedValue({ + id: "88888888-8888-4888-8888-888888888888", + companyId: "company-1", + name: "OpenAI", + key: "openai", + provider: "aws_secrets_manager", + managedMode: "paperclip_managed", + }); + mockSecretService.listBindingReferences.mockResolvedValue([]); + + const res = await request(createApp()).get( + "/api/secrets/88888888-8888-4888-8888-888888888888/usage", + ); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ + secretId: "88888888-8888-4888-8888-888888888888", + bindings: [], + }); + expect(mockSecretService.listBindingReferences).toHaveBeenCalledWith( + "company-1", + "88888888-8888-4888-8888-888888888888", + ); + }); + it("allows DELETE to retry cleanup for already soft-deleted secrets", async () => { const secret = { id: "33333333-3333-4333-8333-333333333333", diff --git a/server/src/__tests__/write-path-membership.test.ts b/server/src/__tests__/write-path-membership.test.ts new file mode 100644 index 0000000000..2a4a765843 --- /dev/null +++ b/server/src/__tests__/write-path-membership.test.ts @@ -0,0 +1,253 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.unmock("http"); +vi.unmock("node:http"); + +// Tests that verify the write-path membership/role checks restored by Fix 1. +// `assertCompanyAccess` (in authz.ts) must reject viewer-role users and +// inactive members on non-safe HTTP methods (POST/PUT/PATCH/DELETE), even when +// `hasCompanyAccess` would let them through the 404 oracle gate. + +const companyId = "11111111-1111-4111-8111-111111111111"; +const goalId = "22222222-2222-4222-8222-222222222222"; + +const baseGoal = { + id: goalId, + companyId, + level: "company" as const, + title: "Q3 goal", + description: null, + parentId: null, + ownerAgentId: null, + createdAt: new Date("2026-04-11T00:00:00.000Z"), + updatedAt: new Date("2026-04-11T00:00:00.000Z"), +}; + +const mockGoalService = vi.hoisted(() => ({ + list: vi.fn(), + getById: vi.fn(), + create: vi.fn(), + update: vi.fn(), + remove: vi.fn(), +})); + +const mockLogActivity = vi.hoisted(() => vi.fn()); +const mockGetTelemetryClient = vi.hoisted(() => vi.fn()); + +vi.mock("@paperclipai/shared/telemetry", () => ({ + trackGoalCreated: vi.fn(), +})); + +vi.mock("../telemetry.js", () => ({ + getTelemetryClient: mockGetTelemetryClient, +})); + +vi.mock("../services/index.js", () => ({ + goalService: () => mockGoalService, + logActivity: mockLogActivity, +})); + +let routeModules: + | Promise<[ + typeof import("../middleware/index.js"), + typeof import("../routes/goals.js"), + ]> + | null = null; + +async function loadRouteModules() { + routeModules ??= Promise.all([ + import("../middleware/index.js"), + import("../routes/goals.js"), + ]); + return routeModules; +} + +async function createApp(actor: Record) { + const [{ errorHandler }, { goalRoutes }] = await loadRouteModules(); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = { ...actor }; + next(); + }); + app.use("/api", goalRoutes({} as any)); + app.use(errorHandler); + return app; +} + +async function requestApp( + app: express.Express, + buildRequest: (baseUrl: string) => request.Test, +) { + const { createServer } = await vi.importActual("node:http"); + const server = createServer(app); + try { + await new Promise((resolve) => { + server.listen(0, "127.0.0.1", resolve); + }); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected HTTP server to listen on a TCP port"); + } + return await buildRequest(`http://127.0.0.1:${address.port}`); + } finally { + if (server.listening) { + await new Promise((resolve, reject) => { + server.close((error) => { + if (error) reject(error); + else resolve(); + }); + }); + } + } +} + +function resetMocks() { + vi.clearAllMocks(); + for (const mock of Object.values(mockGoalService)) mock.mockReset(); + mockGoalService.list.mockImplementation(async () => []); + mockGoalService.getById.mockImplementation(async () => ({ ...baseGoal })); + mockGoalService.create.mockImplementation(async () => ({ ...baseGoal })); + mockGoalService.update.mockImplementation(async () => ({ ...baseGoal })); + mockGoalService.remove.mockImplementation(async () => ({ ...baseGoal })); + mockLogActivity.mockImplementation(async () => undefined); + mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); +} + +describe.sequential("write-path membership checks (viewer / inactive)", () => { + beforeEach(() => { + resetMocks(); + }); + + describe("viewer role", () => { + const viewerActor = { + type: "board" as const, + userId: "viewer-user", + companyIds: [companyId], + source: "session" as const, + isInstanceAdmin: false, + memberships: [ + { companyId, status: "active", membershipRole: "viewer" }, + ], + }; + + it("rejects PATCH on a goal with 403 'Viewer access is read-only'", async () => { + const app = await createApp(viewerActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).patch(`/api/goals/${goalId}`).send({ title: "New title" }), + ); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("Viewer access is read-only"); + expect(mockGoalService.update).not.toHaveBeenCalled(); + }); + + it("rejects DELETE on a goal with 403 'Viewer access is read-only'", async () => { + const app = await createApp(viewerActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).delete(`/api/goals/${goalId}`), + ); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("Viewer access is read-only"); + expect(mockGoalService.remove).not.toHaveBeenCalled(); + }); + + it("rejects POST (create) on a company's goals with 403 'Viewer access is read-only'", async () => { + const app = await createApp(viewerActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post(`/api/companies/${companyId}/goals`) + .send({ level: "company", title: "New goal" }), + ); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("Viewer access is read-only"); + expect(mockGoalService.create).not.toHaveBeenCalled(); + }); + + it("still permits GET on the same goal (read-only access is preserved)", async () => { + const app = await createApp(viewerActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).get(`/api/goals/${goalId}`), + ); + + expect(res.status).toBe(200); + expect(res.body.id).toBe(goalId); + expect(mockGoalService.getById).toHaveBeenCalledWith(goalId); + }); + }); + + describe("inactive membership", () => { + const inactiveActor = { + type: "board" as const, + userId: "ex-employee", + companyIds: [companyId], + source: "session" as const, + isInstanceAdmin: false, + memberships: [ + { companyId, status: "removed", membershipRole: "editor" }, + ], + }; + + it("rejects PATCH on a goal with 403 'User does not have active company access'", async () => { + const app = await createApp(inactiveActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).patch(`/api/goals/${goalId}`).send({ title: "New title" }), + ); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("User does not have active company access"); + expect(mockGoalService.update).not.toHaveBeenCalled(); + }); + + it("rejects DELETE on a goal with 403 'User does not have active company access'", async () => { + const app = await createApp(inactiveActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).delete(`/api/goals/${goalId}`), + ); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("User does not have active company access"); + expect(mockGoalService.remove).not.toHaveBeenCalled(); + }); + + it("rejects POST on a company's goals with 403 'User does not have active company access'", async () => { + const app = await createApp(inactiveActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl) + .post(`/api/companies/${companyId}/goals`) + .send({ level: "company", title: "New goal" }), + ); + + expect(res.status).toBe(403); + expect(res.body.error).toBe("User does not have active company access"); + expect(mockGoalService.create).not.toHaveBeenCalled(); + }); + }); + + describe("active editor (sanity check)", () => { + const editorActor = { + type: "board" as const, + userId: "editor-user", + companyIds: [companyId], + source: "session" as const, + isInstanceAdmin: false, + memberships: [ + { companyId, status: "active", membershipRole: "editor" }, + ], + }; + + it("allows PATCH on a goal", async () => { + const app = await createApp(editorActor); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).patch(`/api/goals/${goalId}`).send({ title: "New title" }), + ); + + expect(res.status).toBe(200); + expect(mockGoalService.update).toHaveBeenCalledWith(goalId, { title: "New title" }); + }); + }); +}); diff --git a/server/src/routes/activity.ts b/server/src/routes/activity.ts index 9648eb4d1d..7fe6894cbf 100644 --- a/server/src/routes/activity.ts +++ b/server/src/routes/activity.ts @@ -4,7 +4,7 @@ import type { Db } from "@paperclipai/db"; import { normalizeIssueIdentifier } from "@paperclipai/shared"; import { validate } from "../middleware/validate.js"; import { activityService, normalizeActivityLimit } from "../services/activity.js"; -import { assertAuthenticated, assertBoard, assertCompanyAccess } from "./authz.js"; +import { assertAuthenticated, assertBoard, assertCompanyAccess, getAccessibleResource, hasCompanyAccess } from "./authz.js"; import { accessService, heartbeatService, issueService } from "../services/index.js"; import { sanitizeRecord } from "../redaction.js"; @@ -102,12 +102,8 @@ export function activityRoutes(db: Db) { router.get("/issues/:id/activity", async (req, res) => { const rawId = req.params.id as string; - const issue = await resolveIssueByRef(rawId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, resolveIssueByRef(rawId), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const result = await svc.forIssue(issue.id); res.json(result); @@ -115,12 +111,8 @@ export function activityRoutes(db: Db) { router.get("/issues/:id/runs", async (req, res) => { const rawId = req.params.id as string; - const issue = await resolveIssueByRef(rawId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, resolveIssueByRef(rawId), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const result = await svc.runsForIssue(issue.companyId, issue.id); res.json(result); @@ -130,7 +122,10 @@ export function activityRoutes(db: Db) { assertAuthenticated(req); const runId = req.params.runId as string; const run = await heartbeat.getRun(runId); - if (!run) { + if (!run || !hasCompanyAccess(req, run.companyId)) { + // Return `200 []` for both "doesn't exist" and "cross-tenant" — preserves the + // legacy API contract while keeping the cross-tenant existence oracle closed + // (both branches yield indistinguishable responses). res.json([]); return; } diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 8de49215b6..21e1970819 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -54,7 +54,7 @@ import { workspaceOperationService, } from "../services/index.js"; import { conflict, forbidden, HttpError, notFound, unprocessable } from "../errors.js"; -import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, assertInstanceAdmin, getAccessibleResource, getActorInfo, hasCompanyAccess } from "./authz.js"; import { assertNoAgentHostWorkspaceCommandMutation, collectAgentAdapterWorkspaceCommandPaths, @@ -803,12 +803,8 @@ export function agentRoutes( } async function getAccessibleAgent(req: Request, res: Response, id: string) { - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return null; - } - assertCompanyAccess(req, agent.companyId); + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return null; if (req.actor.type === "board") { await assertBoardCanManageAgentsForCompany(req, agent.companyId); } @@ -902,6 +898,9 @@ export function agentRoutes( } async function assertCanUpdateAgent(req: Request, targetAgent: { id: string; companyId: string }) { + if (!hasCompanyAccess(req, targetAgent.companyId)) { + throw notFound("Agent not found"); + } assertCompanyAccess(req, targetAgent.companyId); const decision = await access.decide({ actor: req.actor, @@ -913,6 +912,9 @@ export function agentRoutes( } async function assertCanReadAgent(req: Request, targetAgent: { id: string; companyId: string }) { + if (!hasCompanyAccess(req, targetAgent.companyId)) { + throw notFound("Agent not found"); + } assertCompanyAccess(req, targetAgent.companyId); if (req.actor.type === "board") { await assertCanReadConfigurations(req, targetAgent.companyId); @@ -1399,6 +1401,9 @@ export function agentRoutes( targetAgent: { id: string; companyId: string }, targetKeys: string[], ) { + if (!hasCompanyAccess(req, targetAgent.companyId)) { + throw notFound("Agent not found"); + } assertCompanyAccess(req, targetAgent.companyId); const changeScope = { requiresChangeGrant: true }; const decision = await access.decide({ @@ -1869,11 +1874,8 @@ export function agentRoutes( validate(agentSkillSyncSchema), async (req, res) => { const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; await assertCanUpdateAgent(req, agent); const requestedSkills = normalizeDesiredSkillSelections(req.body.desiredSkills); @@ -2185,12 +2187,8 @@ export function agentRoutes( router.get("/agents/:id", async (req, res) => { const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } - assertCompanyAccess(req, agent.companyId); + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; if (!(await assertAgentReadAllowed(req, res, agent))) return; const isSelf = req.actor.type === "agent" && req.actor.agentId === id; if (isSelf) { @@ -2257,11 +2255,8 @@ export function agentRoutes( router.post("/agents/:id/config-revisions/:revisionId/rollback", async (req, res) => { const id = req.params.id as string; const revisionId = req.params.revisionId as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanUpdateAgent(req, existing); const actor = getActorInfo(req); @@ -2292,13 +2287,9 @@ export function agentRoutes( router.get("/agents/:id/runtime-state", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; await assertBoardCanManageAgentsForCompany(req, agent.companyId); - assertCompanyAccess(req, agent.companyId); const state = await heartbeat.getRuntimeState(id); res.json(state); @@ -2307,13 +2298,9 @@ export function agentRoutes( router.get("/agents/:id/task-sessions", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; await assertBoardCanManageAgentsForCompany(req, agent.companyId); - assertCompanyAccess(req, agent.companyId); const sessions = await heartbeat.listTaskSessions(id); res.json( @@ -2327,13 +2314,9 @@ export function agentRoutes( router.post("/agents/:id/runtime-state/reset-session", validate(resetAgentSessionSchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; await assertBoardCanManageAgentsForCompany(req, agent.companyId); - assertCompanyAccess(req, agent.companyId); const taskKey = typeof req.body.taskKey === "string" && req.body.taskKey.trim().length > 0 @@ -2656,12 +2639,8 @@ export function agentRoutes( router.patch("/agents/:id/permissions", validate(updateAgentPermissionsSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; if (req.actor.type === "agent") { const actorAgent = req.actor.agentId ? await svc.getById(req.actor.agentId) : null; @@ -2722,11 +2701,8 @@ export function agentRoutes( } const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanManageInstructionsPath(req, existing); @@ -2800,22 +2776,16 @@ export function agentRoutes( router.get("/agents/:id/instructions-bundle", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanReadAgent(req, existing); res.json(await instructions.getBundle(existing)); }); router.patch("/agents/:id/instructions-bundle", validate(updateAgentInstructionsBundleSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanManageInstructionsPath(req, existing); const actor = getActorInfo(req); @@ -2859,11 +2829,8 @@ export function agentRoutes( router.get("/agents/:id/instructions-bundle/file", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanReadAgent(req, existing); const relativePath = typeof req.query.path === "string" ? req.query.path : ""; @@ -2877,11 +2844,8 @@ export function agentRoutes( router.put("/agents/:id/instructions-bundle/file", validate(upsertAgentInstructionsFileSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanManageInstructionsPath(req, existing); const actor = getActorInfo(req); @@ -2926,11 +2890,8 @@ export function agentRoutes( router.delete("/agents/:id/instructions-bundle/file", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; await assertCanManageInstructionsPath(req, existing); const relativePath = typeof req.query.path === "string" ? req.query.path : ""; @@ -2960,12 +2921,8 @@ export function agentRoutes( router.patch("/agents/:id", validate(updateAgentSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Agent not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!existing) return; if (hasOwn(req.body as object, "permissions")) { res.status(422).json({ error: "Use /api/agents/:id/permissions for permission changes" }); @@ -3446,12 +3403,8 @@ export function agentRoutes( opts: WakeupRouteOpts, ): Promise => { const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } - assertCompanyAccess(req, agent.companyId); + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; if (req.actor.type === "agent") { if (req.actor.agentId !== id) { @@ -3520,12 +3473,8 @@ export function agentRoutes( // an empty body produces the original fixed-arg `heartbeat.invoke()` // shape exactly. const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } - assertCompanyAccess(req, agent.companyId); + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; if (req.actor.type === "agent") { if (req.actor.agentId !== id) { @@ -3598,13 +3547,9 @@ export function agentRoutes( router.post("/agents/:id/claude-login", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const agent = await svc.getById(id); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const agent = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); + if (!agent) return; await assertBoardCanManageAgentsForCompany(req, agent.companyId); - assertCompanyAccess(req, agent.companyId); if (agent.adapterType !== "claude_local") { res.status(400).json({ error: "Login is only supported for claude_local agents" }); return; @@ -3725,12 +3670,8 @@ export function agentRoutes( router.get("/heartbeat-runs/:runId", async (req, res) => { const runId = req.params.runId as string; - const run = await heartbeat.getRun(runId); - if (!run) { - res.status(404).json({ error: "Heartbeat run not found" }); - return; - } - assertCompanyAccess(req, run.companyId); + const run = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); + if (!run) return; const retryExhaustedReason = await heartbeat.getRetryExhaustedReason(runId); const decoratedRun = heartbeat.decorateActiveRunStatus(run); res.json( @@ -3744,10 +3685,8 @@ export function agentRoutes( router.post("/heartbeat-runs/:runId/cancel", async (req, res) => { assertBoard(req); const runId = req.params.runId as string; - const existing = await heartbeat.getRun(runId); - if (existing) { - assertCompanyAccess(req, existing.companyId); - } + const existing = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); + if (!existing) return; const run = await heartbeat.cancelRun(runId); if (run) { @@ -3767,12 +3706,8 @@ export function agentRoutes( router.post("/heartbeat-runs/:runId/watchdog-decisions", async (req, res) => { const runId = req.params.runId as string; - const existing = await heartbeat.getRun(runId); - if (!existing) { - res.status(404).json({ error: "Heartbeat run not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); + if (!existing) return; const decision = typeof req.body?.decision === "string" ? req.body.decision : ""; if (!["snooze", "continue", "dismissed_false_positive"].includes(decision)) { res.status(400).json({ error: "Unsupported watchdog decision" }); @@ -3803,12 +3738,8 @@ export function agentRoutes( router.get("/heartbeat-runs/:runId/events", async (req, res) => { const runId = req.params.runId as string; - const run = await heartbeat.getRun(runId); - if (!run) { - res.status(404).json({ error: "Heartbeat run not found" }); - return; - } - assertCompanyAccess(req, run.companyId); + const run = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); + if (!run) return; const afterSeq = Number(req.query.afterSeq ?? 0); const limit = Number(req.query.limit ?? 200); @@ -3825,12 +3756,8 @@ export function agentRoutes( router.get("/heartbeat-runs/:runId/log", async (req, res) => { const runId = req.params.runId as string; - const run = await heartbeat.getRunLogAccess(runId); - if (!run) { - res.status(404).json({ error: "Heartbeat run not found" }); - return; - } - assertCompanyAccess(req, run.companyId); + const run = await getAccessibleResource(req, res, heartbeat.getRunLogAccess(runId), "Heartbeat run not found"); + if (!run) return; const offset = Number(req.query.offset ?? 0); const limitBytes = readRunLogLimitBytes(req.query.limitBytes); @@ -3845,12 +3772,8 @@ export function agentRoutes( router.get("/heartbeat-runs/:runId/workspace-operations", async (req, res) => { const runId = req.params.runId as string; - const run = await heartbeat.getRun(runId); - if (!run) { - res.status(404).json({ error: "Heartbeat run not found" }); - return; - } - assertCompanyAccess(req, run.companyId); + const run = await getAccessibleResource(req, res, heartbeat.getRun(runId), "Heartbeat run not found"); + if (!run) return; const context = asRecord(run.contextSnapshot); const executionWorkspaceId = asNonEmptyString(context?.executionWorkspaceId); @@ -3860,12 +3783,8 @@ export function agentRoutes( router.get("/workspace-operations/:operationId/log", async (req, res) => { const operationId = req.params.operationId as string; - const operation = await workspaceOperations.getById(operationId); - if (!operation) { - res.status(404).json({ error: "Workspace operation not found" }); - return; - } - assertCompanyAccess(req, operation.companyId); + const operation = await getAccessibleResource(req, res, workspaceOperations.getById(operationId), "Workspace operation not found"); + if (!operation) return; const offset = Number(req.query.offset ?? 0); const limitBytes = readRunLogLimitBytes(req.query.limitBytes); @@ -3882,12 +3801,13 @@ export function agentRoutes( const rawId = req.params.issueId as string; const issueSvc = issueService(db); const identifier = normalizeIssueIdentifier(rawId); - const issue = identifier ? await issueSvc.getByIdentifier(identifier) : await issueSvc.getById(rawId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource( + req, + res, + identifier ? issueSvc.getByIdentifier(identifier) : issueSvc.getById(rawId), + "Issue not found", + ); + if (!issue) return; const liveRuns = await db .select({ @@ -3936,12 +3856,13 @@ export function agentRoutes( const rawId = req.params.issueId as string; const issueSvc = issueService(db); const identifier = normalizeIssueIdentifier(rawId); - const issue = identifier ? await issueSvc.getByIdentifier(identifier) : await issueSvc.getById(rawId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource( + req, + res, + identifier ? issueSvc.getByIdentifier(identifier) : issueSvc.getById(rawId), + "Issue not found", + ); + if (!issue) return; let run = issue.executionRunId ? await heartbeat.getRunIssueSummary(issue.executionRunId) : null; if ( diff --git a/server/src/routes/approvals.ts b/server/src/routes/approvals.ts index 766220a0f2..f75f816215 100644 --- a/server/src/routes/approvals.ts +++ b/server/src/routes/approvals.ts @@ -18,7 +18,7 @@ import { logActivity, secretService, } from "../services/index.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, getAccessibleResource, getActorInfo, hasCompanyAccess } from "./authz.js"; import { redactEventPayload } from "../redaction.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; @@ -55,7 +55,7 @@ export function approvalRoutes( async function requireApprovalAccess(req: Request, id: string) { const approval = await svc.getById(id); - if (!approval) { + if (!approval || !hasCompanyAccess(req, approval.companyId)) { return null; } assertCompanyAccess(req, approval.companyId); @@ -115,12 +115,8 @@ export function approvalRoutes( router.get("/approvals/:id", async (req, res) => { const id = req.params.id as string; - const approval = await svc.getById(id); - if (!approval) { - res.status(404).json({ error: "Approval not found" }); - return; - } - assertCompanyAccess(req, approval.companyId); + const approval = await getAccessibleResource(req, res, svc.getById(id), "Approval not found"); + if (!approval) return; if (!(await assertApprovalAccessAllowed(req, res, approval.companyId))) return; res.json(redactApprovalPayload(approval)); }); @@ -182,12 +178,8 @@ export function approvalRoutes( router.get("/approvals/:id/issues", async (req, res) => { const id = req.params.id as string; - const approval = await svc.getById(id); - if (!approval) { - res.status(404).json({ error: "Approval not found" }); - return; - } - assertCompanyAccess(req, approval.companyId); + const approval = await getAccessibleResource(req, res, svc.getById(id), "Approval not found"); + if (!approval) return; if (!(await assertApprovalAccessAllowed(req, res, approval.companyId))) return; const issues = await issueApprovalsSvc.listIssuesForApproval(id); res.json(issues); @@ -343,12 +335,8 @@ export function approvalRoutes( router.post("/approvals/:id/resubmit", validate(resubmitApprovalSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Approval not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Approval not found"); + if (!existing) return; if (!(await assertApprovalMutationAllowedByRunContext(req, res, existing.companyId))) return; if (req.actor.type === "agent" && req.actor.agentId !== existing.requestedByAgentId) { @@ -382,24 +370,16 @@ export function approvalRoutes( router.get("/approvals/:id/comments", async (req, res) => { const id = req.params.id as string; - const approval = await svc.getById(id); - if (!approval) { - res.status(404).json({ error: "Approval not found" }); - return; - } - assertCompanyAccess(req, approval.companyId); + const approval = await getAccessibleResource(req, res, svc.getById(id), "Approval not found"); + if (!approval) return; const comments = await svc.listComments(id); res.json(comments); }); router.post("/approvals/:id/comments", validate(addApprovalCommentSchema), async (req, res) => { const id = req.params.id as string; - const approval = await svc.getById(id); - if (!approval) { - res.status(404).json({ error: "Approval not found" }); - return; - } - assertCompanyAccess(req, approval.companyId); + const approval = await getAccessibleResource(req, res, svc.getById(id), "Approval not found"); + if (!approval) return; if (!(await assertApprovalMutationAllowedByRunContext(req, res, approval.companyId))) return; const actor = getActorInfo(req); const comment = await svc.addComment(id, req.body.body, { diff --git a/server/src/routes/assets.ts b/server/src/routes/assets.ts index 0a6f857ab5..6b43e39154 100644 --- a/server/src/routes/assets.ts +++ b/server/src/routes/assets.ts @@ -7,7 +7,7 @@ import { createAssetImageMetadataSchema } from "@paperclipai/shared"; import type { StorageService } from "../storage/types.js"; import { assetService, logActivity } from "../services/index.js"; import { isAllowedContentType, MAX_ATTACHMENT_BYTES } from "../attachment-types.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; const SVG_CONTENT_TYPE = "image/svg+xml"; const ALLOWED_COMPANY_LOGO_CONTENT_TYPES = new Set([ "image/png", @@ -311,12 +311,8 @@ export function assetRoutes(db: Db, storage: StorageService) { router.get("/assets/:assetId/content", async (req, res, next) => { const assetId = req.params.assetId as string; - const asset = await svc.getById(assetId); - if (!asset) { - res.status(404).json({ error: "Asset not found" }); - return; - } - assertCompanyAccess(req, asset.companyId); + const asset = await getAccessibleResource(req, res, svc.getById(assetId), "Asset not found"); + if (!asset) return; const object = await storage.getObject(asset.companyId, asset.objectKey); const responseContentType = asset.contentType || object.contentType || "application/octet-stream"; diff --git a/server/src/routes/authz.ts b/server/src/routes/authz.ts index 6e73a3ca17..634aa5f90d 100644 --- a/server/src/routes/authz.ts +++ b/server/src/routes/authz.ts @@ -1,4 +1,4 @@ -import type { Request } from "express"; +import type { Request, Response } from "express"; import { forbidden, HttpError, unauthorized } from "../errors.js"; import { logger } from "../middleware/logger.js"; import { responsibleUserAuthzShadowMode } from "../services/authorization.js"; @@ -119,6 +119,80 @@ export function assertCompanyAccess(req: Request, companyId: string) { } } +/** + * Non-throwing access check for routes that look up a resource by id + * before responding. Prefer this over `assertCompanyAccess` whenever the + * route can reach the access check only after a successful `getById` + * (i.e. after confirming the resource exists). + * + * Using `assertCompanyAccess` in that position leaks resource existence + * across tenants: a 404 means "no such resource" while a 403 means "exists + * in another tenant". Any authenticated user can enumerate IDs and + * distinguish the two responses. + * + * Most routes should use `getAccessibleResource` below, which wraps the + * whole pattern. When composing manually (bespoke not-found responses), + * the shape is: + * + * const issue = await svc.getById(id); + * if (!issue || !hasCompanyAccess(req, issue.companyId)) { + * res.status(404).json({ error: "Issue not found" }); + * return; + * } + * + * so both "does not exist" and "exists but cross-tenant" return the same + * 404, removing the oracle. + * + * Note: this intentionally does not replicate the write-path membership + * checks in `assertCompanyAccess` (active membership, viewer read-only). + * Routes that need those checks for authorized tenants should still call + * `assertCompanyAccess` after the 404 gate — the oracle concern is only + * about the existence check. + * + * The company-scope semantics must stay in lockstep with + * `assertCompanyAccess`: in particular, signed-in instance admins do NOT + * get blanket access to companies they are not a member of. + */ +export function hasCompanyAccess(req: Request, companyId: string): boolean { + if (req.actor.type === "none") return false; + if (req.actor.type === "agent") return req.actor.companyId === companyId; + if (req.actor.source === "local_implicit") return true; + return (req.actor.companyIds ?? []).includes(companyId); +} + +/** + * Preferred way to fetch a company-scoped resource by id inside a route + * handler. Wraps the two-step pattern described on `hasCompanyAccess` so + * new routes cannot accidentally reintroduce the existence oracle: + * + * - missing resource → 404 `{ error: notFoundMessage }`, returns null + * - exists but cross-tenant → identical 404, returns null + * - accessible → runs `assertCompanyAccess` (write-path + * membership checks on non-safe methods) and returns the resource + * + * Usage: + * + * const goal = await getAccessibleResource(req, res, svc.getById(id), "Goal not found"); + * if (!goal) return; + * + * Routes with bespoke not-found behavior (legacy `200 []` contracts, + * audit-logged denials) should still compose `hasCompanyAccess` directly. + */ +export async function getAccessibleResource( + req: Request, + res: Response, + resource: T | null | undefined | Promise, + notFoundMessage: string, +): Promise { + const resolved = await resource; + if (!resolved || !hasCompanyAccess(req, resolved.companyId)) { + res.status(404).json({ error: notFoundMessage }); + return null; + } + assertCompanyAccess(req, resolved.companyId); + return resolved; +} + export function getActorInfo(req: Request): ( { actorType: "agent"; diff --git a/server/src/routes/cases.ts b/server/src/routes/cases.ts index 15a3c43e4e..54c92f7e4e 100644 --- a/server/src/routes/cases.ts +++ b/server/src/routes/cases.ts @@ -31,7 +31,7 @@ import { validate } from "../middleware/validate.js"; import { instanceSettingsService } from "../services/instance-settings.js"; import { documentAnnotationService, logActivity } from "../services/index.js"; import type { StorageService } from "../storage/types.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertCompanyAccess, getActorInfo, hasCompanyAccess } from "./authz.js"; type CaseRouteDb = Db | Parameters[0]>[0]; type CaseActor = ReturnType; @@ -206,7 +206,7 @@ function caseLookupCompanyIds(req: Request) { async function assertCaseAccess(db: Db, req: Request, idOrIdentifier: string) { const row = await loadCaseByIdOrIdentifier(db, idOrIdentifier, caseLookupCompanyIds(req)); - if (!row) throw notFound("Case not found"); + if (!row || !hasCompanyAccess(req, row.companyId)) throw notFound("Case not found"); assertCompanyAccess(req, row.companyId); return row; } @@ -218,7 +218,7 @@ async function assertCaseAccess(db: Db, req: Request, idOrIdentifier: string) { async function resolveSharedPathCase(db: Db, req: Request, idOrIdentifier: string) { const companyIds = caseLookupCompanyIds(req); const row = await loadCaseByIdOrIdentifier(db, idOrIdentifier, companyIds); - if (!row) return null; + if (!row || !hasCompanyAccess(req, row.companyId)) return null; await assertCasesEnabled(db); assertCompanyAccess(req, row.companyId); return row; @@ -1422,7 +1422,7 @@ export function caseRoutes(db: Db, storage: StorageService) { await assertCasesEnabled(db); const issueIdOrIdentifier = (req.params.issueId as string).trim(); const issue = await loadIssueByIdOrIdentifier(db, issueIdOrIdentifier, caseLookupCompanyIds(req)); - if (!issue) throw notFound("Issue not found"); + if (!issue || !hasCompanyAccess(req, issue.companyId)) throw notFound("Issue not found"); assertCompanyAccess(req, issue.companyId); const rows = await db .select({ link: caseIssueLinks, caseRow: cases }) diff --git a/server/src/routes/costs.ts b/server/src/routes/costs.ts index a8e290f2ea..373adf0614 100644 --- a/server/src/routes/costs.ts +++ b/server/src/routes/costs.ts @@ -20,7 +20,7 @@ import { accessService, logActivity, } from "../services/index.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; import { fetchAllQuotaWindows } from "../services/quota-windows.js"; import { badRequest } from "../errors.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; @@ -181,12 +181,8 @@ export function costRoutes( router.get("/issues/:id/cost-summary", async (req, res) => { const rawId = req.params.id as string; - const issue = await resolveIssueByRef(rawId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, resolveIssueByRef(rawId), "Issue not found"); + if (!issue) return; if (!(await assertIssueCostReadAllowed(req, res, issue))) return; const excludeRoot = req.query.excludeRoot === "true" || req.query.excludeRoot === "1"; const summary = await costs.issueTreeSummary(issue.companyId, issue.id, { excludeRoot }); @@ -367,13 +363,9 @@ export function costRoutes( router.patch("/agents/:agentId/budgets", validate(updateBudgetSchema), async (req, res) => { const agentId = req.params.agentId as string; - const agent = await agents.getById(agentId); - if (!agent) { - res.status(404).json({ error: "Agent not found" }); - return; - } + const agent = await getAccessibleResource(req, res, agents.getById(agentId), "Agent not found"); + if (!agent) return; - assertCompanyAccess(req, agent.companyId); assertBoard(req); const updated = await agents.update(agentId, { budgetMonthlyCents: req.body.budgetMonthlyCents }); diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 9ecb9c2a90..86286d4b1a 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -686,22 +686,22 @@ export function environmentRoutes( }); router.get("/environments/:id", async (req, res) => { + assertCanReadInstanceEnvironments(req); const environment = await svc.getById(req.params.id as string); if (!environment) { res.status(404).json({ error: "Environment not found" }); return; } - assertCanReadInstanceEnvironments(req); res.json(presentEnvironmentForRead(req, environment)); }); router.get("/environments/:id/leases", async (req, res) => { + assertCanReadInstanceEnvironments(req); const environment = await svc.getById(req.params.id as string); if (!environment) { res.status(404).json({ error: "Environment not found" }); return; } - assertCanReadInstanceEnvironments(req); const leases = await svc.listLeases(environment.id, { status: req.query.status as string | undefined, }); @@ -709,22 +709,22 @@ export function environmentRoutes( }); router.get("/environment-leases/:leaseId", async (req, res) => { + assertCanReadInstanceEnvironments(req); const lease = await svc.getLeaseById(req.params.leaseId as string); if (!lease) { res.status(404).json({ error: "Environment lease not found" }); return; } - assertCanReadInstanceEnvironments(req); res.json(lease); }); router.patch("/environments/:id", validate(updateEnvironmentSchema), async (req, res) => { + assertCanAccessInstanceEnvironments(req); const existing = await svc.getById(req.params.id as string); if (!existing) { res.status(404).json({ error: "Environment not found" }); return; } - assertCanAccessInstanceEnvironments(req); const actor = getActorInfo(req); const nextDriver = req.body.driver ?? existing.driver; const nextName = req.body.name ?? existing.name; @@ -815,12 +815,12 @@ export function environmentRoutes( }); router.delete("/environments/:id", async (req, res) => { + assertCanAccessInstanceEnvironments(req); const existing = await svc.getById(req.params.id as string); if (!existing) { res.status(404).json({ error: "Environment not found" }); return; } - assertCanAccessInstanceEnvironments(req); const actor = getActorInfo(req); const impact = await svc.getDeleteBlastRadius(existing.id); if (!impact) { @@ -877,12 +877,12 @@ export function environmentRoutes( }); router.post("/environments/:id/probe", async (req, res) => { + assertCanAccessInstanceEnvironments(req); const environment = await svc.getById(req.params.id as string); if (!environment) { res.status(404).json({ error: "Environment not found" }); return; } - assertCanAccessInstanceEnvironments(req); const actor = getActorInfo(req); const companyIdForSecrets = await resolveEnvironmentSecretContextCompanyId(req, environment.id, { required: false }); const companyIdForProbe = companyIdForSecrets diff --git a/server/src/routes/execution-workspaces.ts b/server/src/routes/execution-workspaces.ts index 8c7e27d82a..7d598f722a 100644 --- a/server/src/routes/execution-workspaces.ts +++ b/server/src/routes/execution-workspaces.ts @@ -25,7 +25,7 @@ import { startRuntimeServicesForWorkspaceControl, stopRuntimeServicesForExecutionWorkspace, } from "../services/workspace-runtime.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; import { logger } from "../middleware/logger.js"; import { assertNoAgentHostWorkspaceCommandMutation, @@ -109,24 +109,16 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P router.get("/execution-workspaces/:id", async (req, res) => { const id = req.params.id as string; - const workspace = await svc.getById(id); - if (!workspace) { - res.status(404).json({ error: "Execution workspace not found" }); - return; - } - assertCompanyAccess(req, workspace.companyId); + const workspace = await getAccessibleResource(req, res, svc.getById(id), "Execution workspace not found"); + if (!workspace) return; if (!(await assertExecutionWorkspaceReadAllowed(req, res, workspace.companyId))) return; res.json(workspace); }); router.get("/execution-workspaces/:id/close-readiness", async (req, res) => { const id = req.params.id as string; - const workspace = await svc.getById(id); - if (!workspace) { - res.status(404).json({ error: "Execution workspace not found" }); - return; - } - assertCompanyAccess(req, workspace.companyId); + const workspace = await getAccessibleResource(req, res, svc.getById(id), "Execution workspace not found"); + if (!workspace) return; if (!(await assertExecutionWorkspaceReadAllowed(req, res, workspace.companyId))) return; const readiness = await svc.getCloseReadiness(id); if (!readiness) { @@ -138,12 +130,8 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P router.get("/execution-workspaces/:id/workspace-operations", async (req, res) => { const id = req.params.id as string; - const workspace = await svc.getById(id); - if (!workspace) { - res.status(404).json({ error: "Execution workspace not found" }); - return; - } - assertCompanyAccess(req, workspace.companyId); + const workspace = await getAccessibleResource(req, res, svc.getById(id), "Execution workspace not found"); + if (!workspace) return; if (!(await assertExecutionWorkspaceReadAllowed(req, res, workspace.companyId))) return; const operations = await workspaceOperationsSvc.listForExecutionWorkspace(id); res.json(operations); @@ -157,12 +145,8 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P return; } - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Execution workspace not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Execution workspace not found"); + if (!existing) return; if (!(await assertRuntimeManageAllowed(req, res, existing.companyId))) return; await assertCanManageExecutionWorkspaceRuntimeServices(db, req, { @@ -498,12 +482,8 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P router.post("/execution-workspaces/:id/reconcile-branch", validate(reconcileExecutionWorkspaceBranchSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Execution workspace not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Execution workspace not found"); + if (!existing) return; assertBoard(req); if (!(await assertRuntimeManageAllowed(req, res, existing.companyId))) return; @@ -590,12 +570,8 @@ export function executionWorkspaceRoutes(db: Db, opts: { pluginWorkerManager?: P router.patch("/execution-workspaces/:id", validate(updateExecutionWorkspaceSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Execution workspace not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Execution workspace not found"); + if (!existing) return; if (!(await assertRuntimeManageAllowed(req, res, existing.companyId))) return; assertNoAgentHostWorkspaceCommandMutation( req, diff --git a/server/src/routes/file-resources.ts b/server/src/routes/file-resources.ts index 0d4e8aacd4..6ef567930f 100644 --- a/server/src/routes/file-resources.ts +++ b/server/src/routes/file-resources.ts @@ -10,9 +10,9 @@ import { type WorkspaceFileContent, type WorkspaceFileListResponse, } from "@paperclipai/shared"; -import { HttpError, unprocessable } from "../errors.js"; +import { HttpError, notFound, unprocessable } from "../errors.js"; import { workspaceFileResourceService } from "../services/index.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, getActorInfo, hasCompanyAccess } from "./authz.js"; import { logActivity } from "../services/activity-log.js"; export type WorkspaceFileResourceService = { @@ -352,7 +352,10 @@ export function fileResourceRoutes(db: Db, opts: { const issue = await svc.getIssue(req.params.issueId); const actor = getActorInfo(req); try { - assertCompanyAccess(req, issue.companyId); + if (!hasCompanyAccess(req, issue.companyId)) { + // Same 404 as a missing issue so cross-tenant probes can't tell them apart. + throw notFound("Issue not found"); + } } catch (error) { await logListDeniedAttempt({ companyId: issue.companyId, @@ -458,7 +461,10 @@ export function fileResourceRoutes(db: Db, opts: { const issue = await svc.getIssue(req.params.issueId); const actor = getActorInfo(req); try { - assertCompanyAccess(req, issue.companyId); + if (!hasCompanyAccess(req, issue.companyId)) { + // Same 404 as a missing issue so cross-tenant probes can't tell them apart. + throw notFound("Issue not found"); + } } catch (error) { await logDeniedAttempt({ companyId: issue.companyId, @@ -566,7 +572,10 @@ export function fileResourceRoutes(db: Db, opts: { const issue = await svc.getIssue(req.params.issueId); const actor = getActorInfo(req); try { - assertCompanyAccess(req, issue.companyId); + if (!hasCompanyAccess(req, issue.companyId)) { + // Same 404 as a missing issue so cross-tenant probes can't tell them apart. + throw notFound("Issue not found"); + } } catch (error) { await logDeniedAttempt({ companyId: issue.companyId, diff --git a/server/src/routes/goals.ts b/server/src/routes/goals.ts index 2f090dad8e..9d6e53632e 100644 --- a/server/src/routes/goals.ts +++ b/server/src/routes/goals.ts @@ -4,7 +4,7 @@ import { createGoalSchema, updateGoalSchema } from "@paperclipai/shared"; import { trackGoalCreated } from "@paperclipai/shared/telemetry"; import { validate } from "../middleware/validate.js"; import { goalService, logActivity } from "../services/index.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; import { getTelemetryClient } from "../telemetry.js"; export function goalRoutes(db: Db) { @@ -20,12 +20,8 @@ export function goalRoutes(db: Db) { router.get("/goals/:id", async (req, res) => { const id = req.params.id as string; - const goal = await svc.getById(id); - if (!goal) { - res.status(404).json({ error: "Goal not found" }); - return; - } - assertCompanyAccess(req, goal.companyId); + const goal = await getAccessibleResource(req, res, svc.getById(id), "Goal not found"); + if (!goal) return; res.json(goal); }); @@ -53,12 +49,8 @@ export function goalRoutes(db: Db) { router.patch("/goals/:id", validate(updateGoalSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Goal not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Goal not found"); + if (!existing) return; const goal = await svc.update(id, req.body); if (!goal) { res.status(404).json({ error: "Goal not found" }); @@ -82,12 +74,8 @@ export function goalRoutes(db: Db) { router.delete("/goals/:id", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Goal not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Goal not found"); + if (!existing) return; const goal = await svc.remove(id); if (!goal) { res.status(404).json({ error: "Goal not found" }); diff --git a/server/src/routes/issue-tree-control.ts b/server/src/routes/issue-tree-control.ts index e12683212e..cb452c6390 100644 --- a/server/src/routes/issue-tree-control.ts +++ b/server/src/routes/issue-tree-control.ts @@ -9,7 +9,7 @@ import { } from "@paperclipai/shared"; import { validate } from "../middleware/validate.js"; import { heartbeatService, issueService, issueTreeControlService, logActivity } from "../services/index.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, getAccessibleResource, getActorInfo } from "./authz.js"; const TREE_RUN_CANCELLATION_RESPONSE_WAIT_MS = 1_000; @@ -45,12 +45,8 @@ export function issueTreeControlRoutes(db: Db) { router.post("/issues/:id/tree-control/preview", validate(previewIssueTreeControlSchema), async (req, res) => { assertBoard(req); - const root = await resolveRootIssue(req); - if (!root) { - res.status(404).json({ error: "Root issue not found" }); - return; - } - assertCompanyAccess(req, root.companyId); + const root = await getAccessibleResource(req, res, resolveRootIssue(req), "Root issue not found"); + if (!root) return; const preview = await treeControlSvc.preview(root.companyId, root.id, req.body); const actor = getActorInfo(req); @@ -75,12 +71,8 @@ export function issueTreeControlRoutes(db: Db) { router.post("/issues/:id/tree-holds", validate(createIssueTreeHoldSchema), async (req, res) => { assertBoard(req); - const root = await resolveRootIssue(req); - if (!root) { - res.status(404).json({ error: "Root issue not found" }); - return; - } - assertCompanyAccess(req, root.companyId); + const root = await getAccessibleResource(req, res, resolveRootIssue(req), "Root issue not found"); + if (!root) return; const actor = getActorInfo(req); const actorInput = { @@ -300,24 +292,16 @@ export function issueTreeControlRoutes(db: Db) { router.get("/issues/:id/tree-control/state", async (req, res) => { assertBoard(req); const issueId = req.params.id as string; - const issue = await issuesSvc.getById(issueId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, issuesSvc.getById(issueId), "Issue not found"); + if (!issue) return; const activePauseHold = await treeControlSvc.getActivePauseHoldGate(issue.companyId, issue.id); res.json({ activePauseHold }); }); router.get("/issues/:id/tree-holds", async (req, res) => { assertBoard(req); - const root = await resolveRootIssue(req); - if (!root) { - res.status(404).json({ error: "Root issue not found" }); - return; - } - assertCompanyAccess(req, root.companyId); + const root = await getAccessibleResource(req, res, resolveRootIssue(req), "Root issue not found"); + if (!root) return; const statusParam = typeof req.query.status === "string" ? req.query.status : null; const modeParam = typeof req.query.mode === "string" ? req.query.mode : null; const includeMembers = req.query.includeMembers === "true"; @@ -334,12 +318,8 @@ export function issueTreeControlRoutes(db: Db) { router.get("/issues/:id/tree-holds/:holdId", async (req, res) => { assertBoard(req); - const root = await resolveRootIssue(req); - if (!root) { - res.status(404).json({ error: "Root issue not found" }); - return; - } - assertCompanyAccess(req, root.companyId); + const root = await getAccessibleResource(req, res, resolveRootIssue(req), "Root issue not found"); + if (!root) return; const holdId = req.params.holdId as string; if (!isUuidLike(holdId)) { @@ -360,12 +340,8 @@ export function issueTreeControlRoutes(db: Db) { validate(releaseIssueTreeHoldSchema), async (req, res) => { assertBoard(req); - const root = await resolveRootIssue(req); - if (!root) { - res.status(404).json({ error: "Root issue not found" }); - return; - } - assertCompanyAccess(req, root.companyId); + const root = await getAccessibleResource(req, res, resolveRootIssue(req), "Root issue not found"); + if (!root) return; const holdId = req.params.holdId as string; if (!isUuidLike(holdId)) { diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 401d18482c..80a99b51c0 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -125,7 +125,7 @@ import { import type { TaskWatchdogServiceDeps, taskWatchdogService } from "../services/task-watchdogs.js"; import { logger } from "../middleware/logger.js"; import { conflict, forbidden, HttpError, notFound, unauthorized, unprocessable } from "../errors.js"; -import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; import { assertNoAgentHostWorkspaceCommandMutation, collectIssueWorkspaceCommandPaths, @@ -4976,12 +4976,8 @@ export function issueRoutes( router.delete("/labels/:labelId", async (req, res) => { const labelId = req.params.labelId as string; - const existing = await svc.getLabelById(labelId); - if (!existing) { - res.status(404).json({ error: "Label not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getLabelById(labelId), "Label not found"); + if (!existing) return; const removed = await svc.deleteLabel(labelId); if (!removed) { res.status(404).json({ error: "Label not found" }); @@ -5004,12 +5000,8 @@ export function issueRoutes( router.get("/issues/:id/heartbeat-context", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const wakeCommentId = @@ -5158,12 +5150,8 @@ export function issueRoutes( router.get("/issues/:id/diagnostics/blockers", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const diagnostic = await svc.getBlockerDiagnostics(issue.id); @@ -5193,12 +5181,8 @@ export function issueRoutes( router.get("/issues/:id/diagnostics/wakes", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const [wakeDiagnostic, blockerDiagnostic, includeInternalIds] = await Promise.all([ @@ -5242,12 +5226,8 @@ export function issueRoutes( router.get("/issues/:id/diagnostics/subtree", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const [diagnostic, includeInternalIds] = await Promise.all([ @@ -5296,12 +5276,8 @@ export function issueRoutes( router.get("/issues/:id", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const [ { project, goal }, @@ -5377,24 +5353,16 @@ export function issueRoutes( router.get("/issues/:id/watchdog", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; res.json(await taskWatchdogsSvc.getActiveForIssue(issue.companyId, issue.id)); }); router.put("/issues/:id/watchdog", validate(upsertIssueWatchdogSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (await rejectTaskWatchdogConfigMutation(req, res)) return; @@ -5433,12 +5401,8 @@ export function issueRoutes( router.delete("/issues/:id/watchdog", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (await rejectTaskWatchdogConfigMutation(req, res)) return; @@ -5473,12 +5437,8 @@ export function issueRoutes( router.get("/issues/:id/recovery-actions", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const active = await revalidateActiveSourceRecoveryForRead({ issue, @@ -5493,12 +5453,8 @@ export function issueRoutes( router.post("/issues/:id/recovery-actions/resolve", validate(resolveIssueRecoveryActionSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!existing) return; if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; const activeRecoveryAction = await recoveryActionsSvc.getActiveForIssue(existing.companyId, existing.id); if ( @@ -5662,12 +5618,8 @@ export function issueRoutes( router.get("/issues/:id/work-products", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const workProducts = await workProductsSvc.listForIssue(issue.id); res.json(workProducts); @@ -5675,12 +5627,8 @@ export function issueRoutes( router.get("/issues/:id/external-objects", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const objects = await externalObjectsSvc.listForIssue(issue.id); res.json(objects); @@ -5688,12 +5636,8 @@ export function issueRoutes( router.get("/issues/:id/external-object-summary", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const summary = await externalObjectsSvc.getIssueSummary(issue.id); res.json(summary); @@ -5724,12 +5668,8 @@ export function issueRoutes( router.post("/issues/:id/external-objects/refresh", validate(refreshExternalObjectsSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const actor = getActorInfo(req); const results = await externalObjectsSvc.refreshIssueObjects(issue.id, { @@ -5756,12 +5696,8 @@ export function issueRoutes( router.get("/issues/:id/documents", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const docs = await documentsSvc.listIssueDocuments(issue.id, { includeSystem: req.query.includeSystem === "true", @@ -5771,12 +5707,8 @@ export function issueRoutes( router.get("/issues/:id/documents/:key", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -5801,12 +5733,8 @@ export function issueRoutes( router.get("/issues/:id/documents/:key/annotations", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -5826,12 +5754,8 @@ export function issueRoutes( validate(createDocumentAnnotationThreadSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -5878,12 +5802,8 @@ export function issueRoutes( router.get("/issues/:id/documents/:key/annotations/:threadId", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -5907,12 +5827,8 @@ export function issueRoutes( validate(createDocumentAnnotationCommentSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -5965,12 +5881,8 @@ export function issueRoutes( validate(updateDocumentAnnotationThreadSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -6010,12 +5922,8 @@ export function issueRoutes( router.put("/issues/:id/documents/:key", validate(upsertIssueDocumentSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); @@ -6139,12 +6047,8 @@ export function issueRoutes( router.post("/issues/:id/documents/:key/lock", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6187,12 +6091,8 @@ export function issueRoutes( router.post("/issues/:id/documents/:key/unlock", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6229,12 +6129,8 @@ export function issueRoutes( router.get("/issues/:id/documents/:key/revisions", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); if (!keyParsed.success) { @@ -6251,12 +6147,8 @@ export function issueRoutes( async (req, res) => { const id = req.params.id as string; const revisionId = req.params.revisionId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; const keyParsed = issueDocumentKeySchema.safeParse(String(req.params.key ?? "").trim().toLowerCase()); @@ -6367,12 +6259,8 @@ export function issueRoutes( router.delete("/issues/:id/documents/:key", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6443,12 +6331,8 @@ export function issueRoutes( router.post("/issues/:id/work-products", validate(createIssueWorkProductSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; const actor = getActorInfo(req); @@ -6493,12 +6377,8 @@ export function issueRoutes( router.post("/issues/:id/low-trust/promotions", validate(promoteLowTrustOutputSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return; @@ -6638,12 +6518,8 @@ export function issueRoutes( router.patch("/work-products/:id", validate(updateIssueWorkProductSchema), async (req, res) => { const id = req.params.id as string; - const existing = await workProductsSvc.getById(id); - if (!existing) { - res.status(404).json({ error: "Work product not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, workProductsSvc.getById(id), "Work product not found"); + if (!existing) return; const issue = await svc.getById(existing.issueId); if (!issue) { res.status(404).json({ error: "Issue not found" }); @@ -6698,12 +6574,8 @@ export function issueRoutes( router.delete("/work-products/:id", async (req, res) => { const id = req.params.id as string; - const existing = await workProductsSvc.getById(id); - if (!existing) { - res.status(404).json({ error: "Work product not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, workProductsSvc.getById(id), "Work product not found"); + if (!existing) return; const issue = await svc.getById(existing.issueId); if (!issue) { res.status(404).json({ error: "Issue not found" }); @@ -6739,12 +6611,8 @@ export function issueRoutes( router.post("/issues/:id/read", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6771,12 +6639,8 @@ export function issueRoutes( router.delete("/issues/:id/read", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6803,12 +6667,8 @@ export function issueRoutes( router.post("/issues/:id/inbox-archive", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6835,12 +6695,8 @@ export function issueRoutes( router.delete("/issues/:id/inbox-archive", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Board authentication required" }); return; @@ -6867,12 +6723,8 @@ export function issueRoutes( router.get("/issues/:id/approvals", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (await assertLowTrustControlPlaneDenied(req, res, issue.companyId, issue)) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const approvals = await issueApprovalsSvc.listApprovalsForIssue(id); @@ -6881,12 +6733,8 @@ export function issueRoutes( router.post("/issues/:id/approvals", validate(linkIssueApprovalSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertApprovalMutationAllowedByRunContext(req, res, issue))) return; if (!(await assertCanManageIssueApprovalLinks(req, res, issue.companyId))) return; @@ -6916,12 +6764,8 @@ export function issueRoutes( router.delete("/issues/:id/approvals/:approvalId", async (req, res) => { const id = req.params.id as string; const approvalId = req.params.approvalId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (!(await assertApprovalMutationAllowedByRunContext(req, res, issue))) return; if (!(await assertCanManageIssueApprovalLinks(req, res, issue.companyId))) return; @@ -7182,12 +7026,8 @@ export function issueRoutes( router.post("/issues/:id/children", applyCreateIssueStatusDefault, validate(createChildIssueSchema), async (req, res) => { const parentId = req.params.id as string; - const parent = await svc.getById(parentId); - if (!parent) { - res.status(404).json({ error: "Parent issue not found" }); - return; - } - assertCompanyAccess(req, parent.companyId); + const parent = await getAccessibleResource(req, res, svc.getById(parentId), "Parent issue not found"); + if (!parent) return; if (!isTaskBridgeKeyActor(req) && !(await assertIssueReadAllowed(req, res, parent))) return; if (!(await assertTaskWatchdogCreateIssueAllowed(req, res, parent.companyId, parent))) return; if (await assertLowTrustControlPlaneDenied(req, res, parent.companyId, parent)) return; @@ -7351,24 +7191,16 @@ export function issueRoutes( router.get("/issues/:id/accepted-plan-decompositions", async (req, res) => { const sourceIssueId = req.params.id as string; - const sourceIssue = await svc.getById(sourceIssueId); - if (!sourceIssue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, sourceIssue.companyId); + const sourceIssue = await getAccessibleResource(req, res, svc.getById(sourceIssueId), "Issue not found"); + if (!sourceIssue) return; const decompositions = await svc.listAcceptedPlanDecompositions(sourceIssue.id); res.json(decompositions); }); router.post("/issues/:id/accepted-plan-decompositions", validate(createAcceptedPlanDecompositionSchema), async (req, res) => { const sourceIssueId = req.params.id as string; - const sourceIssue = await svc.getById(sourceIssueId); - if (!sourceIssue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, sourceIssue.companyId); + const sourceIssue = await getAccessibleResource(req, res, svc.getById(sourceIssueId), "Issue not found"); + if (!sourceIssue) return; if (!(await assertAgentIssueMutationAllowed(req, res, sourceIssue))) return; const requestedChildren = []; @@ -7564,12 +7396,8 @@ export function issueRoutes( router.post("/issues/:id/monitor/check-now", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; await assertCanManageIssueMonitor(access, req, issue.companyId, issue.assigneeAgentId, true); const actor = getActorInfo(req); @@ -7586,12 +7414,8 @@ export function issueRoutes( router.post("/issues/:id/scheduled-retry/retry-now", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; const actor = getActorInfo(req); const result = await heartbeat.retryScheduledRetryNow({ @@ -7623,12 +7447,8 @@ export function issueRoutes( router.patch("/issues/:id", validate(updateIssueRouteSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!existing) return; assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body)); if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; if (!(await assertCheapRecoveryIssueAssigneeProfileAllowed(req, res, existing, req.body))) return; @@ -8760,12 +8580,8 @@ export function issueRoutes( router.delete("/issues/:id", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!existing) return; if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; const attachments = await svc.listAttachments(id); @@ -8801,12 +8617,8 @@ export function issueRoutes( router.post("/issues/:id/checkout", validate(checkoutIssueSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (issue.projectId) { const project = await projectsSvc.getById(issue.projectId); @@ -8888,12 +8700,8 @@ export function issueRoutes( router.post("/issues/:id/release", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!existing) return; if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; const actorRunId = requireAgentRunId(req, res); if (req.actor.type === "agent" && !actorRunId) return; @@ -8933,12 +8741,8 @@ export function issueRoutes( } const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!existing) return; const clearAssignee = req.query.clearAssignee === "true"; const result = await svc.adminForceRelease(id, { clearAssignee }); @@ -8971,12 +8775,8 @@ export function issueRoutes( router.get("/issues/:id/comments", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const afterCommentId = typeof req.query.after === "string" && req.query.after.trim().length > 0 @@ -9006,12 +8806,8 @@ export function issueRoutes( router.get("/issues/:id/interactions", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const actor = getActorInfo(req); const interactionSvc = issueThreadInteractionService(db); @@ -9029,12 +8825,8 @@ export function issueRoutes( router.post("/issues/:id/interactions", validate(createIssueThreadInteractionSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type === "agent") { if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; if (await assertLowTrustControlPlaneDenied(req, res, issue.companyId, issue)) return; @@ -9083,12 +8875,8 @@ export function issueRoutes( async (req, res) => { const id = req.params.id as string; const interactionId = req.params.interactionId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (await rejectAgentIssueThreadInteractionResolution(req, res, issue)) return; assertBoard(req); @@ -9230,12 +9018,8 @@ export function issueRoutes( async (req, res) => { const id = req.params.id as string; const interactionId = req.params.interactionId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (await rejectAgentIssueThreadInteractionResolution(req, res, issue)) return; assertBoard(req); @@ -9287,12 +9071,8 @@ export function issueRoutes( async (req, res) => { const id = req.params.id as string; const interactionId = req.params.interactionId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (await rejectAgentIssueThreadInteractionResolution(req, res, issue)) return; assertBoard(req); @@ -9340,12 +9120,8 @@ export function issueRoutes( async (req, res) => { const id = req.params.id as string; const interactionId = req.params.interactionId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (await rejectAgentIssueThreadInteractionResolution(req, res, issue)) return; assertBoard(req); @@ -9410,12 +9186,8 @@ export function issueRoutes( async (req, res) => { const id = req.params.id as string; const interactionId = req.params.interactionId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (await rejectAgentIssueThreadInteractionResolution(req, res, issue)) return; assertBoard(req); @@ -9460,12 +9232,8 @@ export function issueRoutes( router.get("/issues/:id/comments/:commentId", async (req, res) => { const id = req.params.id as string; const commentId = req.params.commentId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const comment = await svc.getComment(commentId); if (!comment || comment.issueId !== id) { @@ -9478,12 +9246,8 @@ export function issueRoutes( router.delete("/issues/:id/comments/:commentId", async (req, res) => { const id = req.params.id as string; const commentId = req.params.commentId as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return; const comment = await svc.getComment(commentId); @@ -9621,12 +9385,8 @@ export function issueRoutes( router.get("/issues/:id/feedback-votes", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Only board users can view feedback votes" }); return; @@ -9638,12 +9398,8 @@ export function issueRoutes( router.get("/issues/:id/feedback-traces", async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Only board users can view feedback traces" }); return; @@ -9701,12 +9457,8 @@ export function issueRoutes( router.post("/issues/:id/comments", validate(addIssueCommentSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; const commentAccessDecision = await assertAgentIssueCommentAllowed(req, res, issue); if (!commentAccessDecision) return; if (!assertStructuredCommentFieldsAllowed(req, res, { @@ -10329,12 +10081,8 @@ export function issueRoutes( router.post("/issues/:id/feedback-votes", validate(upsertIssueFeedbackVoteSchema), async (req, res) => { const id = req.params.id as string; - const issue = await svc.getById(id); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); + if (!issue) return; if (req.actor.type !== "board") { res.status(403).json({ error: "Only board users can vote on AI feedback" }); return; @@ -10428,12 +10176,8 @@ export function issueRoutes( router.get("/issues/:id/attachments", async (req, res) => { const issueId = req.params.id as string; - const issue = await svc.getById(issueId); - if (!issue) { - res.status(404).json({ error: "Issue not found" }); - return; - } - assertCompanyAccess(req, issue.companyId); + const issue = await getAccessibleResource(req, res, svc.getById(issueId), "Issue not found"); + if (!issue) return; if (!(await assertIssueReadAllowed(req, res, issue))) return; const attachments = await svc.listAttachments(issueId); res.json(attachments.map(withContentPath)); @@ -10533,12 +10277,8 @@ export function issueRoutes( router.get("/attachments/:attachmentId/content", async (req, res, next) => { const attachmentId = req.params.attachmentId as string; - const attachment = await svc.getAttachmentById(attachmentId); - if (!attachment) { - res.status(404).json({ error: "Attachment not found" }); - return; - } - assertCompanyAccess(req, attachment.companyId); + const attachment = await getAccessibleResource(req, res, svc.getAttachmentById(attachmentId), "Attachment not found"); + if (!attachment) return; const issue = await svc.getById(attachment.issueId); if (!issue) { res.status(404).json({ error: "Issue not found" }); @@ -10598,12 +10338,8 @@ export function issueRoutes( router.delete("/attachments/:attachmentId", async (req, res) => { const attachmentId = req.params.attachmentId as string; - const attachment = await svc.getAttachmentById(attachmentId); - if (!attachment) { - res.status(404).json({ error: "Attachment not found" }); - return; - } - assertCompanyAccess(req, attachment.companyId); + const attachment = await getAccessibleResource(req, res, svc.getAttachmentById(attachmentId), "Attachment not found"); + if (!attachment) return; const issue = await svc.getById(attachment.issueId); if (!issue) { res.status(404).json({ error: "Issue not found" }); diff --git a/server/src/routes/projects.ts b/server/src/routes/projects.ts index c7e123fc0d..7d472c36d8 100644 --- a/server/src/routes/projects.ts +++ b/server/src/routes/projects.ts @@ -17,7 +17,7 @@ import { accessService, projectService, logActivity, workspaceOperationService } import { conflict, forbidden } from "../errors.js"; import { externalObjectService } from "../services/external-objects.js"; import { instanceSettingsService } from "../services/instance-settings.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; import { buildWorkspaceRuntimeDesiredStatePatch, listConfiguredRuntimeServiceEntries, @@ -135,24 +135,16 @@ export function projectRoutes(db: Db) { router.get("/projects/:id", async (req, res) => { const id = req.params.id as string; - const project = await svc.getById(id); - if (!project) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, project.companyId); + const project = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!project) return; if (!(await assertProjectReadAllowed(req, res, project))) return; res.json(project); }); router.get("/projects/:id/external-object-summary", async (req, res) => { const id = req.params.id as string; - const project = await svc.getById(id); - if (!project) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, project.companyId); + const project = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!project) return; const summary = await externalObjectsSvc.getProjectSummary(project.id); res.json(summary); }); @@ -227,12 +219,8 @@ export function projectRoutes(db: Db) { router.patch("/projects/:id", validate(updateProjectSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!existing) return; const body = { ...req.body }; assertNoAgentHostWorkspaceCommandMutation( req, @@ -287,24 +275,16 @@ export function projectRoutes(db: Db) { router.get("/projects/:id/workspaces", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!existing) return; const workspaces = await svc.listWorkspaces(id); res.json(workspaces); }); router.post("/projects/:id/workspaces", validate(createProjectWorkspaceSchema), async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!existing) return; assertNoAgentHostWorkspaceCommandMutation( req, collectProjectWorkspaceCommandPaths(req.body), @@ -341,12 +321,8 @@ export function projectRoutes(db: Db) { async (req, res) => { const id = req.params.id as string; const workspaceId = req.params.workspaceId as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!existing) return; assertNoAgentHostWorkspaceCommandMutation( req, collectProjectWorkspaceCommandPaths(req.body), @@ -390,12 +366,8 @@ export function projectRoutes(db: Db) { return; } - const project = await svc.getById(id); - if (!project) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, project.companyId); + const project = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!project) return; const workspace = project.workspaces.find((entry) => entry.id === workspaceId) ?? null; if (!workspace) { @@ -662,12 +634,8 @@ export function projectRoutes(db: Db) { router.delete("/projects/:id/workspaces/:workspaceId", async (req, res) => { const id = req.params.id as string; const workspaceId = req.params.workspaceId as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!existing) return; const workspace = await svc.removeWorkspace(id, workspaceId); if (!workspace) { res.status(404).json({ error: "Project workspace not found" }); @@ -694,12 +662,8 @@ export function projectRoutes(db: Db) { router.delete("/projects/:id", async (req, res) => { const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Project not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getById(id), "Project not found"); + if (!existing) return; const project = await svc.remove(id); if (!project) { res.status(404).json({ error: "Project not found" }); diff --git a/server/src/routes/routines.ts b/server/src/routes/routines.ts index 0a16df2465..8a3082dead 100644 --- a/server/src/routes/routines.ts +++ b/server/src/routes/routines.ts @@ -14,7 +14,7 @@ import { import { trackRoutineCreated } from "@paperclipai/shared/telemetry"; import { validate } from "../middleware/validate.js"; import { accessService, documentAnnotationService, logActivity, routineService } from "../services/index.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertCompanyAccess, getAccessibleResource, getActorInfo, hasCompanyAccess } from "./authz.js"; import { forbidden, unauthorized } from "../errors.js"; import { getTelemetryClient } from "../telemetry.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; @@ -106,7 +106,7 @@ export function routineRoutes( async function assertCanManageExistingRoutine(req: Request, routineId: string) { const routine = await svc.get(routineId); - if (!routine) return null; + if (!routine || !hasCompanyAccess(req, routine.companyId)) return null; assertCompanyAccess(req, routine.companyId); if (req.actor.type === "board") return routine; if (req.actor.type !== "agent" || !req.actor.agentId) throw unauthorized(); @@ -189,12 +189,8 @@ export function routineRoutes( }); router.get("/routines/:id", async (req, res) => { - const detail = await svc.getDetail(req.params.id as string); - if (!detail) { - res.status(404).json({ error: "Routine not found" }); - return; - } - assertCompanyAccess(req, detail.companyId); + const detail = await getAccessibleResource(req, res, svc.getDetail(req.params.id as string), "Routine not found"); + if (!detail) return; res.json(detail); }); @@ -450,12 +446,8 @@ export function routineRoutes( }); router.get("/routines/:id/runs", async (req, res) => { - const routine = await svc.get(req.params.id as string); - if (!routine) { - res.status(404).json({ error: "Routine not found" }); - return; - } - assertCompanyAccess(req, routine.companyId); + const routine = await getAccessibleResource(req, res, svc.get(req.params.id as string), "Routine not found"); + if (!routine) return; const limit = Number(req.query.limit ?? 50); const result = await svc.listRuns(routine.id, Number.isFinite(limit) ? limit : 50); res.json(result); @@ -504,7 +496,7 @@ export function routineRoutes( } const routine = await assertCanManageExistingRoutine(req, trigger.routineId); if (!routine) { - res.status(404).json({ error: "Routine not found" }); + res.status(404).json({ error: "Routine trigger not found" }); return; } await assertBoardCanAssignTasks(req, routine.companyId); @@ -546,7 +538,7 @@ export function routineRoutes( } const routine = await assertCanManageExistingRoutine(req, trigger.routineId); if (!routine) { - res.status(404).json({ error: "Routine not found" }); + res.status(404).json({ error: "Routine trigger not found" }); return; } const deleted = await svc.deleteTrigger(trigger.id, { @@ -590,7 +582,7 @@ export function routineRoutes( } const routine = await assertCanManageExistingRoutine(req, trigger.routineId); if (!routine) { - res.status(404).json({ error: "Routine not found" }); + res.status(404).json({ error: "Routine trigger not found" }); return; } const rotated = await svc.rotateTriggerSecret(trigger.id, { diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index 56e6404b2a..884f626c83 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -16,7 +16,7 @@ import { updateUserSecretValueSchema, } from "@paperclipai/shared"; import { validate } from "../middleware/validate.js"; -import { assertBoard, assertCompanyAccess } from "./authz.js"; +import { assertBoard, assertCompanyAccess, getAccessibleResource } from "./authz.js"; import { logActivity, secretService } from "../services/index.js"; import { getConfiguredSecretProvider } from "../secrets/configured-provider.js"; import { forbidden, unauthorized } from "../errors.js"; @@ -154,24 +154,16 @@ export function secretRoutes(db: Db) { router.get("/secret-provider-configs/:id", async (req, res) => { assertBoard(req); - const existing = await svc.getProviderConfigById(req.params.id as string); - if (!existing) { - res.status(404).json({ error: "Provider vault not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getProviderConfigById(req.params.id as string), "Provider vault not found"); + if (!existing) return; res.json(existing); }); router.patch("/secret-provider-configs/:id", validate(updateSecretProviderConfigSchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getProviderConfigById(id); - if (!existing) { - res.status(404).json({ error: "Provider vault not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getProviderConfigById(id), "Provider vault not found"); + if (!existing) return; const updated = await svc.updateProviderConfig(id, { displayName: req.body.displayName, @@ -205,12 +197,8 @@ export function secretRoutes(db: Db) { router.delete("/secret-provider-configs/:id", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getProviderConfigById(id); - if (!existing) { - res.status(404).json({ error: "Provider vault not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getProviderConfigById(id), "Provider vault not found"); + if (!existing) return; const removed = await svc.removeProviderConfig(id); if (!removed) { @@ -238,12 +226,8 @@ export function secretRoutes(db: Db) { router.post("/secret-provider-configs/:id/default", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getProviderConfigById(id); - if (!existing) { - res.status(404).json({ error: "Provider vault not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getProviderConfigById(id), "Provider vault not found"); + if (!existing) return; const updated = await svc.setDefaultProviderConfig(id); if (!updated) { @@ -271,12 +255,8 @@ export function secretRoutes(db: Db) { router.post("/secret-provider-configs/:id/health", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getProviderConfigById(id); - if (!existing) { - res.status(404).json({ error: "Provider vault not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const existing = await getAccessibleResource(req, res, svc.getProviderConfigById(id), "Provider vault not found"); + if (!existing) return; const health = await svc.checkProviderConfigHealth(id); if (!health) { @@ -703,16 +683,14 @@ export function secretRoutes(db: Db) { router.post("/secrets/:id/rotate", validate(rotateSecretSchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Secret not found" }); - return; - } - if (!isCompanyScopedSecret(existing)) { - res.status(404).json({ error: "Secret not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const fetched = await svc.getById(id); + const existing = await getAccessibleResource( + req, + res, + fetched && isCompanyScopedSecret(fetched) ? fetched : null, + "Secret not found", + ); + if (!existing) return; if (existing.status === "deleted") { res.status(404).json({ error: "Secret not found" }); return; @@ -745,16 +723,14 @@ export function secretRoutes(db: Db) { router.patch("/secrets/:id", validate(updateSecretSchema), async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Secret not found" }); - return; - } - if (!isCompanyScopedSecret(existing)) { - res.status(404).json({ error: "Secret not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const fetched = await svc.getById(id); + const existing = await getAccessibleResource( + req, + res, + fetched && isCompanyScopedSecret(fetched) ? fetched : null, + "Secret not found", + ); + if (!existing) return; if (existing.status === "deleted") { res.status(404).json({ error: "Secret not found" }); return; @@ -791,16 +767,14 @@ export function secretRoutes(db: Db) { router.get("/secrets/:id/usage", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Secret not found" }); - return; - } - if (!isCompanyScopedSecret(existing)) { - res.status(404).json({ error: "Secret not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const fetched = await svc.getById(id); + const existing = await getAccessibleResource( + req, + res, + fetched && isCompanyScopedSecret(fetched) ? fetched : null, + "Secret not found", + ); + if (!existing) return; const bindings = await svc.listBindingReferences(existing.companyId, existing.id); res.json({ secretId: existing.id, bindings }); }); @@ -808,16 +782,14 @@ export function secretRoutes(db: Db) { router.get("/secrets/:id/access-events", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Secret not found" }); - return; - } - if (!isCompanyScopedSecret(existing)) { - res.status(404).json({ error: "Secret not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const fetched = await svc.getById(id); + const existing = await getAccessibleResource( + req, + res, + fetched && isCompanyScopedSecret(fetched) ? fetched : null, + "Secret not found", + ); + if (!existing) return; const events = await svc.listAccessEvents(existing.companyId, existing.id); res.json(events); }); @@ -825,16 +797,14 @@ export function secretRoutes(db: Db) { router.delete("/secrets/:id", async (req, res) => { assertBoard(req); const id = req.params.id as string; - const existing = await svc.getById(id); - if (!existing) { - res.status(404).json({ error: "Secret not found" }); - return; - } - if (!isCompanyScopedSecret(existing)) { - res.status(404).json({ error: "Secret not found" }); - return; - } - assertCompanyAccess(req, existing.companyId); + const fetched = await svc.getById(id); + const existing = await getAccessibleResource( + req, + res, + fetched && isCompanyScopedSecret(fetched) ? fetched : null, + "Secret not found", + ); + if (!existing) return; const removed = await svc.remove(id); if (!removed) { diff --git a/server/src/routes/workspace-runtime-service-authz.ts b/server/src/routes/workspace-runtime-service-authz.ts index c638ad4916..d6f3939eb5 100644 --- a/server/src/routes/workspace-runtime-service-authz.ts +++ b/server/src/routes/workspace-runtime-service-authz.ts @@ -3,8 +3,8 @@ import type { Db } from "@paperclipai/db"; import { agents, heartbeatRuns, issues, projects } from "@paperclipai/db"; import { isUuidLike } from "@paperclipai/shared"; import type { Request } from "express"; -import { forbidden } from "../errors.js"; -import { assertCompanyAccess } from "./authz.js"; +import { forbidden, notFound } from "../errors.js"; +import { assertCompanyAccess, hasCompanyAccess } from "./authz.js"; import { parseProjectExecutionWorkspacePolicy } from "../services/execution-workspace-policy.js"; import { isLowTrustRuntimeManagementAllowed } from "../services/low-trust-runtime-containment.js"; import { resolveCoreTrustPreset, type TrustPresetResolution } from "../services/trust-preset-resolver.js"; @@ -305,6 +305,9 @@ export async function assertCanManageProjectWorkspaceRuntimeServices( projectWorkspaceId: string; }, ) { + if (!hasCompanyAccess(req, input.companyId)) { + throw notFound("Project workspace not found"); + } assertCompanyAccess(req, input.companyId); if (req.actor.type === "board") return; await assertAgentCanManageRuntimeServicesForWorkspace(db, req, input); @@ -319,6 +322,9 @@ export async function assertCanManageExecutionWorkspaceRuntimeServices( sourceIssueId?: string | null; }, ) { + if (!hasCompanyAccess(req, input.companyId)) { + throw notFound("Execution workspace not found"); + } assertCompanyAccess(req, input.companyId); if (req.actor.type === "board") return; await assertAgentCanManageRuntimeServicesForWorkspace(db, req, input);