Repository navigation
fix(ci): keep the dispatch PAT out of the job that runs panic-attack - #968
Conversation
estate-rescan built panic-attack from an unpinned HEAD and ran it over ~300 repositories in the same job that held HYPATIA_DISPATCH_PAT. Worse, `Checkout verisimdb-data` cloned with the PAT in the URL, so it sat in that clone's .git/config, readable by every later step, the freshly built scanner included. Split into three jobs: - preflight: the writability probe. Holds the PAT, runs only `gh api`, harden-runner with egress blocked to api.github.com. - scan: builds panic-attack pinned at ac68bba (its current HEAD) and scans. Holds no credential: the repo list is public metadata, read with the ambient github.token. Scans leave as an artifact. - publish: downloads the artifact, commits and publishes exactly as before (two squash-merged PRs). Holds the PAT, runs no fetched code, harden-runner with egress blocked to GitHub and the artifact service. A run that scans nothing is still a green no-op: no artifact, no publish. Dry runs skip preflight and publish, as before. The new `uses:` need lock entries; a one-off on this branch regenerates actions.lock with the tool (as in #958) and is removed before merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1
…-off These are the exact bytes `gh actions-lock` v0.2.0 plus the standards 333b06a repair chain produced in run 40433509902, which also passed update-actions-lock.sh --verify-local, check-lock-sync.sh and the governance gate's check-actions-lock-gate.sh. sha256 42e5e20846fcadb1bf6c3f987b57bb647f4ce246b3e40ec45ec403244623a286, matching the LOCK-SHA256 that run printed. Besides the three estate-rescan entries (harden-runner, upload-artifact, download-artifact), the tool re-keys the five standards callers from hyperpolymath/standards to metadatastician/standards: main's lock still records the old name, because #958 merged without its lock commit. It also records the leftover zz-one-off-lock-regen.yml from #936, which is still in the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…tests (#967) ## Summary Part of the stack to turn `governance / Validate Hypatia Baseline` green (see #966). `Integration Tests` passed `secrets.TEST_GITHUB_TOKEN` into the env of `cargo test --workspace` and `mix test --include integration`. Every dependency's build script and test therefore ran with the credential in reach. Under the risk-scoped RE001 (#966) this is the repo's only **`:high`** finding: the credential and the dependency build share a step. **Nothing reads the token:** - No `.rs`, `.ex`, `.exs` or `.sh` file names `TEST_GITHUB_TOKEN`. - `integration/src/lib.rs` reads only `ARANGODB_*`, `DRAGONFLY_URL` and `MOCK_{GITHUB,GITLAB,BITBUCKET}_URL`. This PR deletes the two env lines. That is least privilege, adds no new action and needs no lock change. **Deviation from the agreed plan:** the plan was to isolate the token and also add harden-runner. With no consumer there is nothing to isolate. After this change the job holds no credential, so it has nothing for harden-runner to protect, and adding it would only mean maintaining an egress allow-list for docker, cargo and hex. ## Type of change - [x] 🔒 Security / least privilege - [x] 🔧 Build / CI / tooling ## How has this been verified? - `grep -rIl TEST_GITHUB` over the tree (excluding deps, `_build`, target) finds only `tests.yml`. - `actionlint` 1.7.7 with shellcheck 0.9.0 gives the same 24 messages as on `main`, all pre-existing. - RE001 v2 (#966) on this branch reports one finding: `estate-rescan.yml`, which #968 fixes. Before this change it reported two. - **Pending CI:** this PR's own `Integration Tests` run is the end-to-end check that nothing needed the token. ## Notes for reviewers - **Owner follow-up, outside the repo:** once this merges, the `TEST_GITHUB_TOKEN` repository secret can be deleted in Settings → Secrets. - **Noticed, not changed:** the workflow sets `ARANGODB_USER`, but `integration/src/lib.rs` reads `ARANGODB_USERNAME`. It falls back to its default, so the variable is ignored. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1
… vouch for (#966) ## Summary Step 1 of the stack that turns `governance / Validate Hypatia Baseline` green. Every finding that gate keeps on main is RE001: 18 of 18, measured on run 39701221313 (main at `9a5db53`, scanner `23dc74d`). RE001 fired on any job that named any `secrets.*` without harden-runner, so it reported every secret-using job and could not tell the risky ones apart. It now treats this as an information-flow question: does a **real credential** share a job with **code the job cannot vouch for**? A job is reported when all of these hold: - it references a secret other than `GITHUB_TOKEN`, in any casing; - it has at least one **exposure**: - an untrusted-input trigger (`pull_request_target`, `issue_comment`, `issues`, `workflow_run`, …). A job-level `if:` made only of `github.event_name == '…'` tests narrows which events reach the job; - a head checkout: `ref:`/`repository:` on an `actions/checkout` step (block or flow form, including `workflow_run.head_sha`/`head_branch`/`head_repository`), or one git checkout/fetch/switch command naming the head; - a piped installer, or a dependency install/build (`npm ci`, `cargo build/test`, `mix deps.get`, …); - it installs no harden-runner. Only executable content counts: the step's own `run:` script (inline, literal or folded, with comments removed), not `env:` values, comments, or an `env:` key named `run`. Severity is **`:high`** when the credential and the code exposure share a step. That includes a job-level `env:` secret the step doesn't override. It is **`:warn`** when they share only the job. | repo | old | new | remaining | |---|---|---|---| | hypatia | 18 | **2** | `estate-rescan` rescan (warn), `tests` integration-tests (high) | | metadatastician/standards | 18 | **2** | `mirror-radicle` (high), `hypatia-scan-reusable` scan (warn) | | hyperpolymath/echidna | 5 | **0** | | The two hypatia findings that remain are real; #968 and #967 fix them. ## Type of change - [x] 🐛 Bug fix (scanner precision) ## How has this been verified? - **Standalone (Elixir 1.14; hex.pm is unreachable here):** `test/research_extensions_test.exs` + `test/kyaml_workflow_test.exs` give **84/84**. - **Negative controls:** each test added for Codex's four review rounds fails against the commit before its fix. 8 of the first 12 new tests fail against the old rule. - **Build checks:** `elixirc --warnings-as-errors` is clean, and `mix format` is clean on the touched files. - **CI on the previous heads:** all green, including `Integration Tests`, which runs the full `mix test --include integration` and so covers the wiring test. - **Pending:** governance can't start on any PR until the Actions allow-list accepts `metadatastician/standards`. ## Notes for reviewers - **Known limits (decided):** - install words inside quoted shell data still count as an exposure; the maintainer kept that trade-off rather than miss `bash -c "npm ci"`; - `git clone` followed by running the clone is not recognised. - **Root cause of the review churn:** the 22 Codex findings were all YAML shapes that a line reader gets wrong. #971 proposes one parsed workflow model for every workflow rule, with a differential experiment before any build. Further shape findings here belong there. - **Pin:** the gate builds hypatia at the `hypatia-ref` pin in `governance.yml`, so this lands in the gate only via the follow-up pin bump. - **Beyond this repo:** every estate repo scans with this rule. Standards' `hypatia-scan-reusable` and `mirror-radicle` are worth a look in standards itself. ## Checklist - [ ] My commits are **signed**. The session's signing shim is platform-mediated. - [x] I ran the project's own checks/tests locally and they pass (scoped as above). - [x] New files carry the correct SPDX (none added). - [x] Docs are updated (moduledoc and `@doc`). - [x] I have not introduced a soundness hole: every dropped finding falls into a class listed above, and the dropped classes are measured. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1
## Summary Step 2 of the stack that turns `governance / Validate Hypatia Baseline` green, after #966, #967 and #968. Both of hypatia's scanner callers pinned `hypatia-ref` to `23dc74d`. A rule fix merged to main therefore reached hypatia's own gate only after a hand bump. #966's risk-scoped RE001 is on main now, but the gate would still build the old rule. This PR replaces the literal with: ```yaml hypatia-ref: ${{ github.event.pull_request.base.sha || github.sha }} ``` | event | scanner commit | |---|---| | `pull_request` | the base-branch commit (main) | | `push`, `schedule`, `workflow_dispatch` | the commit being run | - **Never the PR head.** A PR cannot weaken the rules that gate it, which is the property the fixed pin was protecting. - **Always a 40-hex SHA**, so the reusables' validation passes without change. `HEAD` canary mode is not used, and nothing calls `git ls-remote`. - **Scope:** only hypatia's own callers change. Every other estate repo keeps the reusable's vetted default, so a bad main can't turn the estate red. That was the 09-12..14 and 09-26..30 outage class the default exists for. - **Escape hatch:** if main ships a broken rule, the PR that fixes it can pin a known-good SHA here temporarily. Only the `with:` input and its comment change. `uses:`, the allow-list and `actions.lock` are untouched. ## How has this been verified? - `actionlint` 1.7.7 is clean on both files. - **Differential self-scan:** all 15 `lib/rules` modules with `scan/1,2` were run over `origin/main` and this branch, keeping findings on the two files. Both give the same 10 findings (the ones already on main), with none added or removed. - **Not yet verified:** how GitHub evaluates the expression at run time. Governance still hits `startup_failure` on every PR until the Actions allow-list accepts `metadatastician/standards`, which is handled separately. `Hypatia Security Scan` can confirm the `pull_request` branch of the expression as soon as it runs here. ## Beyond this repo Moving the reusables' own `hypatia-ref` default past `51ab649`, so other repos get #966 too, is a standards decision. This PR does not make it. --- _Generated by [Claude Code](https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1)_
) ## Summary Step 3 of the governance-gate stack, after #966, #967, #968 and #973. Under RE001 as #964 (standards ADR-009) now ships it, every job that reads a real secret is in scope. On main that is **9 `:warn` findings**. Once #973 makes hypatia's gate run main's rules, they hold it red at the `info` threshold. This PR clears them by the safety-triangle order: **eliminate** the credential where we can, then **control** what remains. | job | secret | action | |---|---|---| | `security-policy` / `secret-gitleaks` | `GITLEAKS_LICENSE` | **Eliminated.** gitleaks-action needs a licence only for an organization owner. Run 40442141312 logs `[hyperpolymath] is an individual user. No license key is required.` and shows the secret empty, so it was never set. | | `inbox-steward-intake` / `analyze-patterns` | `FARM_PAT` | Its read-only checkout no longer takes the PAT and keeps no credential (`persist-credentials: false`). The `gh api` read still uses the PAT, so the job is **hardened** with GitHub + API. | | `inbox-steward-intake` / `dispatch-to-farm` | `FARM_PAT` | Dropped a checkout nothing read. **Hardened**, API only. | | `inbox-steward-intake` / `record-report`, `update-ruleset` | `FARM_PAT` | **Hardened**, GitHub + API (push, `gh pr create`). | | `ci-health-sweep` / `sweep` | `HYPATIA_DISPATCH_PAT` | **Hardened**, GitHub + API. Every call in `scripts/ci-health/*.sh` is `gh`. | | `hypatia-remediation-sweep` / `sweep` | `HYPATIA_DISPATCH_PAT` | **Hardened**, API plus the artifact service and its blob storage. | | `roadmap-sync` / `sync` | App private key | **Hardened**, API only. It already used an App token (substitution done before this PR). | | `push-email-notify` / `notify` | `SMTP_*` | **Hardened**: the action's release download, plus `${{ secrets.SMTP_HOST }}:${{ secrets.SMTP_PORT \|\| '465' }}`. The mail host is itself a secret, so it stays masked. | All hardening is `step-security/harden-runner@v2.22.2` with `egress-policy: block`, the same pin #968 introduced. `actions.lock` holds the tool's bytes from run 40443523409 (sha256 `74ebf8be…`, `GATE_RC=0`); it was not hand-written. ## How has this been verified? - **RE001, #964's version** (`653d09c`) on this tree: 9 → **0**. Main's version: 0 → 0. - **Differential self-scan**, all 15 rule modules over `origin/main` and this branch, keeping findings on the six touched workflows: - with #964's rules, only those 9 RE001 findings go and nothing is added; - with main's rules, there is no change; - WH004 is 0 once the lock is in. - **actionlint 1.7.7:** the same 85 issues as main, none new. - **Not verified, and the real risk:** a `block` allow-list that misses a host breaks its job at run time. These jobs run on schedule, dispatch or push, so PR CI can't exercise them. I worked out each list from the job's code (every `gh`/`git`/action call), not by observation. I propose one `workflow_dispatch` per job with its own `dry_run`, before or just after merge, and reading harden-runner's egress report. ## Not done here (needs the owner or another repo) - **Substitution:** the four `FARM_PAT` jobs and the two `HYPATIA_DISPATCH_PAT` sweeps could each move to a GitHub App installation token scoped to the repos they touch, as roadmap-sync already does. That means creating Apps, which this session can't do. - **Attestation:** a `hypatia_chora.deed` with `(egress …)` entries (`:enforcement harden-runner-block`), once #964 lands, agreed with the #964 session. - **Org move:** if the repo moves to an organization (as standards did), gitleaks will need `GITLEAKS_LICENSE` again. --- _Generated by [Claude Code](https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1)_
Summary
Part of the stack to turn
governance / Validate Hypatia Baselinegreen (see #966). Under the risk-scoped RE001,estate-rescanis one of the two real findings left in this repo. It is also a concrete credential exposure:rescanjob builtpanic-attackfrom an unpinnedHEADand ran it over about 300 repositories.HYPATIA_DISPATCH_PATwas used in the same job.Checkout verisimdb-datacloned with the PAT in the URL, so the PAT sat in that clone's.git/config, readable by every later step, the freshly built scanner included.The workflow is now three jobs:
preflight(writability probe, unchanged logic)gh apionly)block:api.github.comscangithub.tokenpanic-attackpinned atac68bba(its current HEAD)publish(commit + two squash-merged PRs, unchanged logic)block:github.com,api.github.com, the artifact service and its blob storageThe scans move from
scantopublishas an artifact with 1-day retention.Behaviour preserved:
preflightandpublish.actions.lockThe lock is tool-owned. As in #958, a one-off workflow on this branch (run 40433509902) did the regeneration:
gh actions-lockv0.2.0 plus the standards333b06arepair chain.update-actions-lock.sh --verify-local,check-lock-sync.shand the governance gate'scheck-actions-lock-gate.shall passed.LOCK-SHA256exactly:42e5e208…a286. The one-off is removed in the same commit.The regenerated lock changes three things:
harden-runner@v2.22.2→dc2769a,upload-artifact@v7.0.2,download-artifact@v8.0.2forestate-rescan.yml.metadatastician/standards@333b06a, but the lock still recordshyperpolymath/standards@333b06a. The tool re-keys all five callers.zz-one-off-lock-regen.ymlfrom Claude/new session th4e9w lockregen #936, which is still in the tree.Type of change
How has this been verified?
actionlint1.7.7 with shellcheck 0.9.0 is clean, except SC2020 on the movedtr ' ,' '\n\n'line, which is identical onmainand intentional.estate-rescan. The one RE001 left istests.yml, which fix(ci): stop passing an unused TEST_GITHUB_TOKEN to the integration tests #967 removes.:infoWH007.uses:cleared once the lock recorded them.blockallow-lists only meet real traffic on the first live (non-dry) run. A wrong entry would fail loudly at publish, not corrupt data.workflow_dispatchdry run on this branch would exercisescanand the artifact upload without publishing. I haven't triggered one.Notes for reviewers
panic-attackis now a deliberate one-line change (PA_SHA) instead of following HEAD. That trades freshness for a reviewable supply chain.repository_dispatch: estate-rescanand is unaffected;verisimdb-datacontract is unchanged.Generated by Claude Code