Audit the environment scope the deploy credentials live in - #22
Conversation
The secrets audit read repository scope alone, which is where the merge bot's credentials live and not where the deploy's do, so an unconfigured deploy passed it. It now asserts both, including that a scoped credential is absent from the environment that must not carry it. The deploy root is no longer declared, since the rsync destination is anchored at the key's confinement root and a declared-but-unread name is drift an audit cannot tell from a missing one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens the repo’s operational audit guidance by explicitly covering environment-scoped deploy credentials (in addition to repository-scoped secrets), and updates the secrets manifest to remove a previously declared-but-unread deploy-root variable.
Changes:
- Extend
AUDIT.mdto describe and enumerate environment-scoped secrets/variables and the required assertions. - Update
spec/secrets.jsonto stop declaringDEPLOY_ROOTand document why it is intentionally absent. - Add a “silent clean pass from empty query results” trap reminder to
TODO.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| TODO.md | Adds a reminder about empty query results falsely reading as “clean”. |
| spec/secrets.json | Removes DEPLOY_ROOT from declared environment variables and adds an explanatory note. |
| AUDIT.md | Adds environment-scope audit guidance and example commands for enumerating environment secrets/variables. |
A guard written in a for header never fires, because command substitution discards the exit status, so the lookup is assigned before it is iterated. The manifest note described the environment split as carrying a deploy root the manifest no longer declares. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
AUDIT.md:62
- The Environment scope section says
gh secret listdoes not reach environment-scoped credentials, butgh secret list --env <env>(andgh variable list --env <env>) can list them. Also, thegh apicalls as written will only fetch the first page of secrets/variables, which can silently truncate results and undermine the “assert against spec” goal if the list ever exceeds the default page size.
`configure.sh check` does not reach these, and neither does `gh secret list`. Assert them against `spec/secrets.json`:
```sh
# Read the lists first, so a moved key fails here rather than emptying the loop below.
envs=$(jq -e -r '.environments.names[]' spec/secrets.json) || exit 1
The section claimed a listing could not reach environment scope, which it can with an environment flag, and the raw API calls read one page. The listing prints every variable's value unless names are requested, so an audit run wrote the deploy endpoint into its own output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both halves correct. Fixed in fd8f8b0.
Pagination was a real risk, and the fix removes the question rather than answering it. Checking that also surfaced something neither of us raised: the bare Verified by running the documented commands verbatim: staging 3 secrets and 4 variables, production 1 and 4. |
Why
The secrets audit read repository scope only. That is where the merge bot's credentials live, and it is correct for them, but the deploy's credentials are environment-scoped. So the audit passed cleanly over a completely unconfigured deploy, and this is the only repo in the fleet where that gap exists.
configure.sh checkdoes not reach environment scope either, and it says so: it defers the secrets question to a manual verification.What it now asserts
Both scopes, separately. The environment half carries three assertions, and the third is the one presence-checking misses:
environments.secretsandenvironments.variablesname is present in every environment.environmentSecrets.<env>name is present in that environment.productionholding a Pangolin access token is a finding rather than a harmless extra, because production answers unauthenticated and a token there means a check could pass through a gate production is not supposed to have.A finding from writing it
spec/secrets.jsondeclaredDEPLOY_ROOTas an environment variable. Neither environment has it and the workflow never reads it, because the rsync destination is anchored at the deploy key's confinement root and names an environment rather than a host path. A declared-but-unread name is drift an audit cannot distinguish from a missing one, so it is no longer declared.A second finding, from running it
My first version of the check used
.environmentSecretswhere the key is.environments.environmentSecrets. The path resolved to null, the loop ran zero times, and the check reported clean. That is the third false pass of this shape here, so every lookup is nowjq -eand the pattern is recorded in the traps.Verification
Run against the live configuration: staging carries the token pair, production correctly does not, and all shared names are present in both.
🤖 Generated with Claude Code