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..e70a5c4db 100644 --- a/scripts/synthetic-journey.mjs +++ b/scripts/synthetic-journey.mjs @@ -24,14 +24,27 @@ * 3. write — `POST /api/measurements` with an `Idempotency-Key`. * 4. read — `GET /api/measurements?type=WEIGHT&limit=1` and assert * the row's id, value, unit and source. - * 5. delete — `DELETE /api/measurements/{id}` so the account stays clean. + * 5. delete — `DELETE /api/measurements/{id}`, then a direct `GET` on + * the same id that must 404, so the account stays clean and + * the leg proves it rather than trusting a 200. + * + * Legs 3 to 5 run inside a `try`/`finally`: from the write on, the run owns + * a row on a real account, and a red read leg must not leave it behind. The + * `finally` sweeps whatever the delete leg did not — including a row whose + * id the write leg never learned, because the response was lost after the + * server had committed. That row is found again by a marker the write puts + * in `notes`, unique per run. + * + * `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 +73,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 @@ -245,17 +288,19 @@ async function signIn(baseUrl, jar, username, password) { return { detail: "session cookie held" }; } -async function writeReading(baseUrl, jar) { +async function writeReading(baseUrl, jar, run) { const measuredAt = new Date().toISOString(); const result = await call(baseUrl, "/api/measurements", { method: "POST", jar, - headers: { "idempotency-key": randomUUID() }, + headers: { "idempotency-key": run.idempotencyKey }, body: { type: PROBE.type, value: PROBE.value, measuredAt, source: PROBE.source, + // The one handle on this row that exists before the response does. + notes: run.marker, }, }); expectStatus(result, 201, "write"); @@ -269,7 +314,7 @@ async function writeReading(baseUrl, jar) { return { id: created.id, measuredAt, detail: `wrote ${created.id}` }; } -async function readBack(baseUrl, jar, written) { +async function readBack(baseUrl, jar, writtenId) { const result = await call( baseUrl, "/api/measurements?type=WEIGHT&limit=1&sortBy=measuredAt&sortDir=desc", @@ -284,22 +329,72 @@ async function readBack(baseUrl, jar, written) { }); } const row = rows[0]; - expectValue(row.id, written.id, "read back: id", result); + expectValue(row.id, writtenId, "read back: id", result); expectValue(row.value, PROBE.value, "read back: value", result); expectValue(row.unit, PROBE.unit, "read back: unit", result); expectValue(row.source, PROBE.source, "read back: source", result); return { detail: `${row.value} ${row.unit} (${row.source})` }; } -async function removeReading(baseUrl, jar, written) { - const result = await call(baseUrl, `/api/measurements/${written.id}`, { +async function removeReading(baseUrl, jar, writtenId) { + const result = await call(baseUrl, `/api/measurements/${writtenId}`, { method: "DELETE", jar, }); expectStatus(result, 200, "delete"); + // A 200 is the instance's claim, not the outcome. An instance that answers + // 200 and keeps the row would have printed "account left clean" — so read + // the id back and require the 404. + const gone = await call(baseUrl, `/api/measurements/${writtenId}`, { jar }); + if (gone.status !== 404) { + throw new LegFailure( + `delete: ${writtenId} still reads back after a 200 delete`, + { status: gone.status, body: gone.text }, + ); + } return { detail: "account left clean" }; } +/** + * The id the write leg never learned. A POST that times out after the server + * committed leaves a row this run owns and cannot name, so ask the instance + * for it: the marker rides in `notes`, is unique per run, and comes back + * decrypted on the list. + */ +async function findLeftover(baseUrl, jar, marker) { + const result = await call( + baseUrl, + "/api/measurements?type=WEIGHT&limit=20&sortBy=measuredAt&sortDir=desc", + { jar }, + ); + if (result.status !== 200) return null; + const rows = result.json?.data?.measurements; + if (!Array.isArray(rows)) return null; + return rows.find((row) => row?.notes === marker)?.id ?? null; +} + +/** + * Best effort, never throws: a red leg is already the run's verdict, and a + * sweep that threw would replace it. It says out loud what it could not + * remove so the maintainer can. + */ +async function sweep(baseUrl, jar, writtenId, marker, log) { + let id = writtenId; + if (!id) { + id = await findLeftover(baseUrl, jar, marker).catch(() => null); + } + if (!id) return; + const result = await call(baseUrl, `/api/measurements/${id}`, { + method: "DELETE", + jar, + }).catch(() => null); + log( + result?.status === 200 + ? `sweep removed ${id} after a failed leg` + : `sweep could not remove ${id} — delete it by hand`, + ); +} + /** * Run the journey. Returns `{ ok: true }` or `{ ok: false, leg, error }` — * the caller decides what an exit code is, which is what lets the self-test @@ -308,6 +403,13 @@ async function removeReading(baseUrl, jar, written) { async function runJourney(config, log = console.log) { const { baseUrl, username, password, expectedVersion } = config; const jar = createCookieJar(); + // Both known BEFORE the write, so a write whose response never arrives is + // still recoverable: the key folds into the server's idempotency cell, the + // marker into the row's own `notes`. + const run = { + idempotencyKey: randomUUID(), + marker: `synthetic-journey ${randomUUID()}`, + }; const warmStarted = performance.now(); await warm(baseUrl, jar); @@ -315,33 +417,19 @@ async function runJourney(config, log = console.log) { `warm ${Math.round(performance.now() - warmStarted)} ms (discarded)`, ); - let written = null; - const legs = [ - { - name: "version", - run: () => assertVersion(baseUrl, jar, expectedVersion), - }, - { name: "sign in", run: () => signIn(baseUrl, jar, username, password) }, - { - name: "write", - run: async () => (written = await writeReading(baseUrl, jar)), - }, - { name: "read back", run: () => readBack(baseUrl, jar, written) }, - { name: "delete", run: () => removeReading(baseUrl, jar, written) }, - ]; - - for (const leg of legs) { + const runLeg = async (name, execute) => { const started = performance.now(); try { - const outcome = await leg.run(); + const outcome = await execute(); const elapsed = Math.round(performance.now() - started); log( - `ok ${leg.name.padEnd(10)} ${String(elapsed).padStart(6)} ms ${outcome?.detail ?? ""}`.trimEnd(), + `ok ${name.padEnd(10)} ${String(elapsed).padStart(6)} ms ${outcome?.detail ?? ""}`.trimEnd(), ); + return { ok: true }; } catch (error) { const elapsed = Math.round(performance.now() - started); log( - `FAILED ${leg.name.padEnd(10)} ${String(elapsed).padStart(6)} ms ${redact(error.message)}`, + `FAILED ${name.padEnd(10)} ${String(elapsed).padStart(6)} ms ${redact(error.message)}`, ); if (error instanceof LegFailure) { if (error.status !== null) log(` status ${error.status}`); @@ -349,8 +437,48 @@ async function runJourney(config, log = console.log) { } else { log(` ${redact(error.stack ?? "")}`); } - return { ok: false, leg: leg.name, error }; + return { ok: false, leg: name, error }; } + }; + + for (const [name, execute] of [ + ["version", () => assertVersion(baseUrl, jar, expectedVersion)], + ["sign in", () => signIn(baseUrl, jar, username, password)], + ]) { + const result = await runLeg(name, execute); + if (!result.ok) return result; + } + + // From the write on, the run owns a row on somebody's real account. A red + // read leg is not a reason to leave it there, so the cleanup sits in a + // `finally` and not in a leg that a `return` can skip. + let writtenId = null; + let removed = false; + try { + for (const [name, execute] of [ + [ + "write", + async () => { + const written = await writeReading(baseUrl, jar, run); + writtenId = written.id; + return written; + }, + ], + ["read back", () => readBack(baseUrl, jar, writtenId)], + [ + "delete", + async () => { + const outcome = await removeReading(baseUrl, jar, writtenId); + removed = true; + return outcome; + }, + ], + ]) { + const result = await runLeg(name, execute); + if (!result.ok) return result; + } + } finally { + if (!removed) await sweep(baseUrl, jar, writtenId, run.marker, log); } return { ok: true }; } @@ -436,8 +564,14 @@ function startMockInstance(breaks) { unit: PROBE.unit, source: parsed.source, measuredAt: parsed.measuredAt, + notes: parsed.notes ?? null, }; rows.set(row.id, row); + // The case the sweep exists for: the server committed and the + // caller never learned the id. Only the marker leads back here. + if (breaks === "write: committed") { + return send(500, { data: null, error: "gateway timed out" }); + } send(201, { data: row, error: null }); }); return; @@ -470,10 +604,26 @@ function startMockInstance(breaks) { if (breaks === "delete") { return send(404, { data: null, error: "Measurement not found" }); } + // 200 and the row still there — the status alone cannot catch this, + // only the read-back the delete leg does afterwards. + if (breaks === "delete: kept") { + return send(200, { data: { success: true }, error: null }); + } rows.delete(url.pathname.split("/").pop()); return send(200, { data: { success: true }, error: null }); } + if ( + url.pathname.startsWith("/api/measurements/") && + request.method === "GET" + ) { + const row = rows.get(url.pathname.split("/").pop()); + if (!row) { + return send(404, { data: null, error: "Measurement not found" }); + } + return send(200, { data: row, error: null }); + } + send(404, { data: null, error: "Not found" }); }); @@ -482,26 +632,104 @@ function startMockInstance(breaks) { const { port } = server.address(); resolve({ baseUrl: `http://127.0.0.1:${port}`, + rows, close: () => new Promise((done) => server.close(done)), }); }); }); } -async function selfTest() { +/** + * 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 = [ - { breaks: null, expect: "pass" }, - { breaks: "version", expect: "fail", leg: "version" }, - { breaks: "sign in", expect: "fail", leg: "sign in" }, - { breaks: "write", expect: "fail", leg: "write" }, - { breaks: "read back: missing", expect: "fail", leg: "read back" }, - { breaks: "read back: value", expect: "fail", leg: "read back" }, - { breaks: "read back: unit", expect: "fail", leg: "read back" }, - { breaks: "read back: source", expect: "fail", leg: "read back" }, - { breaks: "delete", expect: "fail", leg: "delete" }, + { 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; +} + +/** + * The excerpt path is what keeps a token out of a failure log. Prove it + * once, on a body shaped like this project's own key prefix. + */ +function selfTestRedaction() { + const shaped = `hlk_${"a".repeat(24)}`; + const redacted = excerpt(`{"error":"bad token","token":"${shaped}"}`); + const label = "redaction"; + if (redacted.includes("hlk_") || !redacted.includes("[redacted]")) { + console.log(`self-test FAILED ${label.padEnd(44)} → ${redacted}`); + return 1; + } + console.log(`self-test ok ${label.padEnd(44)} → ${redacted}`); + return 0; +} + +async function selfTest() { + const cases = [ + { breaks: null, expect: "pass", clean: true }, + { breaks: "version", expect: "fail", leg: "version" }, + { breaks: "sign in", expect: "fail", leg: "sign in" }, + { breaks: "write", expect: "fail", leg: "write", clean: true }, + // The write committed and the response was lost: no id, and the sweep + // has to find the row again by its marker. + { breaks: "write: committed", expect: "fail", leg: "write", clean: true }, + // The four read cases are also the proof that a red read still deletes: + // `clean` is asserted after the run, and only the `finally` can satisfy + // it once the delete leg has been skipped. + { + breaks: "read back: missing", + expect: "fail", + leg: "read back", + clean: true, + }, + { + breaks: "read back: value", + expect: "fail", + leg: "read back", + clean: true, + }, + { + breaks: "read back: unit", + expect: "fail", + leg: "read back", + clean: true, + }, + { + breaks: "read back: source", + expect: "fail", + leg: "read back", + clean: true, + }, + { breaks: "delete", expect: "fail", leg: "delete" }, + { breaks: "delete: kept", expect: "fail", leg: "delete" }, + ]; + + let failures = selfTestHostAllowlist() + selfTestRedaction(); for (const testCase of cases) { const label = testCase.breaks ?? "nothing broken"; const instance = await startMockInstance(testCase.breaks); @@ -515,19 +743,21 @@ async function selfTest() { }, (line) => lines.push(line), ); + const leftBehind = instance.rows.size; await instance.close(); const wanted = testCase.expect === "pass"; const legMatches = testCase.expect === "pass" || outcome.leg === testCase.leg; - if (outcome.ok === wanted && legMatches) { + const cleanEnough = !testCase.clean || leftBehind === 0; + if (outcome.ok === wanted && legMatches && cleanEnough) { console.log( - `self-test ok ${label.padEnd(20)} → ${outcome.ok ? "green" : `red on "${outcome.leg}"`}`, + `self-test ok ${label.padEnd(44)} → ${outcome.ok ? "green" : `red on "${outcome.leg}"`}${testCase.clean ? ", account clean" : ""}`, ); } else { failures += 1; console.log( - `self-test FAILED ${label.padEnd(20)} → ${outcome.ok ? "green" : `red on "${outcome.leg}"`}, expected ${testCase.expect}${testCase.leg ? ` on "${testCase.leg}"` : ""}`, + `self-test FAILED ${label.padEnd(44)} → ${outcome.ok ? "green" : `red on "${outcome.leg}"`}${cleanEnough ? "" : `, ${leftBehind} row(s) left behind`}, expected ${testCase.expect}${testCase.leg ? ` on "${testCase.leg}"` : ""}`, ); for (const line of lines) console.log(` | ${line}`); } @@ -537,7 +767,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 and redaction proven`, + ); return 0; } @@ -563,6 +795,11 @@ function readConfig() { ); process.exit(2); } + const refusal = hostRefusal(config.baseUrl); + if (refusal) { + console.error(refusal); + process.exit(2); + } return config; }