ci: wire node-suite regression guard (nightly + merge-queue-ready)#5255
ci: wire node-suite regression guard (nightly + merge-queue-ready)#5255TheHypnoo wants to merge 4 commits into
Conversation
…e-ready) Add .github/workflows/node-suite-guard.yml running scripts/node_suite_regression_check.py against the node 26 floor baseline (test-parity/node_suite_baseline.json). The guard fails if any baselined module's pass count drops below its floor — exactly the node:dns 83%->0% class — yet it previously ran in ZERO workflows. Runs nightly and on workflow_dispatch today. The merge_group trigger is inert until the merge queue is enabled in branch protection, at which point the guard gates every merge with no further change. Decoupled from the merge queue on purpose so it delivers value immediately. Uses sccache (now allow-listed after #5221) + Swatinem/rust-cache; node 26 to match the baseline oracle.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new GitHub Actions workflow ( ChangesNode Suite Regression Guard CI Workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/node-suite-guard.yml:
- Line 40: The workflow uses movable action tags (such as `@v6`) in multiple
`uses:` directives, which weakens supply chain security guarantees. Replace each
movable tag reference with a full commit SHA to pin the exact version of each
action. This applies to the actions/checkout action and any other actions
referenced with version tags throughout the workflow file, ensuring
reproducibility and security by pinning to specific, immutable commit hashes
instead of floating tags.
- Line 40: The actions/checkout@v6 step in the workflow should explicitly
disable credential persistence since the workflow only performs read-only
operations (building and testing) that do not require authenticated Git
operations. Add the persist-credentials property set to false in the checkout
action configuration to follow the principle of least privilege and improve the
security posture of the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 77d9257f-6aa3-44a1-9598-ebbd6af36596
📒 Files selected for processing (1)
.github/workflows/node-suite-guard.yml
| runs-on: ubuntu-latest | ||
| timeout-minutes: 120 | ||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflow actions are pinned to full SHAs (non-matching lines are violations).
rg -n --iglob '*.yml' --iglob '*.yaml' '^\s*uses:\s*[^@]+@[^[:space:]]+' .github/workflows \
| rg -n -v '@[0-9a-fA-F]{40}$'Repository: PerryTS/perry
Length of output: 6511
🏁 Script executed:
cat -n .github/workflows/node-suite-guard.yml | sed -n '35,60p'Repository: PerryTS/perry
Length of output: 998
Pin all uses: actions to full commit SHAs.
Lines 40, 43, 46, 48, and 57 use movable tags that weaken workflow supply-chain guarantees and conflict with the stated unpinned-action policy.
Suggested hardening patch
- - uses: actions/checkout@v6
+ - uses: actions/checkout@<40-char-commit-sha>
- uses: dtolnay/rust-toolchain@stable
+ uses: dtolnay/rust-toolchain@<40-char-commit-sha>
- uses: mozilla-actions/sccache-action@v0.0.10
+ uses: mozilla-actions/sccache-action@<40-char-commit-sha>
- - uses: Swatinem/rust-cache@v2
+ - uses: Swatinem/rust-cache@<40-char-commit-sha>
- uses: actions/setup-node@v6
+ uses: actions/setup-node@<40-char-commit-sha>🧰 Tools
🪛 zizmor (1.25.2)
[warning] 40-40: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 40-40: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/node-suite-guard.yml at line 40, The workflow uses movable
action tags (such as `@v6`) in multiple `uses:` directives, which weakens supply
chain security guarantees. Replace each movable tag reference with a full commit
SHA to pin the exact version of each action. This applies to the
actions/checkout action and any other actions referenced with version tags
throughout the workflow file, ensuring reproducibility and security by pinning
to specific, immutable commit hashes instead of floating tags.
Source: Linters/SAST tools
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/node-suite-guard.ymlRepository: PerryTS/perry
Length of output: 3335
🌐 Web query:
GitHub Actions checkout persist-credentials false security best practice
💡 Result:
Setting persist-credentials: false is a recommended security best practice for GitHub Actions workflows that do not require the ability to perform authenticated Git operations (such as pushing commits or tags) [1][2][3]. By default, the actions/checkout action sets persist-credentials to true, which configures the local Git repository with the authentication token (e.g., GITHUB_TOKEN) used to perform the checkout [4][5]. This makes the token accessible to any subsequent step or third-party action in the same job by reading the.git/config file [6][2][3]. Key security considerations: - Principle of Least Privilege: By setting persist-credentials: false, you prevent unnecessary exposure of your authentication token to subsequent workflow steps, reducing the risk of token theft if a step or dependency is compromised [2][3][7]. - When to use: You should set persist-credentials: false for any workflow that only performs read-only operations, such as running tests, building artifacts, or scanning code [3]. - When to avoid: You should only leave persist-credentials as true (or omit it) if your workflow explicitly requires the ability to run authenticated git commands like git push or git tag against the repository [1][2][3]. - Mitigation: Because the default is true, it must be explicitly set to false in your workflow YAML [2][7]: steps: - uses: actions/checkout@v4 with: persist-credentials: false While modern versions of actions/checkout have improved how credentials are stored (using a separate file in $RUNNER_TEMP rather than directly in.git/config in some cases), explicitly disabling persistence remains a standard security recommendation to minimize the attack surface [4][2][8].
Citations:
- 1: https://cheatsheetseries.owasp.org/cheatsheets/GitHub_Actions_Security_Cheat_Sheet.html
- 2: https://actsense.dev/vulnerabilities/unsafe_checkout/
- 3: Add persist-credentials: false to GitHub Actions checkout steps aquasecurity/trivy#10305
- 4: https://github.com/actions/checkout/?tab=readme-ov-file
- 5: https://yossarian.net/til/post/actions-checkout-can-leak-github-credentials/
- 6: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 7: https://github.com/suzuki-shunsuke/ghalint/blob/main/docs/policies/013.md
- 8: [security] Escalate concerning default
persist-credentials=trueplease actions/checkout#2312
Add persist-credentials: false to the checkout action.
The workflow only performs read-only operations (building and testing). Explicitly disabling credential persistence follows the principle of least privilege and is a security best practice recommended by OWASP and security tooling for workflows that don't require authenticated Git operations.
Suggested hardening patch
- - uses: actions/checkout@v6
+ - uses: actions/checkout@v6
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v6 | |
| - uses: actions/checkout@v6 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 40-40: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 40-40: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/node-suite-guard.yml at line 40, The actions/checkout@v6
step in the workflow should explicitly disable credential persistence since the
workflow only performs read-only operations (building and testing) that do not
require authenticated Git operations. Add the persist-credentials property set
to false in the checkout action configuration to follow the principle of least
privilege and improve the security posture of the workflow.
Source: Linters/SAST tools
…eRabbit) The guard job only builds and tests (read-only); it never performs an authenticated git operation. Disabling credential persistence keeps the GITHUB_TOKEN out of the local git config (least privilege).
|
Done — added |
2 participants
What
New
.github/workflows/node-suite-guard.ymlthat runsscripts/node_suite_regression_check.py(roadmap I-02) on a nightly schedule, onworkflow_dispatch, and onmerge_group.Why
The regression guard already existed — it runs the full print-and-diff node-suite and fails if any baselined module's pass count drops below its floor (
test-parity/node_suite_baseline.json, oracle node 26). It was written verbatim becausenode:dnsonce silently went 83% → 0% behind a green build — yet it was wired into zero workflows. This wires it in.Design / scope
merge_grouptrigger is inert until a maintainer enables the merge queue in branch protection — then the guard gates every merge with no further workflow change.sccache(now allow-listed after ci(test): add sccache + bound cargo-test timeout #5221) +Swatinem/rust-cache; allow-listed actions only.pull_requesttrigger by design, so it won't run on this PR; validate before merge withgh workflow run "Node Suite Regression Guard" --ref <branch>if desired — note it builds + runs the full suite, ~1–2 h.)Follow-up (separate, maintainer's call)
Enabling the GitHub merge queue and routing the existing heavy jobs (
parity,compile-smoke, the framework smokes) tomerge_groupis a workflow-wide change, left as a separate decision. Once the queue is enabled, this guard already runs in it.Summary by CodeRabbit