Repository navigation
feat(workflow_audit): report actions.lock records pointing at a moved repository - #1109
Conversation
… repository CASE-054 (CoprocessorRuntime.jl): the required "scan / gitleaks" never reported because secret-scanner and five other workflows died at startup, with no job and no log. The lock still recorded metadatastician/standards@da2c748 with owner_id 6759885; the repository moved to metadatastician (owner_id 297579217, same repo_id 1116521501), so the runner could not verify the record. ActionsLock.moved_repository_keys/1 reads the lock alone against a table of known transfers: a key naming the old location (:old_name), or the new location with the old owner's id (:stale_owner_id). The audit reports one advisory info finding, referred to the relock (cicd-squabbler); nothing is rewritten. One repo_id under two names is already refused by parse/2. Verified on CoprocessorRuntime.jl main e57738b: reports exactly that record. Hypatia's own lock is clean. Owner ruling 14 Nov. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
|
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 22 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. |
Codex Review Summary: ✅ CompletedThis 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: 66fd5cb7e4
ℹ️ 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".
|
The red Generated by Claude Code |
…repo_id Codex on #1109: - The moves now come from Hypatia.Rules.MovedRepositories, which WF028 also reads, so a new transfer is recorded once. The entry gains current_owner_id (additive; the schema version is unchanged). - :old_name needs the moved repository's repo_id, so a new repository created at the old name is not reported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT
## What `Hypatia.Coverage.EvidenceFormatter` turned every string key of an `@tag evidence:` map into an atom with `String.to_atom/1` (line 197). With this change it uses `String.to_existing_atom/1` and keeps the string when no atom exists. ## Why Since #1107 merged, `main` has a high finding from Hypatia's own `code_safety/elixir_atom_from_user` rule. #1107's own baseline run was cancelled, so the gate never saw it. Now `governance / Validate Hypatia Baseline` is red on every open PR, for example #1109: [job](https://github.com/hyperpolymath/hypatia/actions/runs/43625366706/job/132550885569) "1 unfiltered finding(s) at or above 'info'", which is this line. Behaviour is unchanged for the keys the formatter reads. They are atom literals in the module, so they already exist. ## Evidence - New test: "an unknown string key in the tag does not create an atom". It fails on `main` (`23 tests, 1 failure`) and passes with the fix (`23 tests, 0 failures`). - The rule's pattern is `~r/String\.to_atom\(/`, which does not match `to_existing_atom`. - `mix format --check-formatted` passes on both files. `gen-rules-praxis.sh --check` and `check-readme-claims.sh` also pass. ## Not done - 28 other `test/coverage` tests fail in my local copy of the repo. They fail the same way on unchanged `main`, because the copy lacks some modules. CI runs the full suite. --- _Generated by [Claude Code](https://claude.ai/code/session_0136P7ZkGCWGEyedTtdEHAPT)_ Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: feba296276
ℹ️ 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".
| Map.new(Hypatia.Rules.MovedRepositories.entries(), fn m -> | ||
| {String.downcase(m.former), | ||
| %{to: String.downcase(m.current), owner_id: m.current_owner_id, repo_id: m.repo_id}} | ||
| end) |
There was a problem hiding this comment.
When a repository moves twice and the shared registry contains A -> B and B -> C under its one-row-per-move extension model, this map preserves A's immediate destination as B. Consequently, an actions.lock record still naming A is reported as A -> B, and the audit instructs users to relock against an address that is itself obsolete and still fails. Collapse same-repo_id move chains to the final current repository before producing the finding.
Useful? React with 👍 / 👎.
| "#{length(moved)} actions.lock record(s) point at a moved repository's old " <> | ||
| "home (e.g. #{describe_moved(hd(moved))}). The runner cannot verify them, so " <> |
There was a problem hiding this comment.
When the detected reason is :stale_owner_id, the key already names the current repository, so this message incorrectly says it points at the old home and renders a self-mapping such as metadatastician/standards@<sha> -> metadatastician/standards. That obscures the actual owner-ID mismatch the user must diagnose; format this case separately as stale owner metadata, ideally including the observed and expected IDs.
Useful? React with 👍 / 👎.
| Map.new(Hypatia.Rules.MovedRepositories.entries(), fn m -> | ||
| {String.downcase(m.former), | ||
| %{to: String.downcase(m.current), owner_id: m.current_owner_id, repo_id: m.repo_id}} | ||
| end) |
There was a problem hiding this comment.
If repository R1 moves from A to B, a replacement R2 is created at A, and R2 later moves to C, the registry correctly needs two rows with the same former slug but different stable repo_id values. This Map.new/2 silently retains only one of them, so locks for the other moved repository fail the new repo_id guard and are never reported. Group entries by former slug and select the row matching the dependency's repo_id instead of discarding one transfer.
Useful? React with 👍 / 👎.
CASE-054 (CoprocessorRuntime.jl): the required
scan / gitleaksnever reported, becausesecret-scannerand five other workflows died at startup with no job and no log. The repo'sactions.lockstill recordedhyperpolymath/standards@da2c748withowner_id6759885. That repository moved to metadatastician (owner_id297579217, samerepo_id1116521501), so the runner could not verify the record.Owner ruling, 14 Nov: build a static rule from the lock alone, info severity, referred to the squabbler, never autofixed.
What
ActionsLock.moved_repository_keys/1reads the lock against a table of known transfers, currentlyhyperpolymath/standards→metadatastician/standards. It reports::old_name: the key names the old location;:stale_owner_id: the key names the new location with the old owner'sowner_id.repo_idunder two names is already refused byparse/2(:repository_id_reused, reported as an invalid lock), so it isn't repeated here.WorkflowAudit.check_actions_lock_moved_repositories/2, wired intoaudit/3(skipped in partial audits), gives one advisory info finding per audit,hypatia/workflow_audit/actions_lock_moved_repository,action: :report. The referral is a relock against the new owner (cicd-squabbler). Nothing is rewritten.Proof
e57738b: reports exactlyhyperpolymath/standards@da2c748aad55c1a1dcba00b60fe4a35017bc6540(:old_name→metadatastician/standards).actions.lock: clean. Its standards record is alreadymetadatastician/standardswithowner_id297579217.Tests
5 new tests in
test/rules/actions_lock_test.exs:owner_idunder the new name is reported;owner_idis clean;All fail on main.
mix test test/rules: 933 tests, 0 failures.test/soundness_test.exs: 16/16.gen-rules-praxis.sh --check,check-readme-claims.shandrules-praxis-test.sh(22/22) are clean.Extending
When a repository is transferred, add it to
@moved_repositoriesinlib/rules/actions_lock.exwith the new owner's ids.Generated by Claude Code