diff --git a/packages/authz/src/store.ts b/packages/authz/src/store.ts index 89c1fe7e..11480bab 100644 --- a/packages/authz/src/store.ts +++ b/packages/authz/src/store.ts @@ -77,6 +77,14 @@ export function memoryPermissionStore(): PermissionStore { if (at !== -1) roles.splice(at, 1); }, async grant(subjectId, permission, effect, scope) { + // The db adapter enforces this via a CHECK constraint; the memory + // adapter must agree, or a bad effect would silently vanish from both + // the grant and deny buckets on read instead of being refused up front. + if (effect !== "allow" && effect !== "deny") { + throw new TypeError( + `WRN-AUTHZ-EFFECT: effect must be "allow" or "deny", received ${JSON.stringify(effect)}`, + ); + } const key = scopeKey(scope); const at = grants.findIndex( (g) => g.subjectId === subjectId && g.scope === key && g.permission === permission, diff --git a/packages/authz/test/store-conformance.ts b/packages/authz/test/store-conformance.ts index a9154d8e..cb82f99f 100644 --- a/packages/authz/test/store-conformance.ts +++ b/packages/authz/test/store-conformance.ts @@ -154,17 +154,25 @@ export function runStoreConformance(name: string, makeStore: () => Promise { - // A store that wraps one method in a transaction on a shared connection - // will roll back this unrelated write and still resolve successfully. + test("a rejected write leaves unrelated state intact", async () => { await store.assignRole("victim", "admin"); - await Promise.all([ - store.revokeRole("victim", "admin"), - store.grant("other", "post:read", "allow").catch(() => undefined), - ]); - expect((await store.assignmentsFor("victim")).roles).toEqual([]); + await store.grant("victim", "post:read", "allow"); + // An invalid effect must be refused without disturbing anything else. + await expect(store.grant("victim", "post:write", "bogus" as never)).rejects.toThrow(); + const assignments = await store.assignmentsFor("victim"); + expect(assignments.roles).toEqual(["admin"]); + expect(assignments.grants).toEqual(["post:read"]); }); + // NOTE: the shared-connection rollback hazard - where one method's open + // transaction sweeps in a concurrent bare write from another method and + // discards it, so a revoke resolves successfully while the role survives - + // is prevented STRUCTURALLY, by the store using no transactions at all. + // It is deliberately not covered here: reproducing it needs the bare write + // to land inside the open transaction, which a single-process Promise.all + // does not reliably arrange, so any such test would pass against the + // defective implementation and give false assurance. + test("listSubjects with no scope returns global assignees only", async () => { await store.assignRole("g1", "viewer"); await store.assignRole("s1", "editor", { tenantId: "t1" });