feat: implement issue #842 — SonarCloud: GitHub Actions / dependency hardening - #843
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe dependency-audit workflows annotate pinned ChangesGo audit hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new BATS test to verify that all 'go install' commands in workflows are version-pinned and contain the 'NOSONAR(githubactions:S8545)' exemption marker. Feedback on this change highlights that the pattern matching used to detect 'go install' is fragile and can be bypassed if the command appears at the start of a line or within quotes. A more robust pattern-matching suggestion has been provided to ensure all instances are reliably caught without triggering false positives on similar commands like 'cargo install'.
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Pull request overview
Resolves SonarCloud rule githubactions:S8545 findings in the dependency audit workflows by documenting an inline suppression for go install (as a confirmed false positive when version-pinned) and adding a Bats regression guard to prevent future unpinned installs or missing suppressions.
Changes:
- Add a Bats test that enforces
go installusage in workflows is version-pinned and includes an inlineNOSONAR(githubactions:S8545)marker. - Add inline
NOSONAR(githubactions:S8545)comments to thegovulncheckinstall steps in both dependency audit workflows.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/workflows/dependency-hardening.bats | Adds a regression guard for version-pinned go install plus required Sonar suppression marker. |
| .github/workflows/dependency-audit.yml | Adds inline Sonar suppression comment for the pinned govulncheck install. |
| .github/workflows/dependency-audit-reusable.yml | Adds inline Sonar suppression comment for the pinned govulncheck install. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: e441bdf07046d302d5f7adaa683edcb70230f315
Review mode: triage-approved (single reviewer)
Summary
Resolves SonarCloud S8545 findings in the two dependency-audit workflows via inline NOSONAR suppressions (confirmed false positive: go install @vX.Y.Z is fully pinned and verified via the Go checksum DB; Go has no --locked equivalent) and adds a bats regression guard requiring any future 'go install' in workflows to be version-pinned and carry the suppression marker. The workflow changes are YAML-comment-only — the executed command is unchanged and remains pinned at govulncheck@v1.1.4.
Linked issue analysis
Closes #842 (SonarCloud: GitHub Actions / dependency hardening, 2x githubactions:S8545). The issue's acceptance criteria explicitly allow inline NOSONAR for confirmed false positives noted inline — exactly what this PR does, with the justification documented both in the workflow comments and in the new bats test. SonarCloud quality gate on this PR passed with 0 new issues and 0 security hotspots. Substantively addressed.
Findings
- No blocking findings.
- Workflow diff is comment-only: the trailing '# NOSONAR(...)' is a YAML comment, so the run command executed by the shell is byte-identical to before (no behavior change).
- New bats test correctly excludes near-matches like 'cargo install' (word-char-prefix guard) and fails loud on unpinned installs or missing markers; it passed in CI (Dependency Hardening Tests / bats: SUCCESS).
- Prior bot review comments (Gemini pattern fragility, Copilot @v-prefix looseness and justification wording) were addressed in follow-up commits; all 5 review threads are resolved.
- Secret-scanning MCP tool unavailable in this environment; gitleaks CI check passed (SUCCESS).
CI status
All checks green: Lint, ShellCheck, Agent Security Scan, Secret scan (gitleaks), CodeQL (Analyze actions), SonarCloud + SonarCloud Code Analysis (quality gate passed), Dependency Hardening Tests (bats), Dependency audit (npm audit; ecosystem-specific jobs skipped as expected), AgentShield, CodeRabbit. No failures; skipped jobs are conditional ecosystem audits.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
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 `@test/workflows/dependency-hardening.bats`:
- Around line 60-66: Update the workflow file enumeration used by
workflow_command_lines to include both .yml and .yaml files, preserving the
existing command scanning and validation behavior so the go install guard covers
every workflow extension.
- Around line 74-79: Update the command validation in the dependency-hardening
workflow to split each line into command text and inline comment suffix before
checking it. Validate the go install version against only the command tokens,
and require NOSONAR(githubactions:S8545) specifically within the comment suffix,
preserving the existing failure reporting and fail flag behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a4a039cb-321e-43e3-ad39-3aa9a365515b
📒 Files selected for processing (3)
.github/workflows/dependency-audit-reusable.yml.github/workflows/dependency-audit.ymltest/workflows/dependency-hardening.bats
| @test "every 'go install' in a workflow is version-pinned and S8545-exempted" { | ||
| # S8545 flags `go install` as non-lockfile-enforcing. Go has no --locked / | ||
| # --require-hashes equivalent: a pinned `@vX.Y.Z` is the reproducible install | ||
| # (verified via the Go checksum DB, sum.golang.org), so the finding is a confirmed | ||
| # false positive suppressed with an inline `# NOSONAR(githubactions:S8545)` | ||
| # marker. This guard fails loud if a new `go install` is added unpinned | ||
| # (@latest/@main) or without the marker. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover .yaml workflows too.
workflow_command_lines only enumerates .yml files, so this “every workflow” guard will miss unpinned go install commands in .yaml workflows. Extend the helper to scan both extensions.
As per coding guidelines, workflow checks must cover both .yml and .yaml files.
Proposed fix
- for f in "$REPO_ROOT"/.github/workflows/*.yml; do
+ for f in "$REPO_ROOT"/.github/workflows/*.yml \
+ "$REPO_ROOT"/.github/workflows/*.yaml; do
+ [ -f "$f" ] || continue🤖 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 `@test/workflows/dependency-hardening.bats` around lines 60 - 66, Update the
workflow file enumeration used by workflow_command_lines to include both .yml
and .yaml files, preserving the existing command scanning and validation
behavior so the go install guard covers every workflow extension.
Source: Coding guidelines
| case "$cmd" in *"@v"[0-9]*) : ;; *) | ||
| echo "UNPINNED go install (needs @vX.Y.Z): ${file##*/}: $cmd"; fail=1 ;; | ||
| esac | ||
| case "$cmd" in *"NOSONAR(githubactions:S8545)"*) : ;; *) | ||
| echo "MISSING # NOSONAR(githubactions:S8545): ${file##*/}: $cmd"; fail=1 ;; | ||
| esac |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate command tokens, not arbitrary line text.
The checks search the complete raw line, so an unpinned command such as go install ...@latest # @v1 NOSONAR(githubactions:S8545) would pass. Strip the inline comment before validating the package version, and verify the NOSONAR marker specifically in the comment suffix.
Proposed validation shape
- case "$cmd" in *"`@v`"[0-9]*) : ;; *)
+ cmd_without_comment=${cmd%%#*}
+ if [[ "$cmd_without_comment" =~ (^|[[:space:]])go[[:space:]]+install[[:space:]]+[^[:space:]]+@v[0-9]+(\.[0-9]+){2}([[:space:]]|$) ]]; then
+ :
+ else
echo "UNPINNED go install (needs `@vX.Y.Z`): ${file##*/}: $cmd"; fail=1 ;;
- esac
+ fi🤖 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 `@test/workflows/dependency-hardening.bats` around lines 74 - 79, Update the
command validation in the dependency-hardening workflow to split each line into
command text and inline comment suffix before checking it. Validate the go
install version against only the command tokens, and require
NOSONAR(githubactions:S8545) specifically within the comment suffix, preserving
the existing failure reporting and fail flag behavior.
Review — fix requested (cycle 1/3)The automated review identified the following issues. Please address each one: Findings to fixAutomated review — NEEDS HUMAN REVIEWRisk: LOW SummaryPR adds inline NOSONAR(githubactions:S8545) suppressions to two dependency-audit workflows (comment-only; the pinned Downstream impactThis change is consumed by 7 downstream repo(s) that pin the affected reusable workflow / lib / prompt. Impacted consumers: Findings
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5). Reply if you need a human review. Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
Closes #842
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit