fix(spec): require an explicit spec source; drop the production default

.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

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
 

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).");
   }
 

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<Record<string, unknown>>}
+ * @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;
+}