Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion AUDIT.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,45 @@ Two facts specific to this repo:

Names only. Never read, print, or log a secret value.

Two scopes, checked separately, because a name present in one is not present in the other. The **repository** scope carries the merge bot's credentials, and the **environment** scope carries the deploy's. This is the only repo in the fleet whose publishing credentials are environment-scoped, so a check written for repository secrets alone reports a clean pass over an unconfigured deploy.

### Repository scope

```sh
gh secret list --repo ptr727/Blog
gh secret list --repo ptr727/Blog --app dependabot
```

Assert that every name under `baseline.requires` is present in both stores, and that every name under `baseline.forbids` is absent. `CODEGEN_APP_ID` is forbidden: the App-token action takes `client-id`, and the deprecated `app-id` name silently does nothing.

The deploy credentials live in the `staging` and `production` GitHub Environments rather than in repository secrets, so a staging deploy cannot reach production. They are outside the baseline audit.
### Environment scope

`configure.sh check` does not reach these and says so, deferring the secrets question to a manual verification. 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
jq -e '.environments.environmentSecrets' spec/secrets.json > /dev/null || exit 1

for env in $envs; do
gh secret list --repo ptr727/Blog --env "$env" --json name --jq '.[].name'
gh variable list --repo ptr727/Blog --env "$env" --json name --jq '.[].name'
done
```

`--json name` is not decoration. The bare `gh variable list` prints a value column, and when its output is captured rather than shown it prints each value in full, so an audit run without it writes the deploy endpoint into its own log. Never request the `value` field here, and never use `gh variable view`, which prints one by design.

**Assert the query matched before reading what it returned.** A `jq` path that no longer resolves yields nothing, a loop over nothing runs zero times, and a check that counts failures reports none. Every lookup is `jq -e`, which exits non-zero on a null or missing key, and each is **assigned before it is iterated**: command substitution in a `for` header discards the exit status, so a guard written there is a guard that never fires.

Three assertions, and the third is the one presence-checking misses:

- Every name under `environments.secrets` and `environments.variables` is present in **every** environment named in `environments.names`.
- Every name under `environmentSecrets.<env>` is present in that environment.
- A name under `environmentSecrets` is **absent** from an environment that does not list it. `production` holding a Pangolin access token is a finding rather than a harmless extra: production answers unauthenticated, so a token there means a check could pass through a gate production is not supposed to have.

A **declared but unset** name is drift in the same way an undeclared one is. The deploy root is deliberately not declared, because the rsync destination is anchored at the deploy key's confinement root and the workflow names an environment rather than a host path.

Never read, print, or log a value. Every command above lists names.

## 3. The URL Contract

Expand Down
7 changes: 5 additions & 2 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Each of these was hit or nearly hit, and each is cheap to re-trip.
- **Do not name any workflow `build-*-task.yml`** while the repo declares `source-only`, since `detect` is literally `["no build-*-task.yml"]`.
- **Do not edit `.markdownlint-cli2.jsonc`, `repo-config/configure.sh`, or the two ruleset payloads.** They are carried verbatim and byte-matched against the hub. Scope a glob in the workflow instead. A reviewer finding a real defect in one of them is answered by declining locally and filing it at the hub, never by editing the file to satisfy the review.
- **The hub's `main` can promote while a convergence pull request is open**, so ground truth moves underneath work that was correct when it started. It happened twice in one session on 2026-08-03, and the second time added drift the branch could not have known about. Re-run the audit against the hub ref actually carried before claiming convergence, and name that ref in the change, or the claim ages into a false one.
- **A query that matches nothing reads as a clean result.** It has cost three separate false passes: a review-thread poll that could not see suppressed findings, a reviewer filter written in the wrong API's login form, and an audit loop whose `jq` path had moved. Each returned empty, and empty looked like nothing to report. Assert the query matched before reading what it returned, which is what `jq -e` and a non-empty check are for.
- **The hub authors `scripts/pr_review.py`, and hand-rolling the review loop re-discovers its bugs.** One `status` call reports rounds, head coverage, unresolved threads, suppressed findings across every round, and whether a request was ever picked up. `wait` runs the backoff in-process, so a review wait costs one turn rather than one per poll. It is read-only by design and the mutations stay explicit, so fetch and run it rather than reimplementing it. Its README documents the traps below as the reason it exists.
- **A review request can sit forever without being picked up, which looks exactly like patience.** Copilot raises a `copilot_work_started` timeline event within about half a minute of accepting; a request that never draws one is not slow, it is inert, and elapsed time cannot tell them apart. The event is REST-only. Recover by clearing the request with `union: false` and an empty `botIds`, then requesting again, after reading the pending set so a human reviewer is not dropped.
- **The Copilot reviewer's login differs by API, and a wrong-form filter reads as a clean review.** REST reports `copilot-pull-request-reviewer[bot]`, GraphQL omits the suffix. A filter written in the other form matches nothing, and an empty result is indistinguishable from no findings. Assert the filter matched before trusting what it returned.
Expand All @@ -125,11 +126,13 @@ Secrets and variables, per environment. The App-token pair is repository-scoped
| --- | --- |
| `DEPLOY_SSH_PRIVATE_KEY` | secret |
| `DEPLOY_SSH_HOST`, `DEPLOY_SSH_USER`, `DEPLOY_SSH_KNOWN_HOSTS` | variable |
| `DEPLOY_ROOT`, `HUGO_BASEURL` | variable |
| `HUGO_BASEURL` | variable |
| `PANGOLIN_ACCESS_TOKEN_ID`, `PANGOLIN_ACCESS_TOKEN` | secret, staging only |
| `CODEGEN_APP_CLIENT_ID`, `CODEGEN_APP_PRIVATE_KEY` | secret, both stores |

`DEPLOY_SSH_PRIVATE_KEY` now holds the same key in both environments, per the decision above. The environment split still carries the deploy root, the base URL, and the staging-only token pair, so it is not decorative.
`DEPLOY_SSH_PRIVATE_KEY` holds the same key in both environments, per the decision above. The environment split still carries the base URL, the SSH endpoint, and the staging-only token pair, so it is not decorative.

The deploy root is deliberately absent from this table. The rsync destination is anchored at the deploy key's confinement root, so the workflow names an environment rather than a host path, and a declared-but-unread name is drift no audit can tell from a missing one. The local `DEPLOY_ROOT` in `secrets/<environment>.env` is a different value and is still read.

<!-- Repo -->

Expand Down
6 changes: 3 additions & 3 deletions spec/secrets.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"note": "The App-token secrets power the App-signed merge-bot (auto-merge that re-triggers downstream workflows), which every fleet repo runs. Used via actions/create-github-app-token with the client-id input (not the deprecated app-id). The CODEGEN_* name is historical, not codegen-specific."
},
"environments": {
"note": "Not part of the baseline audit. One key covers both environments, a deliberate decision recorded in OPERATIONS.md: the per-environment split only pays where the two keys never share a machine, and both sit on one workstation and in one secret store. The environment split still carries the deploy root, the base URL, and the staging-only access token, so it is not decorative.",
"note": "Not part of the baseline audit. One key covers both environments, a deliberate decision recorded in OPERATIONS.md: the per-environment split only pays where the two keys never share a machine, and both sit on one workstation and in one secret store. The split still carries the base URL, the SSH endpoint, and the staging-only access token, so it is not decorative.",
"names": [
"staging",
"production"
Expand All @@ -30,7 +30,6 @@
"DEPLOY_SSH_HOST",
"DEPLOY_SSH_USER",
"DEPLOY_SSH_KNOWN_HOSTS",
"DEPLOY_ROOT",
"HUGO_BASEURL"
],
"secretsNote": "The 'secrets' and 'variables' lists are required in every environment named above. 'environmentSecrets' names what one environment carries and another does not, so a name audit does not read a staging-only credential as missing from production. Staging keeps its auth gate on and production answers unauthenticated, so the access token exists on staging alone and checks/check-live-urls.sh sends no credential where the pair is absent.",
Expand All @@ -40,6 +39,7 @@
"PANGOLIN_ACCESS_TOKEN"
],
"production": []
}
},
"variablesNote": "The deploy root is deliberately absent. The rsync destination is anchored at the deploy key's confinement root, so the workflow names the environment rather than a host path, and a declared-but-unread name is drift an audit cannot distinguish from a missing one."
Comment thread
ptr727 marked this conversation as resolved.
}
}