diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index d32d03c..a63bfa3 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -15,37 +15,42 @@ jobs: deploy: name: Build, validate, push ACL runs-on: ubuntu-latest + # The CI `validate` job declares this same environment, so both jobs read + # one set of credentials from one scope. See docs/ci-credentials.md. environment: production + env: + TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} + TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }} steps: - uses: actions/checkout@v4 + # The runner context is unavailable in job-level env, so export the Nix + # cache dir once here instead of repeating it on every step. + - name: Configure cache dir + run: echo "XDG_CACHE_HOME=$RUNNER_TEMP/.cache" >> "$GITHUB_ENV" + + # Runs before the Nix install so a dead credential fails in seconds with + # a message naming the secret, the scope, and the fix. + - name: Preflight Tailscale credential wiring + run: python3 scripts/ci_preflight.py + - uses: DeterminateSystems/nix-installer-action@main - uses: DeterminateSystems/magic-nix-cache-action@main - name: Build policy JSON run: nix develop --command just build - env: - XDG_CACHE_HOME: ${{ runner.temp }}/.cache + + # Hard gate: never push a policy the API would reject. + - name: Server-side policy validation + run: python3 scripts/acl_validate.py --prove - name: Validate against live ACL continue-on-error: true run: nix develop --command python3 scripts/validate.py - env: - TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} - TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }} - XDG_CACHE_HOME: ${{ runner.temp }}/.cache - name: Show diff run: | nix develop --command python3 scripts/push.py --dry-run || true - env: - TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} - TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }} - XDG_CACHE_HOME: ${{ runner.temp }}/.cache - name: Push ACL to Tailscale run: nix develop --command python3 scripts/push.py --confirm - env: - TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} - TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }} - XDG_CACHE_HOME: ${{ runner.temp }}/.cache diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91c7885..3bf44f8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,16 +92,38 @@ jobs: runs-on: ubuntu-latest needs: [check, secrets] if: github.event_name == 'pull_request' + # Deliberately the same environment the CD deploy job uses, so this job + # reads the *same* TAILSCALE_API_KEY and TS_OAUTH_CLIENT_ID values the push + # will read. Without it, a green run here says nothing about the deploy + # path. See docs/ci-credentials.md. + environment: production + env: + TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} + TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }} steps: - uses: actions/checkout@v4 + # The runner context is unavailable in job-level env, so export the Nix + # cache dir once here instead of repeating it on every step. + - name: Configure cache dir + run: echo "XDG_CACHE_HOME=$RUNNER_TEMP/.cache" >> "$GITHUB_ENV" + + # Runs before the Nix install so a dead credential fails in seconds with + # a message naming the secret, the scope, and the fix. + - name: Preflight Tailscale credential wiring + run: python3 scripts/ci_preflight.py + - uses: DeterminateSystems/nix-installer-action@main - uses: DeterminateSystems/magic-nix-cache-action@main - name: Build policy + id: build run: nix develop --command just build - env: - XDG_CACHE_HOME: ${{ runner.temp }}/.cache + + # Server-side grammar check. push.py --dry-run only diffs locally; this + # asks Tailscale whether the policy would be accepted at all. + - name: Server-side policy validation + run: python3 scripts/acl_validate.py --prove - name: Validate against live id: validate @@ -127,12 +149,12 @@ jobs: fi exit "$EXIT_CODE" - env: - TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} - XDG_CACHE_HOME: ${{ runner.temp }}/.cache + # Only meaningful once the toolchain and policy exist. When the preflight + # fails fast, Nix is never installed and these steps would otherwise post + # a PR comment whose entire content is "nix: command not found". - name: Diff summary - if: always() + if: always() && steps.build.outcome == 'success' id: diff run: | set +e @@ -142,12 +164,9 @@ jobs: echo "$OUTPUT" echo 'EOF' } >> "$GITHUB_OUTPUT" - env: - TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }} - XDG_CACHE_HOME: ${{ runner.temp }}/.cache - name: Comment on PR - if: always() + if: always() && steps.build.outcome == 'success' uses: actions/github-script@v7 with: script: | diff --git a/docs/ci-credentials.md b/docs/ci-credentials.md new file mode 100644 index 0000000..f5c11d1 --- /dev/null +++ b/docs/ci-credentials.md @@ -0,0 +1,132 @@ +# CI/CD credentials for the tailnet policy + +This repo pushes the live tailnet policy for `taila4c78d.ts.net`. Two GitHub +Actions jobs talk to the Tailscale API: + +| Job | Workflow | Trigger | What it does | +| --- | --- | --- | --- | +| `validate` | `.github/workflows/ci.yml` | `pull_request` | Reads the live ACL, diffs it, asks Tailscale to type-check the built policy | +| `deploy` | `.github/workflows/cd.yml` | `push` to `main` | Same checks, then `POST /acl` to apply | + +## One scope, one credential set + +Both jobs declare `environment: production`. That is deliberate, and it is the +whole point of this document. + +GitHub resolves `secrets.X` from the job's environment first and falls back to +the repository scope, so a repository secret and an environment secret with the +**same name** are two different values with no visible sign that they differ. +Before this was unified, `validate` read the repository-scoped +`TAILSCALE_API_KEY` and `deploy` read the `production`-scoped one. A red +`validate` therefore said nothing about whether the deploy would work, and a +rotation applied to one scope silently left the other dead. + +The rule now: **the `production` environment is the only scope that matters.** +A repository-level secret of the same name is dead weight and should be deleted +so it cannot be rotated by mistake. + +| Name | Type | Scope | Required | +| --- | --- | --- | --- | +| `TAILSCALE_API_KEY` | Environment **secret** | `production` | always | +| `TS_OAUTH_CLIENT_ID` | Environment **variable** | `production` | only when the secret is an OAuth client secret | + +`TS_OAUTH_CLIENT_ID` is a *variable*, not a secret — the client id is not +sensitive, and storing it as a secret makes it unreadable in logs for no gain. +Environment variables live under +*Settings → Environments → production → Environment variables*. + +## Two kinds of credential + +`scripts/ts_auth.py` accepts either form in `TAILSCALE_API_KEY` and branches on +the prefix: + +- **`tskey-api-…`** — a direct admin API key. Used as the bearer token as-is. + Needs no client id. **Expires after at most 90 days**, so it guarantees a + future outage on a timer. +- **`tskey-client-…`** — an OAuth client secret. Tailscale rejects it as a + bearer token (HTTP 403); it is exchanged for a short-lived access token via + the `client_credentials` grant. **Does not expire.** The exchange requires the + client id, so `TS_OAUTH_CLIENT_ID` becomes mandatory. + +Either kind must carry the `policy_file` scope. Without it the credential +authenticates but the ACL read returns HTTP 403. + +The OAuth pair is the preferred configuration: non-expiring, and scopeable to +`policy_file` alone rather than full admin. + +## Rotation runbook + +### Preferred: OAuth client pair + +1. Tailscale admin console → **Settings → OAuth clients → Generate OAuth + client**. Grant **only** the `policy_file` scope (read **and** write — the CD + job pushes). +2. Copy both halves. The secret (`tskey-client-…`) is shown once. +3. GitHub → **Settings → Environments → production**: + - **Environment secrets** → update `TAILSCALE_API_KEY` to the client secret. + - **Environment variables** → add/update `TS_OAUTH_CLIENT_ID` to the client + id. +4. Delete the old OAuth client in the Tailscale console. +5. Re-run CI on any open PR. `Preflight Tailscale credential wiring` proves the + new pair before anything else runs. + +### Interim: direct API key + +1. Tailscale admin console → **Settings → Keys → Generate access token**. +2. GitHub → **Settings → Environments → production → Environment secrets** → + update `TAILSCALE_API_KEY`. +3. Leave `TS_OAUTH_CLIENT_ID` unset (or accept the preflight warning that it is + ignored). +4. Diary the 90-day expiry, or move to the OAuth pair. + +Do **not** set the repository-scoped `TAILSCALE_API_KEY`. It is not read by +either job. + +## What the preflight tells you + +`scripts/ci_preflight.py` runs first in both jobs, on the runner's system +python3, before the Nix toolchain is installed — a dead credential costs +seconds, not a full dev-shell build followed by an opaque `API error 401`. It +never prints a credential value, only the kind inferred from the prefix. + +| Symptom | Meaning | Fix | +| --- | --- | --- | +| `Tailscale credential is not wired` | `TAILSCALE_API_KEY` unset in the `production` environment | Create the environment secret | +| `Tailscale OAuth client id is missing` | Secret is `tskey-client-…`, `TS_OAUTH_CLIENT_ID` unset | Add the environment **variable** | +| `Tailscale OAuth token exchange failed` | Client id and client secret are not a matching pair | Re-copy both halves from one OAuth client | +| `…present but rejected (HTTP 401)` | Expired or revoked credential | Rotate the value | +| `…lacks policy-file permission (HTTP 403)` | Credential lacks `policy_file` scope | Regenerate with the scope | + +## Server-side policy validation + +`scripts/acl_validate.py` POSTs the built policy to +`POST /api/v2/tailnet/{tailnet}/acl/validate`, which type-checks it without +applying anything. This is the only pre-merge check that can catch a policy +Tailscale will refuse — `push.py --dry-run` merely diffs the local build against +the live ACL and never asks whether the result is legal. + +The endpoint has a trap: **policy errors are returned with HTTP 200** and a +JSON body carrying `message` / `data`, matching +`tailscale.com/cmd/gitops-pusher`. A naive status-code check passes everything. +So the script treats a non-empty `message` or `data` as failure, and `--prove` +first submits a policy that must be rejected (unknown action plus an undefined +group reference). If that known-bad policy comes back clean, the checker is +blind and the step fails rather than reporting a pass it cannot justify. + +Without credentials the step emits a loud warning and skips, which only happens +locally — in CI the preflight has already failed the job. + +## Caveats of `environment:` on a pull-request job + +- **Fork PRs get no secrets.** This is GitHub behaviour for any scope, not a + consequence of using an environment. Policy changes have to come from a + branch on this repository. +- **Adding required reviewers to the `production` environment would gate every + PR**, because the `validate` job would then wait for a deployment approval. + If that protection is ever wanted for deploys only, split the environments and + update the `environment:` key plus the `SCOPE` string in + `scripts/ci_preflight.py` together. +- **A deployment-branch policy restricting `production` to `main` would break + PR validation** for the same reason. +- Each PR run records a `production` deployment in the Environments UI. That is + cosmetic noise; `validate` applies nothing. diff --git a/scripts/acl_validate.py b/scripts/acl_validate.py new file mode 100644 index 0000000..9745d00 --- /dev/null +++ b/scripts/acl_validate.py @@ -0,0 +1,232 @@ +#!/usr/bin/env python3 +"""Ask Tailscale to type-check generated/policy.json without applying it. + +``push.py --dry-run`` only diffs the local build against the live ACL. Nothing +in CI asks Tailscale whether the policy it is about to push is *acceptable*, so +a grammar or reference error survives review and first surfaces as a failed CD +push after merge. This closes that gap with + + POST /api/v2/tailnet/{tailnet}/acl/validate + +which validates a policy server-side and applies nothing. + +Response contract, mirroring tailscale.com/cmd/gitops-pusher (testNewACLs): + + - a non-2xx status is a failure; + - a 2xx response whose body carries a non-empty ``message`` or a non-empty + ``data`` array is ALSO a failure -- Tailscale reports policy errors with + HTTP 200; + - an empty body, or ``{}``, is a pass. + +Because "HTTP 200" does not mean "valid" here, ``--prove`` first submits a +policy Tailscale must reject. If that known-bad policy comes back clean, this +checker cannot see failures at all, and we fail loudly rather than report a +pass we have no basis for. +""" + +import argparse +import json +import os +import sys +import urllib.error +import urllib.request +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from ts_auth import resolve_bearer # noqa: E402 + +REPO_ROOT = Path(__file__).resolve().parent.parent +GENERATED_POLICY = REPO_ROOT / "generated" / "policy.json" +TAILNET = "taila4c78d.ts.net" +API_BASE = f"https://api.tailscale.com/api/v2/tailnet/{TAILNET}" + +IN_ACTIONS = os.environ.get("GITHUB_ACTIONS") == "true" + +# Two independent grammar errors: an action Tailscale does not define, and a +# reference to a group that is not declared anywhere. Used by --prove to +# confirm this checker can actually observe a rejection. +KNOWN_BAD_POLICY = { + "acls": [ + { + "action": "definitely-not-a-valid-action", + "src": ["group:tailnet-acl-selftest-undefined-group"], + "dst": ["*:*"], + } + ] +} + + +def _annotate(level: str, title: str, lines: "list[str]") -> None: + sys.stdout.flush() + if IN_ACTIONS: + print(f"::{level} title={title}::{' '.join(lines)}") + print(title, file=sys.stderr) + for line in lines: + print(f" {line}", file=sys.stderr) + + +def post_validate(bearer: str, policy: dict) -> "tuple[int, str]": + """POST a policy to the validate endpoint. Returns (status, body).""" + data = json.dumps(policy).encode("utf-8") + req = urllib.request.Request(f"{API_BASE}/acl/validate", data=data, method="POST") + req.add_header("Authorization", f"Bearer {bearer}") + # gitops-pusher posts a whole policy file with this content type; strict + # JSON is valid HuJSON, so the body below is what upstream would send. + req.add_header("Content-Type", "application/hujson") + req.add_header("Accept", "application/json") + try: + with urllib.request.urlopen(req, timeout=60) as resp: + return resp.status, resp.read().decode(errors="replace") + except urllib.error.HTTPError as exc: + return exc.code, exc.read().decode(errors="replace") + + +def interpret(status: int, body: str) -> "tuple[bool, list[str]]": + """Decide whether a /acl/validate response is a pass, and why not.""" + problems: list[str] = [] + text = body.strip() + parsed = None + + if text: + try: + parsed = json.loads(text) + except json.JSONDecodeError: + problems.append(f"unparseable response body: {text[:500]}") + + if isinstance(parsed, dict): + message = parsed.get("message") or "" + if message: + problems.append(str(message)) + for entry in parsed.get("data") or []: + if not isinstance(entry, dict): + problems.append(str(entry)) + continue + prefix = f"user {entry['user']}: " if entry.get("user") else "" + for err in entry.get("errors") or []: + problems.append(f"{prefix}error: {err}") + for warning in entry.get("warnings") or []: + problems.append(f"{prefix}warning: {warning}") + elif parsed is not None: + problems.append(f"unexpected response shape: {text[:500]}") + + if status // 100 != 2: + problems.append(f"HTTP {status}") + + return (not problems), problems + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--prove", + action="store_true", + help=( + "Before validating the real policy, submit a deliberately invalid one and " + "require that Tailscale rejects it. Guards against reporting a vacuous pass." + ), + ) + args = parser.parse_args() + + secret = os.environ.get("TAILSCALE_API_KEY", "").strip() + if not secret: + _annotate( + "warning", + "Server-side ACL validation SKIPPED", + [ + "TAILSCALE_API_KEY is not set, so the policy was not checked against the " + "Tailscale grammar.", + "In CI this cannot happen: scripts/ci_preflight.py fails the job first. " + "Locally, export TAILSCALE_API_KEY to enable this check.", + ], + ) + return 0 + + if not GENERATED_POLICY.exists(): + _annotate( + "error", + "Nothing to validate", + [f"{GENERATED_POLICY} does not exist. Run 'just build' first."], + ) + return 1 + + try: + bearer = resolve_bearer(secret) + except Exception as exc: # noqa: BLE001 -- surfaced verbatim to the operator + _annotate( + "error", + "Tailscale token could not be resolved", + [str(exc), "See docs/ci-credentials.md."], + ) + return 1 + + if args.prove: + status, body = post_validate(bearer, KNOWN_BAD_POLICY) + ok, problems = interpret(status, body) + if status in (404, 405): + # The endpoint is not available for this tailnet. That is not a + # policy problem and must not fail the build -- but say so loudly, + # because the pre-merge grammar gate is then absent. + _annotate( + "warning", + "Server-side ACL validation UNAVAILABLE", + [ + f"POST {API_BASE}/acl/validate returned HTTP {status}, so this " + "tailnet does not expose the validate endpoint.", + "Skipping the grammar gate. Policy errors will surface at CD push " + "time instead of on the pull request.", + ], + ) + return 0 + if status // 100 != 2: + # A transport or auth failure would also "reject" the known-bad + # policy, which would make the self-test vacuous. Say so plainly + # instead of claiming the validator works. + _annotate( + "error", + "Could not run the ACL validator self-test", + [ + f"The validate endpoint returned HTTP {status}, so the self-test " + "proves nothing about grammar checking.", + f"Body: {body.strip()[:300] or '(empty)'}", + "This is a credential or availability problem, not a policy problem. " + "See docs/ci-credentials.md.", + ], + ) + return 1 + if ok: + _annotate( + "error", + "Server-side ACL validation is blind", + [ + "Tailscale accepted a policy that is deliberately invalid (unknown " + "action plus an undefined group reference), so a clean result from " + "this endpoint proves nothing about the real policy.", + f"Endpoint returned HTTP {status} with body: {body.strip()[:300] or '(empty)'}", + "Refusing to report a pass. Re-check the endpoint contract before " + "trusting this step.", + ], + ) + return 1 + print("Self-test OK: the known-bad policy was rejected as expected.") + print(f" rejection: {problems[0][:300]}") + + policy = json.loads(GENERATED_POLICY.read_text(encoding="utf-8")) + print(f"Validating generated/policy.json against {TAILNET} ...") + status, body = post_validate(bearer, policy) + ok, problems = interpret(status, body) + + if ok: + print(f"Server-side validation PASSED (HTTP {status}).") + return 0 + + _annotate( + "error", + "Tailscale rejected the generated policy", + [f"HTTP {status}."] + problems, + ) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/ci_preflight.py b/scripts/ci_preflight.py new file mode 100644 index 0000000..664cb29 --- /dev/null +++ b/scripts/ci_preflight.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +"""Fail fast, and legibly, when the Tailscale credential wiring is wrong. + +This runs on the runner's system python3 before the Nix toolchain is installed, +so a missing or dead credential costs seconds rather than a full dev-shell +build followed by an opaque ``API error 401`` a few minutes later. + +Every failure names the exact secret or variable, and the exact scope it has to +live in, so a rotation lands in the right place on the first attempt. + +No credential value is ever printed -- only its kind, inferred from the prefix. +""" + +import os +import sys +import urllib.error +import urllib.request +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from ts_auth import resolve_bearer # noqa: E402 + +TAILNET = "taila4c78d.ts.net" +API_BASE = f"https://api.tailscale.com/api/v2/tailnet/{TAILNET}" + +SECRET_NAME = "TAILSCALE_API_KEY" +CLIENT_ID_VAR = "TS_OAUTH_CLIENT_ID" + +# The CI `validate` job and the CD `deploy` job both declare +# `environment: production`, so both resolve their credentials from this one +# scope. Keep this string in sync with the `environment:` key in +# .github/workflows/ci.yml and .github/workflows/cd.yml. +SCOPE = 'the "production" environment (repo Settings -> Environments -> production)' + +IN_ACTIONS = os.environ.get("GITHUB_ACTIONS") == "true" + + +def _annotate(level: str, title: str, lines: "list[str]") -> None: + """Emit a GitHub annotation (single line) plus readable stderr output.""" + sys.stdout.flush() + if IN_ACTIONS: + # Annotation bodies cannot contain raw newlines. + print(f"::{level} title={title}::{' '.join(lines)}") + print(f"{title}", file=sys.stderr) + for line in lines: + print(f" {line}", file=sys.stderr) + + +def fail(title: str, *lines: str) -> int: + _annotate("error", title, list(lines)) + return 1 + + +def warn(title: str, *lines: str) -> None: + _annotate("warning", title, list(lines)) + + +def main() -> int: + secret = os.environ.get(SECRET_NAME, "").strip() + client_id = os.environ.get(CLIENT_ID_VAR, "").strip() + + if not secret: + return fail( + "Tailscale credential is not wired", + f"{SECRET_NAME} is unset or empty in this job.", + f"Create it as an ENVIRONMENT secret named {SECRET_NAME} in {SCOPE}.", + "A repository-level secret of the same name is NOT what this job reads: " + "the job declares `environment: production`, and an environment secret of " + "the same name shadows the repository secret.", + ) + + if secret.startswith("tskey-client-"): + kind = "OAuth client secret (tskey-client-...)" + if not client_id: + return fail( + "Tailscale OAuth client id is missing", + f"{SECRET_NAME} holds an OAuth client secret, which cannot be used as a " + "bearer token on its own -- it has to be exchanged for an access token, " + "and that exchange requires the client id.", + f"Set a variable (not a secret; the client id is not sensitive) named " + f"{CLIENT_ID_VAR} in {SCOPE}, under 'Environment variables'.", + f"Alternatively, swap {SECRET_NAME} back to a direct API key " + "(tskey-api-...), which needs no client id.", + ) + elif secret.startswith("tskey-api-"): + kind = "direct API key (tskey-api-...)" + if client_id: + warn( + "TS_OAUTH_CLIENT_ID is set but unused", + f"{SECRET_NAME} holds a direct API key, so {CLIENT_ID_VAR} is ignored. " + "This is harmless, but it means the OAuth path is untested.", + ) + else: + kind = "unrecognised prefix" + warn( + "Tailscale credential has an unrecognised prefix", + f"{SECRET_NAME} starts with neither 'tskey-api-' nor 'tskey-client-'. " + "This is usually a truncated or mis-pasted value. The live probe below is " + "the authority.", + ) + + print(f"Credential scope: {SCOPE}") + print(f"Credential kind: {kind}") + print(f"{CLIENT_ID_VAR}: {'set' if client_id else 'unset'}") + + try: + bearer = resolve_bearer(secret) + except urllib.error.HTTPError as exc: + detail = exc.read().decode(errors="replace")[:400] + return fail( + "Tailscale OAuth token exchange failed", + f"The client_credentials grant returned HTTP {exc.code}: {detail}", + f"Check that {CLIENT_ID_VAR} in {SCOPE} is the client id belonging to the " + f"client secret currently stored in {SECRET_NAME} -- a mismatched pair fails " + "here.", + ) + except Exception as exc: # noqa: BLE001 -- surfaced verbatim to the operator + return fail("Tailscale token could not be resolved", str(exc)) + + req = urllib.request.Request(f"{API_BASE}/acl") + req.add_header("Authorization", f"Bearer {bearer}") + req.add_header("Accept", "application/json") + try: + with urllib.request.urlopen(req, timeout=30) as resp: + resp.read() + status = resp.status + except urllib.error.HTTPError as exc: + detail = exc.read().decode(errors="replace")[:400] + if exc.code == 401: + return fail( + "Tailscale credential is present but rejected (HTTP 401)", + f"The API returned: {detail}", + f"Rotate the value of the {SECRET_NAME} environment secret in {SCOPE}.", + "Direct API keys (tskey-api-...) expire after at most 90 days. An " + "ACL-scoped OAuth client secret (tskey-client-...) does not expire and is " + f"the preferred replacement, but it additionally needs its client id in " + f"the {CLIENT_ID_VAR} variable in {SCOPE}.", + "See docs/ci-credentials.md for the rotation runbook.", + ) + if exc.code == 403: + return fail( + "Tailscale credential is valid but lacks policy-file permission (HTTP 403)", + f"The API returned: {detail}", + "The API key or OAuth client must carry the 'policy_file' scope " + "(Tailscale admin console -> Settings -> Keys / OAuth clients).", + ) + return fail( + f"Tailscale API probe failed (HTTP {exc.code})", + detail, + f"Probed {API_BASE}/acl.", + ) + except urllib.error.URLError as exc: + return fail("Could not reach the Tailscale API", str(exc.reason)) + + print(f"Preflight OK: the credential authenticates against {TAILNET} (HTTP {status}).") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_ci_credential_wiring.py b/tests/test_ci_credential_wiring.py new file mode 100644 index 0000000..22ae7c4 --- /dev/null +++ b/tests/test_ci_credential_wiring.py @@ -0,0 +1,208 @@ +"""Contract tests for the CI/CD credential wiring. + +Two things are pinned here: + +1. The response contract of Tailscale's ``POST /acl/validate`` endpoint, which + reports policy errors with **HTTP 200**. A status-code check alone passes + everything, so ``acl_validate.interpret`` is the load-bearing logic and it + cannot be exercised live from a test. +2. That ``validate`` (ci.yml) and ``deploy`` (cd.yml) read their credentials + from one scope and one variable set. Splitting them again is the specific + regression this repo already suffered. +""" + +import json +import os +import re +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import acl_validate # noqa: E402 + + +class ValidateResponseContractTest(unittest.TestCase): + def test_empty_body_is_a_pass(self) -> None: + ok, problems = acl_validate.interpret(200, "") + self.assertTrue(ok) + self.assertEqual(problems, []) + + def test_empty_object_is_a_pass(self) -> None: + ok, problems = acl_validate.interpret(200, "{}") + self.assertTrue(ok) + self.assertEqual(problems, []) + + def test_message_on_http_200_is_a_failure(self) -> None: + # The trap: Tailscale reports grammar errors with a 200 status. + ok, problems = acl_validate.interpret( + 200, '{"message": "line 3, column 5: unknown action"}' + ) + self.assertFalse(ok) + self.assertIn("line 3, column 5: unknown action", problems) + + def test_data_errors_on_http_200_are_a_failure(self) -> None: + ok, problems = acl_validate.interpret( + 200, '{"data": [{"user": "alice", "errors": ["cannot reach tag:x"]}]}' + ) + self.assertFalse(ok) + self.assertIn("user alice: error: cannot reach tag:x", problems) + + def test_data_warnings_on_http_200_are_a_failure(self) -> None: + # Parity with tailscale.com/cmd/gitops-pusher, which fails on any + # non-empty data array. + ok, problems = acl_validate.interpret( + 200, '{"data": [{"user": "bob", "warnings": ["unused group"]}]}' + ) + self.assertFalse(ok) + self.assertIn("user bob: warning: unused group", problems) + + def test_unauthorized_is_a_failure(self) -> None: + ok, problems = acl_validate.interpret(401, '{"message": "API token invalid"}') + self.assertFalse(ok) + self.assertIn("API token invalid", problems) + self.assertIn("HTTP 401", problems) + + def test_non_2xx_with_empty_body_is_still_a_failure(self) -> None: + ok, problems = acl_validate.interpret(503, "") + self.assertFalse(ok) + self.assertEqual(problems, ["HTTP 503"]) + + def test_unparseable_body_is_a_failure(self) -> None: + ok, problems = acl_validate.interpret(200, "gateway") + self.assertFalse(ok) + self.assertTrue(any("unparseable" in p for p in problems)) + + def test_known_bad_policy_is_actually_malformed(self) -> None: + # --prove is only meaningful if the probe policy really is invalid. + rule = acl_validate.KNOWN_BAD_POLICY["acls"][0] + self.assertNotIn(rule["action"], {"accept", "check"}) + self.assertTrue(rule["src"][0].startswith("group:")) + self.assertIn("undefined", rule["src"][0]) + + +class ProveGateTest(unittest.TestCase): + """The --prove self-test decides whether a clean result may be trusted.""" + + def _run(self, responses: "list[tuple[int, str]]") -> "tuple[int, int]": + calls = {"n": 0} + + def fake_post(_bearer: str, _policy: dict) -> "tuple[int, str]": + response = responses[calls["n"]] + calls["n"] += 1 + return response + + with tempfile.TemporaryDirectory() as tmp: + policy_path = Path(tmp) / "policy.json" + policy_path.write_text(json.dumps({"acls": []}), encoding="utf-8") + with mock.patch.object(acl_validate, "post_validate", fake_post), mock.patch.object( + acl_validate, "resolve_bearer", lambda secret: "bearer" + ), mock.patch.object( + acl_validate, "GENERATED_POLICY", policy_path + ), mock.patch.dict( + os.environ, {"TAILSCALE_API_KEY": "tskey-api-stub"}, clear=False + ), mock.patch.object( + sys, "argv", ["acl_validate.py", "--prove"] + ): + return acl_validate.main(), calls["n"] + + def test_known_bad_rejected_then_real_policy_clean_passes(self) -> None: + code, calls = self._run( + [(200, '{"message": "group:... is not defined"}'), (200, "")] + ) + self.assertEqual(code, 0) + self.assertEqual(calls, 2) + + def test_known_bad_accepted_means_blind_and_fails(self) -> None: + # The endpoint said a deliberately invalid policy is fine. A clean + # result on the real policy would then be worthless. + code, calls = self._run([(200, "{}")]) + self.assertEqual(code, 1) + self.assertEqual(calls, 1, "must not go on to validate the real policy") + + def test_endpoint_absent_skips_without_failing(self) -> None: + code, calls = self._run([(404, '{"message": "404 page not found"}')]) + self.assertEqual(code, 0) + self.assertEqual(calls, 1) + + def test_auth_failure_during_self_test_fails(self) -> None: + code, calls = self._run([(401, '{"message": "API token invalid"}')]) + self.assertEqual(code, 1) + self.assertEqual(calls, 1) + + def test_real_policy_rejection_fails(self) -> None: + code, calls = self._run( + [ + (200, '{"message": "unknown action"}'), + (200, '{"message": "line 12, column 3: tag:nope is not defined"}'), + ] + ) + self.assertEqual(code, 1) + self.assertEqual(calls, 2) + + +class WorkflowScopeTest(unittest.TestCase): + CI = REPO_ROOT / ".github" / "workflows" / "ci.yml" + CD = REPO_ROOT / ".github" / "workflows" / "cd.yml" + + @classmethod + def setUpClass(cls) -> None: + cls.ci_text = cls.CI.read_text(encoding="utf-8") + cls.cd_text = cls.CD.read_text(encoding="utf-8") + + def test_both_jobs_declare_the_production_environment(self) -> None: + for name, text in (("ci.yml", self.ci_text), ("cd.yml", self.cd_text)): + with self.subTest(workflow=name): + self.assertIn("environment: production", text) + + def test_both_jobs_map_the_same_credential_pair(self) -> None: + secret = "TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }}" + client_id = "TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }}" + for name, text in (("ci.yml", self.ci_text), ("cd.yml", self.cd_text)): + with self.subTest(workflow=name): + self.assertIn(secret, text) + self.assertIn(client_id, text) + + def test_credentials_are_mapped_exactly_once_per_workflow(self) -> None: + # Job-level env only. A per-step remap is how the two scopes drifted + # apart in the first place. + for name, text in (("ci.yml", self.ci_text), ("cd.yml", self.cd_text)): + with self.subTest(workflow=name): + self.assertEqual( + len(re.findall(r"secrets\.TAILSCALE_API_KEY", text)), 1 + ) + self.assertEqual( + len(re.findall(r"vars\.TS_OAUTH_CLIENT_ID", text)), 1 + ) + + def test_preflight_runs_in_both_workflows(self) -> None: + for name, text in (("ci.yml", self.ci_text), ("cd.yml", self.cd_text)): + with self.subTest(workflow=name): + self.assertIn("scripts/ci_preflight.py", text) + + def test_server_side_validation_runs_with_prove_in_both_workflows(self) -> None: + for name, text in (("ci.yml", self.ci_text), ("cd.yml", self.cd_text)): + with self.subTest(workflow=name): + self.assertIn("scripts/acl_validate.py --prove", text) + + def test_pr_comment_steps_are_gated_on_a_successful_build(self) -> None: + # A bare `if: always()` here posts a PR comment whose whole content is + # "nix: command not found" whenever the preflight fails before Nix is + # installed, which is now the common failure mode. + self.assertEqual( + len(re.findall(r"steps\.build\.outcome == 'success'", self.ci_text)), 2 + ) + self.assertNotIn(" if: always()\n", self.ci_text) + + def test_preflight_scope_string_matches_the_declared_environment(self) -> None: + import ci_preflight + + self.assertIn("production", ci_preflight.SCOPE) + + +if __name__ == "__main__": + unittest.main()