feat(security): R9 release-hardening-p0 — five P0 findings closed (auth · uploads · ZDR · audit) - #211
Open
TinDang97 wants to merge 9 commits into
Open
feat(security): R9 release-hardening-p0 — five P0 findings closed (auth · uploads · ZDR · audit)#211TinDang97 wants to merge 9 commits into
TinDang97 wants to merge 9 commits into
Conversation
…ion revocation (P0-1) Task 1 of release-hardening-p0 (auth-hardening-login-sessions, SECURITY, gated PASS by Tin Dang; adversarial security lens: appsec-engineer, no HARD-STOP). Closes the three P0 auth gaps from the 2026-08-18 deep review: S1 — hardened login: - POST /admin/auth/login now rate-limited per-IP + per-email (fixed-window InvitePublicRateLimiter, fail-open on Redis outage) BEFORE the argon2 verify — a limited attempt costs zero hasher work. Client IP only via resolve_trusted_client_ip; 429s byte-identical for known/unknown emails. - Lockout is limiter decay only — no persistent lock state (no lockout DoS). S2 — password reset: - POST /admin/auth/password-reset: uniform 202 (no enumeration oracle); 256-bit CSPRNG token, sha256-at-rest, TTL-bounded (password_reset_tokens). - POST /admin/auth/password-reset/confirm: validity -> expiry -> weak-LAST order (weak never consumes the token); atomic single-use consume + new password_hash + users.sessions_not_before watermark in ONE transaction — every pre-reset session dies at the identity seam. S3 — server-side revocation: - Every newly issued session JWT carries a jti; POST /admin/auth/logout denylists it (revoked_auth_sessions, idempotent, self-GC'ing). - ensure_session_not_revoked enforced at ALL 5 identity seams (authz _resolve_identity, keys/catalog deps, usage router, GetIdentityUseCase — the latter with a REQUIRED revocation_guard_factory kwarg across all 6 construction sites). Store failure -> 503 ERR_AUTH_UNAVAILABLE (fail closed in the domain helper), never a 401 and never a silent allow. - Legacy no-jti tokens still decode (watermark-only revocable). - auth.password_reset / auth.logout audit events, secret-free metadata. Migration b6d2e8f19a45 (additive): users.sessions_not_before + password_reset_tokens + revoked_auth_sessions; both table manifests updated (SANCTIONED EDIT). Two sibling exact-claim-set asserts gained the jti claim (SANCTIONED EDIT). One red-check repaired (XFF test authored on a false premise about trusted_proxy_hops default 0 — config coerces <=0 to 1; repaired to attacker-prepended-token semantics, assertion unchanged, disclosed at gate). Evidence: 14/14 frozen checks green (receipt 4 bound); 326 sibling tests green; pyright 0 errors; ruff clean. refs: release-hardening-p0 task 1/8 author: Tin Dang
…(P0-6) Task 2 of release-hardening-p0 (upload-bounds-audio, SECURITY, gated PASS by Tin Dang; adversarial security lens: appsec-engineer, no HARD-STOP). The deep review's "audio uploads have no size cap" was partly false — the edge BodySizeLimitMiddleware has capped /v1/audio/ total-body since #65. The real defects fixed here: - The route cap EQUALED max_audio_upload_bytes, so a legal file of exactly the cap was wrongly refused (multipart framing overflow) and no precise per-file 413 could ever fire (boundary theater). /v1/audio/ now carries 1 MiB multipart headroom via main._audio_route_cap (the _files_route_cap precedent; 0 -> large finite ceiling, never unlimited). - TranscriptionUseCase gains a per-file cap (keyword-only max_file_bytes, 0=off, prod DI injects GATEWAY_MAX_AUDIO_UPLOAD_BYTES): bounded read(cap+1) — the files-router idiom, oversize never materializes — refusing 413 ERR_PAYLOAD_AUDIO_TOO_LARGE before the upstream call and before any usage record, on BOTH /v1/audio/transcriptions and /v1/audio/translations. - New structural guard (tests/upload_bounds): every multipart route prefix must carry a finite middleware cap + finite default_cap, and every upload-field `.read()` under proxy/application + files/api must sit in an allowlisted bounded reader — a future unbounded read site fails CI. One SANCTIONED EDIT: edge_input_hardening's audio-cap probe moved to the new cap+headroom+1 edge boundary (intent unchanged, mandated by frozen M3). Residue (Tin-accepted at gate): /v1/realtime WS->STT bridge constructs the use case without the cap (own utterance_too_large gate) — follow-up task upload-bounds-realtime-stt created. Evidence: 7/7 frozen checks green (receipt 2 bound); 135 sibling tests green (edge_input_hardening, audio_endpoints, audio_translations, azure_audio, files_uploads_api); pyright 0 errors; ruff clean. Implemented by opus add-worker delegation; verify completed by orchestrator. refs: release-hardening-p0 task 2/8 author: Tin Dang
…bridge Follow-up task upload-bounds-realtime-stt (release-hardening-p0, SECURITY, gated PASS by Tin Dang; appsec-engineer lens: close, no HARD-STOP, 0.93). Directed by Tin at upload-bounds-audio's gate from that task's residue. realtime_ws.py constructed TranscriptionUseCase without max_file_bytes, leaving the per-file cap off on the WS->STT leg (bounded only by the protocol-level utterance_too_large gate). It now receives max_file_bytes=settings.max_audio_upload_bytes — the SAME knob the HTTP STT path injects (no realtime-specific twin; utterance gate untouched). Checks (tests/upload_bounds/test_realtime_stt_cap.py, driving _real_stt directly with a fake websocket): over-cap buffer raises the per-file ERR_PAYLOAD_AUDIO_TOO_LARGE with zero upstream/usage-record calls (red->green — a transcript came back pre-fix); at-cap still transcribes (green-by-design regression pin, disclosed at freeze). Evidence: 106 tests green across upload_bounds + realtime suites; pyright 0 errors; ruff clean. refs: release-hardening-p0 follow-up to task 2/8 author: Tin Dang
…+ inventory guard (P0-2) Closes deep-review P0 #2: the ZDR purge pass hand-enumerated nine tables and silently missed the three newest payload stores, so a zdr_enabled tenant's chunk text and embeddings, eval request bodies and response text, and finetune hyperparameters and provider error text survived every sweep tick. Recon during direction found a second, unreported hole: finetune had NO ZDR gate anywhere on its write path (router -> use_cases.create_job -> repository.create were all unguarded), unlike vector_stores and evals which already carry the locked re-check. Purging at rest while the write path stays open is a leaky bucket, so both are closed here. - ZDR_PURGE_TABLES: ONE declared tuple in retention_policy.py that the sweeper itself CONSUMES (the structural guard binds it by object identity, so an equal-but-separate copy fails) = the existing nine + vector_store_chunks, eval_cases, eval_case_results, finetune_job_events, finetune_jobs. - The tuple names WHICH tables, never HOW. The three blob-backed tables (artifacts, files, compliance_report_runs) keep their object-store-aware purgers: delete the blob first, DEFER the row when the store is unreachable. Collapsing them into a generic row-DELETE would drop the object_key that is the only handle on those bytes -- a permanent orphan no later tick can find. - Import-time invariant: a table declared without a registered purger (or a purger with no inventory row) fails at import rather than being silently skipped every tick. Pure module constants, so it can only fire in CI. - Explicit per-table tenant-scoped DELETEs; children before containers, never relying on FK cascade -- a re-parented or denormalized row escapes a cascade. - finetune ZDR gate, both halves: entry raise_if_zdr as the first statement in create_job (before any provider round-trip), and raise_if_zdr_locked as the LAST statement before the router commits -- after the provider await returns, never at the insert, which would hold the tenants row lock across an outbound round-trip and trade a TOCTOU for a per-tenant head-of-line stall. - A structural guard walks Base.metadata: any tenant_id-carrying table with a Text/JSONB/Vector column must sit in the consumed inventory or on a named (table, reason, task-citation) exemption row. 26 exemptions, each justified. Direction was pressure-tested before freeze, which caught two defects a compliant Build would otherwise have shipped: the blob-orphan path above, and a TOCTOU check that failed to discriminate (its harness took the row lock before the request started, so an entry-only locked gate would have passed it). Verify: 5/5 frozen checks green; 361 sibling tests green; pyright 0 errors. Both M4 clauses proved load-bearing by mutation -- each gate removed in turn made the specific check that must go red, go red. Residue on record at gate: the shared FOR UPDATE lock wait is not time-bounded (six call sites share it; a follow-up bounds the primitive once); enabling ZDR drains vector-store chunks while file status still reads "completed"; a ZDR tenant's cancel_job still writes one event row that lives at most one tick. ADD: .add/tasks/zdr-retention-inventory-extension.md (frozen sha256:4dbc95a9, gate PASS, appsec-engineer lens, receipt runs/1.md) author: Tin Dang
… — /admin/catalog/sync and /admin/teams 500 Repairs a regression introduced on this branch by 4222420 (auth-hardening P0-1). Not shipped: 4222420 is not an ancestor of origin/main, so there was no production exposure -- this simply must never merge broken. Cause: 4222420 wired `ensure_session_not_revoked` into five call sites. Its SELECT autobegins a transaction on whatever session it is handed. A repository that later calls `async with self._session.begin()` on that SAME session then raises InvalidRequestError ("A transaction is already begun on this Session"). Two seams share the request session AND pair with such a repository, and both were live 500s: catalog/api/deps.py -> SqlAlchemyCatalogRepository.sync_catalog tests/catalog_sync_trigger: 9/9 green at cd17ccf, 5/9 red at 4222420 keys/api/deps.py -> teams/api/deps.py -> SqlAlchemyTeamRepository tests/teams: 29 failed / 2 passed at 4222420 The sibling impersonation guard never tripped this because it queries only for impersonation sessions; the revocation guard queries on EVERY request. Fix, in the SHARED PRIMITIVES rather than at the dependencies -- all THREE read-only auth guards (DbSessionRevocationGuard.is_revoked, DbImpersonationSessionGuard.ensure_live, DbUserLivenessGuard.ensure_active) now sample `opened = not session.in_transaction()` before their read and roll back ONLY if they opened one. A caller that already holds a transaction keeps it, so a read-only guard can never discard someone else's pending work. Rollback, never commit (R:BLIND_COMMIT): the guards only read, and a commit on the auth path can persist half-formed state a later failure should have discarded. The restore never displaces the verdict the caller is owed: on the store-failure branch it is best-effort (a rollback failure there is the same outage and must not turn the 503 ERR_AUTH_UNAVAILABLE into a 500); on the success branch it is NOT suppressed, because swallowing it would leave the transaction open and silently resurrect the clash. REJECTED, on record, because this branch tried it first: closing the transaction in each dependency (catalog/api/deps.py, keys/api/deps.py). Two refute passes killed it. It does not COMPOSE -- tenants/domain/authz.py::_resolve_identity runs after keys/api/deps.py::get_identity has already closed and simply re-opens one, and require_active_user's liveness SELECT re-opens it again after that; both proven at runtime on POST /admin/keys. Worse, keeping even two of them as "defence in depth" MASKS the runtime sweep below: an unconditional close at the dependency hides a primitive that failed to restore, so a deliberately broken primitive turned only 1 of 5 seams red instead of 5 of 5. Both rollbacks are therefore REMOVED, not kept. Two guards, because one shape of evidence was not enough: - A RUNTIME seam sweep (test_seam_session_state.py) drives the real app through the real FastAPI dependency graph and samples the shared session at the moment a repository would be constructed on it -- after the auth seam, before the handler body, via an observer appended to the route's dependant. The earlier AST guard resolved reachability by IMPORT ADJACENCY and was structurally blind to 3 of the 5 seams. Sampling point is the whole check: after client.request returns, the session is closed and every seam reads clean; at dependency teardown, the handler's own reads have re-opened one and every seam reads dirty. Both wrong points pass against the pre-fix tree and prove nothing. - A guard CENSUS (test_no_unguarded_autobegin_seam.py) discovers its own population -- any class constructed with both `session=` and `timeout_seconds=`, the house shape of all three guards and of no read repository -- and fails if one lacks the restore. RED against the pre-fix tree with the population there resolving to exactly the three known guards, all three flagged. An anti-vacuity assert fails loudly if discovery ever stops seeing them, so a broken census cannot report a clean sweep. Unparseable source is a finding, never a skip. The task's A3 was WRONG THREE TIMES before it held -- "catalog only", then "catalog + keys", then "two guards" -- each time because the class of affected seams was enumerated BY HAND, and each time corrected by a refute pass rather than frozen as written. The census exists so the fourth guess is not needed: the class is now closed by construction. Also in this commit, from the same 4222420 blast radius: - tests/auth_hardening: drop a dead `count_rows` helper (no call sites; it was the only S608 finding), reformat 3 files, and declare the TIME BUDGET on the outage assertion that repo_hygiene's wall-clock guard requires. Still open from the same cause, booked as its own node with the diagnosis recorded: registrar-hint-zero-db-io -- the same SELECTs break the shipped domain-capture M12 "zero database IO" invariant. That is a query-count clash, not a transaction-state one, and every resolution is a real design decision. Verify: affected scope 143/143 green (tests/catalog_session_autobegin, catalog_sync_trigger, teams, keys, auth_hardening, impersonation_live_session_guard, impersonation_session_lifecycle, tenants); pyright 0 errors; ruff clean. ADD: .add/tasks/catalog-sync-session-autobegin.md (frozen sha256:ab3da018acfdbc07, plan authority, receipt runs/5.md — 6/6 checks bound) author: Tin Dang
…th a structural guard (P0-7) Closes deep-review P0 #7: evals, vector_stores, finetune, memory and conversations mutations emitted ZERO audit events, and nothing structurally prevented the next mutating route from shipping silent. Census moves 104 audited / 10 exempt / 25 violations -> 129 / 10 / 0, every one of the 25 earned on its OWN package's evidence, none by exemption. - Route walker: every mutating route must be audited or carry a named exemption row. Evidence is an AST `record_audit(...)` Call node in the handler's own IMMEDIATE package. Unclassifiable is a finding, never a skip. DESIGN CONSTRAINT this imposed, and the reason the diff looks the way it does: routing emissions through one shared `emit_audit()` facade is functionally correct but makes every retrofitted package look SILENT to its own guard. So `build_audit_event()` is shared and the `await record_audit(...)` call stays AT the call site. `build_audit_event`'s docstring says why, so it is not "cleaned up" later. - 25 emission sites, one DISTINCT action per mutation. /v1 rows carry the key actor; the nine JWT-authed /admin routes carry the Identity user actor -- the key rule would raise audit_missing_actor OUTSIDE record_audit's try and 500 the route. - Audits are INLINE-awaited, never fire-and-forget: a fire-and-forget write loses the read-race, and record_audit is fail-open by design. Verified no call site uses the request session -- all 20 use app.state.sessionmaker, so a failing audit cannot poison the mutation's own transaction. - Evidence envelope grows three NULLABLE actor fields across all four projection sites. Without them a retrofit row is actor_email: null everywhere, and the export's actor filter is exact-match -- an auditor filtering by actor would silently lose every key-actor row. - gateway_audit_write_failed_total: a swallowed audit-write failure was previously invisible (CC7.2). Unlabelled -- one increment is one lost event. The registry is resolved from the sessionmaker AT INCREMENT TIME, not bound eagerly, so the fail-open check is adversarial rather than vacuous. SANCTIONED EDIT: tests/audit_export/test_audit_export.py `_ITEM_FIELDS` gains the three new names. The assertion stays an EQUALITY -- it was not relaxed to a subset. The envelope grew; the guard did not. Mutation-proven still load-bearing: dropping actor_key_id from the projection turns audit_export red. Direction was pressure-tested before freeze and the build after it. The pre-freeze pass caught the anonymous-evidence defect above; the build's own refute pass caught a substring scan for `record_audit(` matching FIVE PROSE COMMENTS, which would have turned all 17 violations green -- replaced with AST Call-node detection. Verify: 10/10 frozen checks; 185 audit-family + 245 evals/memory/conversations/ vector-store/finetune siblings; pyright 0 errors; ruff clean. appsec-engineer lens on record, SAFE-TO-GATE, no HARD-STOP -- it refuted the counter-vacuity trap by instrumenting the raising double and observing it reached, and proved the walker discriminating by three mutations that each go red naming their victims. Residue on record at gate: M9 leg 3 cannot detect an eager-binding regression; the inline audit await is unbounded (bare create_async_engine, no pool/command timeout); observability/metrics.py and main.py carry M9's wiring but sit OUTSIDE the frozen scope_digest, so they will not stale this receipt if they drift. Follow-ups booked, not fixed here: impersonation-audit-actor-attribution (audit rows drop the real superadmin behind an impersonated identity -- pre-existing, ~40 emitters, and it qualifies the R8 CC6 claim); audit-coverage-v1-crud (the 7 deferred exemption rows); build_audit_event's own build-failure branch has no counter; the guard vouches per-package, so a new silent router inside an already-audited package stays invisible. ADD: .add/tasks/audit-coverage-structural-guard.md (frozen sha256:aed72dd9e1e31bdd, appsec-engineer lens) author: Tin Dang
…sweep both passed on a live 500 A fourth adversarial refute pass against the committed tree (aae83bd) found both anti-regression guards reporting GREEN while DbImpersonationSessionGuard was broken and the motivating InvalidRequestError 500 was live. The production fix is unchanged and was never in question; the EVIDENCE for it was. Reproduced before fixing. Deleting only the success-path restore from `ensure_live` (nothing else) left: test_catalog_sync_succeeds_for_authenticated_console_user FAILED (the real 500) test_every_read_only_session_guard_restores_what_it_opened passed <- vacuous test_no_seam_leaves_the_shared_session_in_a_transaction passed <- vacuous Two independent causes, one per guard: - CENSUS: `_has_conditional_restore` asked only whether `rollback` appeared anywhere in the ClassDef. Every guard also carries a best-effort rollback inside its `except` handler (M5), which satisfied that on its own. Ran the predicate against mutated source to confirm: ORIGINAL=True, MUTATED=True. Now the rollback must be reachable when NOTHING was raised — at least one not nested in an ast.ExceptHandler. Re-proven per guard: all three ORIGINAL=True / MUTATED=False. - SWEEP: authz.py::ensure_impersonation_session_live calls the guard IFF `identity.impersonation is not None`, and every SEAMS row drove a plain signup_and_login token — so that guard never executed in the sweep at all. Added an impersonated row (conftest already shipped mint_impersonated_owner_token). It now fails naming the seam: 'DbImpersonationSessionGuard (impersonated identity) (GET /admin/models -> 200)' Also fixed in the sweep, same class of defect: - The verdict is now read BEFORE the 5xx drivability triage. The defect manifests as exactly a 500, so triaging first filed the offending seam as "undrivable" and discarded the sample that proved it. - The anti-vacuity floor was `len(undrivable) < len(SEAMS)` — five of six seams could regress into undrivable and still report green. Now `not undrivable`: a seam that cannot be driven is a finding to fix, never a row to tolerate. Declared residue, recorded in the task node's EVIDENCE, each verified before filing: - REFUTED, not a defect: "an unsuppressed success-path rollback yields a 401 that lies about a live token". authz.py:236-242 already maps ANY non- SessionRevocationUnavailableError from the guard — a failed rollback included — to a 503. The impersonation and liveness guards surface it as a 500, which is what M5 asks for. - OPEN, no exposure: the population that shares the request session with these guards is wider than the four modules the contract named — five more (domain_claims_router.py:97, provider_keys_admin_router.py:113, saml_admin_router.py:145, oidc_admin_router.py:143, device_approval_router.py:119). The primitive-level fix covers them all by construction; this is the strongest argument yet for M4 refusing a per-dependency close. - OPEN, accepted: M5's success-branch non-suppression has no dedicated check; the autobegin CLASS is not closed, only its guard-opened subset (A11 explains why the census population is deliberately narrow). No production source changed in this commit. Task reopened to direction and re-gated rather than amended, so the ledger carries the reason the first gate was not enough: a green gate proves the declared checks ran, passed and are bound — never that they were sufficient. Verify: affected scope 143/143 green, 6/6 checks bound; pyright 0 errors; ruff clean. ADD: .add/tasks/catalog-sync-session-autobegin.md (refrozen sha256:45b868dc51bce41c, plan authority, receipt runs/7.md) author: Tin Dang
One tenant's upstream failures could deny service to every other tenant on the
same provider. This closes that on all four layers and adds two complementary
guards so the class cannot recur a fourth time.
The deep review filed this citing deps.py (breaker) and redis_cooldown_gate.py
(cooldown). Both citations were imprecise and both missed the live exposure.
deps.py was already keyed per PROVIDER (audit-remediation C1) — tenant-blind,
not global. The cooldown gate IS tenant-blind and cross-replica but is
DEFAULT-OFF (cooldown_failure_threshold defaults to 0). The actual live
cross-tenant DoS sat one layer down: ELEVEN process-wide adapter breakers, each
`self._breaker = CircuitBreaker()` in __init__, each constructed once in
create_app() while serving per-tenant BYOK credentials resolved per-request from
a contextvar. The two layers also compound — deps.py catches CircuitOpenError
from the adapter and counts it as the wrapper's own failure, so one tenant's
adapter trip drives the shared wrapper breaker toward opening too.
This defect class has been HARD-STOPPED at a verify gate twice before
(residency-service-tiers, api-surface-parity R4) and fixed in one seam only
(moderations CR-1). The remedy primitive already existed, private, at
ml_moderation_evaluator.py — this promotes it and applies it everywhere.
What changed:
- TenantScopedBreakerRegistry — ONE shared bounded-LRU registry (cap documented,
strict LRU so a hot tenant's OPEN breaker is never the eviction victim; an
uncapped dict keyed by tenant is a memory-growth vector under self-serve
signup). The private _TenantBreakerRegistry is replaced by it, not forked.
- breaker_tenant_key() resolves credential tenant, ELSE guardrail tenant, ELSE a
reserved sentinel. The guardrail fallback is load-bearing: the moderation seam
is already per-tenant but sets its credential untagged, so a credential-only
resolver would have silently undone the CR-1 fix.
- Every adapter entry point resolves through the registry — 31 entry points
across 14 classes, not 11 adapters. The difference is where the defect hid.
- Cooldown keys are tenant-partitioned on all FOUR kinds (fails/open/half/probe)
with the tenant segment FIRST. model_id is caller-controlled and may contain
":", so tenant-first makes cross-partition addressing unreachable by
construction rather than by validation. tenant_key_segment REJECTS a ":"
bearing key — the earlier replace(":", "_") was not injective ("a:b" and "a_b"
collided).
- Unattributed calls get a reserved sentinel partition, never the legacy
unprefixed key, and never share with a real tenant.
- GET /admin/routing reports an honest cross-partition aggregate — never
"closed" over a partition it did not inspect.
- cost_recovery.py now passes the tenant it already held, so background polling
cannot accumulate failures on a bucket shared with live traffic.
- Semantics unchanged: same threshold, same cooldown, same fail-open contract,
same zero-Redis-command fast path at threshold 0, no new hot-path await.
Isolation comes from partitioning the key, never from weakening protection.
Two complementary guards, deliberately using DISJOINT predicates:
- A static AST census over proxy/ for CircuitBreaker( construction.
- A boot-time census walking the LIVE OBJECT GRAPH by isinstance. This must not
share the static census's AST rule — `_CLS = CircuitBreaker; _CLS()` yields
name == "_CLS", so a shared predicate would report green on four of the five
evasion shapes the second guard exists to catch. It also catches a breaker
held only in a closure cell, which is how main.py wires the eval executor.
Also fixes a monitoring regression this diff would otherwise have caused: once
the realtime path moved off app.state.circuit_breaker, NOTHING in production
drove that breaker, so gateway_circuit_breaker_state was pinned at 0.0 =
"closed" forever. The gauge now reads the same population the boot census walks,
worst-state-wins, with NO tenant label — a tenant label would be unbounded and
attacker-influenceable, i.e. the memory-exhaustion vector reappearing as
Prometheus cardinality.
Evidence: 18/18 task checks; 703 sibling tests green across proxy, moderations,
realtime, routing, cooldown, observability and provider suites; pyright 0
errors; ruff clean. The green was proven non-hollow by mutating the fix in three
independent places (reversed key order; collapsed tenant segment; pinned gauge)
and confirming the corresponding guard went red each time — a guard proven only
against total absence is unproven against partial removal.
Contract amended mid-build and refrozen at human authority: three anti-vacuity
floors were phrased over the live tree and so were satisfiable only WHILE the
defect existed, and four checks bound R:KEY_COLLISION / R:SHARED_BUCKET to
signature reads rather than behaviour. Both are now fixed.
Refs: R9 release-hardening-p0, deep review artifact 6816985f (P0 #3)
author: Tin Dang
…t and STT work First CI run of the R9 branch went red on three tests across two shards. Two are fixed here; the third is a real design conflict and is deliberately left failing (see below). 1. tests/preset_resolution_ingress — a stale test double. `_FakeSettings` did not carry `max_audio_upload_bytes`, but upload-bounds-realtime-stt (318ab5c) made `_real_stt` pass `max_file_bytes=settings.max_audio_upload_bytes` into TranscriptionUseCase, so the WS-STT path raised AttributeError against the double. The double now mirrors the production default (core/config.py, 25 MiB) rather than inventing a value — a different number would silently test a bound production never uses. This is the same blast-radius class the milestone has hit before: a per-task suite structurally cannot see a shared dependency's other consumers. 2. tests/audit_export::test_export_empty_result_set — a latent flake, now deterministic, caused by a WRONG TEST PREMISE rather than by any regression. The export endpoint AUDITS ITSELF (router.py, action="audit.export"), which it also does on origin/main. The test exported twice for the SAME tenant and asserted the second call saw an empty set. That could only ever pass because the fire-and-forget audit write lost the read-race. The R9 audit retrofit routes emissions through the shared writer so the row now lands reliably, which flipped a latent flake into a deterministic failure under CI load. Diagnosed by DELAY INJECTION rather than by re-running: injecting a 1.5s sleep before the second export reproduced it locally every time, showing `items: [{'action': 'audit.export', ...}]`. The fix gives the JSON assertion a VIRGIN tenant, so "empty result set" means what it says instead of depending on losing a race. Proven by re-injecting the same 1.5s delay against the fixed test: it passes. A fix that only holds at native speed would not be a fix. Note this is a strengthening, not a weakening: the assertion is unchanged, and it now actually tests an empty set. NOT fixed here — needs a human decision, so CI stays red on it: tests/domain_capture::test_registrar_hint_zero_db_io A frozen ZERO-DB-IO invariant on GET /admin/domain-claims/registrar-hint is in direct conflict with the per-request session-revocation guard that auth-hardening-login-sessions (4222420) added for security. The route now issues 2 SELECTs per request (revoked_auth_sessions.jti, users.sessions_not_before). Both are shipped, deliberate, and correct in isolation; they cannot both stand unchanged. Resolving it means either amending a frozen invariant or accepting revocation lag from a cached verdict — a security tradeoff that is not mine to make silently. Tracked as the booked node `registrar-hint-zero-db-io`. Evidence: tests/audit_export + preset_resolution_ingress + audit_coverage + domain_capture = 130 passed, 1 failed (the registrar-hint conflict above). Refs: R9 release-hardening-p0 author: Tin Dang
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
R9
release-hardening-p0— first five gated tasksCloses five of the eight P0 findings from the 2026-08-18 seven-agent depth review
(artifact 6816985f), plus one regression the first task shipped. Every task went through
the full ADD 3-beat loop and carries a PASS gate at human authority (
sensitivity: security/datafloors) with acovers:-bound receipt.Commits
42224201auth-hardening-login-sessions/admin/auth/loginhad no rate limit or lockout; no password reset, no server-side session revocation anywhere. Adds ajticlaim to every new JWT,ensure_session_not_revokedat 5 seams, a guarded single-use reset consume (UPDATE … WHERE used_at IS NULL+ watermark in one txn), and a logout denylist with self-GC. Store failure maps fail-closed to503 ERR_AUTH_UNAVAILABLEin the domain helper, so a monkeypatched adapter cannot open it. 14/14 checks.f0dae847upload-bounds-audio_audio_route_capheadroom (cap+1MiB,0→ finite ceiling) so a new boundedread(cap+1)returns413 ERR_PAYLOAD_AUDIO_TOO_LARGE, plus a structural sweep pinning finite caps on every multipart prefix.318ab5c3upload-bounds-realtime-sttmax_file_bytesinto the/v1/realtimeWS-STT bridge, which had been relying on its ownutterance_too_largegate.b5200718zdr-retention-inventory-extensionvector_store_*,eval_*andfinetune_*, so ZDR tenants' payloads survived the sweep. Extends the inventory and adds a guard. Blob-backed tables keep the delete-object-first / defer-row ordering so anobject_keyis never orphaned.4f08916baudit-coverage-structural-guardaae83bd1catalog-sync-session-autobeginSELECTautobegins a transaction on the shared session, so any repository later callingasync with session.begin()raisedInvalidRequestError— two live 500s (/admin/catalog/sync, every/admin/teamsmutation). Fix is a conditional restore inside all three shared read-only guards: recordopened = not session.in_transaction()before the read, roll back only if this guard opened one.bb4b8b79exceptbranch, and the runtime sweep never executed the impersonation guard because every row drove an ordinary login. Both now provenORIGINAL=True / MUTATED=Falseper population member.Evidence
Each task's receipt lives under
.add/tasks/<slug>.md→verified:. Gates: 14/14, 5/5,5/5, 10/10, 6/6 checks bound. Wide sibling suites run per task (the per-task suites
structurally cannot see a shared auth dependency's blast radius — that is exactly how the
four
aae83bd1regressions escaped).pyright0 errors on every commit.⚠ Review status — please read
This branch has not had a second human reviewer.
required_approving_review_countonmainis0, so merging this on my approval alone is byte-approval, not four-eyes —the same disclosure that applies to #117/#118 and #199–#210. For SOC 2 CC8.1 this is the
open exposure, not a formality. It should get a real second reader before merge.
Not in this branch
tenant-scoped-breaker-cooldown(P0-3) is frozen with its build HARD-STOPPED on achange-request — three of its 17 checks are unsatisfiable by construction (an anti-vacuity
floor tied to the live tree is satisfiable only while the defect exists). It lands
separately.
vision-fidelity-bedrock-messages,credits-gate-fail-closedandkeycloak-external-idpare still open.