Problem
ResolveApproval() can resolve the same approval more than once under concurrency.
The engine reads a pending approval under lock, mutates a local copy, unlocks, writes to SQLite, and only then writes the resolved state back into memory. During that window, a second resolver can still read the same approval as pending and also succeed.
The store layer makes this worse by using unconditional UPSERT semantics for both approval_states and attempt_grants, so the last writer wins instead of the second request failing closed.
Impact
- Two operators can both receive a successful response while submitting conflicting decisions for the same approval
- Final approval state depends on write ordering rather than first successful resolution
- Attempt grants can be overwritten after an approval was already denied or approved differently
- This breaks approval integrity in the most security-sensitive state transition in the system
Expected
Resolving an approval should be atomic and one-way:
- Exactly one resolver should be able to transition
pending -> approved|denied|expired
- Concurrent resolvers should get a conflict once the first resolution commits
- Approval state and attempt grant persistence should be updated transactionally
- Tests should cover concurrent approve/deny races against the same approval id
Key code
internal/core/engine.go — ResolveApproval() unlocks before persistence and final in-memory updateinternal/store/sqlite.go — SaveApproval() uses unconditional UPSERTinternal/store/sqlite.go — SaveAttemptGrant() also overwrites existing rows on conflict
Problem
ResolveApproval()can resolve the same approval more than once under concurrency.The engine reads a pending approval under lock, mutates a local copy, unlocks, writes to SQLite, and only then writes the resolved state back into memory. During that window, a second resolver can still read the same approval as
pendingand also succeed.The store layer makes this worse by using unconditional UPSERT semantics for both
approval_statesandattempt_grants, so the last writer wins instead of the second request failing closed.Impact
Expected
Resolving an approval should be atomic and one-way:
pending -> approved|denied|expiredKey code
internal/core/engine.go—ResolveApproval()unlocks before persistence and final in-memory updateinternal/store/sqlite.go—SaveApproval()uses unconditional UPSERTinternal/store/sqlite.go—SaveAttemptGrant()also overwrites existing rows on conflict