diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 548e8e6..83b6ba0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,9 @@ on: pull_request: branches: [main] schedule: - # Mondays 09:00 UTC — surface live-spec drift even without a push. + # Mondays 09:00 UTC — surface spec drift even without a push. Only does + # anything once the SENDLY_OPENAPI_URL repository variable is set; see the + # drift step at the end of this file. - cron: "0 9 * * 1" concurrency: @@ -53,9 +55,19 @@ jobs: run: pnpm build # Non-blocking: warn (do not fail) when the committed spec has drifted - # from the live public API, so external contributors are never blocked - # by a production change they did not make. The weekly schedule surfaces - # drift without a push; `pnpm sync-spec` refreshes the committed copy. - - name: Live spec drift (non-blocking) + # from the reference contract, so external contributors are never blocked + # by a platform change they did not make. `pnpm sync-spec` refreshes the + # committed copy. + # + # This step previously fetched https://api.sendly.now on every push, every + # pull request (forks included) and every weekly cron. Syncing the SDK spec + # from production is forbidden, so the source is now explicit: the step + # compares against whatever the SENDLY_OPENAPI_URL repository variable + # names, and SKIPS with a notice when that variable is unset. Until a + # maintainer sets it to a non-production contract, this step and the weekly + # schedule above are inert by design. + - name: Spec drift (non-blocking) run: pnpm check-spec-drift + env: + SENDLY_OPENAPI_URL: ${{ vars.SENDLY_OPENAPI_URL }} continue-on-error: true diff --git a/README.md b/README.md index 11bed67..e9759a6 100644 --- a/README.md +++ b/README.md @@ -380,11 +380,42 @@ pnpm build # regenerate types from openapi.json, then bundle with tsup ``` The type definitions in `src/types.generated.ts` are generated from -`openapi.json` via `pnpm build:types`. `openapi.json` is a committed snapshot -of Sendly's public OpenAPI spec; refresh it from the live API with -`pnpm sync-spec`, then regenerate the types (`pnpm build:types`, which -`pnpm build` runs for you). The SDK surface is verified against this snapshot -by the contract suite in `src/__tests__/contract.test.ts`. +`openapi.json` via `pnpm build:types`. `openapi.json` is a committed snapshot of +Sendly's OpenAPI contract, and the SDK surface is verified against it by the +contract suite in `src/__tests__/contract.test.ts`. + +### Refreshing `openapi.json` + +`pnpm sync-spec` requires `SENDLY_OPENAPI_URL`. There is **no default**, and in +particular it does not default to production: + +```bash +SENDLY_OPENAPI_URL=/path/to/sendly/apps/web/openapi/openapi.json pnpm sync-spec +pnpm build:types # regenerate types (pnpm build runs this for you) +``` + +`SENDLY_OPENAPI_URL` accepts a filesystem path (the normal case — the committed +contract in the Sendly platform monorepo at `apps/web/openapi/openapi.json`) or +an `http(s)://` URL of a local or staging API. Running `pnpm sync-spec` with it +unset exits non-zero and prints what to set. + +**Do not point it at `https://api.sendly.now`.** Vendoring the spec from the +deployed API makes the SDK mirror what is _running_ rather than what the repo +_declares_, so any drift between the platform's code and its committed contract +is laundered into "correct" on the way in — the SDK regenerates to match the +deployment and the mismatch vanishes silently. That destroys the vendored spec's +only job: it is the fixed reference the contract suite compares against, so an +SDK synced from production can no longer detect the very drift it exists to +catch. It is also unreproducible and unreviewable. + +This is not hard-blocked — "what does production actually serve?" is a legitimate +one-off. Doing it prints an unmissable warning (and a CI annotation), because +_quiet_ is what made the old default dangerous, not the host. Never commit the +result, and never wire that host into CI or any unattended job. + +`pnpm check-spec-drift` compares the committed `openapi.json` to the same source +and never fails the build. With `SENDLY_OPENAPI_URL` unset it skips with a notice +rather than erroring, so CI and fork pull requests stay green. ## License diff --git a/scripts/check-spec-drift.mjs b/scripts/check-spec-drift.mjs index 62a8209..8cdade8 100644 --- a/scripts/check-spec-drift.mjs +++ b/scripts/check-spec-drift.mjs @@ -1,14 +1,22 @@ #!/usr/bin/env node -// Non-blocking drift check: compare the committed ./openapi.json to the live -// public spec and emit a GitHub Actions warning annotation if they differ. +// Non-blocking drift check: compare the committed ./openapi.json to the Sendly +// OpenAPI contract and emit a GitHub Actions warning annotation if they differ. // -// Never fails — a moved production API must not block external contributors. -// CI runs this with continue-on-error; run it locally with -// `pnpm check-spec-drift`. Zero-dependency, Node 20+ global fetch. +// Never fails — a moved contract must not block external contributors. +// +// The source is SENDLY_OPENAPI_URL, with no default (see scripts/spec-source.mjs). +// Unlike `sync-spec`, an unconfigured run SKIPS with a notice instead of failing: +// this runs unattended in CI on every pull request, including from forks that +// cannot supply a source, and turning that into a red step would report a +// configuration gap as if it were spec drift. +// +// Run it locally with: +// SENDLY_OPENAPI_URL=/path/to/sendly/apps/web/openapi/openapi.json pnpm check-spec-drift import { readFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; -const SPEC_URL = process.env.SENDLY_OPENAPI_URL ?? "https://api.sendly.now/api/openapi.json"; +import { SPEC_SOURCE_ENV, loadSpec, resolveSpecSource, warnIfProduction } from "./spec-source.mjs"; + const COMMITTED_PATH = fileURLToPath(new URL("../openapi.json", import.meta.url)); function warn(message) { @@ -25,44 +33,59 @@ function operationSet(spec) { } async function main() { + let source; + try { + source = resolveSpecSource(); + } catch { + console.log( + `spec drift check: skipped — ${SPEC_SOURCE_ENV} is not set. ` + + `Set it to the committed contract (apps/web/openapi/openapi.json in the platform monorepo) to compare.`, + ); + return; + } + + // This one matters most: it is the step that runs unattended in CI, so a + // production source here is precisely the thing that must never be quiet. + warnIfProduction(source); + const committed = JSON.parse(await readFile(COMMITTED_PATH, "utf8")); - let live; + let reference; try { - const response = await fetch(SPEC_URL, { headers: { Accept: "application/json" } }); - if (!response.ok) { - warn(`could not fetch live spec (HTTP ${response.status}); skipping drift check`); - return; - } - live = await response.json(); + reference = await loadSpec(source); } catch (error) { const reason = error instanceof Error ? error.message : String(error); - warn(`could not reach ${SPEC_URL} (${reason}); skipping drift check`); + warn(`could not read ${source.display} (${reason}); skipping drift check`); return; } // Re-serialize both (compact) so formatting/line-endings never register as drift. - if (JSON.stringify(committed) === JSON.stringify(live)) { - console.log(`spec drift check: committed openapi.json matches ${SPEC_URL}`); + if (JSON.stringify(committed) === JSON.stringify(reference)) { + console.log(`spec drift check: committed openapi.json matches ${source.display}`); return; } const committedOps = operationSet(committed); - const liveOps = operationSet(live); - const onlyLiveOps = [...liveOps].filter((op) => !committedOps.has(op)).sort(); - const onlyCommittedOps = [...committedOps].filter((op) => !liveOps.has(op)).sort(); + const referenceOps = operationSet(reference); + const onlyReferenceOps = [...referenceOps].filter((op) => !committedOps.has(op)).sort(); + const onlyCommittedOps = [...committedOps].filter((op) => !referenceOps.has(op)).sort(); const committedSchemas = new Set(Object.keys(committed.components?.schemas ?? {})); - const liveSchemas = new Set(Object.keys(live.components?.schemas ?? {})); - const onlyLiveSchemas = [...liveSchemas].filter((name) => !committedSchemas.has(name)).sort(); - const onlyCommittedSchemas = [...committedSchemas].filter((name) => !liveSchemas.has(name)).sort(); + const referenceSchemas = new Set(Object.keys(reference.components?.schemas ?? {})); + const onlyReferenceSchemas = [...referenceSchemas].filter((name) => !committedSchemas.has(name)).sort(); + const onlyCommittedSchemas = [...committedSchemas].filter((name) => !referenceSchemas.has(name)).sort(); - const parts = [`committed openapi.json differs from ${SPEC_URL} — run \`pnpm sync-spec\` to refresh.`]; - if (onlyLiveOps.length) parts.push(`operations only live: ${onlyLiveOps.join(", ")}`); + const parts = [`committed openapi.json differs from ${source.display} — run \`pnpm sync-spec\` to refresh.`]; + if (onlyReferenceOps.length) parts.push(`operations only in source: ${onlyReferenceOps.join(", ")}`); if (onlyCommittedOps.length) parts.push(`operations only committed: ${onlyCommittedOps.join(", ")}`); - if (onlyLiveSchemas.length) parts.push(`schemas only live: ${onlyLiveSchemas.join(", ")}`); + if (onlyReferenceSchemas.length) parts.push(`schemas only in source: ${onlyReferenceSchemas.join(", ")}`); if (onlyCommittedSchemas.length) parts.push(`schemas only committed: ${onlyCommittedSchemas.join(", ")}`); - if (!onlyLiveOps.length && !onlyCommittedOps.length && !onlyLiveSchemas.length && !onlyCommittedSchemas.length) { + if ( + !onlyReferenceOps.length && + !onlyCommittedOps.length && + !onlyReferenceSchemas.length && + !onlyCommittedSchemas.length + ) { parts.push("field-level changes only (same operations and schemas)."); } diff --git a/scripts/spec-source.mjs b/scripts/spec-source.mjs new file mode 100644 index 0000000..2621c95 --- /dev/null +++ b/scripts/spec-source.mjs @@ -0,0 +1,163 @@ +// Resolve where the OpenAPI spec is read from. Shared by sync-spec.mjs and +// check-spec-drift.mjs so the two can never disagree about the source. +// +// WHY PRODUCTION IS BANNED AS A SOURCE — this is the reason, not a superstition, +// and it is written down so nobody deletes the guardrail for lack of one: +// +// Vendoring the spec from the deployed API makes the SDK mirror whatever is +// RUNNING rather than what the repo DECLARES. Any drift between the platform's +// code and its committed contract is then laundered into "correct" on the way +// in — the SDK regenerates itself to match the deployment and the mismatch +// disappears silently. That destroys the one job the vendored spec has: it is +// the fixed reference `src/__tests__/contract.test.ts` compares against, so +// an SDK synced from production can no longer detect the very drift it exists +// to catch. It is also unreproducible (two maintainers on the same commit can +// get different files) and unreviewable (the diff traces to no merged change). +// +// So there is deliberately NO default, and a script that silently picks *some* +// remote when unconfigured is the same class of bug — an unset +// SENDLY_OPENAPI_URL is an error naming exactly what to set, not a fallback. +// +// Production is NOT hard-blocked: "what does production actually serve?" is a +// legitimate one-off check. It is made LOUD instead (see productionWarning), +// because quiet is the property that made the old default dangerous, not the +// host itself. +// +// SENDLY_OPENAPI_URL accepts either form: +// - a filesystem path (absolute or relative) to a committed spec <- normal case +// - an http(s):// URL of a local or staging API <- occasional +import { readFile } from "node:fs/promises"; +import { fileURLToPath } from "node:url"; + +export const SPEC_SOURCE_ENV = "SENDLY_OPENAPI_URL"; + +/** Host of the deployed production API. Never a legitimate unattended source. */ +export const PRODUCTION_HOST = "api.sendly.now"; + +/** Canonical location of the contract inside the Sendly platform monorepo. */ +export const MONOREPO_SPEC_PATH = "apps/web/openapi/openapi.json"; + +/** + * The message shown when no source is configured. Kept as one exported string + * so `sync-spec` and `check-spec-drift` report the missing configuration + * identically, and so it stays in step with sendly-python's sync_spec.py. + */ +export const UNCONFIGURED_MESSAGE = [ + `${SPEC_SOURCE_ENV} is not set, and there is no default.`, + "", + "Point it at the committed contract in the Sendly platform monorepo:", + ` ${SPEC_SOURCE_ENV}=/path/to/sendly/${MONOREPO_SPEC_PATH} pnpm sync-spec`, + "", + "An http(s):// URL of a local or staging API works too. Do NOT point it at", + "production (https://api.sendly.now): the SDK spec is synced from the committed", + "contract, never live-synced from the deployed API.", +].join("\n"); + +/** + * Describe the configured spec source without reading it. + * @returns {{ kind: "file" | "http", value: string, display: string }} + * @throws {Error} when SENDLY_OPENAPI_URL is unset or empty. + */ +export function resolveSpecSource() { + const raw = process.env[SPEC_SOURCE_ENV]?.trim(); + if (!raw) throw new Error(UNCONFIGURED_MESSAGE); + + if (/^https?:\/\//i.test(raw)) return { kind: "http", value: raw, display: raw }; + + // Everything else is a path on disk. A file:// URL is accepted because it is + // what a shell tab-completion or a URL-shaped habit tends to produce; Node's + // fetch() rejects that scheme outright, which is why this cannot simply be + // handed to fetch. + const path = /^file:\/\//i.test(raw) ? fileURLToPath(raw) : raw; + return { kind: "file", value: path, display: path }; +} + +/** + * True when the source is the deployed production API. + * @param {{ kind: "file" | "http", value: string }} source + */ +export function isProductionSource(source) { + if (source.kind !== "http") return false; + try { + return new URL(source.value).hostname.toLowerCase() === PRODUCTION_HOST; + } catch { + return false; + } +} + +/** + * Shout — do not refuse — when the resolved source is production. + * + * A refusal would block the legitimate "verify what production actually serves" + * one-off. What must not happen is this occurring QUIETLY, which is exactly how + * the old default went unnoticed while running on every push, every PR and a + * weekly cron. So it is unmissable in a scrolling log, and it annotates the run + * when it happens inside GitHub Actions. + * + * @param {{ kind: "file" | "http", value: string, display: string }} source + */ +export function warnIfProduction(source) { + if (!isProductionSource(source)) return false; + + const banner = [ + "!!!===========================================================================!!!", + "!!! WARNING: reading the OpenAPI spec from PRODUCTION !!!", + `!!! ${source.display}`, + "!!! !!!", + "!!! This is the BANNED path. Vendoring a spec from the deployed API makes !!!", + "!!! the SDK mirror what is RUNNING instead of what the repo DECLARES, which !!!", + "!!! launders code-vs-contract drift into 'correct' and destroys the SDK's !!!", + "!!! ability to detect the very drift it exists to catch. !!!", + "!!! !!!", + "!!! Only ever do this as a DELIBERATE one-off (e.g. 'what does production !!!", + "!!! actually serve right now?'). NEVER commit the result, and never wire !!!", + "!!! this host into CI or any unattended job. !!!", + "!!!===========================================================================!!!", + ].join("\n"); + console.error(banner); + + if (process.env.GITHUB_ACTIONS) { + console.log( + `::warning title=OpenAPI spec read from PRODUCTION::${source.display} is the deployed API. ` + + `Syncing an SDK spec from production is banned — it launders code-vs-contract drift into "correct". ` + + `An unattended job must never be pointed at this host.`, + ); + } + return true; +} + +/** + * Read and parse the OpenAPI document from the configured source. + * @param {{ kind: "file" | "http", value: string, display: string }} source + * @returns {Promise>} + * @throws {Error} on any read/fetch/parse failure, or if the document is not OpenAPI. + */ +export async function loadSpec(source) { + let text; + if (source.kind === "file") { + try { + text = await readFile(source.value, "utf8"); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error(`could not read ${source.display}: ${reason}`); + } + } else { + const response = await fetch(source.value, { headers: { Accept: "application/json" } }); + if (!response.ok) { + throw new Error(`fetch failed with HTTP ${response.status} ${response.statusText} for ${source.display}`); + } + text = await response.text(); + } + + let spec; + try { + spec = JSON.parse(text); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error(`${source.display} did not contain valid JSON: ${reason}`); + } + if (!spec || typeof spec !== "object" || spec.openapi == null || spec.paths == null) { + throw new Error(`${source.display} is not a valid OpenAPI document`); + } + return spec; +} diff --git a/scripts/sync-spec.mjs b/scripts/sync-spec.mjs index f107642..04a0ebe 100644 --- a/scripts/sync-spec.mjs +++ b/scripts/sync-spec.mjs @@ -1,30 +1,44 @@ #!/usr/bin/env node -// Refresh ./openapi.json from Sendly's live public OpenAPI spec. +// Refresh ./openapi.json from the Sendly OpenAPI contract. // -// Zero-dependency: uses Node 20+ global fetch. Run via `pnpm sync-spec`. +// The source is REQUIRED and comes from SENDLY_OPENAPI_URL — normally a path to +// the committed contract in the platform monorepo (apps/web/openapi/openapi.json). +// There is no default: syncing from the live production API is forbidden, so +// running this unconfigured fails with instructions rather than reaching for a +// remote of its own choosing. See scripts/spec-source.mjs. +// +// Zero-dependency: Node 20+ only. Run via `pnpm sync-spec`. // The committed openapi.json is the contract the SDK is tested against // (see src/__tests__/contract.test.ts) and the input to `pnpm build:types`; -// this script is the sync mechanism when the platform API changes. +// this script is the sync mechanism when the platform contract changes. import { writeFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; -const SPEC_URL = process.env.SENDLY_OPENAPI_URL ?? "https://api.sendly.now/api/openapi.json"; +import { loadSpec, resolveSpecSource, warnIfProduction } from "./spec-source.mjs"; + const OUT_PATH = fileURLToPath(new URL("../openapi.json", import.meta.url)); -const response = await fetch(SPEC_URL, { headers: { Accept: "application/json" } }); -if (!response.ok) { - console.error(`sync-spec: fetch failed with HTTP ${response.status} ${response.statusText} for ${SPEC_URL}`); +let source; +try { + source = resolveSpecSource(); +} catch (error) { + console.error(`sync-spec: ${error.message}`); process.exit(1); } -const spec = await response.json(); -if (!spec || typeof spec !== "object" || spec.openapi == null || spec.paths == null) { - console.error(`sync-spec: ${SPEC_URL} did not return a valid OpenAPI document`); +// Loud, but not a refusal — see warnIfProduction in spec-source.mjs. +warnIfProduction(source); + +let spec; +try { + spec = await loadSpec(source); +} catch (error) { + console.error(`sync-spec: ${error.message}`); process.exit(1); } // Pretty-printed (2-space) with a trailing newline for a stable, reviewable diff. await writeFile(OUT_PATH, `${JSON.stringify(spec, null, 2)}\n`, "utf8"); console.log( - `sync-spec: wrote openapi.json (OpenAPI ${spec.openapi}, ${Object.keys(spec.paths).length} paths) from ${SPEC_URL}`, + `sync-spec: wrote openapi.json (OpenAPI ${spec.openapi}, ${Object.keys(spec.paths).length} paths) from ${source.display}`, );