Repository navigation
feat(secret-scanning): dedicated token, fail-loud sweep, secret-alerts command - #969
Conversation
…alerts` command
A workflow's GITHUB_TOKEN cannot read secret-scanning alerts (no workflow
permission covers them), so per-repo scans never see them. They will be
gathered by one central sweep from a private repo, with a dedicated
read-only token. This adds what that sweep needs, without changing any
existing caller.
- Token: `HYPATIA_SECRET_ALERTS_TOKEN` is read first, then GITHUB_TOKEN,
so the dedicated token never has to be exported under the general name.
- `sweep/2`: one API call; returns {:error, reason} when the alerts cannot
be fetched. The per-rule SSA001-004 functions log and return [] on a
fetch failure, which an estate sweep would read as "clean".
- Errors now carry the HTTP status and GitHub's message (curl no longer
uses -f, which discarded the body), e.g. "GitHub API 404: Secret
scanning is disabled on this repository."
- The finding builders are pure functions of the alert list, so they are
tested from fixtures without the network.
- `hypatia secret-alerts --owner X --repo Y`: one JSON line; exit 0 when
the alerts were read, 3 when they could not be.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P4yFkxW5Heb8p2YW7ZYs9F
The local Elixir 1.14 formatter rejoined two pre-existing assertions; CI formats with 1.19.4. Restore them so this change only adds lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4yFkxW5Heb8p2YW7ZYs9F
|
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc9c20d74c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case fetch_alerts(owner, repo) do | ||
| {:ok, alerts} -> {:ok, result(findings_from_alerts(owner, repo, alerts))} |
There was a problem hiding this comment.
For repositories with more than 100 historical alerts, fetch_alerts/2 returns only the newest 100 because GitHub caps per_page at 100 and defaults to the first page (API documentation). Nevertheless, sweep/2 treats this partial list as complete and emits status: "ok", so older open, stale, or improperly dismissed alerts are silently omitted and totals are understated. Fetch subsequent pages or follow GitHub's pagination cursors before building the findings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid. This PR had already merged, so the fix is in #970. The fetch now follows Link: rel="next" pages, but only on api.github.com. Past 50 pages it returns an error rather than a truncated ok.
Generated by Claude Code
| [ | ||
| "-s", | ||
| "-w", | ||
| "\n%{http_code}", |
There was a problem hiding this comment.
When GitHub or an intermediate network connection accepts the request but stops delivering data, this synchronous curl invocation has neither --max-time nor another transfer deadline, so System.cmd/3 can block indefinitely instead of letting sweep/2 return an error and the CLI exit 3. This can stall the entire central estate sweep on a single repository; the curl manual specifically describes --max-time as preventing batch jobs from hanging for hours. Add a finite transfer timeout and let the existing nonzero-exit path report it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid. Fixed in #970: each request now has --max-time 30, and a timeout (curl exit 28) is reported as an error, so secret-alerts exits 3 instead of hanging the sweep.
Generated by Claude Code
…LICENSE.adoc (#970) ## Summary This fixes three defects from #963 and #969. Codex flagged them after those PRs had already merged. It also tightens the REUSE licence rule after CodeRabbit's review here. ## Changes `lib/rules/secret_scanning_alerts.ex` (from #969) - **Pagination:** the fetch read only the first page of 100 alerts, so `sweep/2` returned `status: ok` with older open, stale or badly dismissed alerts missing. It now follows GitHub's `Link: rel="next"` header until the last page. - It only follows links on `api.github.com`, because the token goes with every request. - Past 50 pages (5,000 alerts) it returns an error rather than a truncated list. - **Timeout:** the curl call had no deadline, so one stalled repository could hang an estate-wide sweep. Each request now has `--max-time 30`, and a timeout (curl exit 28) is reported as `GitHub API request timed out after 30s`. - **Header parsing:** responses are read with `curl -i` so the `Link` header is available. If an HTTPS proxy adds its own "Connection established" header block first, it's skipped. `lib/scorecard_ingestor.ex` and `lib/rules/root_hygiene.ex` (from #963) - **SC-010 now agrees with root hygiene.** The local Scorecard licence check still rejected `LICENSE.adoc` and REUSE `LICENSES/`, so a single scan reported both "licence present" (root hygiene) and "No LICENSE file found" (SC-010). It now accepts `LICENSE.adoc` and calls root hygiene's REUSE check. - **`RootHygiene.reuse_licenses?/1` is now public.** The REUSE rule has one definition instead of two copies that could drift apart. REUSE strictness (e5f5a7a, owner decision on CodeRabbit's finding) - **The problem:** a `LICENSES/` file used to count if it was any non-empty `<name>.<ext>`. So `LICENSES/notes.txt` alone passed both licence checks. - **The fix:** its stem must now be on the SPDX License List or be a REUSE `LicenseRef-` id. Matching is exact. - **Why a list, not a filename rule:** real identifiers such as `curl` and `blessing` are lowercase, so no heuristic separates them from `notes`. The list is vendored instead: - `priv/spdx/license-ids.txt` holds SPDX License List 3.29.0: 740 identifiers, deprecated ones included. Its header records the source tag. - `scripts/refresh-spdx-license-ids.sh <tag>` regenerates the file and refuses a suspiciously short download. - `Hypatia.SPDX` reads the file at compile time, so the escript needs no `priv/` at run time. - **Owner action:** the vendored list is third-party data. I have not added a `.reuse/dep5` entry for it, because licence records are yours to make. ## Testing Run with plain Elixir 1.14 and a local JSON stand-in, as for #969. Hex isn't reachable from here, and CI's `mix test` uses the real Jason. - **Overall:** 84 of 84 pass across `secret_scanning_alerts_test.exs`, `root_hygiene_test.exs`, `scorecard_ingestor_license_test.exs`, `scorecard_ingestor_security_policy_test.exs` (regression guard) and the new `spdx_test.exs`. - **Behaviour tests fail on the old code.** The 10 tests from the first commit fail against `main`. The two `notes.txt` tests fail against the previous `reuse_licenses?/1`. - `parse_page/1`: the next link, the last page, the proxy header block, and an error page. - `collect_pages/2`: two pages joined, a next link to another host refused, a failed later page failing the whole fetch, the page cap raising an error, and exactly 50 requests before the cap. - SC-010: `LICENSE.adoc`, REUSE and `LicenseRef-` pass. A `.gitkeep`-only `LICENSES/` and a `notes.txt` fail. - `Hypatia.SPDX`: listed, lowercase, deprecated and `LicenseRef-` ids count; other names and case variants do not. - **Formatting:** `mix format --check-formatted` passes on every changed file. `shellcheck` passes on the refresh script. --- _Generated by [Claude Code](https://claude.ai/code/session_01P4yFkxW5Heb8p2YW7ZYs9F)_
Summary
A workflow's own
GITHUB_TOKENcan't read secret-scanning alerts, because no workflow permission covers them. So the per-repo Hypatia scan never sees them (raised by Codex on hyperpolymath/rsr-template-repo#258).The owner decided these alerts will be collected by one central sweep, run from a private repo, using a dedicated read-only token, and not by adding a secret to every repo. The results stay private: an "open leaked token in repo X" list must not land in the public
verisimdb-datastore or in a public repo's Actions logs.This PR adds what that sweep needs on the Hypatia side. Existing callers behave exactly as before.
Changes
lib/rules/secret_scanning_alerts.exHYPATIA_SECRET_ALERTS_TOKENfirst, thenGITHUB_TOKEN. The dedicated token never has to be exported under the general name, and an empty value counts as unset.sweep/2: fetches the alerts in one API call and returns{:error, reason}when they can't be fetched.[], as the other API-backed rules do.[]would make a refused token or a repo with secret scanning disabled look clean.sweep/2never does.-f, which threw away GitHub's error body. Errors now carry the HTTP status and GitHub's message, for exampleGitHub API 404: Secret scanning is disabled on this repository.lib/hypatia/cli.exhypatia secret-alerts --owner X --repo Y: prints one JSON line (status: okwith findings, orstatus: errorwith the reason).Not in this PR
The private sweep workflow itself is not here. It belongs in a private repo, and this session can't attach
hyperpolymath/.git-private-farm: repo names starting with.are refused. The owner still has to decide where it goes.Testing
Hex isn't reachable from this environment, so I ran the test files standalone with plain Elixir 1.14. A minimal local JSON shim stood in for Jason; it isn't committed. CI's full
mix testruns with real Jason.test/secret_scanning_alerts_test.exs: 16 of 16 pass, 6 existing and 10 new. The new tests cover:sweep/2returning an error without a token;test/cli_test.exs: the 3 newsecret-alertstests and the help tests pass. One existing test needs rule modules my harness can't load. It fails the same way on unmodifiedmainin this harness, so it isn't affected by this change.mix format --check-formattedpasses locally. Two existing assertions that the local formatter rewrapped are restored to their original form, sotest/cli_test.exsonly gains lines.Generated by Claude Code