[calvin-archastro]

Review on ArchCode

Merge order: rebased onto #188 (aster-srv hardening) and #183/#184/#187; ships in 0.24.0 with #182 (person-token login), already merged. #185 rebases onto this after it merges.

Problem and author intent

Calvin and the overseer need to read dashboard data (flaky, broken, clusters, quarantine, cache) with the token aster cache login already gives them.

• /ui is IAP-gated HTML only, so a script or agent cannot read it.
• A person token (feat: laptop cache login through aster-srv (aster cache login, person tokens) #182) is accepted on /v1/cache/summary|entries|lookup and 403s on every /v1/test-results/* route.
• Failure clusters exist only as HTML.

Intent: every dashboard dataset is readable over /v1 JSON by a person token, no write is, and a CLI prints it. Review of #182 also found auth gaps, fixed here (checklist below):

POST /v1/cache/commit (64 MiB body)  ->  body buffered + JSON-parsed  ->  then auth   (before)
POST /v1/cache/commit                ->  gate (headers only) -> 401      body never read (after)

What changed

Dataset	/ui page	Route	CLI	Person	CI token
Flaky tests	/ui	GET /v1/test-results/tests?state=flaky	aster srv flaky	200 (was 403)	own repo (unchanged)
Broken tests	/ui	GET /v1/test-results/tests?state=broken	aster srv broken	200 (was 403)	unchanged
Failure clusters	/ui/clusters	new GET /v1/test-results/clusters (same ui::data::clusters query)	aster srv clusters	200	own repo
Quarantine	/ui/quarantine	GET /v1/test-results/quarantine	aster srv quarantine	200 (was 403)	unchanged
One test	/ui/tests/<id>	GET /v1/test-results/tests/<id>	aster srv test <id>	200 (was 403)	unchanged
Run failures	n/a	GET /v1/test-results/runs/<id>/failures	aster srv get	200 (was 403)	unchanged
Durations	n/a	GET /v1/test-results/durations	aster srv get	200 (was 403)	unchanged
Cache summary / entries (target=)	/ui/cache	GET /v1/cache/summary, /entries	aster srv cache	200 (existing, verified)	unchanged
Cache lookup	n/a	POST /v1/cache/lookup	n/a	200, no hit counting	counts hits
Any write	n/a	POST /v1/test-results/runs, /v1/cache/upload, /commit	n/a	403	writer rules

• Person reads name the repository with ?repo=; it must be on ASTER_SRV_REPOSITORIES and the email domain rule applies. The handlers' own-repo rule then runs on an identity for that repository, so there is no second copy of the SQL.
• Scope: tokens are minted with read. cache:read is still accepted as the same scope (person.rs, LEGACY_SCOPE): feat(aster-srv): harden the non-IAP /v1 channel #188 rejects feat: laptop cache login through aster-srv (aster cache login, person tokens) #182-shape tokens, but tokens it issued carry cache:read for up to 12 hours, and refusing them would force a re-login on deploy. Drop LEGACY_SCOPE after 12 hours in production. feat(aster-srv): harden the non-IAP /v1 channel #188's strict claims (sub, nbf, iat, jti, email, HS256, 30 s leeway, key entropy) are unchanged.
• aster srv get|cache|flaky|broken|quarantine|clusters|test (src/test_results/read.rs) uses CI credentials when the env has them, else the stored sign-in. Repo defaults like the cache client (GITHUB_REPOSITORY, else origin). Non-zero exit with the reason on not-signed-in, 401, 403, 404.
• Single route table routes::ROUTES (method, pattern, auth class Public|Iap|SignedUrl|Ci|CiOrPerson|CiWriter, handler). routes::gate is the only auth step: the main listener and feat(aster-srv): harden the non-IAP /v1 channel #188's /v1-only listener (ASTER_SRV_API_ADDR) both run the same route(), so both mount it.

Rebase onto #188

• Replaced feat(aster-srv): harden the non-IAP /v1 channel #188's hand-written is_v1_authed, preauth and audit::route_label with the table. routes::gate now does what preauth did (audit and count every 401/403, charge the subject's rate limit, 429) and what authenticate did (audit person_token_not_allowed). routes::label derives the metric/audit route label from ROUTES; label strings are unchanged.
• Body caps come from the route's handler (http.rs::body_limit), used both for the declared-length check and the read: forms 8 KiB, lookup/upload 1 MiB, commit and results upload max_body_bytes, everything else none. feat(aster-srv): harden the non-IAP /v1 channel #188's check on the declared length still runs before auth, so a declared 100 MB body is 413 (no body read) rather than 401; neither reads the body. A known path with the wrong method is 405 before the size check.
• /v1-only listener: still 404 for non-/v1 paths and for any route whose class is Iap; /v1/healthz is answered there without auth as before.
• Behavior changes from the table's deny-by-default: an unlisted /v1/ path (including /v1/healthz on the main port, and on the API port) is 401 without a token and 404 only with one; it was 404. Tests in hardening.rs updated for this.
• Config: both PRs' checks apply (writers need workflows; ASTER_SRV_ALLOW_HTTP_PUBLIC_URL; key entropy). Test fixtures that used the bare backend-service prefix now use /projects/123456/global/backendServices/4242, since feat(aster-srv): person tokens read every dashboard dataset; aster srv; route-table auth gate #189 refuses the bare prefix.

Scope, risk, user impact

• Scope: backend (aster-srv) plus CLI. No frontend.
• Risk: medium to high. It touches auth on every /v1 route and tightens write rules; no schema change. Read access is widened to people, and the deny-by-default gate narrows everything else.
• Behavior changes to know about:
  • Cache writes now need event_name == push; tokens without it are 403.
  • ASTER_SRV_CACHE_WRITERS without ASTER_SRV_CACHE_WRITER_WORKFLOWS fails startup (opt-out ASTER_SRV_ALLOW_ANY_WRITER_WORKFLOW=1).
  • ASTER_SRV_IAP_AUDIENCE must be the exact backend-service audience (a trailing-/ prefix fails startup).
  • A POST to a read route is now 401 without a token (was 405) and 405 with one.
• User impact: Calvin and the overseer can run aster srv ... after one aster cache login. Nothing changes for CI uploads that already come from push on main through ci.yml.

Security checklist

Paths under crates/aster-srv/. Tests are in tests/routes.rs unless noted.

Item	Where	Test
4. Router-level deny-by-default gate; route table is the source of truth	src/routes.rs (ROUTES :71, lookup :197, label :217, gate :251), src/http.rs:404-430 (route)	every_v1_route_answers_401_to_every_kind_of_bad_credential walks ROUTES (+ an unlisted path + an unsupported method) with: none, garbage, empty, wrong scheme, expired, wrong key, nbf in the future, alg=none, RS/HS confusion (public and private PEM as HMAC key), wrong aud, wrong iss, person token with wrong aud/iss, valid IAP assertion with no token, forged x-goog-* headers
4. Exception: code exchange	src/routes.rs (Public)	the_code_exchange_is_the_only_v1_route_without_a_token_and_never_says_200_without_a_code asserts 400 invalid_grant, never 200; routes::tests::every_v1_route_but_the_code_exchange_needs_a_token
6. Person scope is an explicit read allowlist	src/routes.rs (CiOrPerson)	a_person_reaches_only_the_read_allowlist_and_every_other_route_is_403 pins the allowlist and walks ROUTES: reads allowed for a served repo, 403 for an unserved repo, 403 on every Ci/CiWriter route; tests/cli_auth.rs::a_person_reads_every_dashboard_dataset_and_writes_nothing
7. CI writes need OIDC iss/aud/repo allowlist, main, ci.yml, push	src/auth.rs (OIDC verify), src/cache/policy.rs:82 (pushed), ci_reader	cache_writes_need_a_push_to_main_from_the_ci_workflow_in_an_allowed_repository: PR, PR claiming main, pull_request_target, feature branch, merge queue, workflow_dispatch, schedule, workflow_run, no event, other workflow, no workflow, other/no repository_owner_id, repo off the allowlist: all 403 on upload and commit; push to main from ci.yml is 200
7. Test-results upload	src/cache/policy.rs:92	test_results_uploads_need_the_ci_workflow_and_refuse_pull_request_target
7. JWKS cached, no refetch storm	src/auth.rs (7dcd475), src/iap.rs:180-200	unit tests concurrent_requests_for_an_unseen_key_share_one_slow_fetch, concurrent_assertions_for_an_unseen_key_share_one_slow_fetch
Redaction	src/auth.rs:119 (into_reply sends a generic message, logs the reason)	same walk asserts no body echoes the token and every body is generic
F1. Auth before body; per-route caps	src/http.rs:404-430 (gate), :34 (SMALL_BODY_BYTES), :311 (body_limit: 1 MiB lookup/upload, max_body_bytes only for commit)	a_request_is_authenticated_before_its_body_is_read_and_capped_after: declared 100 MB body never sent gets 401 or 413 within 5 s on every POST route; 2 MiB lookup/upload 413
F3. Exact IAP audience	src/iap.rs:234, src/config.rs	iap::tests::the_audience_must_be_the_one_configured_backend_service, refuses_other_audiences... (another backend of the project), config::tests::reads_the_dashboard_settings (prefix refused at startup)
F7. Writer policy	src/cache/policy.rs, src/config.rs (OwnerIds, writers need workflows)	rows above; config::tests::writers_need_workflows_and_owner_ids_pin_repositories
F9. Stale keys evicted	src/auth.rs, src/iap.rs	both slow-fetch tests assert a withdrawn key is dropped
F12. Person lookups do not mutate	src/cache/routes.rs:556	a_persons_lookup_does_not_count_or_keep_an_entry_alive_but_ci_does
F13. nbf, generic errors	src/auth.rs:284, src/iap.rs:162	nbf and generic-body assertions in the walk
Blob stand-in cannot be mounted in production	src/http.rs:536, src/cache/routes.rs:63, ASTER_SRV_DEV_BLOB_ROUTE	the_signed_blob_stand_in_exists_only_when_the_dev_flag_is_set

Choices to confirm:

• Writer events: push only (not schedule or workflow_dispatch).
• Test-results uploads still accept pull_request (and other non-pull_request_target events). A PR run's retry-then-pass is the flake signal, so refusing PR uploads would stop flake detection. Tell me if you want uploads restricted like cache writes.
• Not done: workflow_ref/job_workflow_ref are not matched when ASTER_SRV_CACHE_WRITER_WORKFLOWS is unset for results (unchanged); repository_owner_id pinning is opt-in via ASTER_SRV_REPOSITORY_OWNER_IDS (firstlanding must set it).

Also fixed

• iap.rs JWKS refetch race: same fix as 7dcd475 (lock held across the fetch, re-check after acquiring) plus the regression test above.

Testing

Canonical end-to-end proof: tests/cache_login_e2e.rs::a_signed_in_laptop_reads_every_dashboard_dataset_with_aster_srv.

• Real aster binary, real aster-srv, real Postgres.
• CI uploads 3 runs through aster results upload (OIDC token from a stand-in endpoint); the classifier pass runs.
• Before sign-in, aster srv flaky exits non-zero with not signed in; run aster cache login.
• The scripted browser signs in (IAP assertion injected), then aster srv reads flaky, broken, quarantine, clusters, a test, a run's failures and durations, each as JSON, with the repo derived from the checkout.
• --repo evil/other is 403; POST /v1/test-results/runs, /v1/cache/upload, /v1/cache/commit are 403.
• Sibling a_laptop_signs_in_through_aster_srv_and_restores_what_ci_wrote still passes.

Server tests: crates/aster-srv/tests/{routes,cli_auth,cache,query}.rs, src/** unit tests. CLI unit tests: src/test_results/read.rs.

Automatic coverage: CI job aster-srv runs cargo test --locked --test cache_remote_e2e --test cache_login_e2e -- --test-threads=1 and cargo test --locked -p aster-srv against a Postgres service, so all of the above run on PRs (confirm each prints ... ok, not skipped).

Integration matrix

Entry	Person	CI own repo	No/bad token	Other repo
7 test-results reads + clusters	200	200	401	person 403 if not served; CI 403
cache summary/entries/lookup	200	200	401	same
writes (runs, upload, commit)	403	writer rules	401	n/a
unknown /v1 path, wrong method	401/405	401/405	401	n/a

Covered: same instance, one caller. Not covered: concurrent callers across replicas for reads, dependency down mid-flight, old/new client versions beyond the legacy cache:read token.

Fidelity: real Postgres, real HTTP server, real aster binary, real JWT verification. Faked: IAP (a signed assertion injected, signed with a test P-256 key), the browser (scripted HTTP client), GCS (in-memory store behind the dev flag), GitHub OIDC (test RSA key and a stand-in token endpoint). The fakes share the verifier code with production.

Not proven here: real IAP and a real browser; the url-map /v1 split (firstlanding PR B); the dedicated listener, audit, rate limits and HSTS are #188's and now share this gate (hardening.rs and routes.rs both pass).

Follow-ups

• Quarantine change log (/ui/quarantine "Recent changes") has no JSON route yet.
• Large PR (about 1,200 lines, mostly tests and the route-table refactor); the iap.rs fix, CLI and route table could be split if you prefer.

🤖 Generated with Claude Code

[calvin-archastro]

ArchDev is watching this pull request. State: settled (cycle 2 of 2 for head 7cb70b4).

when	trigger	decision	result
2026-10-23T17:44:43.946Z	failed ci	enqueue job_275912f65b476a78836e7ac1	published 7cb70b4
2026-10-23T20:32:03.519Z	merge conflict	enqueue job_857990a754660799246e6c71	cancelled pull request converted to draft

Paused by the no-autofix label or a draft; never resolves reviewer threads. Jobs: job_275912f65b476a78836e7ac1, job_857990a754660799246e6c71.