fix!: order privilege events by sequence number and merge snapshots by position #333

2026-09-16T18:28:31Z

argoyle commented

2026-09-16 18:28:31 +00:00

Why

The privilege cache could keep a grant authz-service had revoked:

Unordered keys: each routing key has its own transient queue, so a late Privilege.Added/User.Added resurrected a revoked grant.
Startup gap: services fetched /authz before binding their queues, so revocations published in between were lost until restart.

Design: ADR-0015 (docs PR, Proposed).

What

Process orders events by authz-service's global sequenceNo per (email, company). All four events come from the Company aggregate, so seq order equals commit order. An event only overrides older facts, and User.Removed stamps every privilege.
Events without a sequence number fail closed: additions are dropped, and removals hold until the next snapshot. Negative or huge sequence numbers are dropped.
Fetch checks the status and retries 503 (60×1 s, 30 s HTTP timeout). It reads X-Authz-Sequence, merges the snapshot as facts at that position, and raises a floor; snapshots older than the floor are ignored. A missing header merges at 0 with a warning (rollout window only).
CompaniesByUser returns [], and unknown privileges create no state. CLAUDE.md is rewritten.

BREAKING: Process without SequenceNo no longer grants, so service tests must set it. Services must call Fetch() after conn.Start.

Verification

go test -race: 98.3% coverage, including table-driven reorderings, snapshot-merge cases and a revocation-during-Fetch race test.
26 mutants on the ordering, merge and retry checks: all killed (each compiled and produced --- FAIL).
prek passes.

Expert review: two rounds. Round 1: Security, Go Backend, Event Sourcing and Database experts reviewed both diffs. Round 2: Security and Event Sourcing re-reviewed the fixes. A final Event Sourcing review covered the committed-events wrapper. Fixed from the reviews: older snapshots merged after newer ones (floor check); a lagging read view serving snapshots that miss revocations (503 plus catch-up); a reset's TRUNCATE emptying a REPEATABLE READ snapshot (LOCK TABLE privileges, verified on PostgreSQL); seq-0 removals undone by older additions (pending stamp); late-committing events skipped by read view backfills (CommittedEventStore with LOCK TABLE events IN SHARE MODE, verified on PostgreSQL 18); an unbounded lock wait (lock_timeout plus retries); catch-up firing on ordinary lag (5 s stall, 10 s cooldown, 10 min deadline); plus smaller items (unknown privileges, invalid seqs, require in a goroutine, wrapped errors, [] not nil). Deliberately deferred (tracked in Ambix): stored-but-unpublished revocations (user decision: authz-service outbox); the readview library's commit-order gap for other services (upstream); removing the missing-header fallback and alerting on /authz 503s; moving Fetch after conn.Start in the 13 consumers (separate bump PRs).

🤖 Generated with Claude Code

https://claude.ai/code/session_01DVGsVQ8AMFR4NZoxyCoEqS

## Why The privilege cache could keep a grant authz-service had revoked: - **Unordered keys:** each routing key has its own transient queue, so a late `Privilege.Added`/`User.Added` resurrected a revoked grant. - **Startup gap:** services fetched `/authz` before binding their queues, so revocations published in between were lost until restart. Design: ADR-0015 (docs PR, Proposed). ## What - `Process` orders events by authz-service's global `sequenceNo` per (email, company). All four events come from the Company aggregate, so seq order equals commit order. An event only overrides older facts, and `User.Removed` stamps every privilege. - Events without a sequence number fail closed: additions are dropped, and removals hold until the next snapshot. Negative or huge sequence numbers are dropped. - `Fetch` checks the status and retries 503 (60×1 s, 30 s HTTP timeout). It reads `X-Authz-Sequence`, merges the snapshot as facts at that position, and raises a floor; snapshots older than the floor are ignored. A missing header merges at 0 with a warning (rollout window only). - `CompaniesByUser` returns `[]`, and unknown privileges create no state. CLAUDE.md is rewritten. **BREAKING:** `Process` without `SequenceNo` no longer grants, so service tests must set it. Services must call `Fetch()` after `conn.Start`. ## Verification - `go test -race`: 98.3% coverage, including table-driven reorderings, snapshot-merge cases and a revocation-during-Fetch race test. - 26 mutants on the ordering, merge and retry checks: all killed (each compiled and produced `--- FAIL`). - prek passes. **Expert review:** two rounds. Round 1: Security, Go Backend, Event Sourcing and Database experts reviewed both diffs. Round 2: Security and Event Sourcing re-reviewed the fixes. A final Event Sourcing review covered the committed-events wrapper. Fixed from the reviews: older snapshots merged after newer ones (floor check); a lagging read view serving snapshots that miss revocations (503 plus catch-up); a reset's TRUNCATE emptying a REPEATABLE READ snapshot (LOCK TABLE privileges, verified on PostgreSQL); seq-0 removals undone by older additions (pending stamp); late-committing events skipped by read view backfills (CommittedEventStore with LOCK TABLE events IN SHARE MODE, verified on PostgreSQL 18); an unbounded lock wait (lock_timeout plus retries); catch-up firing on ordinary lag (5 s stall, 10 s cooldown, 10 min deadline); plus smaller items (unknown privileges, invalid seqs, `require` in a goroutine, wrapped errors, `[]` not nil). **Deliberately deferred (tracked in Ambix):** stored-but-unpublished revocations (user decision: authz-service outbox); the readview library's commit-order gap for other services (upstream); removing the missing-header fallback and alerting on /authz 503s; moving Fetch after conn.Start in the 13 consumers (separate bump PRs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01DVGsVQ8AMFR4NZoxyCoEqS

argoyle added 1 commit 2026-09-16 18:28:33 +00:00

fix!: order privilege events by sequence number and merge snapshots by position

authz_client / test (push) Skipped

authz_client / vulnerabilities (push) Skipped

pre-commit / pre-commit (push) Skipped

authz_client / vulnerabilities (pull_request) Successful in 1m0s

Details

authz_client / test (pull_request) Successful in 1m10s

Details

pre-commit / pre-commit (pull_request) Successful in 3m47s

Details

84e9309901

The four privilege keys arrive on separate transient queues, so a late
Privilege.Added or User.Added could resurrect a revoked grant. Services also
fetched /authz before binding their queues, losing revocations published in
between.

Process now orders events by authz-service's global sequenceNo per
(email, company): an event only overrides older facts, User.Removed stamps
every privilege, and events without a sequence number fail closed (additions
dropped, removals held until the next snapshot). Fetch checks the status,
retries 503 while authz-service's read view is behind, reads the
X-Authz-Sequence header and merges the snapshot as facts at that position,
ignoring snapshots older than one already merged. CompaniesByUser returns []
instead of nil.

BREAKING CHANGE: events without SequenceNo no longer grant anything; tests that
seed the handler through Process must set SequenceNo. Call Fetch() after
conn.Start (ADR-0015).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVGsVQ8AMFR4NZoxyCoEqS

argoyle scheduled this pull request to auto merge when all checks succeed 2026-09-16 18:28:35 +00:00

argoyle canceled auto merging this pull request when all checks succeed 2026-09-16 18:32:32 +00:00

gitea-actions Bot commented

2026-09-16 18:32:35 +00:00