Skip to content

fix: exempt internal reusable workflow refs from action-pinning check#242

Open
don-petry wants to merge 30 commits into
mainfrom
claude/issue-239-20260511-1242
Open

fix: exempt internal reusable workflow refs from action-pinning check#242
don-petry wants to merge 30 commits into
mainfrom
claude/issue-239-20260511-1242

Conversation

@don-petry
Copy link
Copy Markdown
Contributor

Summary

  • The check_action_pinning function in scripts/compliance-audit.sh was generating false-positive findings for petry-projects/.github reusable workflow refs (e.g. @v1 tags)
  • standards/ci-standards.md#exception-internal-reusable-workflow-references explicitly exempts these refs from SHA pinning — they intentionally use tags and must NOT be SHA-pinned
  • Added a grep -vE 'petry-projects/\.github/' exclusion to filter out these refs from the unpinned-action detection

Test plan

  • Verify the compliance audit no longer flags dependabot-automerge.yml (or any other Tier 1 caller stub) for using @v1 on internal reusable workflow refs
  • Verify the audit still flags genuine third-party actions pinned to tags (e.g. actions/checkout@v4)
  • Run scripts/compliance-audit.sh with DRY_RUN=true to confirm no false-positive action-pinning findings in the .github repo

Closes #239

Generated with Claude Code

Copilot AI review requested due to automatic review settings May 11, 2026 12:46
@don-petry don-petry requested a review from a team as a code owner May 11, 2026 12:46
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Warning

Rate limit exceeded

@don-petry has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 49 minutes and 18 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5c7bc202-d918-4b67-8131-23f7acdbceab

📥 Commits

Reviewing files that changed from the base of the PR and between cb9fac2 and 774e6df.

📒 Files selected for processing (1)
  • scripts/compliance-audit.sh
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-239-20260511-1242

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@don-petry
Copy link
Copy Markdown
Contributor Author

CI is green (Lint, ShellCheck, AgentShield, SonarCloud all pass). CodeQL is still running but typically passes for bash-only changes.

Self-review notes:

  • The grep -vE 'petry-projects/\.github/' exclusion correctly exempts all petry-projects/.github reusable workflow refs (both @v1 and @main) from the SHA-pinning check
  • Uses an escaped dot (\.) so it only matches a literal dot, not any character
  • Trailing / ensures we match the path separator and don't accidentally exempt an unrelated petry-projects/.github-other repo
  • Consistent with other exemptions already in the script (e.g., -reusable.yml skip in check_workflow_permissions, .github repo skip in check_centralized_workflow_stubs)

@petry-projects/org-leads — this PR fixes a false-positive in the compliance audit. Ready for review and merge.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the compliance audit script to exempt internal reusable workflows from SHA-pinning requirements, aligning with organizational policy. The review feedback identifies a potential issue where the broad regex used for the exemption could lead to false negatives if the exclusion string appears within a comment on a line containing an unpinned third-party action. A more robust, anchored regex was suggested to ensure the exemption only applies to the action reference itself.

# standards/ci-standards.md#exception-internal-reusable-workflow-references
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 'petry-projects/\.github/' || true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The regex petry-projects/\.github/ is applied to the entire line, which can lead to false negatives if a workflow file contains comments referencing the internal repository on the same line as an unpinned third-party action (e.g., uses: actions/checkout@v4 # see petry-projects/.github/).

To ensure the exemption only applies to the action reference itself and to improve the robustness of the existing exclusions, I suggest anchoring the regex to the uses: directive and consolidating the exclusions into a single anchored pattern.

Suggested change
unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '(docker://|\.\/)' | grep -vE 'petry-projects/\.github/' || true)
unpinned=$(echo "$decoded" | grep -E '^\s*-?\s*uses:\s+[^#]*@' | grep -vE '@[0-9a-f]{40}' | grep -vE '^\s*-?\s*uses:\s+(docker://|\.\/|petry-projects/\.github/)' || true)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the org-wide compliance audit script to align check_action_pinning behavior with the documented exception that internal petry-projects/.github reusable workflows may use tag refs (e.g., @v1) instead of SHA pins, avoiding false-positive compliance findings.

Changes:

  • Excludes petry-projects/.github reusable workflow references from the unpinned-action detection pipeline in scripts/compliance-audit.sh.
  • Adds inline comments linking the exclusion to the corresponding standards exception section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

donpetry-bot
donpetry-bot previously approved these changes May 11, 2026
Copy link
Copy Markdown
Contributor

@donpetry-bot donpetry-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: dc45fcd0a7cc56e045e81aad46b41a1f9b52e3fc
Review mode: triage-approved (single reviewer)

Summary

Confirms the triage assessment. This is a 4-line change to scripts/compliance-audit.sh adding a grep -vE 'petry-projects/\.github/' exclusion to check_action_pinning, with a clear inline comment pointing at the relevant exception in standards/ci-standards.md. Internal reusable workflow refs intentionally use @v1 tags rather than SHA pins, and the previous logic was generating false-positive compliance findings against caller stubs like dependabot-automerge.yml.

Linked issue analysis

Closes #239 (Compliance: unpinned-actions-dependabot-automerge.yml). The issue is itself an auto-filed false-positive — it flagged petry-projects/.github/.github/workflows/dependabot-automerge-reusable.yml@v1, which is exactly the kind of internal ref the new exclusion targets. Fix is substantively aligned with the issue.

Findings

  • Scope is appropriately narrow. Only one function (check_action_pinning) is touched; existing exclusions for docker:// and ./ are preserved.
  • Regex correctness. petry-projects/\.github/ correctly escapes the dot and includes a trailing /, so it won't spuriously exempt a hypothetical petry-projects/.github-other repo.
  • Minor nit (non-blocking, noted by gemini-code-assist). The new grep -vE runs against full lines rather than the captured uses: ref. A line like uses: actions/checkout@v4 # see petry-projects/.github/... would be silently filtered, masking a real third-party finding. Probability is very low in practice, and the surrounding pipeline already partially mitigates via [^#]*@. A future hardening could anchor the exclusion to the @ portion of the ref. Not a blocker for this fix.
  • No security regression. The exemption is policy-aligned (documented in standards/ci-standards.md#exception-internal-reusable-workflow-references); it does not weaken pinning for any third-party action.

CI status

All required checks green: Lint, ShellCheck, Agent Security Scan, AgentShield, SonarCloud, CodeQL, Claude Code, CodeRabbit, Dependency audit (ecosystem detector). Skipped jobs are conditional (Dependabot auto-merge, mention triggers, language-specific audits not applicable to this repo). SonarCloud Quality Gate passed with 0 new issues.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

@donpetry-bot donpetry-bot enabled auto-merge (squash) May 11, 2026 13:52
@don-petry don-petry closed this May 11, 2026
auto-merge was automatically disabled May 11, 2026 21:35

Pull request was closed

@don-petry don-petry reopened this May 11, 2026
@don-petry don-petry closed this May 12, 2026
@don-petry don-petry reopened this May 12, 2026
@don-petry don-petry enabled auto-merge (squash) May 12, 2026 01:42
@don-petry don-petry dismissed donpetry-bot’s stale review May 12, 2026 02:50

The merge-base changed after approval.

@github-actions
Copy link
Copy Markdown
Contributor

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved manually.

Please resolve the conflicts and push:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

The check_action_pinning function was flagging `petry-projects/.github`
reusable workflow refs (e.g. `@v1`) as unpinned. However, ci-standards.md
explicitly exempts these from the SHA-pinning policy — internal reusable
workflow refs use tags by design and must NOT be SHA-pinned.

Add a grep exclusion for `petry-projects/.github/` refs so the audit
no longer generates false-positive findings like issue #239.

Closes #239

Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
@don-petry don-petry force-pushed the claude/issue-239-20260511-1242 branch from c17229f to 8913355 Compare May 13, 2026 16:53
This was referenced May 13, 2026
donpetry-bot
donpetry-bot previously approved these changes May 14, 2026
Copy link
Copy Markdown
Contributor

@donpetry-bot donpetry-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: 802ad9bcaa94c13aa7383878957083951ad35997
Review mode: triage-approved (single reviewer)

Summary

Confirms the triage assessment. This is a 4-line change to scripts/compliance-audit.sh adding a grep -vE 'petry-projects/\.github/' exclusion to check_action_pinning, with an inline comment pointing at the documented exemption in standards/ci-standards.md#exception-internal-reusable-workflow-references. Since the prior (dismissed) review at c17229f, the only new commits are two merge-from-main commits (72305e4a, 802ad9bc) — no functional drift, the fix itself is unchanged.

Linked issue analysis

Closes #239 (Compliance: unpinned-actions-dependabot-automerge.yml), an auto-filed false-positive flagging petry-projects/.github/.github/workflows/dependabot-automerge-reusable.yml@v1. That ref is exactly the kind of internal reusable workflow the new exclusion targets, and standards/ci-standards.md explicitly exempts these refs from SHA pinning. Substantively addressed.

Findings

  • Scope is appropriately narrow. Only one function (check_action_pinning) is touched; existing exclusions for docker:// and ./ are preserved.
  • Regex correctness. petry-projects/\.github/ correctly escapes the dot and includes a trailing /, so it won't spuriously exempt a hypothetical petry-projects/.github-other repo.
  • Minor nit (non-blocking, previously noted by gemini-code-assist). The grep -vE runs against full lines rather than the captured uses: ref. A line like uses: actions/checkout@v4 # see petry-projects/.github/... would be silently filtered, masking a real third-party finding. Probability is very low in practice and the upstream [^#]*@ partially mitigates. A future hardening could anchor the exclusion to the @-ref portion; not a blocker.
  • No security regression. The exemption is policy-aligned; it does not weaken pinning for any third-party action.
  • Merge state. mergeable: MERGEABLE; mergeStateStatus: BLOCKED reflects review-required, not conflict. The earlier auto-rebase-conflict comment from 2026-05-12 has since been resolved by the two merge-from-main commits.

CI status

Green: CodeQL (Analyze actions) SUCCESS, CodeRabbit SUCCESS. Author-reported additional checks (Lint, ShellCheck, AgentShield, SonarCloud Quality Gate — 0 new issues) also passed.


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

Copy link
Copy Markdown
Contributor

@donpetry-bot donpetry-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: LOW
Reviewed commit: 146bf90460a6ca6dfbd7b76cb94d9d9421d23b4c
Review mode: triage-approved (single reviewer)

Summary

Confirms the triage assessment. This is a 4-line change to scripts/compliance-audit.sh adding a grep -vE 'petry-projects/\.github/' exclusion to check_action_pinning, with an inline comment pointing at the documented exemption in standards/ci-standards.md#exception-internal-reusable-workflow-references. Since the prior approved review at 802ad9bc, the only new commits are two merge-from-main commits (a70a6ea0, 146bf904) that pull in unrelated upstream work (#281 auto-rebase fallback, #275 check-suite trigger fix); the fix itself is unchanged.

Linked issue analysis

Closes #239 (Compliance: unpinned-actions-dependabot-automerge.yml), an auto-filed false-positive flagging petry-projects/.github/.github/workflows/dependabot-automerge-reusable.yml@v1. That ref is exactly the kind of internal reusable workflow the new exclusion targets, and standards/ci-standards.md explicitly exempts these refs from SHA pinning. Substantively addressed.

Findings

  • Scope is appropriately narrow. Only one function (check_action_pinning) is touched; existing exclusions for docker:// and ./ are preserved.
  • Regex correctness. petry-projects/\.github/ correctly escapes the dot and includes a trailing /, so it won't spuriously exempt a hypothetical petry-projects/.github-other repo.
  • Minor nit (non-blocking, previously noted by gemini-code-assist). The grep -vE runs against full lines rather than the captured uses: ref. A line like uses: actions/checkout@v4 # see petry-projects/.github/... would be silently filtered, masking a real third-party finding. Probability is very low in practice and the upstream [^#]*@ partially mitigates. A future hardening could anchor the exclusion to the @-ref portion; not a blocker.
  • No security regression. The exemption is policy-aligned; it does not weaken pinning for any third-party action.
  • Merge state. mergeable: MERGEABLE; mergeStateStatus: BLOCKED reflects review-required, not conflict. The earlier auto-rebase-conflict comment from 2026-05-12 has since been resolved by the merge-from-main commits.

CI status

All required checks green: Lint, ShellCheck, Agent Security Scan, Secret scan (gitleaks), AgentShield, SonarCloud (Quality Gate passed — 0 new issues), CodeQL (Analyze actions), Claude Code, CodeRabbit, Dependency audit (ecosystem detector). Skipped jobs are conditional (Dependabot auto-merge, language-specific audits not applicable, mention-triggered Claude jobs).


Reviewed automatically by the PR-review agent (single-reviewer mode: opus 4.7). Reply if you need a human review.

@don-petry
Copy link
Copy Markdown
Contributor Author

@dev-lead - please fix this PR

@sonarqubecloud
Copy link
Copy Markdown

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry
Copy link
Copy Markdown
Contributor Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

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:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compliance: unpinned-actions-dependabot-automerge.yml

3 participants