From a139868f43ee08d8c39228ce5a80e278342c6461 Mon Sep 17 00:00:00 2001 From: Willie Chalmers III Date: Wed, 23 Sep 2026 18:32:13 -0700 Subject: [PATCH] fix(dx): show every bootstrap value that is not secret The readiness report listed PUBLIC_LVBT_CWA_TOKEN and PUBLIC_LVBT_LABS_CWA_TOKEN as ready with only their hostname, so nobody could check that the right site's token was stored, although the token is public and sent with every page view. The final CI message likewise named CLOUDFLARE_ACCOUNT_ID without its value. Both reports, and `pnpm preflight`, now print these values, and storing a Web Analytics token echoes what was stored. The Cloudflare API token stays the only masked prompt and is still shown only as set or not set. The io seam's masked prompt says it is for real credentials only. A new test runs the bootstrap, runs it again, and runs preflight. It checks that the token never appears in any output or command, that the account id, database ids and Web Analytics tokens do appear in the second run and in preflight, and that only one prompt is masked. Co-Authored-By: Claude Opus 5.5 --- docs/operations/how-to/set-up-production.md | 5 +++ scripts/bootstrap/lib/io.ts | 7 +++- scripts/bootstrap/phases/analytics.ts | 40 +++++++++++++------ scripts/bootstrap/phases/ci-secrets.ts | 2 +- scripts/tests/bootstrap-idempotency.test.ts | 27 +++++++++++++ .../support/fake-bootstrap-services.test.ts | 27 +++++++++++-- 6 files changed, 89 insertions(+), 19 deletions(-) diff --git a/docs/operations/how-to/set-up-production.md b/docs/operations/how-to/set-up-production.md index 9aae287a..6d4868d6 100644 --- a/docs/operations/how-to/set-up-production.md +++ b/docs/operations/how-to/set-up-production.md @@ -161,6 +161,11 @@ them and asks first, because the migrations come from your checkout. `pnpm preflight` runs the same checks and only reports. It never installs, creates, writes or asks anything. +Both commands print every value that is not secret, such as the account ID, +the database ids and the Web Analytics tokens, so you can check that each is +the one you expect. The deploy token is the only value they hide: it is shown +as set or not set, never printed, and typed at a prompt that does not echo. + ## Replacing a value The bootstrap never replaces a value that is already set unless you ask for diff --git a/scripts/bootstrap/lib/io.ts b/scripts/bootstrap/lib/io.ts index dbc891bf..e22f6d46 100644 --- a/scripts/bootstrap/lib/io.ts +++ b/scripts/bootstrap/lib/io.ts @@ -30,7 +30,12 @@ export interface BootstrapIo { /** Writes a file by its path relative to the repository root. */ writeFile: (relativePath: string, content: string) => void; confirm: (message: string, initialValue: boolean) => Promise; - /** Masked prompt. The answer is never echoed or logged. */ + /** + * Masked prompt, for real credentials only: API keys, tokens, client and + * signing secrets, private keys. The answer is never echoed or logged. + * Anything else is asked with `text` and shown in every report, because + * hiding a value that is not secret only stops people checking it. + */ secret: (message: string) => Promise; /** Plain prompt for a value that is not secret, checked by `check`. */ text: (message: string, check: (value: string) => string | undefined) => Promise; diff --git a/scripts/bootstrap/phases/analytics.ts b/scripts/bootstrap/phases/analytics.ts index 8d568383..c31dcd86 100644 --- a/scripts/bootstrap/phases/analytics.ts +++ b/scripts/bootstrap/phases/analytics.ts @@ -62,20 +62,31 @@ function analyticsSteps(account: CloudflareAccount, variable: AnalyticsVariable) ].join('\n'); } -/** Names of the variables the environment already holds a value for, or - * null when they could not be read. */ -function variablesSet(io: BootstrapIo): string[] | null { +/** The environment's variables that hold a value, by name, or null when + * they could not be read. */ +function variablesSet(io: BootstrapIo): Map | null { const listed = io.run(`gh variable list --env ${ENVIRONMENT} --json name,value`); if (!listed.ok) return null; const rows = parseJsonOutput(listed.stdout); if (!Array.isArray(rows)) return null; - return (rows as { name?: unknown; value?: unknown }[]).flatMap((row) => - typeof row.name === 'string' && typeof row.value === 'string' && row.value.trim() - ? [row.name] - : [], + return new Map( + (rows as { name?: unknown; value?: unknown }[]).flatMap((row) => + typeof row.name === 'string' && typeof row.value === 'string' && row.value.trim() + ? [[row.name, row.value] as const] + : [], + ), ); } +/** + * The token itself, beside the host it belongs to. It is public, so hiding + * it would only stop somebody checking that the right site's token is + * stored, which is the one mistake this report can catch. + */ +function tokenDetail(variable: AnalyticsVariable, token: string): string { + return `${token} (${variable.host})`; +} + async function askAndStore( io: BootstrapIo, account: CloudflareAccount, @@ -91,6 +102,8 @@ async function askAndStore( }); if (!result.ok) { io.log('error', `Failed to set ${variable.name}: ${result.stderr || result.stdout}`); + } else { + io.log('success', `${variable.name} is set to ${tokenDetail(variable, token)}.`); } return result.ok; } @@ -114,12 +127,13 @@ export async function runAnalyticsPhase({ doctor, io }: PhaseContext): Promise

!set.includes(variable.name)); - const rows: ToolRow[] = ANALYTICS_VARIABLES.map((variable) => - missing.includes(variable) - ? { label: variable.name, status: 'failed', detail: 'not set' } - : { label: variable.name, status: 'ready', detail: variable.host }, - ); + const missing = ANALYTICS_VARIABLES.filter((variable) => !set.has(variable.name)); + const rows: ToolRow[] = ANALYTICS_VARIABLES.map((variable) => { + const token = set.get(variable.name); + return token === undefined + ? { label: variable.name, status: 'failed', detail: `not set (${variable.host})` } + : { label: variable.name, status: 'ready', detail: tokenDetail(variable, token) }; + }); io.table('Analytics variables', rows); if (missing.length === 0) return { success: true }; if (doctor) return { success: false }; diff --git a/scripts/bootstrap/phases/ci-secrets.ts b/scripts/bootstrap/phases/ci-secrets.ts index 10e0259c..b8eef354 100644 --- a/scripts/bootstrap/phases/ci-secrets.ts +++ b/scripts/bootstrap/phases/ci-secrets.ts @@ -351,7 +351,7 @@ export async function runCiSecretsPhase({ io.log( 'success', - `${TOKEN_SECRET} (secret) and ${ACCOUNT_VARIABLE} (variable) are set on ${states.map((state) => state.environment).join(' and ')}. CI deploys should work on the next push to main.`, + `${TOKEN_SECRET} (secret) and ${ACCOUNT_VARIABLE} (variable, ${accountId}) are set on ${states.map((state) => state.environment).join(' and ')}. CI deploys should work on the next push to main.`, ); return { success: true }; } diff --git a/scripts/tests/bootstrap-idempotency.test.ts b/scripts/tests/bootstrap-idempotency.test.ts index b0890cf8..2d1365fe 100644 --- a/scripts/tests/bootstrap-idempotency.test.ts +++ b/scripts/tests/bootstrap-idempotency.test.ts @@ -11,6 +11,7 @@ import { } from '../bootstrap/lib/wrangler-config.js'; import { ACCOUNT, + FAKE_ANALYTICS_TOKEN, FAKE_TOKEN, FakeServices, isMutation, @@ -352,6 +353,32 @@ describe('credentials that are already set', () => { }); }); +describe('what the bootstrap shows', () => { + it('masks the token and nothing else', async () => { + const first = await run(services); + + expect(first.maskedPrompts).toHaveLength(1); + expect(first.prompts.length).toBeGreaterThan(first.maskedPrompts.length); + }); + + it('never prints the token, and shows every value that is not secret', async () => { + const first = await run(services); + const second = await run(services); + const report = await run(services, { doctor: true }); + + for (const record of [first, second, report]) { + expect(record.output.join('\n')).not.toContain(FAKE_TOKEN); + expect(record.calls.some((call) => call.command.includes(FAKE_TOKEN))).toBe(false); + } + for (const record of [second, report]) { + const printed = record.output.join('\n'); + expect(printed).toContain(ACCOUNT.id); + expect(printed).toContain(FAKE_ANALYTICS_TOKEN); + for (const database of services.databases) expect(printed).toContain(database.uuid); + } + }); +}); + describe('preflight', () => { it('reports a brand-new setup without changing or asking anything', async () => { const report = await run(services, { doctor: true }); diff --git a/scripts/tests/support/fake-bootstrap-services.test.ts b/scripts/tests/support/fake-bootstrap-services.test.ts index c761cb85..60511a31 100644 --- a/scripts/tests/support/fake-bootstrap-services.test.ts +++ b/scripts/tests/support/fake-bootstrap-services.test.ts @@ -69,8 +69,12 @@ interface RecordedCall { export interface RunRecord { calls: RecordedCall[]; prompts: string[]; + /** The prompts that masked their answer. */ + maskedPrompts: string[]; writes: string[]; tables: ToolRow[][]; + /** Everything printed: notes, table rows, and log lines. */ + output: string[]; } /** One GitHub API request as the fake routes it. */ @@ -130,7 +134,14 @@ export class FakeServices { /** A fresh seam for one run, recording what that run does. */ io(): BootstrapIo & { record: RunRecord } { - const record: RunRecord = { calls: [], prompts: [], writes: [], tables: [] }; + const record: RunRecord = { + calls: [], + prompts: [], + maskedPrompts: [], + writes: [], + tables: [], + output: [], + }; const ask = (message: string, answer: T): Promise => { record.prompts.push(message); return Promise.resolve(answer); @@ -150,18 +161,26 @@ export class FakeServices { writeFileSync(path.join(this.root, relativePath), content, 'utf8'); }, confirm: (message) => ask(message, true), - secret: (message) => ask(message, FAKE_TOKEN), + secret: (message) => { + record.maskedPrompts.push(message); + return ask(message, FAKE_TOKEN); + }, text: (message, check) => { const refused = check(FAKE_ANALYTICS_TOKEN); if (refused) throw new Error(`the fake analytics token was refused: ${refused}`); return ask(message, FAKE_ANALYTICS_TOKEN); }, openUrl: () => true, - note: () => undefined, + note: (body) => { + record.output.push(body); + }, table: (_title, rows) => { record.tables.push([...rows]); + record.output.push(...rows.map((row) => `${row.label} ${row.detail ?? ''}`)); + }, + log: (_level, message) => { + record.output.push(message); }, - log: () => undefined, }; }