auditdb/locker: misc medium/low findings (empty-lock semantics, key-namespace collision, retry storms) #2040

Summary

A batch of medium/low-severity findings from a systematic bug-hunting review of token/services/storage/auditdb and its locker/** subpackages, not severe enough to warrant individual issues but worth tracking and fixing. (Two more severe findings from the same review — Postgres locker self-theft and empty-Owner cluster-wide lock disablement — are tracked separately as #2033 and #2035.)

1. Inconsistent empty-enrollment-IDs semantics between memory and postgres lockers (MEDIUM)

• auditdb/locker/memory/memory.go:87-89 — AssertLocksHeld always returns nil regardless of whether any locks were ever acquired.
• auditdb/locker/postgres/postgres.go:107-111 — AcquireLocks returns early with no session recorded when the deduplicated enrollment-ID set is empty; AssertLocksHeld (postgres.go:259-261) then treats "no session" as ErrLockNotHeld unconditionally.

A token request whose inputs+outputs yield zero enrollment IDs succeeds at AcquireLocks but then fails Append ("locks lost before write") only under the Postgres backend — a backend-dependent correctness divergence for an edge case that is rare but real and verified in code.

Suggested fix: align behavior — either both backends should treat zero-locks as always-satisfied (mirroring memory), or both should require an explicit no-op session marker so AssertLocksHeld succeeds consistently.

2. Shared sync.Map namespace risks a panic on key collision (MEDIUM)

auditdb/locker/memory/memory.go — m.locks sync.Map stores both eID -> *semaphore.Weighted (line 46) and anchor -> []string (line 59) in the same map, with no prefix/namespace separation. If an anchor string ever collides with an enrollment-ID string, AcquireLocks's type assertion sem.(*semaphore.Weighted) (line 47) or ReleaseLocks's equivalent (line 79) panics. Both namespaces are unconstrained strings; not currently exercised by tests.

Suggested fix: use two separate maps, or prefix keys by namespace (e.g. "eid:" + eID vs "anchor:" + anchor) before storing.

3. Doubled, uncoordinated retry loops can cause ~10 minutes of blocking under contention (LOW/MEDIUM)

services/auditor/auditor.go:189-210 wraps AcquireLocks in its own exponential-backoff retry (default MaxRetries=10), while the Postgres locker (auditdb/locker/postgres/postgres.go:107-144) already retries internally for up to AcquireDeadline (default 1 minute) using a fixed, non-exponential, non-jittered poll interval (AcquireBackoff, default 100ms — up to ~600 DB round trips per attempt). Nesting these means worst-case blocking of ~10 minutes under sustained contention, plus constant-rate DB polling that can thunder under load.

Suggested fix: make the outer retry Postgres-aware (skip it entirely for the Postgres backend, or shorten AcquireDeadline), and add exponential backoff + jitter to the inner poll loop.

Perf suggestions

• AssertLocksHeld/lease-renewal heartbeats run one query per anchor per tick — batching across anchors on one node would reduce connection churn under many concurrent audits.
• Consider capping/evicting the memory locker's semaphore map if enrollment-ID cardinality is unbounded in some deployments.

Severity

MEDIUM (items 1, 2), LOW/MEDIUM (item 3) — correctness edge cases and a latent robustness/perf issue, not immediately exploitable but worth fixing alongside the related critical/high issues from the same review.