diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ffd1e6f..ab9caec 100644 --- a/.github/workflows/ci.yml +++ b/.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." diff --git a/README.md b/README.md index 52b87d3..8f990ce 100644 --- a/README.md +++ b/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: diff --git a/scripts/sync_spec.py b/scripts/sync_spec.py index 97a9c50..d8c5f39 100644 --- a/scripts/sync_spec.py +++ b/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,8 +214,11 @@ 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 @@ -98,25 +226,58 @@ def write_spec() -> None: 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")