From 742eeaad3ca49ddb7a70f2f5ef19928a01dbec99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 01:25:01 +0200 Subject: [PATCH] fix(ops): refuse a synthetic-journey target that is not ours S1. The journey signs in with a real account and POSTs that account's password to whatever BASE_URL names, and the workflow took BASE_URL as free text. Anyone with repo write, or a leaked token, could dispatch the run at a collector of their own and keep a working credential; secret masking guards the log, never the egress. The host now has to be on a short const allowlist in the script, checked before anything is sent, with exit 2 and a named refusal otherwise. The workflow input is a choice between the two hosted instances, so the form cannot offer a third; the script's own check still stands behind it for a dispatch that goes round the form. --self-test covers the refusal, the prefix lookalike a startsWith check would have waved through included. --- .github/workflows/synthetic-journey.yml | 16 ++++- scripts/synthetic-journey.mjs | 81 ++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/.github/workflows/synthetic-journey.yml b/.github/workflows/synthetic-journey.yml index 6354bdcc4..2d096f9b0 100644 --- a/.github/workflows/synthetic-journey.yml +++ b/.github/workflows/synthetic-journey.yml @@ -21,10 +21,19 @@ name: Synthetic journey on: workflow_dispatch: inputs: + # A closed list, not free text. The dispatcher chooses where the probe + # account's password is POSTed, so the choice is between the hosted + # instances and nothing else. The script re-checks the host against its + # own const allowlist and exits 2 on anything else, so a dispatch that + # goes round this form still cannot reach a stranger's collector. base_url: - description: Instance to walk, e.g. https://review.healthlog.dev + description: Instance to walk required: true - type: string + type: choice + default: https://review.healthlog.dev + options: + - https://review.healthlog.dev + - https://healthlog.bombeck.io expected_version: description: Version the instance must serve, e.g. v1.38.13 required: true @@ -53,7 +62,8 @@ jobs: run: node scripts/synthetic-journey.mjs --self-test # Every input and secret rides an env var — never interpolated into - # the shell command. + # the shell command. The script refuses a `BASE_URL` whose host is off + # its allowlist before it sends the password anywhere. - name: Walk the journey env: BASE_URL: ${{ inputs.base_url }} diff --git a/scripts/synthetic-journey.mjs b/scripts/synthetic-journey.mjs index e59d7fa21..eae3442d4 100644 --- a/scripts/synthetic-journey.mjs +++ b/scripts/synthetic-journey.mjs @@ -26,12 +26,16 @@ * the row's id, value, unit and source. * 5. delete — `DELETE /api/measurements/{id}` so the account stays clean. * + * `BASE_URL` decides where the probe account's password is POSTed, so it is + * checked against a closed in-repo host allowlist and refused with exit 2 + * otherwise. + * * The first failing leg ends the run with a non-zero exit, the response * status and a short body excerpt. Anything token-shaped in that excerpt * is redacted before it reaches the log. * * Usage: - * BASE_URL=https://review.example.test \ + * BASE_URL=https://review.healthlog.dev \ * SYNTHETIC_USERNAME=… SYNTHETIC_PASSWORD=… EXPECTED_VERSION=v1.38.13 \ * node scripts/synthetic-journey.mjs * @@ -60,6 +64,36 @@ const BODY_EXCERPT_LIMIT = 400; /** The reading the journey writes. Distinctive, plausible, and short-lived. */ const PROBE = { type: "WEIGHT", value: 77.7, unit: "kg", source: "MANUAL" }; +/** + * The journey signs in with a real account and POSTs that account's password + * to whatever `BASE_URL` names, so `BASE_URL` alone decides where the + * password goes. Anyone able to name the host walks off with a working + * credential; masking guards the log, not the egress. The allowlist is the + * defence, and it is deliberately a const: it changes by a commit and a + * review, never by a form field. + */ +const ALLOWED_HOSTS = [ + "review.healthlog.dev", + "healthlog.bombeck.io", + "localhost", + "127.0.0.1", +]; + +/** + * `null` when the host may receive the probe credentials, otherwise the line + * the caller prints before exiting 2. + */ +function hostRefusal(baseUrl) { + let host; + try { + host = new URL(baseUrl).hostname; + } catch { + return `BASE_URL is not a URL: ${JSON.stringify(baseUrl)}`; + } + if (ALLOWED_HOSTS.includes(host)) return null; + return `BASE_URL host ${JSON.stringify(host)} is not on the allowlist — the probe password is only ever sent to ${ALLOWED_HOSTS.join(", ")}`; +} + /** * Patterns that must never reach a log line. The first three are this * project's own token prefixes plus the OpenAI-style key shape the @@ -488,6 +522,40 @@ function startMockInstance(breaks) { }); } +/** + * The allowlist is the only thing standing between a dispatcher and the + * probe account's password, so prove it refuses — including the prefix + * lookalike, which a `startsWith` check would have waved through. + */ +function selfTestHostAllowlist() { + const cases = [ + { url: "https://review.healthlog.dev", refused: false }, + { url: "https://healthlog.bombeck.io/", refused: false }, + { url: "http://localhost:3000", refused: false }, + { url: "http://127.0.0.1:3000", refused: false }, + { url: "https://healthlog.bombeck.io.example.test", refused: true }, + { url: "https://collector.example.test", refused: true }, + { url: "not-a-url", refused: true }, + ]; + + let failures = 0; + for (const testCase of cases) { + const refusal = hostRefusal(testCase.url); + const label = `host ${testCase.url}`; + if (Boolean(refusal) === testCase.refused) { + console.log( + `self-test ok ${label.padEnd(44)} → ${refusal ? "refused" : "allowed"}`, + ); + } else { + failures += 1; + console.log( + `self-test FAILED ${label.padEnd(44)} → ${refusal ? "refused" : "allowed"}, expected ${testCase.refused ? "refused" : "allowed"}`, + ); + } + } + return failures; +} + async function selfTest() { const cases = [ { breaks: null, expect: "pass" }, @@ -501,7 +569,7 @@ async function selfTest() { { breaks: "delete", expect: "fail", leg: "delete" }, ]; - let failures = 0; + let failures = selfTestHostAllowlist(); for (const testCase of cases) { const label = testCase.breaks ?? "nothing broken"; const instance = await startMockInstance(testCase.breaks); @@ -537,7 +605,9 @@ async function selfTest() { console.error(`\nself-test: ${failures} case(s) behaved wrongly`); return 1; } - console.log(`\nself-test: ${cases.length} cases, every leg proven breakable`); + console.log( + `\nself-test: ${cases.length} journey cases, every leg proven breakable; host allowlist proven`, + ); return 0; } @@ -563,6 +633,11 @@ function readConfig() { ); process.exit(2); } + const refusal = hostRefusal(config.baseUrl); + if (refusal) { + console.error(refusal); + process.exit(2); + } return config; }