feat: implement issue #549 — feat(ci-standards): adopt inline-NOSONAR S7637 exemption in caller-stub templates (org-wide; supersedes per-file approach) — spike-validated - #551
Conversation
… S7637 exemption in caller-stub templates (org-wide; supersedes per-file approach) — spike-validated
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 44 minutes and 4 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds inline ChangesInline NOSONAR S7637 exemption
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![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 canonical inline # NOSONAR(githubactions:S7637) marker mechanism for first-party channel-pinned reusable-ref workflows, removing the need for per-repo sonar-project.properties files. It updates the compliance audit script, documentation, standard workflow templates, and adds corresponding BATS tests. The review feedback suggests avoiding a false positive in the compliance script when no channel-pinned stubs are present, and recommends using explicit return 0 instead of bare return statements in Bash functions to ensure robust exit code handling.
Dev-Lead — review-changes (applied)Changes committed and pushed. |
![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 `@test/scripts/compliance-audit/sonar-s7637-exemption.bats`:
- Around line 168-180: The drift-guard test currently uses a hardcoded template
list, so new shipped caller-stub workflows can slip through without coverage.
Update the `inline: every shipped channel-pinned caller-stub template is
present` test to iterate over `standards/workflows/*.yml` instead of enumerating
names manually, and keep using `classify_inline` to assert the result is never
`missing` while allowing `present` and `n/a` for non-stub workflows. This change
should be made in the Bats test around `classify_inline` so the guard
self-maintains as templates are added.
🪄 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: ca46aa93-9cb6-4be8-99c5-1c7389649e00
📒 Files selected for processing (15)
scripts/compliance-audit.shstandards/ci-standards.mdstandards/workflows/add-to-project.ymlstandards/workflows/agent-shield.ymlstandards/workflows/auto-rebase.ymlstandards/workflows/dependabot-automerge.ymlstandards/workflows/dependabot-rebase.ymlstandards/workflows/dependency-audit.ymlstandards/workflows/dev-lead.ymlstandards/workflows/idea-enhancer.ymlstandards/workflows/idea-triage.ymlstandards/workflows/initiative-planner.ymlstandards/workflows/pr-auto-review.ymlstandards/workflows/pr-review-mention.ymltest/scripts/compliance-audit/sonar-s7637-exemption.bats
|
@coderabbitai resolve |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
✅ Action performedComments resolved and changes approved. |
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
…dling Replace all bare `return` statements in compliance-audit.sh with explicit `return 0` to ensure consistent and clear exit code handling across functions. This is particularly important for check_sonar_s7637_exemption's early exit when sonarcloud.yml is not present — now it correctly returns success (0) instead of propagating the gh_api failure code. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
|
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: MEDIUM
Reviewed commit: a68bc087ed3ffa12992ecb53a002cda9107262b7
Cascade: triage → deep (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5)
Summary
PR #549 adds an inline # NOSONAR(githubactions:S7637) exemption mechanism for first-party channel-pinned reusable-workflow caller stubs, reworking check_sonar_s7637_exemption in compliance-audit.sh, updating ci-standards.md, marking all caller-stub templates, and adding bats coverage. The triage escalation (Gemini's false-positive logic bug) is RESOLVED in the final commit: both missing finding sites are guarded by [ "$inline_missing" -eq 1 ], and since classify_inline_s7637_marker only sets inline_missing together with inline_seen, that guard is equivalent to Gemini's suggested inline_seen check at the legacy branch — a repo with no channel-pinned stubs is no longer flagged. No downstream consumers were reported (DOWNSTREAM_IMPACT=none).
Findings
- INFO [correctness]: Verified RESOLVED: Gemini's flagged false positive (flagging repos with no channel-pinned first-party stubs as S7637-missing) does not occur in the final code. The legacy
missing)and no-properties branches both gate add_finding on[ "$inline_missing" -eq 1 ]; classify_inline_s7637_marker only emitsmissing(setting inline_missing=1) alongside inline_seen=1, and the inline_seen=1/inline_missing=0 case returns early as exempt. So at the legacy branch inline_missing==inline_seen, making this guard equivalent to the suggested inline_seen guard. (scripts/compliance-audit.sh:948) - INFO [security]: The NOSONAR markers are narrowly scoped to first-party channel-pinned reusable-ref
uses:lines only (regex anchored to petry-projects/.github(-private)?/.github/workflows/...@). SHA-pinned and third-party actions get no marker and remain S7637-enforced, consistent with the documented AGENTS.md mutable-ref exception. Not a new security regression; mirrors the prior sonar-project.properties exemption. SonarCloud, CodeQL, gitleaks, and ShellCheck all passed. (scripts/compliance-audit.sh) - INFO [tooling]: run_secret_scanning MCP tool was not available in this environment; skipped per instructions. The gitleaks CI secret-scan check passed (SUCCESS) on the head SHA.
- MINOR [robustness]: In check_sonar_s7637_exemption,
for wf in $workflowsrelies on unquoted word-splitting over gh_api-returned filenames. Workflow filenames don't contain whitespace in practice, so this is benign, but a while-read loop would be marginally more robust. Non-blocking. (scripts/compliance-audit.sh)
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.
2 participants
Closes #549
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
New Features
uses:line.Bug Fixes
Documentation