From e6e8ec0e93d281ed2c2faa31deee2949e15b1ccb Mon Sep 17 00:00:00 2001 From: Babissimo Date: Thu, 10 Sep 2026 16:52:51 +0100 Subject: [PATCH 1/3] Stop gating a production rollback on the storage scan The production smoke test polled GET /api/admin/storage on api.retina.fm for two minutes and failed the job if it never answered 200 or 202. A failed production smoke test SSHes to retina-prod and runs deploy/rollback.sh, so this probe could revert a release on its own. It has to go before require_admin starts enforcing identity, because it would then answer 401 to CI's unauthenticated curl and revert every deploy. A service token cannot rescue it either: api.retina.fm is the fleet's ingest hostname and must never carry an Access application, so there is nothing for a token to authenticate against, and the 401 comes from this codebase rather than the edge. Deleting rather than relocating it, because what it measures does not belong in that gate. It reports whether a background du scan has finished, which is a capacity signal on a timer, not evidence that the build now running is healthy; everything else in the block probes public routes and asserts on the request path a user actually takes. If the storage figure is worth watching it belongs in alerting, where noticing it late costs nothing. Nothing pinned this probe. test_towers_vhost_coverage.py constrains the staging smoke list, and test_smoke_tally_shared.py constrains only the shared counters, which the block still sources and still uses for the elevation and aircraft checks. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 806749cb..d83392e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1277,16 +1277,6 @@ jobs: fi done - # Wait for storage scan (background du task, 202 = still scanning) - printf " %-40s " "GET /api/admin/storage" - for i in $(seq 1 24); do - SCODE=$(curl -sk -o /dev/null -w "%{http_code}" "${BASE_URL}/api/admin/storage") - if [ "$SCODE" = "200" ]; then echo "OK (200)"; PASS=$((PASS+1)); break; fi - if [ "$SCODE" = "202" ] && [ "$i" = "24" ]; then echo "WARN (202 still scanning after 120s)"; WARN=$((WARN+1)); break; fi - if [ "$i" = "24" ]; then echo "FAIL ($SCODE != 200 after 120s)"; FAIL=$((FAIL+1)); break; fi - sleep 5 - done - # Check aircraft data is flowing AIRCRAFT=$(curl -sk "${BASE_URL}/api/test/dashboard" \ | python3 -c "import sys,json; print(json.load(sys.stdin)['pipeline']['aircraft_on_map'])" 2>/dev/null) || AIRCRAFT=0 From 9b72860d247091e642a36c2420c679b5727992c8 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Thu, 10 Sep 2026 17:15:15 +0100 Subject: [PATCH 2/3] Let the smoke suites present an Access service token The tower contract is asserted against admin.retina.fm and staging-admin, and once those hostnames carry an Access application a browserless request meets a 302 to the login page. curl runs without -L, so assert_tower_contract reports `got HTTP 302` and the suite reads a missing credential as a routing fault. Both suites already share this file, so the headers go here rather than at each call site: the production block in ci.yml and staging-smoke-test.sh pick them up by sourcing it, and the two cannot drift on what they present. Inert until both variables are set, which is what makes it safe to land before the Access applications exist: an ungated hostname sees exactly the request it sees today, and so does a developer running the script by hand. Half a credential contributes nothing either, so a secret configured on its own cannot produce a request that is refused for a reason nobody can see. The array is expanded with the `+` guard at every call site because expanding an empty one is an unbound-variable error under `set -u` before bash 4.4, and staging-smoke-test.sh runs with `set -euo pipefail`. The token buys passage through the edge and nothing more. A service-token assertion carries common_name rather than email, so the origin's verifier finds no identity in it and it cannot reach an admin route; /api/towers, which is what these probes actually assert, needs none. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 13 +++++++++++++ deploy/tower-contract.sh | 31 ++++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d83392e4..784bbf73 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -760,6 +760,11 @@ jobs: done - name: Run smoke tests against staging timeout-minutes: 15 + # Same service token as production: one token, admitted by a Service Auth + # policy on each admin application. Inert while the secrets are unset. + env: + CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} + CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} run: bash deploy/staging-smoke-test.sh deploy-production: @@ -1194,6 +1199,14 @@ jobs: - name: Run production smoke tests id: smoke timeout-minutes: 15 + # Passage through Cloudflare Access for the admin vhost. An unset secret + # renders empty, and tower-contract.sh sends no headers at all in that + # case, so this is inert until the Access application and the service + # token both exist. Failing this step rolls production back, so it must + # not become the reason a deploy reverts. + env: + CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} + CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} run: | CURL="curl -s --connect-timeout 10 --max-time 60" # PASS/FAIL/WARN and smoke_summary, shared with deploy/staging-smoke-test.sh. diff --git a/deploy/tower-contract.sh b/deploy/tower-contract.sh index c204471f..9ad7e963 100644 --- a/deploy/tower-contract.sh +++ b/deploy/tower-contract.sh @@ -19,6 +19,28 @@ TOWER_CONTRACT_ECHO='"user_frequencies_mhz":[1234.5]' # and measures ~3s; this is the outage threshold, not the expected time. TOWER_CONTRACT_MAX_TIME=45 +# Cloudflare Access service-token credentials, for the admin vhosts that sit +# behind an Access application. Without them the edge answers a browserless +# request with a 302 to its login page, which arrives here as `got HTTP 302` +# and reads like a routing fault rather than a missing credential. +# +# Empty unless the environment supplies both, so an ungated hostname and a +# developer running this by hand behave exactly as before. The token only buys +# passage through the edge: it carries no email claim, so the origin treats it +# as nobody and it cannot reach an admin route. +TOWER_CONTRACT_CF_HEADERS=() +if [ -n "${CF_ACCESS_CLIENT_ID:-}" ] && [ -n "${CF_ACCESS_CLIENT_SECRET:-}" ]; then + TOWER_CONTRACT_CF_HEADERS=( + -H "CF-Access-Client-Id: ${CF_ACCESS_CLIENT_ID}" + -H "CF-Access-Client-Secret: ${CF_ACCESS_CLIENT_SECRET}" + ) +fi + +# Every call site expands it as ${TOWER_CONTRACT_CF_HEADERS[@]+"${...[@]}"}: +# expanding an empty array is an unbound-variable error under `set -u` before +# bash 4.4, and staging-smoke-test.sh sources this with `set -euo pipefail`. +# The `+` guard expands to nothing at all when the array is empty. + # assert_tower_contract # Endpoint, not host: the api vhost publishes this as /towers, everyone else as # /api/towers. Prints why it failed on stdout; returns non-zero. @@ -27,7 +49,8 @@ assert_tower_contract() { # Two attempts: the endpoint depends on third-party APIs, and a blip there # must not read as a routing fault and block a release. for attempt in 1 2; do - resp=$(curl -s --connect-timeout 10 --max-time "$TOWER_CONTRACT_MAX_TIME" \ + resp=$(curl -s ${TOWER_CONTRACT_CF_HEADERS[@]+"${TOWER_CONTRACT_CF_HEADERS[@]}"} \ + --connect-timeout 10 --max-time "$TOWER_CONTRACT_MAX_TIME" \ -w '\n%{http_code}' "${endpoint}?${TOWER_CONTRACT_QUERY}" 2>/dev/null) || { [ "$attempt" = 1 ] && { sleep 5; continue; } echo "unreachable after 2 attempts: ${endpoint}" @@ -87,7 +110,8 @@ _assert_json_keys() { # Two attempts, same reasoning as assert_tower_contract: elevation fans out # to a third party, and a blip there must not read as a routing fault. for attempt in 1 2; do - resp=$(curl -s --connect-timeout 10 --max-time "$TOWER_CONTRACT_MAX_TIME" \ + resp=$(curl -s ${TOWER_CONTRACT_CF_HEADERS[@]+"${TOWER_CONTRACT_CF_HEADERS[@]}"} \ + --connect-timeout 10 --max-time "$TOWER_CONTRACT_MAX_TIME" \ -w '\n%{http_code}' "$url" 2>/dev/null) || { [ "$attempt" = 1 ] && { sleep 5; continue; } echo "${label}: unreachable after 2 attempts: ${url}" @@ -136,7 +160,8 @@ assert_elevation_contract() { local url="${1}?${TOWER_CONTRACT_ELEVATION_QUERY}" body code resp attempt # Two attempts, as the siblings above: a blip must not read as a routing fault. for attempt in 1 2; do - resp=$(curl -s --connect-timeout 10 --max-time "$TOWER_CONTRACT_MAX_TIME" \ + resp=$(curl -s ${TOWER_CONTRACT_CF_HEADERS[@]+"${TOWER_CONTRACT_CF_HEADERS[@]}"} \ + --connect-timeout 10 --max-time "$TOWER_CONTRACT_MAX_TIME" \ -w '\n%{http_code}' "$url" 2>/dev/null) || { [ "$attempt" = 1 ] && { sleep 5; continue; } echo "elevation: unreachable after 2 attempts: ${url}" From a17f18100e163f65f67bb17fd00f69196284bfc0 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Thu, 10 Sep 2026 17:22:49 +0100 Subject: [PATCH 3/3] Let Playwright through Access on the admin surface The admin vhost will sit behind an Access application, and Playwright follows redirects, so a browser test against it would fetch Cloudflare's login page and then fail somewhere unhelpful: parsing that HTML as the app, rather than saying it was never admitted. The headers go on `use` so page navigation carries them, and are exported as well because request.newContext() does not inherit `use` and a spec building its own context against a gated host has to pass them itself. Empty unless both variables are set, so an ungated hostname and anyone running the suite locally see exactly what they see today. Both or neither: half a credential is refused at the edge just like none, and sending one would only make the refusal harder to read. Note this buys passage and not identity. A service-token assertion carries common_name rather than email, so the SPA's own /api/auth/me still comes back unauthenticated behind it, which is why this alone does not keep the admin surface-selection test alive once the bypass is closed. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 5 +++++ frontend/playwright.config.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 784bbf73..d3f8243c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -340,6 +340,11 @@ jobs: env: CI: "true" RADAR_API_KEY: ${{ secrets.STAGING_RADAR_API_KEY }} + # Passage through Access for the admin surface. Playwright follows the + # edge's 302, so without these the admin tests would parse Cloudflare's + # login page as the app. Empty secrets render empty and send nothing. + CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} + CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} e2e-prod: name: Playwright E2E (production) diff --git a/frontend/playwright.config.ts b/frontend/playwright.config.ts index 42a8d19f..f917e067 100644 --- a/frontend/playwright.config.ts +++ b/frontend/playwright.config.ts @@ -65,6 +65,31 @@ const HOSTS = { export const env = ENV; export const hosts = HOSTS[ENV]; +/** + * Cloudflare Access service-token headers, empty unless CI supplies both. + * + * The admin vhost sits behind an Access application, which answers a request + * carrying no session with a 302 to its login page. Playwright follows that + * redirect and lands on Cloudflare's HTML, so without these a test against the + * admin surface fails somewhere unhelpful — parsing a login page as the app — + * rather than saying it was never let in. + * + * Empty on an ungated hostname and for anyone running the suite locally, so + * this changes nothing until the Access applications and the token both exist. + * Both halves or neither: half a credential is refused at the edge exactly like + * none, and sending one would only make the failure harder to read. + * + * Exported because `request.newContext()` does not inherit `use`, so a spec + * building its own context against a gated host has to pass these itself. + */ +export const accessHeaders: Record = + process.env.CF_ACCESS_CLIENT_ID && process.env.CF_ACCESS_CLIENT_SECRET + ? { + "CF-Access-Client-Id": process.env.CF_ACCESS_CLIENT_ID, + "CF-Access-Client-Secret": process.env.CF_ACCESS_CLIENT_SECRET, + } + : {}; + export default defineConfig({ testDir: "./e2e", timeout: 30_000, @@ -74,6 +99,7 @@ export default defineConfig({ reporter: process.env.CI ? "github" : "list", use: { baseURL: hosts.frontend, + extraHTTPHeaders: accessHeaders, trace: "on-first-retry", screenshot: "only-on-failure", headless: true,