Repository navigation
fix(actions-lock): accept lockfile v0.0.3 (github.com dependencies) - #928
hyperpolymath wants to merge 3 commits into
Conversation
…ammar gh-actions-lock v0.2.0 writes lockfile v0.0.3, and the estate gate now pins v0.2.0 (standards#1250). ActionsLock accepted only v0.0.2, so any relocked repository raised a high-severity invalid_actions_lock from workflow_audit and workflow_hardening (palimpsest-license alerts 347 and 349). Hypatia's own lock has been v0.0.3 since #926, so its own "parses the repository's authoritative generated lockfile" test failed. v0.0.3 has the v0.0.2 line grammar: same sections, same record fields, same `'<path>': []` form (measured on this repo's lock and on rsr-template-repo's). Both versions are now parsed by the one strict grammar; every other version string is still refused, and a second version line is still a duplicate. Tests: one case for v0.0.3 (accepted; malformed v0.0.3 still fails closed), one for unsupported versions and a duplicate line. Run standalone against the module: 14/14 pass; the unpatched module fails 3 of them, including the existing own-lock test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LET2eHeDbN3b4WuxYjMUX9
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely states the main change: accepting lockfile v0.0.3 for github.com dependencies. |
| Description check | |
The description directly explains the v0.0.3 compatibility change, hostname handling, tests, validation results, and known test limitations. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
✨ Finishing Touches
-
📝 Generate docstrings
-
- 🤖 Coding Agent task started for docstring generation.
- 🤖 Coding Agent task started for docstring generation.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
The red checks here do not come from this PR's change, which touches only
The checks that cover this change passed: Generated by Claude Code |
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. |
…suites pass E2E — Elixir Scanner Pipeline and Integration Tests fail on main since #926: the repository's own lock is now v0.0.3, and Hypatia.Rules.ActionsLock rejects it ("parses the repository's authoritative generated lockfile": unsupported_version "v0.0.3"). #928 fixes that. This brings in its commit unchanged, so the two PRs merge in either order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LET2eHeDbN3b4WuxYjMUX9
|
#929 now contains this PR's commit (114d015, merged in unchanged). With the standards repin, the UUID fixture fix and the gitleaks fingerprints, that gives one PR that should take the governance, secret-scanner, E2E and Integration checks green together. Merge order does not matter:
This PR's own remaining reds are covered in #929's triage comment:
Generated by Claude Code |
…generic error
v0.0.3 is not quite the v0.0.2 grammar. gh-actions-lock v0.2.0 adds one
optional per-dependency field, `hostname` (`json:"hostname,omitempty"`,
confirmed in the v0.2.0 binary). It writes the field only for a GHE.com
data-residency tenant, never for github.com, so neither real v0.0.3 lock
in the estate (this repo's, rsr-template-repo's) carries it.
Make the parser say so instead of reporting a generic
:malformed_dependency_entry:
- In a v0.0.3 lock, `hostname: 'github.com'` is accepted. It states the
default.
- In a v0.0.3 lock, any other host is refused as
{:unsupported_dependency_hostname, host}. Repository identity
(owner_id/repo_id uniqueness) is validated per host, and this parser
models github.com only.
- In a v0.0.2 lock, a hostname line is still malformed: the field did not
exist then.
The moduledoc now describes the restricted schema accurately.
Tests: a new case covers all three behaviours and a duplicate hostname
line. The real locks still parse: hypatia v0.0.3 (40 deps),
rsr-template-repo v0.0.3 (37), palimpsest-license v0.0.2 (38), standards
v0.0.2 (26).
Addresses CodeRabbit's finding on #929.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LET2eHeDbN3b4WuxYjMUX9
…tions-lock code Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LET2eHeDbN3b4WuxYjMUX9
hyperpolymath
changed the title
![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: a50c0e5ea8
ℹ️ 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".
| match = state.version == "v0.0.3" && Regex.run(~r/^ hostname: '([^']+)'$/, line) -> | ||
| [_, host] = match | ||
| line_error(line_number, {:unsupported_dependency_hostname, host}) |
There was a problem hiding this comment.
When scanning a v0.0.3 lock generated on a GHE.com data-residency repository, home-tenant dependencies omit hostname: the authoritative schema notes that producers must omit it for home-host pins. This condition rejects only an explicitly non-GitHub hostname, while parse/1 has no home-host context, so omitted tenant dependencies finalize successfully and pinned?/3 can suppress WH004 as though their repository identities belonged to github.com. Pass the repository host into parsing or otherwise fail closed for ambiguous omitted-host v0.0.3 locks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The ambiguity is real, but only for a repository this scanner doesn't see. So I've left the code as is.
The schema README (github/actions-lockfile) gives the rules:
- An omitted
hostnamebinds a pin to the repository's home host. - On github.com,
hostnamemust be omitted orgithub.com, and any other value is rejected. - Producers write
github.comexplicitly only on a GHE.com data-residency instance, and never write a*.ghe.comvalue.
parse/1 assumes the home host is github.com. On that host it now enforces the same rule GitHub does: omitted or github.com is accepted, and anything else is {:unsupported_dependency_hostname, host}.
The misread you describe needs Hypatia to scan a lock produced in a GHE.com tenant repository. Every repository in this estate is on github.com, and no lock in it has a hostname line. Neither suggested fix is in proportion:
- Fail closed on omitted-host v0.0.3 locks. That would refuse every github.com v0.0.3 lock, including this repo's own, because they omit
hostnametoo. - Pass the repository host into parsing. That needs host plumbing through
Rules.scan_workflowand every scan path, for a host the estate doesn't use.
Even in that case, the effect is limited to cross-host owner/repo identity checks. The workflow-ref-to-commit pinning that pinned?/3 relies on is still checked structurally. I've raised this with the repository owner; if they want tenant support, the host parameter is the route.
Generated by Claude Code
|
Add Carrot credits or activate Agent usage billing to use Autofix |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
❌ Failed to create Coding Agent finishing-touch task. Please try again. |
This branch has not been deployed
2 participants
Why
gh-actions-lock v0.2.0 writes lockfile v0.0.3, and the estate's lock gate now pins v0.2.0 (hyperpolymath/standards#1250).
Hypatia.Rules.ActionsLockaccepts only v0.0.2.workflow_auditandworkflow_hardening, at high severity. It happened on ci: add actions.lock, run governance at standards b26d3fb, repin dtolnay/rust-toolchain palimpsest-license#177 as code-scanning alerts 347 and 349.actions.lockhas been v0.0.3 since chore(deps): bump the actions group across 1 directory with 7 updates #926. Its existing test"parses the repository's authoritative generated lockfile"therefore fails against the unpatched module.What
114d015. For github.com dependencies, v0.0.3 uses the v0.0.2 line grammar. I measured this on this repo's lock and on
rsr-template-repo's, both v0.0.3:workflows:anddependencies:sections;ref,commit,owner_id,repo_id,uses);'<path>': []form for a workflow with nothing to lock.So:
@supported_version "v0.0.2"becomes@supported_versions ["v0.0.2", "v0.0.3"], both parsed by the one strict grammar.{:unsupported_version, v}, and a secondversion:line is still:duplicate_version.a50c0e5 is for CodeRabbit's finding on #929. v0.0.3 adds one optional per-dependency field,
hostname. The v0.2.0 binary hasjson:"hostname,omitempty", and it is written only for GHE.com data-residency tenants, never for github.com. Neither real v0.0.3 lock in the estate carries it. Rather than model multi-host identity for hosts the estate doesn't use:hostname: 'github.com'is accepted, as the default it states;{:unsupported_dependency_hostname, host};The moduledoc records both points.
Tests
test/rules/actions_lock_test.exsgains three cases:v0.0.1,v0.0.4,v0.1.0and a bare0.0.3are each refused, and a second version line is a duplicate.hostnamebehaves as listed above, including a duplicatehostnameline.I couldn't run the full suite in my sandbox, because repo.hex.pm is blocked there, so
mix deps.getcannot run. Instead I ran this test file standalone against the compiled module with Elixir 1.14. I left out only the legacyHypatia.Rulescase, which needs the full engine.main)The patched parser also reads four real locks:
lib/rules/actions_lock.exis formatter-clean. I left the untouched test lines as CI's Elixir 1.17 formatter has them.Relation to #929
#929 merges this branch unchanged (114d015 and a50c0e5), so CI there exercises this code too. The two PRs merge in either order.
Follow-up
When this merges, standards'
hypatia-scan-reusable.ymlandgovernance-reusable.ymlcan move their defaulthypatia-reffrom51ab649to the merge commit. That makes the estate scanner accept v0.0.3 locks.🤖 Generated with Claude Code
https://claude.ai/code/session_01LET2eHeDbN3b4WuxYjMUX9