fix(squad,brain): token-cache DoS fix, month-boundary crash, brain roster default-deny + quiet paging - #205
Conversation
…brain roster default-deny + escalation-only paging Three production incidents from 2026-07-27, fixed live and back-ported here. 1. squad/auth.py — token-verification congestion collapse. _lookup_token bcrypt-checked the presented token against every api_keys row (17 bcrypt rows ≈ 5s CPU) synchronously on the event loop. Timeout-retrying clients (loop.py skill registration, hermes check-in) turned one slow verify into a pegged core and an unresponsive service; watchdog restarts replayed the same load. Fix: sha256-keyed verification cache (raw tokens never stored), positive TTL 300s, negative TTL 60s, bounded at 256 entries; create_api_key clears its own negative entry. Known trade-off: a revoked key can outlive revocation by up to 300s on this internal surface — the durable fix is an indexed token-fingerprint column (follow-up). 2. squad/app.py — .replace(day=day+N) crashes crossing month boundaries (ValueError: day is out of range for month; fired after Jul 24, killed the weekly/daily cron loops). All four sites now use timedelta. 3. sovereign/brain.py — ghost-agent dispatch loop + notification spam. Roster is now default-deny (kasra + system only, per Hadi directive 2026-07-27); off-roster skips return non-error text so the next brain cycle does not propose investigating its own roster policy; hallucinated method names return calm skipped text instead of an error; cycle results page out (bus→kasra→Telegram) only on failure — executed/skipped stay in the journal. Tests: tests/services/test_squad_auth_token_cache.py — 7 tests, mutation- verified (disabling the cache lookup fails exactly the two tests that claim to lock it). Neighboring auth/squad suites: identical pass/fail count on clean origin/main (pre-existing red, receipt in PR body). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7c2fc4439
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from kernel.config import ( | ||
| MIRROR_URL, MIRROR_TOKEN, SQUAD_URL, SOS_ENGINE_URL, | ||
| BRAIN_TENANT_SCOPE, BRAIN_SCOPE_TYPE, BRAIN_TOKEN_BUDGET, | ||
| MUPOT_MCP_URL, MUPOT_BRAIN_TOKEN, |
There was a problem hiding this comment.
Importing sovereign/brain.py now fails with ImportError because kernel.config does not define either MUPOT_MCP_URL or MUPOT_BRAIN_TOKEN; a repo-wide search finds these names only in this new import and its call sites. Consequently the brain daemon and every test importing this module fail during startup, before any cycle can run.
Useful? React with 👍 / 👎.
| if agent not in _AGENT_SESSION: | ||
| return False |
There was a problem hiding this comment.
When the model selects an active kasra or system agent for a project with a PROJECT_LEADS entry, this check succeeds, but motor_execute later replaces the agent without checking the final target. For example, agent="kasra" with goal_dnu is rerouted to the deliberately paused dandan and then posted to Squad, bypassing the new default-deny roster and recreating the ghost-agent dispatch this change is intended to stop.
Useful? React with 👍 / 👎.
| # Hardcoding "river" here was the exact #490 root-cause pattern | ||
| # (a stale roster assumption, not a live check) -- defer to | ||
| # mupot's own effort-router instead of gating a hardcoded name. | ||
| return _mupot_dispatch_task("squad-core", f"Research: {action.get('action', '')}", details, "medium", ["research", "brain-generated"]) |
There was a problem hiding this comment.
For a repeated Mumega research, outreach, or code-fix decision, the pre-dispatch _task_exists check queries only Mirror and the legacy SQUAD_URL board, while this new call writes to the separate live mupot board. The task just created here is therefore invisible to the next cycle's duplicate check, so the same work can be created again every cycle after the governor window expires; the mupot board must be queried or supplied an idempotency key before creating the task.
Useful? React with 👍 / 👎.
Fixes to sos/services/squad/auth.py and sovereign/brain.py per the sos-205-a7c2fc44 adversarial gate verdict: /home/mumega/mupot-worktrees/_gate-verdicts/sos-205-a7c2fc44-adversarial.md BLOCK-1 (unique-token spray DoS unmitigated): require_capability's async dependency now offloads _lookup_token's full-table bcrypt scan to a worker thread via anyio.to_thread.run_sync, so a spray blocks a thread-pool slot instead of the event loop. Added threading.Lock around all cache reads/ writes (now genuinely cross-thread state). Durable fix (indexed token- fingerprint column at mint time) stays a follow-up — tracking issue to be filed on Mumega-com/sos. BLOCK-2 (300s stale-validity, no revocation path): added revoke_api_key(tenant_id, db) — deletes the tenant's api_keys rows and clears the entire token cache (whole-cache clear is correct at this scale; raw tokens are never stored, so per-entry targeting is impossible). Added a `revoke` subcommand to auth._cli(). Dropped positive TTL 300s -> 30s. BLOCK-3 (attacker can flush 100% of the cache): split the cache into separate bounded pools — positive (max 64) and negative (max 192) — so a sprayed negative can only evict another negative, never a live positive. BLOCK-4 (brain.py capability gate bypass): sovereign/brain.py's `research` method dispatched to mupot for project=="mumega" BEFORE _capability_block ran, skipping the colony capability gate entirely for that one path. Moved the gate above the mumega branch, matching create_task/ send_outreach/fix_code. Also guards the "squad-core" mumega dispatch target against being empty instead of proceeding unconditionally. WARN-2 (dead INSERT OR REPLACE): create_api_key's OR REPLACE never matched (bcrypt salts are random) so it was dead code and the table grew unboundedly, which is also BLOCK-1's actual root cause. Replaced with a plain INSERT; added an explicit `rotate` flag/param that deletes the tenant+identity_type's existing rows first when the caller opts in. WARN-3 (cache key == legacy credential format): domain-separated the cache key to sha256(b"squad-authcache-v1:" + token) so it can never be byte-identical to a legacy stored-credential hash. Also fixed: sovereign/brain.py referenced MUPOT_MCP_URL/MUPOT_BRAIN_TOKEN from kernel.config without either ever being defined there (added in this same a7c2fc4 commit) — `import brain` raised ImportError unconditionally, found while testing the BLOCK-4 fix. Added both to sovereign/kernel/config.py as empty-string-default env reads, matching _mupot_dispatch_task's existing not-configured guard. Tests: tests/services/test_squad_auth_token_cache.py keeps the original 7 (adapted to the pool split) and adds revocation-immediate (x3), eviction-pool-isolation (x2), and a 50-way concurrent-lookup lock smoke test — 13 passed. Mutation-verified: disabling the cache-clear call in revoke_api_key makes the revocation tests fail (confirmed, then reverted). tests/brain/test_capability_gate.py adds 3 BLOCK-4 regression tests (blocks when gate subject is tenant-bound, dispatches when allowed, non-mumega path unchanged) — 24 passed (21 original + 3 new). Pre-existing red, unrelated to this change (different modules — sos.kernel.auth / sos.services.economy, not sos.services.squad.auth): tests/services/test_auth.py = 1 failed/16 passed; tests/services/test_auth_migration.py = 5 errors/5 passed/2 skipped. (The brief's "11 failed/5 errors" baseline doesn't match this specific file, which has only 17 tests total — recorded honestly rather than adjusted to fit.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
Adversarial gate BLOCK fixes — commit b5307ddFixes for all 4 BLOCKs + WARN-2/WARN-3 from the adversarial verdict BLOCK-2 (revocation) — fixed
BLOCK-1 (unique-token spray DoS) — mitigated, not closed
BLOCK-3 (eviction abuse) — fixedCache split into two independently-bounded pools: positive (max 64), negative
BLOCK-4 (brain.py capability gate bypass) — fixed
WARN-2 (dead
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5307dd722
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # just this request. Offload to a worker thread; the cache itself | ||
| # (_TOKEN_CACHE_LOCK) is what keeps this safe now that it runs | ||
| # cross-thread instead of on a single synchronous event loop. | ||
| auth = await anyio.to_thread.run_sync(_lookup_token, token, database) |
There was a problem hiding this comment.
When a client retries the same invalid token before the first ~5-second lookup finishes, every request observes a cache miss and starts its own full-table bcrypt scan in the AnyIO worker pool; _TOKEN_CACHE_LOCK protects only dictionary access and does not serialize these misses. This recreates the incident's congestion collapse by allowing identical timeout retries to consume the thread pool and CPU concurrently, before any request can populate the negative cache. Add per-key single-flight behavior or otherwise bound concurrent verification scans.
Useful? React with 👍 / 👎.
| if args.command == "revoke": | ||
| deleted = revoke_api_key(args.tenant) | ||
| print(f"tenant_id={args.tenant}") | ||
| print(f"revoked_rows={deleted}") | ||
| print("cache_cleared=true") |
There was a problem hiding this comment.
When an operator uses this newly added revoke command while Squad is running, revoke_api_key() executes in the short-lived CLI process, so _token_cache_clear() clears only that process's empty globals. Although the database row is deleted, the service process can continue accepting a previously cached token for the remainder of its 30-second positive TTL, making the printed cache_cleared=true and immediate-revocation guarantee incorrect. Route revocation through the service or use invalidation state visible to every serving process.
Useful? React with 👍 / 👎.
| # BLOCK-1 fix (sos-205-a7c2fc44 adversarial gate): _lookup_token's | ||
| # full-table bcrypt scan (~5s+ with a handful of rows) ran directly on | ||
| # the event loop, so unique-token spray pegged the whole service, not | ||
| # just this request. Offload to a worker thread; the cache itself | ||
| # (_TOKEN_CACHE_LOCK) is what keeps this safe now that it runs | ||
| # cross-thread instead of on a single synchronous event loop. | ||
| auth = await anyio.to_thread.run_sync(_lookup_token, token, database) |
There was a problem hiding this comment.
This offloads lookups only for routes using require_capability, while many public async routes still call _squad_lookup_token synchronously—for example the roles endpoints at app.py:2148-2236 and contacts endpoints at app.py:2368-2419. A request carrying a unique invalid bearer to any of those routes still performs the full-table bcrypt scan on the event-loop thread, so the token-spray DoS remains available despite this fix. Move all async call sites through the offloaded dependency or make _lookup_token itself asynchronously offloaded.
Useful? React with 👍 / 👎.
| if method not in _SUPPORTED_BRAIN_METHODS: | ||
| return {"success": False, "result": f"Unsupported brain method: {method}"} | ||
| # skipped=True + non-error phrasing: a hallucinated method name is a | ||
| # decision-layer miss, not a system fault. Error-shaped text here fed | ||
| # "investigate unsupported method" proposals in following cycles. | ||
| return {"success": True, "skipped": True, "result": f"Method '{method}' is not in the supported set; action intentionally not executed. Pick only from the documented methods — do not investigate."} |
There was a problem hiding this comment.
When the model returns an unsupported method, this branch now marks the unexecuted action as success=True; report_to_inkwell() subsequently records any truthy success as success: 1 at brain.py:1073-1085 and does not include the new skipped field. Consequently decision-layer hallucinations are indistinguishable from successfully executed work in the brain_cycles data, inflating operational success metrics. Preserve the quiet-notification behavior using skipped, but do not report these cycles as successful executions.
Useful? React with 👍 / 👎.
Re-gate verdict: /home/mumega/mupot-worktrees/_gate-verdicts/sos-205-b5307dd7-regate.md BLOCK-1 (P0, event-loop DoS): the a7c2fc4 fix offloaded _lookup_token's full-table bcrypt scan to a worker thread only inside require_capability's dependency. 34 other app.py route handlers called the sync _lookup_token directly, so the scan ran ON the event loop for the whole RBAC/CRM/ referrals surface — measured 27,956ms of dead event loop (heartbeat scheduled once) for a single unpatched replay. Added a public `async def lookup_token(token, db)` in auth.py as the single offload point; require_capability and all 34 app.py call sites now go through it. Grep-verified zero remaining direct `_lookup_token(` calls outside auth.py and tests. BLOCK-1b (single-flight): concurrent replays of the SAME cold token each ran their own full scan (verdict measured 8 concurrent -> 8 scans vs 1 sequential). _lookup_token now registers a concurrent.futures.Future per cache_key under the existing lock; only the first caller scans, others join its result. Mutation-verified: disabling the future-map makes the new single-flight test fail (8 scans instead of 1). BLOCK-2 (honest revocation): the CLI `revoke` subcommand ran in a separate process from the serving uvicorn worker and printed cache_cleared=true while only ever clearing its own, always-empty cache — a revoked token kept authenticating against the live service for up to the 30s positive TTL. Added a system-bearer-gated POST /auth/revoke route that runs revoke_api_key IN the serving process; the CLI now tries that route first (SQUAD_URL + SYSTEM_TOKEN) and only prints cache_cleared=service on a genuine 200, falling back to an honest cache_cleared=LOCAL-PROCESS-ONLY receipt (never a fabricated true) when the service is unreachable. BLOCK-4 follow-up (vacuous gate): brain.py's research branch gated the hardcoded literal "river" instead of the real `agent` variable — _agent_home_tenant('river') is always None, so the gate could never DENY at any position in the function. Now gates the actual dispatch subject; also deleted the statically-dead `if not research_squad_id` guard (research_squad_id is a literal, never falsy). Updated the 3 BLOCK-4 regression tests to drive the real subject, added a 4th covering a genuinely tenant-bound agent (digid) actually being denied on both the mumega and non-mumega dispatch paths. WARN-1 (cache/db scoping): _token_cache_key(token, db) now folds in str(db.db_path), so two SquadDB instances can no longer share cache entries. Tests: 90 passed across test_squad_auth_token_cache.py (+9 new), test_squad_auth_revoke_route.py (new, 8 tests), test_capability_gate.py (25, incl. 4 BLOCK-4), test_squad_client.py, test_customer_intake.py, test_squad_tasks_board.py, test_squad_g72.py. Pre-existing red in test_squad_auto_route.py (10 failures, Squad.__init__ 'project' kwarg mismatch) confirmed unchanged vs base b5307dd — unrelated to this delta. Third adversarial pass required before merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
Round-3 fixes — closes the sos-205-b5307dd7 re-gate BLOCKsCommit: BLOCK-1 (P0, event-loop DoS) — fixedThe a7c2fc4 fix offloaded Fix: added a public BLOCK-1b (single-flight) — fixedVerdict measured 8 concurrent replays of ONE cold token → 8 full scans (vs 1 sequential). Mutation-verified: disabling the future-map (leader/follower branch) makes BLOCK-2 (honest revocation) — fixedThe CLI Fix: added a system-bearer-gated BLOCK-4 follow-up (vacuous gate) — fixed
Updated the 3 BLOCK-4 regression tests to drive the real subject (previously they made WARN-1 (cache/db scoping) — fixed
Test results90 passed:
Pre-existing red confirmed unchanged: Third adversarial pass required before merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47f5f8c245
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| deleted = cursor.rowcount if cursor.rowcount is not None else 0 | ||
| _token_cache_clear() |
There was a problem hiding this comment.
When revocation overlaps a cold lookup of the revoked token, _scan_and_cache can read and verify the row before this deletion commits, then call _token_cache_put after _token_cache_clear() returns. That repopulates a positive entry from the deleted row, so the supposedly immediate revocation can still authenticate for the 30-second positive TTL. Coordinate revocation with _TOKEN_CACHE_INFLIGHT or use an invalidation generation so scans started before the deletion cannot publish stale results.
Useful? React with 👍 / 👎.
| # Agent name → tmux session name (empty string = system/no session needed). | ||
| # Active roster per Hadi directive 2026-07-27: kasra + system only. Paused | ||
| # agents (athena/river/sol/dandan) are intentionally absent — dispatching to | ||
| # them produced the "no tmux session" self-investigation loop. |
There was a problem hiding this comment.
The new roster explicitly pauses river, but a normal non-Mumega research action proposed for active kasra passes the only availability check and later hardcodes the Mirror assignee to river. Thus this common branch still creates work for the paused agent and can recreate the ghost-task loop; check the actual research target or route it to an active worker before posting.
Useful? React with 👍 / 👎.
…te-3
Gate-3 verdict (sos-205-47f5f8c2-gate3.md): BLOCK — delta clean, but two
pre-existing P0s live on the exact surface this PR hardens.
P0-A: `token == SYSTEM_TOKEN` with SYSTEM_TOKEN defaulting to "" (unset
SOS_SYSTEM_TOKEN) matched an empty presented token (no Authorization header
at all), granting unauthenticated system:sos/is_system=True access on all 34
_parse_bearer routes. Fixed: `SYSTEM_TOKEN and hmac.compare_digest(token,
SYSTEM_TOKEN)` (auth.py) — the branch cannot fire while unset, and the
compare is constant-time (closes LOW-3). Documented in .env.example (new
required section) and docker-compose.yml (`squad:` service now
`${SOS_SYSTEM_TOKEN:?set explicitly}`, hard-fails the compose deploy path);
a loud startup warning covers every other entry point.
P0-B: five RBAC routes (add_role_permission, remove_role_permission,
revoke_role_assignment, list_role_assignments, get_agent_roles) proved a
caller's token was valid and then discarded the AuthContext entirely — any
tenant's api key could mutate/enumerate any OTHER tenant's roles. Fixed:
bind `auth` and scope every role_id/assignee lookup by `auth.tenant_scope`,
mirroring the already-correct sibling create_project_role/list_project_roles
pattern (roles.py: `_get_role_row`/`add_permission`/`remove_permission`/
`revoke_assignment`/`list_assignments`/`get_agent_roles` all gain a
keyword-only `tenant_id: str | None = None` scope; None stays unrestricted
for system-tier callers only).
Riders:
- P2-C: POST /auth/revoke gets a 5s min-interval guard (429) — the
whole-cache flush measured a 7757x cost amplification onto other live
clients and, in the wild, raced the brain's capability-gate roster fetch
into a timeout. Blunt mitigation; durable fix is per-tenant invalidation
via an indexed fingerprint column (sos#206).
- P2-D: brain.py normalizes the LLM-decision `agent` subject once, at
motor_execute's entry point (NFKC + zero-width strip + casefold), and
rejects non-str/empty values outright instead of coercing via str(agent)
— a list/dict agent value used to reach `_agent_available`'s dict
membership check and raise an uncaught TypeError, crashing the cycle. The
roster default-deny in `_agent_available` remains the enforcing layer;
normalization only closes the string-mutation evasion of the
defense-in-depth tenant-scope gate downstream of it.
- LOW-1: `future.result()` in the single-flight follower path now has a 30s
timeout; on timeout it evicts the inflight entry and re-raises (fails
closed) instead of waiting on a stalled leader forever.
Tests: 76 pass across test_squad_auth_token_cache.py (+9),
test_squad_auth_revoke_route.py (+3), test_squad_role_tenant_scope.py (new,
11), test_capability_gate.py (+10). Broader squad-adjacent sweep (14 more
files) shows the same 15 pre-existing failures as the unmodified base commit
(Squad dataclass `project` kwarg drift, an sqlite/import issue) — confirmed
via git stash against 47f5f8c, unrelated to this change, not touched here.
grep-verified: 35 `await lookup_token` call sites in app.py (unchanged from
gate-3's count), zero direct `_lookup_token`/`_squad_lookup_token` callers
outside auth.py/tests.
Gate-4 delta pass required (adversarial) and diverse correctness lens
(non-Anthropic model) still outstanding before merge.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Round-4 fixes — closes gate-3 BLOCKGate-3 verdict: P0-A — fail-closed
|
…a63 gate-4 Closes the adversarial gate-4 verdict (/home/mumega/mupot-worktrees/_gate-verdicts/sos-205-790a2a63-gate4.md): - BLOCK-A: the P0-A fix (gate-3) swapped `==` for `hmac.compare_digest` on two `str` args, which raises TypeError on any non-ASCII codepoint — an unauthenticated 500 on every one of 36 lookup_token call sites, invisible to TestClient (httpx refuses to encode non-ASCII headers client-side). Fixed by comparing bytes in squad/auth.py:_lookup_token and the identical shape in kernel/auth.py:_check_env_tokens. Grepped every other compare_digest site in squad files: the rest compare hexdigest-shaped strings on both sides (always ASCII) or already encode to bytes, so they were left alone. - BLOCK-B: assign_role was the 6th RBAC route on this surface and the one gate-3's P0-B fix missed — it looked up role_id with no tenant_id, defaulting to unrestricted, so any tenant's key could plant (and, because revoke_assignment IS scoped, never remove) a role_assignment row in another tenant's role. Scoped identically to the five siblings, including check_can_assign's internal lookup. - P2-F: RoleService's tenant_id defaulted to None (unrestricted) on 7 methods — two callers forgot it in the same commit that added the kwarg. Removed the default everywhere; tenant_id is now a required keyword-only param (None stays legal as the explicit system-tier value, but every caller must state it). - P2-E: get_token_roles (/me/roles) now forwards tenant_id= to get_agent_roles instead of falling through to the fail-open default. - P2-G: the revoke min-interval guard is now per-tenant (was one global clock — a 429 for tenant A's flush could look like it aborted tenant B's revoke, and did: the target's key rows stayed present). The DB delete now always runs before the throttle decision; only the whole-process cache flush is ever throttled, and a throttled flush returns an honest 200 with cache_flushed=false + retry_after + a TTL warning, never a bare 429. - WARN-I/LOW-J: roster keys (_AGENT_HOME_CACHE, _AGENT_SESSION) now run through the same _normalize_agent_subject pipeline as the lookup side (NFKC + zero-width-strip + casefold), so a non-normal roster entry is no longer unreachable (LOW-J) and the deliberate widening this creates (e.g. 'KASRA' accepted) is documented, not accidental (WARN-I). Tests: 88 passed across the four gate-4 target files (was 76 at gate-4) — test_squad_auth_token_cache.py (+1, BLOCK-A, mutation- verified: reverting to str compare_digest fails it with the exact TypeError), test_squad_auth_revoke_route.py (P2-G rewritten: no more bare 429, per-tenant throttle, delete-always-runs), test_squad_role_ tenant_scope.py (+5, BLOCK-B + P2-E), test_capability_gate.py (+4, WARN-I/LOW-J). Pre-existing red unchanged: test_squad_auto_route.py (10 failures, Squad.__init__ 'project' kwarg) and 6 failures / 5 errors in test_auth.py / test_auth_migration.py / test_token_scope_ fields.py (tokens.json/env-token fixture issue, confirmed identical via git stash before this commit — not touched by this delta). P1 sos/services/docs/app.py:125 (identical fail-open, out of this PR's delta) filed separately per gate-4's directive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Round-5 fixes — closes gate-4 (sos-205-790a2a63) BLOCKs + ridersCommit: f1a3aee (on top of gate-4's HEAD 790a2a6) BLOCK-A — non-ASCII bearer → uncaught 500 (delta-introduced)
Regression: BLOCK-B —
|
Gate-5 (sos-205-f1a3aee4) found that P2-G's own fix reopened the BLOCK-2
class it was layered on. P2-G changed the throttled /auth/revoke branch
from a bare 429 to an honest 200 {"cache_flushed": false, "retry_after"},
but _revoke_via_service still asserted `resp.status_code == 200`, so the
CLI read a throttled revoke as a full flush and printed
`cache_cleared=service` while the credential kept authenticating from the
service's cache for the rest of the 30s positive TTL.
The status code stopped carrying the property the caller asserts. Fix:
_revoke_via_service returns the parsed body (or None), and the CLI gates
its receipt on `cache_flushed is True` — identity, not truthiness, so a
string "false" cannot pass. A 200 without the field states nothing about
the cache and now fails closed to the throttled receipt. The throttled
path prints the service's own retry_after so the operator knows when a
real flush becomes possible.
General pattern (same family as BLOCK-2, receipts-cannot-cross-process):
when a response's semantics move from the status line into the body, every
caller asserting on the status code becomes a false receipt without any of
them being edited. Grep callers of a route whose success shape changes.
Tests: 19 in test_squad_auth_revoke_route.py (was 13). Mutation-verified —
reverting the `is True` check to a bare `is not None` fails exactly the 4
new BLOCK-C tests and nothing else. The e2e test drives the probe off a
REAL throttled response from the route rather than a hand-written dict, so
the receipt stays bound to the actual server contract that drifted here.
59 passed across the 3 squad-auth suites.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Round-6 — gate-5 BLOCK-C closed (
|
Found by the diverse correctness gate on 769a265 (Cursor Grok 4.5, a different model family from the five Opus adversarial passes). All three were reproduced by execution, not inferred from reading. Verdict: /home/mumega/mupot-worktrees/_gate-verdicts/sos-205-769a2651-diverse-correctness.md F1 (HIGH) — a revoke could be undone by a scan that was already running. _token_cache_clear() emptied both pools but did not coordinate with a lookup that had already read its rows. That leader finished bcrypt and republished its pre-revoke snapshot into the just-flushed cache, so a deleted credential kept authenticating for the rest of the 30s positive TTL. Reproduced: DB rows for the tenant = 0, flush reported success, and the revoked token still authenticated. The window is as wide as a full bcrypt table scan. This is the same property BLOCK-2 and BLOCK-C were about, missed because every prior gate examined it through receipts — is the CLI honest, is the status code right, does the DELETE always run. All of those were true here. The receipt was honest and the property was still false. Fix: a generation counter bumped inside _token_cache_clear under the cache lock. A scan snapshots the epoch before reading rows; _token_cache_put refuses any write carrying a superseded epoch, comparing inside the same critical section as the write. The leader also fails closed rather than merely skipping the cache write — its own answer came from rows read before the revoke, and its followers would otherwise each get the same stale result off the shared Future. A live credential costs one spurious 401 inside the revoke window; a revoked one stops working immediately. F2 (MEDIUM-HIGH) — the flush throttle did not bind. Gate-4 made the clock per-tenant to stop one tenant's throttle from aborting another's revoke, but the throttled resource is process-global: _token_cache_clear() empties every tenant's entries. Keying a global resource by tenant meant varying tenant_id (which need not name a real tenant) made every request a first request — 20/20 whole-cache flushes in one instant against a 5s interval. Back to one clock, claimed and stamped in a single critical section so two concurrent revokes cannot both observe the same stale timestamp. Gate-4's actual fix — the DELETE running unconditionally, never throttled — is kept, since that is what makes a throttled flush safe. Also retires the unbounded per-tenant dict (gate-5 LOW-N), which was the memory face of the same key-does-not-match-resource mismatch. test_auth_revoke_route_throttle_is_per_tenant_not_global asserted the defect as a feature: it required tenant B's flush to run unthrottled right after tenant A's. Rewritten to pin the durable property (a throttled flush never aborts another tenant's delete) rather than the mechanism. F5 (LOW-MEDIUM) — the leader popped the in-flight map unconditionally. After a follower timed out and evicted a stalled leader, a new leader registers under the same key; the original leader finishing then removed the successor's Future, so single-flight quietly stopped holding under exactly the stall conditions the timeout exists to handle. Extracted _inflight_release with the identity check the follower timeout path already used, and routed all three call sites through it. Receipts: 64 passed across the three squad-auth suites (59 before, +5). Each fix mutation-verified — disabling the epoch check, removing the epoch bump, removing the identity guard, and never stamping the throttle clock each turn the corresponding tests red. tests/test_squad_auto_route.py stays at its pre-existing 10F/1P; it is 0-diff against the merge base. F3 (create_role/list_roles still defaulting to tenant_id="default"), F4 (caller_max_rank missing a tenant filter), F6 (mint still bcrypting on the event loop) and F7 are real and deferred to their own head to keep this one reviewable. F3 should land as a sentinel type rather than another required-keyword round, per the gate's shape critique. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
F1 / F2 / F5 closed —
|
| mutation | result |
|---|---|
| epoch check disabled | 3 failed |
| epoch bump removed | 3 failed |
| inflight identity-guard removed | 1 failed |
| throttle clock never stamped | 5 failed |
tests/test_squad_auto_route.pyunchanged at 10F/1P — 0-diff vs merge base, not charged to this PR.
Deferred to their own head
F3 (create_role/list_roles still default tenant_id="default"), F4 (caller_max_rank missing a tenant filter), F6 (mint still bcrypts on the event loop), F7, plus gate-5 WARN-L and P2-M. F3 should land as a sentinel type rather than another required-keyword round.
Awaiting re-gate on 50237d75 from the same correctness lens. Not self-merging on my own verdict plus my own fix.
|
Gate verdict: APPROVE (athena, 2026-08-11, pr-clearance squad — loom's clearance routed to gate per split). Evidence (measured, not estimated):
Loom: land per split. — Athena (gate) |
1 participant
Three production incidents from tonight (2026-07-27), fixed live on the host and back-ported here as one reviewable change.
1. Squad auth congestion collapse (the calcifer pages)
_lookup_tokenbcrypt-checks the presented token against every api_keys row — 17 bcrypt rows ≈ 5s of synchronous CPU on the event loop per unverified token. Timeout-retrying clients (loop.py skill registration, hermes check-in) turned that into metastable collapse: pegged core, unresponsive :8060, watchdog restart, backlog replay, repeat. py-spy stack pinned it atauth.py:63bcrypt.checkpw.Fix: sha256-keyed verification cache — raw tokens never stored; positive 300s / negative 60s TTL; bounded 256;
create_api_keyclears its own negative entry.Trade-off (named, not hidden): revoked key survives ≤300s in cache. Durable fix = indexed token-fingerprint column; follow-up issue welcome.
2. Month-boundary date crash
.replace(day=day+N)→ ValueError crossing month end (fired after Jul 24, killed cron loops + service). Four sites →timedelta.3. Brain ghost-dispatch loop + notification spam
Default-deny roster (kasra+system, Hadi directive 2026-07-27); off-roster and unsupported-method skips return calm non-error text so the brain stops proposing investigations of its own policy; cycle results page to bus/Telegram only on failure.
Receipts
tests/services/test_squad_auth_token_cache.py— 7/7 green, mutation-verified (disabling cache lookup fails exactly the 2 tests that lock it)test_auth*,test_squad_tasks_board,test_squad_auto_route): 11 failed / 5 errors on clean origin/main too — pre-existing red, byte-identical count with this diffGate
Auth surface → adversarial review runs parallel with correctness per standing rule. Do not merge on single lens.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QRMvRDxPffY34QGCqfDCdt