N1: scopeKey guarded the empty-string VALUE but not the TYPE. A
non-string tenantId (null, 0, false, an object) flowed through
un-normalised, and the adapters disagreed about the result - db
rejects null on NOT NULL, memory accepts it as an unreachable row; 0
and false stringify differently and could collide. Now
`typeof tenantId !== "string" || tenantId === ""` is refused with the
same WRN-AUTHZ-SCOPE error. Added a conformance case covering
null/0/false/{}.
N2: nothing failed if grant() were re-wrapped in db.tx, reintroducing
the shared-connection rollback from C1/C2 - timing-based tests can't
reliably prove a transaction is never opened. Added
db-no-transaction.test.ts: a fake Db with a spied driver.transaction
and statement-recording all/exec, driving every PermissionStore method
and asserting zero transaction calls and no "BEGIN" in any recorded
statement. Verified it fails when grant() is temporarily re-wrapped in
db.tx, then restored.
Also documents two things in db.ts as comments only: the UNIQUE
constraints are now load-bearing for ON CONFLICT/ON DUPLICATE KEY
target inference, and MySQL's VALUES(effect) upsert syntax is
deprecated since 8.0.20 (no MySQL server in CI to catch its removal).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fix-round-1 test "a concurrent write is not lost to another
method's failure" was vacuous: a single-process Promise.all cannot
reliably land a bare write inside another method's open transaction,
so it passed against both the fixed and the (previously) defective
grant() implementation. The shared-connection rollback hazard it was
meant to catch is real (confirmed separately by forcing the
transaction open before the write), but this specific test could
never reach that state and gave false assurance either way.
Replaced it with "a rejected write leaves unrelated state intact",
which asserts a grant() call with an invalid effect is refused without
disturbing the subject's existing roles/grants, plus a NOTE
documenting that the rollback hazard is now prevented structurally (no
transactions) rather than by a dedicated concurrency test.
memoryPermissionStore.grant() had no effect validation, so it failed
the new test; added a guard mirroring the db adapter's CHECK
constraint so both adapters agree on rejecting anything other than
"allow"/"deny".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
C1/C2: grant() wrapped its delete+insert in db.tx on a shared,
unserialized sqlite connection, so a concurrent bare write from another
method (e.g. revokeRole) got swept into the open transaction and
discarded on rollback - a revoke could report success while the
privilege survived. Also broke concurrent grants on distinct keys
("cannot start a transaction within a transaction"). Replaced with
single-statement upserts (ON CONFLICT / ON DUPLICATE KEY UPDATE),
atomic without a transaction.
I1: assignRole's check-then-act SELECT lost 19/20 concurrent identical
calls to a UNIQUE violation; switched to ON CONFLICT DO NOTHING.
I2: an unrecognised `effect` value was dropped from both the grant and
deny buckets on read. Added a CHECK constraint and made anything not
literally "allow" count as a deny (fail closed).
I3: ensureAuthzTables defaulted to sqlite instead of the Db's own
dialect. I4: scopeKey now refuses an explicitly empty tenantId rather
than treating it as global (shared with the memory adapter). I5: added
migrations.test.ts asserting the generated DDL per dialect, including
MySQL's binary collation on identity columns. M1: DDL is now a
statement list instead of a blob split on a formatting-dependent
separator. M3: declared @wrnexus/db as a workspace dependency.
Extends the conformance suite with four concurrency/empty-scope tests
(23 total, up from 19) that all three adapters now pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ClintchizandClaude Opus 5