From e2a465747163d8715f61db52424edd597795f424 Mon Sep 17 00:00:00 2001 From: Danijel Martinek Date: Fri, 10 Jul 2026 17:44:51 +0200 Subject: [PATCH] fix(core-dsr): scope DSR operations to the caller's own subject Non-admin callers may export/rectify/restrict/delete ONLY their own subjectId; a mismatch is rejected with FORBIDDEN instead of being honored verbatim (IDOR, audit finding A1). Cross-subject operations require the admin role; cascade-hard stays admin-only on top. Co-Authored-By: Claude Fable 5 --- .../core-dsr/src/__tests__/dsr.router.test.ts | 87 +++++++++++++++++++ packages/core-dsr/src/dsr.router.ts | 46 +++++++--- 2 files changed, 122 insertions(+), 11 deletions(-) diff --git a/packages/core-dsr/src/__tests__/dsr.router.test.ts b/packages/core-dsr/src/__tests__/dsr.router.test.ts index 063642b..e1825b1 100644 --- a/packages/core-dsr/src/__tests__/dsr.router.test.ts +++ b/packages/core-dsr/src/__tests__/dsr.router.test.ts @@ -208,6 +208,93 @@ describe("dsrRouter.restrict", () => { }); }); +describe("dsrRouter subject scoping (A1 — IDOR)", () => { + it("rejects a non-admin export for another subject with FORBIDDEN", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, authenticatedUser); + await expect( + caller.export({ subjectId: "bob", format: "json" }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + expect(binding.dataExport.calls).toHaveLength(0); + }); + + it("rejects a non-admin delete for another subject with FORBIDDEN", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, authenticatedUser); + await expect( + caller.delete({ subjectId: "bob", mode: "soft" }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + expect(binding.dataDelete.calls).toHaveLength(0); + }); + + it("rejects a non-admin rectify for another subject with FORBIDDEN", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, authenticatedUser); + await expect( + caller.rectify({ + subjectId: "bob", + collection: "users", + field: "name", + value: "x", + }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + expect(binding.dataRectify.calls).toHaveLength(0); + }); + + it("rejects a non-admin restrict for another subject with FORBIDDEN", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, authenticatedUser); + await expect( + caller.restrict({ subjectId: "bob", granted: true }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + expect(binding.processingRestriction.sets).toHaveLength(0); + }); + + it("rejects a non-admin user without an id acting on any subject", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, { roles: ["user"] }); + await expect( + caller.export({ subjectId: "alice", format: "json" }), + ).rejects.toMatchObject({ code: "FORBIDDEN" }); + }); + + it("allows a non-admin to act on themselves for every operation", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, authenticatedUser); + await caller.export({ subjectId: "alice", format: "json" }); + await caller.delete({ subjectId: "alice", mode: "soft" }); + await caller.rectify({ + subjectId: "alice", + collection: "users", + field: "name", + value: "Alice", + }); + await caller.restrict({ subjectId: "alice", granted: true }); + expect(binding.dataExport.calls).toHaveLength(1); + expect(binding.dataDelete.calls).toHaveLength(1); + expect(binding.dataRectify.calls).toHaveLength(1); + expect(binding.processingRestriction.sets).toHaveLength(1); + }); + + it("allows an admin to act cross-subject on every operation", async () => { + const binding = makeBinding(); + const caller = makeCaller(binding, adminUser); + await caller.export({ subjectId: "alice", format: "json" }); + await caller.delete({ subjectId: "alice", mode: "soft" }); + await caller.rectify({ + subjectId: "alice", + collection: "users", + field: "name", + value: "Alice", + }); + await caller.restrict({ subjectId: "alice", granted: true }); + expect(binding.dataExport.calls).toHaveLength(1); + expect(binding.dataDelete.calls).toHaveLength(1); + expect(binding.dataRectify.calls).toHaveLength(1); + expect(binding.processingRestriction.sets).toHaveLength(1); + }); +}); + describe("dsrRouter singleton guard", () => { it("throws when procedures are called without a real DsrBinding", async () => { // The singleton uses a Proxy that throws on any binding property access. diff --git a/packages/core-dsr/src/dsr.router.ts b/packages/core-dsr/src/dsr.router.ts index 2600baa..635cc2c 100644 --- a/packages/core-dsr/src/dsr.router.ts +++ b/packages/core-dsr/src/dsr.router.ts @@ -29,6 +29,28 @@ const dsrProcedure = t.procedure .use(requireAuthenticated) .use(defineErrorMiddleware([])); +/** + * Subject-scope guard (audit finding A1 — DSR IDOR). + * + * Non-admin callers may act ONLY on themselves: a request whose + * `input.subjectId` differs from the authenticated user's id is rejected + * with FORBIDDEN (rejected, not silently rewritten, so the mismatch is + * visible to the caller). Any cross-subject operation requires the + * "admin" role. + */ +function assertSubjectScope(user: DsrTrpcUser, subjectId: string): void { + if (user.roles?.includes("admin")) return; + if (user.id !== undefined && user.id === subjectId) return; + throw new TRPCError({ + code: "FORBIDDEN", + message: "DSR operations on another subject require the admin role", + }); +} + +function userFromCtx(ctx: object): DsrTrpcUser { + return (ctx as { user: DsrTrpcUser }).user; +} + /** * Creates the DSR tRPC router. * @@ -54,7 +76,8 @@ export function createDsrRouter(binding: DsrBinding) { }) .strict(), ) - .query(async ({ input }) => { + .query(async ({ ctx, input }) => { + assertSubjectScope(userFromCtx(ctx), input.subjectId); const res = await createExportHandler(binding.dataExport)(input); return res.body; }), @@ -69,14 +92,13 @@ export function createDsrRouter(binding: DsrBinding) { .strict(), ) .mutation(async ({ ctx, input }) => { - if (input.mode === "cascade-hard") { - const user = (ctx as { user: DsrTrpcUser }).user; - if (!user.roles?.includes("admin")) { - throw new TRPCError({ - code: "FORBIDDEN", - message: "Admin role required for cascade-hard deletion", - }); - } + const user = userFromCtx(ctx); + assertSubjectScope(user, input.subjectId); + if (input.mode === "cascade-hard" && !user.roles?.includes("admin")) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "Admin role required for cascade-hard deletion", + }); } const res = await createDeleteHandler(binding.dataDelete)(input); return res.body; @@ -93,7 +115,8 @@ export function createDsrRouter(binding: DsrBinding) { }) .strict(), ) - .mutation(async ({ input }) => { + .mutation(async ({ ctx, input }) => { + assertSubjectScope(userFromCtx(ctx), input.subjectId); // tRPC infers z.unknown() as value?: unknown; cast to assert presence const res = await createRectifyHandler(binding.dataRectify)( input as RectifyHandlerInput, @@ -110,7 +133,8 @@ export function createDsrRouter(binding: DsrBinding) { }) .strict(), ) - .mutation(async ({ input }) => { + .mutation(async ({ ctx, input }) => { + assertSubjectScope(userFromCtx(ctx), input.subjectId); const res = await createRestrictHandler(binding.processingRestriction)( input, );