Repository navigation
fix(ci): stop passing an unused TEST_GITHUB_TOKEN to the integration tests - #967
Conversation
…tests Integration Tests handed secrets.TEST_GITHUB_TOKEN to `cargo test` and to `mix test --include integration`, so every dependency's build script and test ran with the credential in its environment. Nothing reads it: no Rust, Elixir or shell file in the tree names TEST_GITHUB_TOKEN, and the integration crate's config (integration/src/lib.rs) reads only the ArangoDB, Dragonfly and mock-forge URLs. Removing it leaves the job with no credential, which clears this repo's only :high RE001 finding (risk-scoped rule, #966) without adding an action. 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 |
|
Add Carrot credits or activate Agent usage billing to use Autofix |
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. |
…968) ## Summary Part of the stack to turn `governance / Validate Hypatia Baseline` green (see #966). Under the risk-scoped RE001, `estate-rescan` is one of the two real findings left in this repo. It is also a concrete credential exposure: - The `rescan` job built `panic-attack` from an **unpinned `HEAD`** and ran it over about 300 repositories. - `HYPATIA_DISPATCH_PAT` was used in the same job. - `Checkout verisimdb-data` cloned 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: | job | credential | runs fetched code | egress | |---|---|---|---| | `preflight` (writability probe, unchanged logic) | PAT | no (`gh api` only) | harden-runner `block`: `api.github.com` | | `scan` | **none**: the repo list is public metadata, read with the ambient `github.token` | yes: `panic-attack` **pinned at `ac68bba`** (its current HEAD) | unrestricted | | `publish` (commit + two squash-merged PRs, unchanged logic) | PAT | no | harden-runner `block`: `github.com`, `api.github.com`, the artifact service and its blob storage | The scans move from `scan` to `publish` as an artifact with 1-day retention. **Behaviour preserved:** - Dry runs skip `preflight` and `publish`. - A run that scans nothing is still a green no-op: no artifact and no publish. - The run still fails fast if the PAT can't write. ### `actions.lock` The lock is tool-owned. As in #958, a one-off workflow on this branch (run 40433509902) did the regeneration: - It ran `gh actions-lock` v0.2.0 plus the standards `333b06a` repair chain. - `update-actions-lock.sh --verify-local`, `check-lock-sync.sh` and the governance gate's `check-actions-lock-gate.sh` all passed. - It printed the lock's diff and sha256. I applied that diff, and the file matches the run's `LOCK-SHA256` exactly: `42e5e208…a286`. The one-off is removed in the same commit. The regenerated lock changes three things: 1. **This PR's entries:** `harden-runner@v2.22.2` → `dc2769a`, `upload-artifact@v7.0.2`, `download-artifact@v8.0.2` for `estate-rescan.yml`. 2. **Main's lock is stale:** #958 squash-merged without its lock commit. Main's workflows call `metadatastician/standards@333b06a`, but the lock still records `hyperpolymath/standards@333b06a`. The tool re-keys all five callers.⚠️ This overlaps the session unblocking estate CI after the standards move. Whichever lands first, the other PR should take main and regenerate. 3. It records the leftover `zz-one-off-lock-regen.yml` from #936, which is still in the tree. ## Type of change - [x] 🔒 Security / least privilege - [x] 🔧 Build / CI / tooling ## How has this been verified? - `actionlint` 1.7.7 with shellcheck 0.9.0 is clean, except SC2020 on the moved `tr ' ,' '\n\n'` line, which is identical on `main` and intentional. - Hypatia's own rules on this branch, compiled standalone: - RE001 v2 no longer reports `estate-rescan`. The one RE001 left is `tests.yml`, which #967 removes. - WorkflowHardening shows only the 4 pre-existing `:info` WH007. - The 4 WH004 hits on the new `uses:` cleared once the lock recorded them. - **Not yet verified:** - The two `block` allow-lists only meet real traffic on the first live (non-dry) run. A wrong entry would fail loudly at publish, not corrupt data. - A `workflow_dispatch` dry run on this branch would exercise `scan` and the artifact upload without publishing. I haven't triggered one. - Governance CI is pending on the allow-list fix. ## Notes for reviewers - **Bumping `panic-attack`** is now a deliberate one-line change (`PA_SHA`) instead of following HEAD. That trades freshness for a reviewable supply chain. - **Beyond this repo:** - gitbot-fleet's dispatch-runner fires `repository_dispatch: estate-rescan` and is unaffected; - the `verisimdb-data` contract is unchanged. --- _Generated by [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).Integration Testspassedsecrets.TEST_GITHUB_TOKENinto the env ofcargo test --workspaceandmix 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:highfinding: the credential and the dependency build share a step.Nothing reads the token:
.rs,.ex,.exsor.shfile namesTEST_GITHUB_TOKEN.integration/src/lib.rsreads onlyARANGODB_*,DRAGONFLY_URLandMOCK_{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
How has this been verified?
grep -rIl TEST_GITHUBover the tree (excluding deps,_build, target) finds onlytests.yml.actionlint1.7.7 with shellcheck 0.9.0 gives the same 24 messages as onmain, all pre-existing.estate-rescan.yml, which fix(ci): keep the dispatch PAT out of the job that runs panic-attack #968 fixes. Before this change it reported two.Integration Testsrun is the end-to-end check that nothing needed the token.Notes for reviewers
TEST_GITHUB_TOKENrepository secret can be deleted in Settings → Secrets.ARANGODB_USER, butintegration/src/lib.rsreadsARANGODB_USERNAME. It falls back to its default, so the variable is ignored.🤖 Generated with Claude Code
https://claude.ai/code/session_01J4QvDfStHgMdZGTeqaBgM1