Skip to content

fix(ci): read Tailscale credentials from one scope, fail fast, validate server-side - #23

Merged
Jesssullivan merged 2 commits into
mainfrom
fix/ci-credential-wiring
Aug 22, 2026
Merged

fix(ci): read Tailscale credentials from one scope, fail fast, validate server-side#23
Jesssullivan merged 2 commits into
mainfrom
fix/ci-credential-wiring

Conversation

@Jesssullivan

@Jesssullivan Jesssullivan commented Aug 22, 2026

Copy link
Copy Markdown
Owner

The two-scope trap

TAILSCALE_API_KEY exists twice, under one name, in two scopes:

Scope Value last set Read by Status
Repository secret validate in ci.yml dead — 401
production environment secret 2026-06-24 deploy in cd.yml (it declares environment: production) dead — 401

GitHub resolves secrets.X from the job's environment first and silently falls
back to the repository scope. Nothing in the logs distinguishes the two. The
consequences, observed rather than theorised:

  • The repo-scoped key is dead. CI run
    32587939240
    on feat(acl): grant node_exporter egress proxies scrape access (TIN-3970) #22: API error 401: {"message":"API token invalid"}.
  • The env-scoped key was last known-good at the CD push of 2026-07-27 (run
    30311167443, success) and had not been exercised since.
  • So a red validate was never evidence about the deploy path, and a rotation
    applied to one scope leaves the other dead with no signal.

The first run of this branch found something worse

Because validate now reads the production scope, this PR's own CI is the
first thing in four weeks to test the deploy credential. It is also dead
(run 32596036086,
step 4, 5 seconds in):

Tailscale credential is present but rejected (HTTP 401)
  The API returned: {"message":"API token invalid"}
  Rotate the value of the TAILSCALE_API_KEY environment secret in the
  "production" environment (repo Settings -> Environments -> production).
  ...
Credential kind:  direct API key (tskey-api-...)
TS_OAUTH_CLIENT_ID:   unset

Both scopes are dead. The next merge to main would have failed its CD push
regardless of this PR — the old wiring simply had no way to tell you until after
the merge. That is the whole argument for this change, and it is why the red
check on this PR is the intended result rather than a defect: the failure is
five seconds in, names the secret, the scope and the fix, and skips the Nix
build entirely.

There is a second, latent break. cd.yml referenced vars.TS_OAUTH_CLIENT_ID,
but at the time of writing:

repository variables            total_count = 0
production env variables        total_count = 0

It is set nowhere. scripts/ts_auth.py raises RuntimeError when
TAILSCALE_API_KEY holds an OAuth client secret (tskey-client-) and no client
id is present — so the moment anyone swaps the secret to the non-expiring OAuth
form (which #15 and #16 built the support for), the push fails. ci.yml never
mapped the variable at all, so the OAuth path was unreachable from the PR side
even in principle.

What this changes

1. One scope, one credential set. Both validate and deploy now declare
environment: production and map the pair once, at job level:

    environment: production
    env:
      TAILSCALE_API_KEY: ${{ secrets.TAILSCALE_API_KEY }}
      TS_OAUTH_CLIENT_ID: ${{ vars.TS_OAUTH_CLIENT_ID }}

A green validate is now evidence about the credential the deploy will use,
because it is the same credential. Per-step env: blocks are removed — that
scattering is how the scopes drifted apart, and a contract test now pins the
mapping to exactly one occurrence per workflow.

2. Fail-fast preflight (scripts/ci_preflight.py), first step in both jobs,
before the Nix install so a dead credential costs seconds instead of minutes.
Every failure names the exact secret or variable and the scope it must live
in
, so a rotation lands correctly first try. It ends with a live auth probe,
so an expired key reports as "rotate this, here" instead of an API error 401
buried four minutes into a build. It never prints a credential value — only the
kind inferred from the prefix.

All four failure paths were exercised against the real API before opening this
PR:

Input Output
unset Tailscale credential is not wired + which scope to create it in
tskey-client-…, no client id Tailscale OAuth client id is missing + "variable, not a secret"
mismatched OAuth pair Tailscale OAuth token exchange failed (real HTTP 401 from the grant)
dead tskey-api-… …present but rejected (HTTP 401) + rotation instructions (real 401)

3. Pre-merge server-side policy validation (scripts/acl_validate.py).
Nothing validated policy grammar before merge: push.py --dry-run only diffs
the local build against live and never asks whether the result is legal. This
POSTs the built policy to POST /api/v2/tailnet/{tailnet}/acl/validate, which
type-checks without applying.

The endpoint has a trap worth stating plainly: it reports policy errors with
HTTP 200
and a body carrying message / data. A status-code check passes
everything. The implementation follows
cmd/gitops-pusher
testNewACLs exactly — non-empty message or data is a failure regardless of
status, and the same application/hujson content type.

Because "200 means pass" is false here, --prove first submits a policy
Tailscale must reject (unknown action and 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. Graceful gates:

  • creds absent → loud ::warning::, skip (can only happen locally; in CI the
    preflight has already failed the job);
  • endpoint returns 404/405 → loud warning, skip (not a policy problem);
  • auth failure during the self-test → hard fail, framed as a credential
    problem, not reported as a validation pass;
  • grammar rejection → hard fail.

4. docs/ci-credentials.md — the trap, both credential kinds, the rotation
runbook, the preflight message → fix table, and the caveats of putting
environment: on a PR job.

5. Diff summary and Comment on PR are gated on
steps.build.outcome == 'success'. With a bare if: always() they still ran
after a fast preflight failure and posted a PR comment whose entire content was
nix: command not found — noise that reads like a policy problem. Found by this
branch's own first run; fixed in the second commit.

What the operator must rotate, and where

This PR does not fix the 401. It makes the 401 legible and puts both jobs
behind one credential. The rotation is yours.

Preferred — OAuth pair (non-expiring)

  1. Tailscale admin → Settings → OAuth clients → Generate OAuth client, scope
    policy_file (read and write; CD pushes).
  2. GitHub → Settings → Environments → production:
    • Environment secretsTAILSCALE_API_KEY = the client secret
      (tskey-client-…)
    • Environment variablesTS_OAUTH_CLIENT_ID = the client id
      (a variable, not a secret — the client id is not sensitive)
  3. Delete the old OAuth client in Tailscale.

Interim — direct API key

  • Settings → Environments → production → Environment secrets
    TAILSCALE_API_KEY = tskey-api-…. No client id needed. Expires in ≤ 90
    days
    , i.e. it schedules the next outage.

Either way

  • Delete the repository-scoped TAILSCALE_API_KEY. After this PR nothing
    reads it, and leaving it in place preserves the exact trap this PR removes —
    a future rotation could land on the dead copy again.
  • Do not add required reviewers or a deployment-branch policy to the
    production environment: validate would then block on every PR. If deploy
    approval is ever wanted, split the environments and update the SCOPE string
    in scripts/ci_preflight.py in the same change.

Validation

check result
actionlint .github/workflows/*.yml exit 0
YAML parse, job/env structure asserted both jobs environment: production, identical env keys
python3 -m unittest discover -s tests 24 tests, OK (3 pre-existing + 21 new)
preflight failure paths all four exercised against the live API
acl_validate skip / blind / auth-failure paths exercised
live CI run on this branch preflight fired at step 4 in 5s; Nix install, build and validate correctly skipped
.dhall, grants.json, justfile, flake.nix untouched — no policy change

actionlint caught a real bug during authoring: ${{ runner.temp }} is not
available in job-level env. XDG_CACHE_HOME therefore moves to a one-line
Configure cache dir step using $RUNNER_TEMP, which also removes the
per-step repetition.

The 20 new tests were mutation-tested — each re-run against a deliberately
broken tree to prove it can fail rather than pass vacuously:

mutation result
drop environment: production from ci.yml FAIL
re-add a per-step secrets.TAILSCALE_API_KEY mapping FAIL
drop --prove from cd.yml FAIL
replace interpret() with a status-code-only check FAIL (6)
make KNOWN_BAD_POLICY valid FAIL
remove the preflight step FAIL
remove the blind-validator guard FAIL
make an absent endpoint hard-fail FAIL
make --prove a no-op FAIL (4)
revert the steps.build.outcome gate to bare if: always() FAIL

Restored, all 24 pass.

Not verified here

The happy path — a valid credential returning HTTP 200 with a clean body —
could not be exercised: no working Tailscale credential is available outside
CI. The first green validate run after rotation is the proof, and --prove
exists precisely so that run cannot be a vacuous green.

Ordering against #22

#22 is correct on its merits and unrelated to this. But its validate job is
red for a credential reason, so merging it would push a policy through a lane
nothing has confirmed. Suggested order:

  1. Merge this PR.
  2. Rotate per the table above — required either way, since the deploy
    credential is dead and the next CD push would fail without it.
  3. Re-run CI on feat(acl): grant node_exporter egress proxies scrape access (TIN-3970) #22validate should go green, and that green now means the
    deploy credential works.
  4. Merge feat(acl): grant node_exporter egress proxies scrape access (TIN-3970) #22; CD pushes the grant.

Not merging

This repo drives the live tailnet. Held for operator review and merge.

Refs: #15, #16, #22

…te server-side

The CI `validate` job read the repository-scoped TAILSCALE_API_KEY while the CD
`deploy` job, declaring `environment: production`, read the environment-scoped
secret of the same name. Two different values behind one name, with nothing to
show they differ: a red validate said nothing about the deploy path, and a
rotation applied to one scope silently left the other dead.

cd.yml also referenced `vars.TS_OAUTH_CLIENT_ID`, which is set in neither scope
(repo variables and production environment variables are both empty), so any
swap of the secret to an OAuth client secret would have failed the push with a
RuntimeError from ts_auth.resolve_bearer. ci.yml never mapped the variable at
all, so the OAuth path was unreachable from the pull request side.

- Both jobs now declare `environment: production` and map TAILSCALE_API_KEY and
  TS_OAUTH_CLIENT_ID exactly once, at job level. A green validate is now
  evidence about the credential the deploy will use.
- scripts/ci_preflight.py runs before the Nix install and fails with a message
  naming the exact secret or variable and the scope it must live in. It ends
  with a live auth probe, so an expired key reports as "rotate this, here"
  rather than an opaque `API error 401` several minutes into the job.
- scripts/acl_validate.py adds the missing pre-merge gate: Tailscale
  type-checks the built policy via POST /acl/validate, which nothing did
  before (push.py --dry-run only diffs local against live). That endpoint
  reports policy errors with HTTP 200, so --prove first submits a policy the
  server must reject and refuses to claim a pass if it comes back clean.
- docs/ci-credentials.md records the two-scope trap, both credential kinds and
  the rotation runbook for each.

XDG_CACHE_HOME moves to a `Configure cache dir` step because the `runner`
context is not available in job-level `env` (caught by actionlint).

No policy change: no .dhall file and no grants.json entry is touched.
The first run of this branch proved the preflight works -- and exposed a
wart. `Diff summary` and `Comment on PR` carried a bare `if: always()`, so when
the preflight fails before the Nix install they still ran, and posted a PR
comment whose entire content was:

    /home/runner/work/_temp/....sh: line 2: nix: command not found

Gate both on `steps.build.outcome == 'success'`. Fast-failing on a dead
credential now leaves the PR clean instead of adding noise that reads like a
policy problem.

Pinned by a contract test that also refuses a bare `if: always()` in ci.yml.
Repository owner deleted a comment from github-actions Bot Aug 22, 2026
@Jesssullivan
Jesssullivan merged commit d188c8e into main Aug 22, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant