fix(compliance-audit): exempt internal reusable workflow refs from SHA pinning check#263
fix(compliance-audit): exempt internal reusable workflow refs from SHA pinning check#263don-petry wants to merge 0 commit into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe compliance audit script updates its unpinned GitHub Actions detection to exclude internal ChangesCompliance Audit Exception
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
🚥 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)
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 |
|
CI is green. @petry-projects/org-leads — this PR is ready for review and merge. |
![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 updates the compliance audit script to exempt internal reusable workflows from SHA-pinning requirements, which prevents OIDC validation failures. A review comment suggests anchoring the exclusion regex to the start of the line to ensure the audit remains robust against false negatives in commented-out code.
| # cause OIDC validation failures for claude.yml on PR branches. | ||
| local unpinned | ||
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' || true) | ||
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'uses:\s+petry-projects/\.github/' || true) |
There was a problem hiding this comment.
The exclusion regex for internal reusable workflows is not anchored to the start of the line. This can lead to false negatives if a uses: directive for a different action contains the exempt string within a comment (e.g., uses: actions/checkout@v4 # uses: petry-projects/.github/...). Anchoring the regex ensures that only the actual action reference is evaluated for exemption, maintaining the integrity of the audit for other actions.
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'uses:\s+petry-projects/\.github/' || true) | |
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE '^\s*-?\s*uses:\s+petry-projects/\.github/' || true) |
There was a problem hiding this comment.
Pull request overview
Updates the compliance audit’s action SHA-pinning check to stop reporting false positives for Tier 1 workflow caller stubs that legitimately reference petry-projects/.github reusable workflows via tag refs (@v1, @main), aligning the audit behavior with the documented exception in ci-standards.md.
Changes:
- Exempts
petry-projects/.githubreferences from thecheck_action_pinning()“unpinned actions” scan. - Adds inline documentation in
compliance-audit.shexplaining the internal reusable workflow exception and OIDC validation motivation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # cause OIDC validation failures for claude.yml on PR branches. | ||
| local unpinned | ||
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' || true) | ||
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'uses:\s+petry-projects/\.github/' || true) |
| # cause OIDC validation failures for claude.yml on PR branches. | ||
| local unpinned | ||
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' || true) | ||
| unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'uses:\s+petry-projects/\.github/' || true) |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 49c39329fe2d6ca5c7ae3c68d48be75c2ba243d6
Review mode: triage-approved (single reviewer)
Summary
A 5+/1- change to scripts/compliance-audit.sh that extends check_action_pinning() with a grep -vE 'uses:\s+petry-projects/\.github/' filter so internal reusable workflow refs are no longer flagged as unpinned. The exemption is explicitly documented in standards/ci-standards.md#action-pinning-policy ("Exception: Internal Reusable Workflow References") and is required because Anthropic OIDC validates that claude.yml on a PR branch is byte-for-byte identical to main — any diff, including a SHA pin on uses:, triggers 401 Workflow validation failed. The org controls the @v1 tag, so the standard supply-chain rationale for SHA-pinning does not apply here.
Linked issue analysis
The PR is scoped against #261 (umbrella compliance-audit findings, 2026-05-12) and addresses a specific subset: the false-positive unpinned-actions-agent-shield.yml, unpinned-actions-claude.yml, and unpinned-actions-dependabot-automerge.yml findings on the .github repo (and equivalents in downstream repos). The PR description is explicit that the remaining categories (repo settings, secret-scanning, workflow additions, genuine SHA-pinning, and secret_scan_ci_job_present) require admin API credentials or workflows write permission and are out of scope here. Partial closure of #261 is appropriate.
Findings
No blocking issues.
- Style nit (non-blocking):
gemini-code-assistsuggested anchoring the new exclusion regex to the start of the line for robustness against commented-out code. This is unnecessary: the upstream filter in the same pipeline (^\s*-?\s*uses:\s+[^#]*@) is start-anchored and does not match comment lines like# uses: ..., so the exemption filter never sees commented-out content. The current form is correct and readable; no change required. - Scope of regex:
uses:\s+petry-projects/\.github/targets only refs under thepetry-projects/.githuborg's own path. This matches the documented exception verbatim and does not over-exempt otherpetry-projects/*repos. - Branch is BEHIND
main: non-blocking, just needs a rebase before merge.
CI status
All required checks green: AgentShield, CI (Lint, ShellCheck, Agent Security Scan), Claude Code, CodeQL (Analyze actions), SonarCloud (Quality Gate passed, 0 new issues), Dependency audit (Detect ecosystems), CodeRabbit. No security warnings raised by any scanner on this diff.
Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/compliance-audit.sh`:
- Line 217: The grep exclusion is too broad—update the unpinned assignment
(variable unpinned) so it only ignores reusable workflow references under
petry-projects/.github (e.g. paths containing ".github/workflows/"), not every
petry-projects/.github usage; replace the final grep -vE
'uses:\s+petry-projects/\.github/' with a more specific pattern like -vE
'uses:\s+petry-projects/\.github/.*workflows/' so only reusable workflow calls
are exempted while other petry-projects/.github action uses remain checked.
🪄 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: 24247e1c-5fce-4c54-8f04-25a725b947bd
📒 Files selected for processing (1)
scripts/compliance-audit.sh
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/compliance-audit.sh (1)
217-217:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNarrow the exemption to internal reusable workflows only.
At Line 217, the filter excludes every
uses: petry-projects/.github/...reference, which is broader than the documented exception and can mask real unpinned findings. Restrict the exemption to reusable workflow paths (.github/workflows/...@ref) only.Suggested diff
- unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'uses:\s+petry-projects/\.github/' || true) + unpinned=$(echo "$decoded" \ + | grep -E '^\s*-?\s*uses:\s+[^#]*@' \ + | grep -vE '@[0-9a-f]{40}' \ + | grep -vE '(docker://|\.\/)' \ + | grep -vE '^[[:space:]]*-?[[:space:]]*uses:[[:space:]]*petry-projects/\.github/\.github/workflows/[^@]+@' \ + || true)🤖 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 `@scripts/compliance-audit.sh` at line 217, The current unpinned detection command (assigning to variable unpinned) is too broad because it excludes any uses: petry-projects/.github/... references; update the grep exclusion so it only exempts reusable workflow calls under .github/workflows (e.g., match uses:\s+petry-projects/\.github/workflows/[^@]+@) instead of the existing petry-projects/\.github/ pattern, leaving all other filters intact so genuine unpinned uses are still reported.
🤖 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.
Duplicate comments:
In `@scripts/compliance-audit.sh`:
- Line 217: The current unpinned detection command (assigning to variable
unpinned) is too broad because it excludes any uses: petry-projects/.github/...
references; update the grep exclusion so it only exempts reusable workflow calls
under .github/workflows (e.g., match
uses:\s+petry-projects/\.github/workflows/[^@]+@) instead of the existing
petry-projects/\.github/ pattern, leaving all other filters intact so genuine
unpinned uses are still reported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 045a96b0-303a-4c4f-b172-f00f12fb1847
📒 Files selected for processing (1)
scripts/compliance-audit.sh
Superseded by automated re-review at
|
donpetry-bot
dismissed
their stale review
Superseded by automated re-review at 88636c7.
Review — fix requested (cycle 3/3)The automated review identified the following issues. Please address each one: Findings to fix[Findings would be inserted here] Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
Automated review — human attention neededThis PR has been through 3 automated review cycles (cap: 3) without converging on an approval-and-merge state. Further automated review has been paused to avoid infinite loops. Please take a look manually, or close this PR if it's no longer needed. Once a human review resolves the situation, remove the Posted by the donpetry-bot PR-review cascade. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/compliance-audit.sh (1)
224-224:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnresolved issue from previous review: Exemption pattern is too broad.
This concern was previously raised and remains unfixed. The pattern
uses:\s+petry-projects/\.github/excludes allpetry-projects/.github/references, not just reusable workflows. This deviates from the documented policy in the comments (lines 219-222) which specifies "internal reusable workflow refs" only.Risk: If
petry-projects/.githubever contains composite actions or other GitHub Actions (not reusable workflows), references to them with tag pins like@v1would be incorrectly exempted from SHA-pinning checks, creating a compliance gap.Fix: Narrow the pattern to match only reusable workflow paths (
petry-projects/.github/.github/workflows/):🔧 Suggested fix
- unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'uses:\s+petry-projects/\.github/' || true) + unpinned=$(echo "$decoded" \ + | grep -E '^\s*-?\s*uses:\s+[^#]*@' \ + | grep -vE '@[0-9a-f]{40}' \ + | grep -vE '(docker://|\.\/)' \ + | grep -vE '^[[:space:]]*-?[[:space:]]*uses:[[:space:]]*petry-projects/\.github/\.github/workflows/[^@]+@' \ + || true)🤖 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 `@scripts/compliance-audit.sh` at line 224, The current exclusion in the construction of the unpinned variable uses a too-broad pattern `uses:\s+petry-projects/\.github/` which exempts all references to petry-projects/.github (including composite actions); update the grep/regex in the unpinned assignment that builds the unpinned variable to only exclude internal reusable workflow refs by matching the reusable-workflow path (e.g., `petry-projects/.github/.github/workflows/`) instead of the entire `.github` repo root so only reusable workflow usages are skipped.
🤖 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.
Duplicate comments:
In `@scripts/compliance-audit.sh`:
- Line 224: The current exclusion in the construction of the unpinned variable
uses a too-broad pattern `uses:\s+petry-projects/\.github/` which exempts all
references to petry-projects/.github (including composite actions); update the
grep/regex in the unpinned assignment that builds the unpinned variable to only
exclude internal reusable workflow refs by matching the reusable-workflow path
(e.g., `petry-projects/.github/.github/workflows/`) instead of the entire
`.github` repo root so only reusable workflow usages are skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d8f8f26-ba01-4b6c-a716-6ea7bf122a4c
📒 Files selected for processing (1)
scripts/compliance-audit.sh
|
@dev-lead - please fix this PR |
|
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
|
Auto-rebase failed — merge conflict — this branch has conflicts with dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention. To resolve manually instead: |
Pull request was closed
don-petry
force-pushed
the
claude/issue-261-20260512-1901
branch
from
59b6003 to
b6f0535
Compare
|
Automated rebase result: the PR's fix was already independently applied to After rebasing onto current No action needed; the intent of issue #261 is already satisfied on main. |
Dev-Lead — rebase (no-changes)Agent reasoning |
Summary
unpinned-actions-*findings in compliance audit for Tier 1 caller stubs that referencepetry-projects/.githubinternal reusable workflowsagent-shield.yml,claude.yml,dependabot-automerge.yml(and equivalents in all downstream repos) because the@v1/@maintag refs used for internal reusable workflows are explicitly exempt from SHA pinning perci-standards.md#action-pinning-policyRoot cause
check_action_pinning()inscripts/compliance-audit.shscanned alluses:lines for non-SHA refs but did not implement the documented exemption forpetry-projects/.githubinternal reusable workflow references. This produced false positives for every repo that correctly uses Tier 1 stub workflows.Why internal reusable refs must NOT be SHA-pinned
.github/workflows/claude.ymlon a PR branch is byte-for-byte identical tomain— any diff (including a SHA pin onuses:) causes401 Workflow validation failedpetry-projects/.githuborg controls the@v1tag — no supply-chain risk from tag-force-push@v1allows the org to push improvements to all downstream repos simultaneously by bumping the tagCompliance findings addressed (
.githubrepo)unpinned-actions-agent-shield.ymlunpinned-actions-claude.ymlunpinned-actions-dependabot-automerge.ymlRelated PRs created in other repos
CLAUDE.md+dependabot.yml(fixesclaude-md-missing-agents-ref,missing-github-actions-ecosystem,missing-security-label,missing-dependencies-label)wrong-limit-npm(setsopen-pull-requests-limit: 0)Remaining findings requiring human action (cannot be fixed via PR alone)
The following categories across all 8 repos require either admin API credentials or
workflowswrite permission:allow_auto_merge,delete_branch_on_merge,check-suite-auto-trigger-*)bash scripts/apply-repo-settings.sh <repo>with admin tokensecret_scanning_ai_detection,secret_scanning_non_provider_patterns)bash scripts/apply-repo-settings.sh <repo>workflowswrite permission — not available to this agentunpinned-actions-*genuine findings)workflowswrite permissionsecret_scan_ci_job_presentci.yml— needsworkflowspermissionCloses #261
Generated with Claude Code
Summary by CodeRabbit