fix(governance): verify reusable workflow locks - #696
Conversation
|
Warning Review limit reachedNext included review available in 7 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
Although the PR is marked as 'up to standards' by Codacy, the review reveals critical logical contradictions in the test suite that undermine confidence in the fix. Specifically, the test for 'reusable-exact' matches is set up with a failing mock status yet asserts success, indicating the test may be producing false positives.
Additionally, there is a lack of verification for the 'Safety boundary' requirement. The PR description explicitly states that only 'stale' findings should pass, but there are no test cases to confirm that 'missing' or 'invalid' findings are still blocked. The implementation details in scripts/update-actions-lock.sh are also missing from the review context, preventing a full validation of the normalization logic.
About this PR
- The changes to
scripts/update-actions-lock.share not visible in this diff, which makes it impossible to verify the implementation of the normalization logic or the safety constraint ensuring only 'stale' findings are bypassed. - The test suite lacks scenarios to verify that findings with categories other than 'stale' (e.g., missing, invalid) are still blocked as required by the safety boundary specifications.
Test suggestions
- Exact reusable-workflow dependency match (normalized) is accepted
- Reusable-workflow with mismatched git ref is blocked
- Finding with category other than 'stale' is blocked even if ref matches
- Handling of malformed JSON findings
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Finding with category other than 'stale' is blocked even if ref matches
2. Handling of malformed JSON findings
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
a01d080 to
d8814e4
Compare
d8814e4 to
4ed2cef
Compare
|
## Problem Ephapax Rust CI has produced only zero-job `startup_failure` runs across the measured latest 20-run history. The called Standards reusable is valid and its action commits resolve, but `gh actions-lock` v0.1.6 does not model cross-repository reusable-workflow dependencies. Ephapax's caller lock therefore omitted the reusable and its transitive action evidence. ## Change - Record the exact immutable Standards revision under each caller workflow. - Record the union of transitive action SHAs from the six called Standards reusables. - Preserve Ephapax's direct local-job dependencies in `rust-ci.yml`. The dependency shape follows the already-proven Chronicles remediation recorded in standards#573. It does not change workflow source or broaden permissions. ## Verification - All referenced action commits and the Standards commit resolve through GitHub. - All 15 Ephapax workflows pass Actionlint, excluding only two pre-existing `SC2129` style notices in Rust summary blocks. - The patched Standards verifier in hyperpolymath/standards#696 accepts the exact reusable path/ref while retaining a failing wrong-ref control. ## Landing order This PR is intentionally stacked on hyperpolymath/standards#696. After that source fix merges, this lock entry and all six workflow callers will be repinned to its exact merge SHA. The required end-to-end control is a Rust CI run with real jobs, followed by a green post-merge push run.
1 participant
Closes #695.
Outcome
The Standards actions-lock wrapper now recognizes the exact caller-side dependency shape GitHub requires for cross-repository reusable workflows, without weakening ordinary stale-entry detection.
Safety boundary
Only a
stalefinding is eligible, and only when the finding's own workflow file contains an exactowner/repo/.github/workflows/file@refcall that normalizes to the reportedowner/repo@refdependency. A different ref, other category, malformed JSON, or genuinely unused dependency remains blocking.Verification
bash -n scripts/update-actions-lock.sh scripts/tests/actions-lock-update-test.shshellcheck scripts/update-actions-lock.sh scripts/tests/actions-lock-update-test.shbash scripts/tests/actions-lock-update-test.shbash scripts/tests/check-trusted-base-test.sh(PASS=13 FAIL=0)rust-ci.yml -> standards@6b38eb...dependency.git diff --checkThe remaining end-to-end acceptance control is the Ephapax PR/push run: Rust CI must create jobs rather than a zero-job
startup_failure.