chore/port-solidtime-audit-fixes #1

Merged
danijel merged 54 commits from chore/port-solidtime-audit-fixes into main 2026-07-12 21:13:09 +00:00
151 changed files with 3533 additions and 1048 deletions
Showing only changes of commit e2a4657471 - Show all commits

View File

@@ -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.

View File

@@ -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,
);