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

.github/workflows/ci.yml

@@ -5,7 +5,9 @@ on:
     branches: [main]
   pull_request:
   schedule:
-    # Mondays 06:00 UTC -- surface live OpenAPI drift even without a push.
+    # Mondays 06:00 UTC -- surface OpenAPI 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 6 * * 1"
 
 # Public repo -> GitHub-hosted standard runners (free for public repos).
@@ -38,15 +40,25 @@ jobs:
         run: pytest tests/test_contract.py
       - name: Pytest
         run: pytest
-      # Non-blocking: diff the vendored spec against the live API so drift shows
-      # up as a warning annotation without failing the build. Runs once (on the
-      # newest interpreter) to avoid duplicate live fetches and annotations.
-      - name: Live OpenAPI drift check (non-blocking)
+      # Non-blocking: diff the vendored spec against the reference contract so
+      # drift shows up as a warning annotation without failing the build. Runs
+      # once (on the newest interpreter) to avoid duplicate reads and annotations.
+      #
+      # 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: OpenAPI drift check (non-blocking)
         id: spec_drift
         if: matrix.python-version == '3.13'
         continue-on-error: true
+        env:
+          SENDLY_OPENAPI_URL: ${{ vars.SENDLY_OPENAPI_URL }}
         run: python scripts/sync_spec.py --check
       - name: Annotate OpenAPI drift
         if: matrix.python-version == '3.13' && steps.spec_drift.outcome == 'failure'
         run: |
-          echo "::warning title=OpenAPI spec drift::Vendored tests/fixtures/openapi.json differs from the live spec at https://api.sendly.now/api/openapi.json. Run 'python scripts/sync_spec.py' and commit the refreshed copy."
+          echo "::warning title=OpenAPI spec drift::Vendored tests/fixtures/openapi.json differs from the contract at \$SENDLY_OPENAPI_URL. Run 'SENDLY_OPENAPI_URL=... python scripts/sync_spec.py' and commit the refreshed copy."

README.md

@@ -442,6 +442,45 @@ pytest
 
 Tests are fully hermetic (httpx `MockTransport`) and hit no network.
 
+### Refreshing the vendored OpenAPI spec
+
+`tests/fixtures/openapi.json` is a committed snapshot of Sendly's OpenAPI
+contract; the contract suite (`tests/test_contract.py`) verifies the SDK surface
+against it and never touches the network.
+
+`scripts/sync_spec.py` 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 \
+    python scripts/sync_spec.py
+
+SENDLY_OPENAPI_URL=... python scripts/sync_spec.py --check   # is the copy stale?
+```
+
+`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 the script 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 re-vendors to match the
+deployment and the mismatch vanishes silently. That destroys the vendored spec's
+only job: it is the fixed reference `tests/test_contract.py` 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.
+
+`--check` is the exception to the fail-loud rule: it never runs unattended
+against an unknown source, so with `SENDLY_OPENAPI_URL` unset it skips with a
+notice and exits 0, keeping CI and fork pull requests green.
+
 ## Documentation
 
 Full API reference: <https://docs.sendly.now>

scripts/sync_spec.py

@@ -1,21 +1,48 @@
 #!/usr/bin/env python3
 """Sync the vendored Sendly OpenAPI spec.
 
-Fetches the live OpenAPI document from the public API and writes a
-pretty-printed, deterministic copy to ``tests/fixtures/openapi.json``. The
-contract test suite (``tests/test_contract.py``) reads that committed copy and
-never touches the network, so this script is the single place where the vendored
-spec is refreshed.
+Reads the Sendly OpenAPI contract and writes a pretty-printed, deterministic copy
+to ``tests/fixtures/openapi.json``. The contract test suite
+(``tests/test_contract.py``) reads that committed copy and never touches the
+network, so this script is the single place where the vendored spec is refreshed.
+
+The source is **required** and comes from the ``SENDLY_OPENAPI_URL`` environment
+variable.
+
+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 re-vendors itself to match the deployment and the mismatch
+    disappears silently. That destroys the one job the vendored spec has: it is
+    the fixed reference ``tests/test_contract.py`` 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. Production is NOT hard-blocked
+-- "what does production actually serve?" is a legitimate one-off check. It is
+made LOUD instead (see ``warn_if_production``), 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
 
 Usage::
 
-    python scripts/sync_spec.py            # fetch + overwrite the vendored copy
-    python scripts/sync_spec.py --check    # fail if the vendored copy is stale
+    SENDLY_OPENAPI_URL=/path/to/sendly/apps/web/openapi/openapi.json \\
+        python scripts/sync_spec.py            # overwrite the vendored copy
+
+    SENDLY_OPENAPI_URL=... python scripts/sync_spec.py --check
+                                               # fail if the vendored copy is stale
 
-The spec URL can be overridden with the ``SENDLY_OPENAPI_URL`` environment
-variable (useful for staging or self-hosted deployments). Standard-library only
--- no third-party dependencies -- so the script runs in a bare Python 3.10+
-environment.
+Standard-library only -- no third-party dependencies -- so the script runs in a
+bare Python 3.10+ environment.
 """
 
 from __future__ import annotations
@@ -26,11 +53,36 @@
 import os
 import sys
 import urllib.request
+from collections.abc import Iterable
 from pathlib import Path
 from typing import Any, NoReturn
+from urllib.parse import urlparse
+from urllib.request import url2pathname
+
+#: Environment variable naming the OpenAPI source. No default -- see module docstring.
+SPEC_SOURCE_ENV = "SENDLY_OPENAPI_URL"
+
+#: Canonical location of the contract inside the Sendly platform monorepo.
+MONOREPO_SPEC_PATH = "apps/web/openapi/openapi.json"
+
+#: Host of the deployed production API. Never a legitimate unattended source.
+PRODUCTION_HOST = "api.sendly.now"
 
-#: Live OpenAPI 3.1 document for the public Sendly REST API.
-DEFAULT_SPEC_URL = "https://api.sendly.now/api/openapi.json"
+#: Shown when no source is configured. Kept in step with sendly-js's
+#: scripts/spec-source.mjs so both SDKs report the same missing configuration
+#: the same way.
+UNCONFIGURED_MESSAGE = "\n".join(
+    [
+        f"{SPEC_SOURCE_ENV} is not set, and there is no default.",
+        "",
+        "Point it at the committed contract in the Sendly platform monorepo:",
+        f"  {SPEC_SOURCE_ENV}=/path/to/sendly/{MONOREPO_SPEC_PATH} python scripts/sync_spec.py",
+        "",
+        "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.",
+    ]
+)
 
 #: Committed copy consumed by the contract tests. Kept relative to this file so
 #: the script works from any working directory.
@@ -42,33 +94,106 @@
 _TIMEOUT_SECONDS = 30
 
 
-def spec_url() -> str:
-    """Resolve the spec URL, honouring the ``SENDLY_OPENAPI_URL`` override."""
-    return os.environ.get("SENDLY_OPENAPI_URL", DEFAULT_SPEC_URL)
-
-
 def _fail(message: str) -> NoReturn:
     print(f"sync_spec: {message}", file=sys.stderr)
     raise SystemExit(1)
 
 
-def fetch_spec(url: str) -> dict[str, Any]:
-    """Fetch and parse the live OpenAPI document. Fail loud on any error."""
-    request = urllib.request.Request(url, headers={"User-Agent": "sendly-spec-sync"})
-    try:
-        with urllib.request.urlopen(request, timeout=_TIMEOUT_SECONDS) as response:
-            status = getattr(response, "status", 200)
-            if status != 200:
-                _fail(f"{url} returned HTTP {status}")
-            payload = response.read().decode("utf-8")
-    except (OSError, ValueError) as exc:
-        _fail(f"could not fetch {url}: {exc}")
+def spec_source() -> str:
+    """Resolve the configured spec source, or fail loudly naming what to set."""
+    raw = os.environ.get(SPEC_SOURCE_ENV, "").strip()
+    if not raw:
+        _fail(UNCONFIGURED_MESSAGE)
+    return raw
+
+
+def _is_http(source: str) -> bool:
+    return source.lower().startswith(("http://", "https://"))
+
+
+def _as_local_path(source: str) -> Path:
+    """Interpret a non-http source as a path on disk.
+
+    A ``file://`` URL is accepted alongside a plain path because it is what a
+    shell completion or a URL-shaped habit tends to produce.
+    """
+    if source.lower().startswith("file://"):
+        return Path(url2pathname(urlparse(source).path))
+    return Path(source)
+
+
+def is_production_source(source: str) -> bool:
+    """True when the source is the deployed production API."""
+    if not _is_http(source):
+        return False
+    return (urlparse(source).hostname or "").lower() == PRODUCTION_HOST
+
+
+def warn_if_production(source: str) -> bool:
+    """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.
+    """
+    if not is_production_source(source):
+        return False
+
+    banner = "\n".join(
+        [
+            "!!!===========================================================================!!!",
+            "!!!  WARNING: reading the OpenAPI spec from PRODUCTION                        !!!",
+            f"!!!  {source}",
+            "!!!                                                                          !!!",
+            "!!!  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.                                 !!!",
+            "!!!===========================================================================!!!",
+        ]
+    )
+    print(banner, file=sys.stderr)
+
+    if os.environ.get("GITHUB_ACTIONS"):
+        print(
+            f"::warning title=OpenAPI spec read from PRODUCTION::{source} 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
+
+
+def load_spec(source: str) -> dict[str, Any]:
+    """Read and parse the OpenAPI document from ``source``. Fail loud on any error."""
+    if _is_http(source):
+        request = urllib.request.Request(source, headers={"User-Agent": "sendly-spec-sync"})
+        try:
+            with urllib.request.urlopen(request, timeout=_TIMEOUT_SECONDS) as response:
+                status = getattr(response, "status", 200)
+                if status != 200:
+                    _fail(f"{source} returned HTTP {status}")
+                payload = response.read().decode("utf-8")
+        except (OSError, ValueError) as exc:
+            _fail(f"could not fetch {source}: {exc}")
+    else:
+        path = _as_local_path(source)
+        try:
+            payload = path.read_text(encoding="utf-8")
+        except OSError as exc:
+            _fail(f"could not read {path}: {exc}")
+
     try:
         spec: Any = json.loads(payload)
     except json.JSONDecodeError as exc:
-        _fail(f"{url} did not return valid JSON: {exc}")
+        _fail(f"{source} did not contain valid JSON: {exc}")
     if not isinstance(spec, dict) or "openapi" not in spec or "paths" not in spec:
-        _fail(f"{url} did not return a valid OpenAPI document")
+        _fail(f"{source} is not a valid OpenAPI document")
     return spec
 
 
@@ -89,34 +214,70 @@ def _operation_count(spec: dict[str, Any]) -> int:
 
 
 def write_spec() -> None:
-    """Fetch the live spec and overwrite the vendored copy."""
-    spec = fetch_spec(spec_url())
+    """Read the configured spec and overwrite the vendored copy."""
+    source = spec_source()
+    # Loud, but not a refusal -- see warn_if_production.
+    warn_if_production(source)
+    spec = load_spec(source)
     text = render(spec)
     SPEC_PATH.parent.mkdir(parents=True, exist_ok=True)
     # newline="\n" keeps the vendored copy byte-identical on every platform
     # (Windows text-mode writes would otherwise emit CRLF).
     SPEC_PATH.write_text(text, encoding="utf-8", newline="\n")
     print(
         f"sync_spec: wrote {SPEC_PATH} "
-        f"({len(text)} bytes, {len(spec['paths'])} paths, {_operation_count(spec)} operations)"
+        f"({len(text)} bytes, {len(spec['paths'])} paths, {_operation_count(spec)} operations) "
+        f"from {source}"
     )
 
 
+def _print_diff(lines: Iterable[str]) -> None:
+    """Print a unified diff without dying on the console's encoding.
+
+    The spec contains non-ASCII characters (e.g. U+21D2 in descriptions) that a
+    legacy Windows console codepage cannot encode, which turned every drifted
+    ``--check`` on Windows into a UnicodeEncodeError traceback instead of the
+    diff it exists to show.
+    """
+    text = "".join(lines)
+    try:
+        sys.stdout.write(text)
+    except UnicodeEncodeError:
+        encoding = sys.stdout.encoding or "ascii"
+        sys.stdout.write(text.encode(encoding, "replace").decode(encoding))
+
+
 def check_spec() -> None:
-    """Fail (exit 1) if the vendored copy differs from the live spec."""
+    """Report whether the vendored copy differs from the configured source.
+
+    Unlike a write, an unconfigured ``--check`` SKIPS (exit 0) rather than
+    failing: it runs unattended in CI on every pull request, including from forks
+    that cannot supply a source, and a red step there would report a
+    configuration gap as if it were spec drift.
+    """
+    source = os.environ.get(SPEC_SOURCE_ENV, "").strip()
+    if not source:
+        print(
+            f"sync_spec: skipped -- {SPEC_SOURCE_ENV} is not set. Set it to the committed "
+            f"contract ({MONOREPO_SPEC_PATH} 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.
+    warn_if_production(source)
     if not SPEC_PATH.exists():
         _fail(f"vendored spec missing at {SPEC_PATH}; run `python scripts/sync_spec.py`")
-    live = render(fetch_spec(spec_url()))
+    current = render(load_spec(source))
     vendored = SPEC_PATH.read_text(encoding="utf-8")
-    if live == vendored:
-        print(f"sync_spec: vendored spec is in sync with {spec_url()}")
+    if current == vendored:
+        print(f"sync_spec: vendored spec is in sync with {source}")
         return
-    sys.stdout.writelines(
+    _print_diff(
         difflib.unified_diff(
             vendored.splitlines(keepends=True),
-            live.splitlines(keepends=True),
+            current.splitlines(keepends=True),
             fromfile="vendored tests/fixtures/openapi.json",
-            tofile=f"live {spec_url()}",
+            tofile=f"source {source}",
         )
     )
     _fail("vendored spec is STALE -- run `python scripts/sync_spec.py` and commit the result")