fix(ci): remediate .github compliance findings — 2026-04-17 audit#147
fix(ci): remediate .github compliance findings — 2026-04-17 audit#147don-petry wants to merge 5 commits 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 (7)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Remediates the 2026-04-17 compliance audit findings for the .github repo by tightening GitHub Actions SHA pinning, centralizing dependency-audit logic into the org reusable workflow, and adding a CI secret-scanning job.
Changes:
- Converted
dependency-audit.ymlfrom an inline workflow into a thin caller stub that delegates to the org reusable workflow. - SHA-pinned Tier-1 stub workflow
uses:references (claude.yml,agent-shield.yml,dependency-audit.yml). - Added
secret-scanCI job usinggitleaks/gitleaks-actionwith full-history checkout.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| .github/workflows/dependency-audit.yml | Replaced inline audit implementation with reusable-workflow caller and added “thin stub” header block. |
| .github/workflows/claude.yml | Pinned reusable workflow reference from branch/tag to commit SHA. |
| .github/workflows/ci.yml | Added secret-scan job running gitleaks with fetch-depth: 0. |
| .github/workflows/agent-shield.yml | Pinned reusable workflow reference from tag to commit SHA. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| dependency-audit: | ||
| uses: petry-projects/.github/.github/workflows/dependency-audit-reusable.yml@ae9709f4466dec60a5733c9e7487f69dcd004e05 # v1 |
There was a problem hiding this comment.
This stub claims standards/workflows/dependency-audit.yml as the “SOURCE OF TRUTH”, but that template currently uses @v1 (tag) while this file pins the reusable to a commit SHA. Please reconcile this by updating the standards template (and any related docs) to match the SHA-pinning requirement, otherwise future “copy verbatim” adoptions will drift and/or fail the compliance audit.
| jobs: | ||
| claude-code: | ||
| uses: petry-projects/.github/.github/workflows/claude-code-reusable.yml@main | ||
| uses: petry-projects/.github/.github/workflows/claude-code-reusable.yml@ae9709f4466dec60a5733c9e7487f69dcd004e05 # v1 |
There was a problem hiding this comment.
standards/ci-standards.md describes Tier-1 stub workflows as delegating to reusables via ...@v1, and standards/workflows/claude.yml also uses @v1. This stub is now SHA-pinned instead, which creates a standards/templates mismatch. Please update the standards template/docs (or the audit exemption rules) so adopters can still “copy verbatim” without creating drift.
| uses: petry-projects/.github/.github/workflows/claude-code-reusable.yml@ae9709f4466dec60a5733c9e7487f69dcd004e05 # v1 | |
| uses: petry-projects/.github/.github/workflows/claude-code-reusable.yml@v1 |
| jobs: | ||
| agent-shield: | ||
| uses: petry-projects/.github/.github/workflows/agent-shield-reusable.yml@v1 | ||
| uses: petry-projects/.github/.github/workflows/agent-shield-reusable.yml@ae9709f4466dec60a5733c9e7487f69dcd004e05 # v1 |
There was a problem hiding this comment.
standards/ci-standards.md and standards/workflows/agent-shield.yml currently define the Tier-1 stub as calling the reusable via ...@v1. This file now pins to a SHA, which means the “SOURCE OF TRUTH” template and documentation are out of sync with what compliance requires. Please reconcile by updating the standards template/docs (or the audit script) so downstream repos can adopt verbatim without failing audits.
| steps: | ||
| - name: Checkout (full history) | ||
| # Pin to SHA per Action Pinning Policy (ci-standards.md#action-pinning-policy). | ||
| # Look up current SHA: gh api repos/actions/checkout/git/refs/tags/v4 --jq '.object.sha' |
There was a problem hiding this comment.
The checkout SHA comment and the “Look up current SHA” command disagree: it says to query refs/tags/v4, but the pinned version comment is # v6.0.2. Please update the lookup command (and/or version comment) so maintainers don’t fetch the wrong ref when refreshing the pin.
| # Look up current SHA: gh api repos/actions/checkout/git/refs/tags/v4 --jq '.object.sha' | |
| # Look up current SHA: gh api repos/actions/checkout/git/refs/tags/v6.0.2 --jq '.object.sha' |
| # Standard: https://github.com/petry-projects/.github/blob/main/standards/dependabot-policy.md#vulnerability-audit-ci-check | ||
| # ───────────────────────────────────────────────────────────────────────────── | ||
| # SOURCE OF TRUTH: petry-projects/.github/standards/workflows/dependency-audit.yml | ||
| # Standard: petry-projects/.github/standards/ci-standards.md#5-dependency-audit-dependency-auditym |
There was a problem hiding this comment.
The header’s “Standard:” link looks incorrect (#5-...dependency-auditym doesn’t match the actual “### 6. Dependency Audit (dependency-audit.yml)” section in standards/ci-standards.md). Please update this anchor so the reference is accurate and doesn’t 404/mislead future adopters.
| # Standard: petry-projects/.github/standards/ci-standards.md#5-dependency-audit-dependency-auditym | |
| # Standard: petry-projects/.github/standards/ci-standards.md#6-dependency-audit-dependency-audityml |
Outdated review (superseded by re-review at
|
|
Auto-rebase failed — merge conflict — this branch has conflicts with Please resolve the conflicts and push: |
![petry-projects-pr-review-agent[bot]](https://avatars.githubusercontent.com/u/258516906?s=60&v=4){kind=small}
|
Auto-rebase blocked — the base branch contains Please rebase this branch manually: |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Reviewed by the don-petry PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: gpt-5.4 → audit: opus 4.6). Reply with @don-petry if you need a human.
|
@claude Please address all open review comments on this PR from CodeRabbit and Copilot. |
|
Claude finished @don-petry's task in 4m 4s —— View job Addressing open review comments
What was fixedCommitted to
Two items require manual fixes (workflow file permission limitation)The GitHub App token used by Claude does not have the 1. # Before (wrong):
# Look up current SHA: gh api repos/actions/checkout/git/refs/tags/v4 --jq '.object.sha'
# After (correct):
# Look up current SHA: gh api repos/actions/checkout/git/refs/tags/v6.0.2 --jq '.object.sha'2. # Before (wrong):
# Standard: petry-projects/.github/standards/ci-standards.md#5-dependency-audit-dependency-auditym
# After (correct):
# Standard: petry-projects/.github/standards/ci-standards.md#6-dependency-audit-dependency-auditymlBoth are cosmetic comment fixes. They can be applied in a follow-up commit by anyone with the |
…anchor Address Copilot review comments on PR #147: - standards/workflows/{dependency-audit,claude,agent-shield}.yml: SHA-pin the reusable workflow `uses:` references to match what the actual `.github/workflows/` stubs already do (ae9709f = v1). The Action Pinning Policy applies to reusable workflow refs, not just third-party actions. - standards/workflows/dependency-audit.yml: fix the "Standard:" anchor from the broken `#5-...dependency-auditym` to the correct `#6-dependency-audit-dependency-audityml`. - standards/ci-standards.md: update the Tier 1 table to show `@<SHA> # v1` instead of `@v1`, and rewrite the "Why pin?" rationale to explain that SHA pinning is required per policy while the version comment preserves human readability. Two items in .github/workflows/ files require `workflows` permission and must be fixed manually: - ci.yml:107 — "Look up current SHA" comment cites refs/tags/v4, should be refs/tags/v6.0.2 - dependency-audit.yml:3 — same broken anchor as fixed in the template Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
|
@claude This PR has 5 unresolved Copilot review threads from 2026-04-17 (predating the
Note: this PR may be largely superseded by #216 + #217 — feel free to close it as obsolete if all of its findings are already addressed there. Generated by Claude Code |
|
Claude finished @don-petry's task in 5m 8s —— View job Addressing open review threads + rebasing onto
|
Addresses all 7 compliance findings for this repository from the 2026-04-17 audit: **SHA pinning (3 findings)** - claude.yml: pin reusable workflow ref from @main → SHA (v1) - agent-shield.yml: pin reusable workflow ref @v1 → SHA - dependency-audit.yml: replace full inline workflow with thin caller stub per template; removes unpinned dtolnay/rust-toolchain@stable **Secret scan CI job (1 finding)** - ci.yml: add secret-scan job using gitleaks/gitleaks-action@SHA **API-applied (3 findings — applied directly via gh CLI)** - Enabled CodeQL default setup (codeql-default-setup-not-configured) - Set allow_auto_merge=true (allow_auto_merge) - Set delete_branch_on_merge=true (delete_branch_on_merge) Note: security_and_analysis_unavailable warnings require GitHub Advanced Security (org plan upgrade) — not actionable at the workflow level. Closes #146 Co-authored-by: don-petry <don-petry@users.noreply.github.com>
gitleaks/gitleaks-action v2 now requires a paid GITLEAKS_LICENSE for organisation repositories (petry-projects is an org). Replace the action with a direct install of the gitleaks v8.24.0 CLI binary, verified against its published SHA256 checksum. This satisfies the push-protection standard's intent while working on the free plan. Note: the compliance audit's secret_scan_ci_job_present check looks for 'gitleaks/gitleaks-action' in ci.yml. This change satisfies the security requirement (gitleaks full-history scan) but the check pattern may need updating in the audit script to also accept CLI-based scans. Co-authored-by: don-petry <don-petry@users.noreply.github.com>
…anchor Address Copilot review comments on PR #147: - standards/workflows/{dependency-audit,claude,agent-shield}.yml: SHA-pin the reusable workflow `uses:` references to match what the actual `.github/workflows/` stubs already do (ae9709f = v1). The Action Pinning Policy applies to reusable workflow refs, not just third-party actions. - standards/workflows/dependency-audit.yml: fix the "Standard:" anchor from the broken `#5-...dependency-auditym` to the correct `#6-dependency-audit-dependency-audityml`. - standards/ci-standards.md: update the Tier 1 table to show `@<SHA> # v1` instead of `@v1`, and rewrite the "Why pin?" rationale to explain that SHA pinning is required per policy while the version comment preserves human readability. Two items in .github/workflows/ files require `workflows` permission and must be fixed manually: - ci.yml:107 — "Look up current SHA" comment cites refs/tags/v4, should be refs/tags/v6.0.2 - dependency-audit.yml:3 — same broken anchor as fixed in the template Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
… note SHA-pinning the claude.yml `uses:` ref breaks Anthropic's OIDC token exchange (401 Unauthorized). Internal reusable workflow refs in this stub are exempt from the Action Pinning Policy per the OIDC constraint. Document the exemption in both the template header and ci-standards.md Tier-1 table to prevent future agents from re-introducing the SHA pin. Closes discussion_r3100768048 Co-authored-by: Don Petry <don-petry@users.noreply.github.com>
don-petry
force-pushed
the
claude/issue-146-20260417-1337
branch
from
c6fb9fb to
d823b31
Compare
|
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 2842908b741ebab9896c4a7e444cf7667c8ca7c5
Cascade: triage → deep (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7)
Summary
All seven compliance findings from the 2026-04-17 audit are addressed: SHA pins for three reusable workflow stubs are correct (ae9709f = v1, previously verified), the gitleaks CLI install follows a sound download-checksum-verify pattern (cb49b7de SHA256 verified against official release), and standards templates are reconciled. Two Copilot threads remain open only because the GitHub App token lacks the 'workflows' permission — both are cosmetic comment fixes (wrong tag in a lookup-SHA comment, a broken section anchor) with no functional or security impact. CI is green on CodeQL and CodeRabbit; the SonarCloud Security Hotspot on the curl+tar install pattern is properly mitigated by the SHA256 checksum step and was vetted in the prior review. The head merge commit (2842908) is a clean github-actions merge of main; the diff shows no unexpected changes beyond the PR's intended seven files.
Findings
- minor: ci.yml:107 — the 'Look up current SHA' comment references 'refs/tags/v4' but the action is pinned to v6.0.2 (SHA de0fac2e). Cosmetic mismatch; cannot be fixed by Claude App token (no 'workflows' permission). Should be corrected in a follow-up commit by someone with that permission.
- minor: dependency-audit.yml:3 — header anchor '#5-dependency-audit-dependency-auditym' is stale; the correct anchor is '#6-dependency-audit-dependency-audityml'. Already fixed in standards/workflows/dependency-audit.yml; cannot be fixed in .github/workflows/ without 'workflows' permission.
- info: Reusable workflow SHA ae9709f verified (annotated tag v1) across agent-shield.yml, claude.yml, and dependency-audit.yml. Standards templates reconciled to match.
- info: Gitleaks v8.24.0 SHA256 cb49b7de5ee986510fe8666ca0273a6cc15eb82571f2f14832c9e8920751f3a4 verified against official release (asset ID 230829272). Download-verify-execute pattern is correctly implemented with sha256sum -c check before extraction.
- info: secret-scan job permissions are minimal: contents: read only. No write permissions, no token exposure in run steps.
- info: SonarCloud Quality Gate failed with 1 Security Hotspot on the curl+tar binary download pattern. Properly mitigated by SHA256 checksum verification; recommend reviewing and marking 'Safe' in SonarCloud dashboard.
- info: standards/workflows/claude.yml intentionally retains @v1 (not SHA-pinned): SHA-pinning the claude.yml reusable ref breaks Anthropic OIDC token exchange (401 Unauthorized). Exemption is documented in both the template header and ci-standards.md Tier-1 table.
- info: Head commit 2842908 is a clean merge commit (github-actions[bot], 2026-05-13) bringing in current main. Diff is consistent with the PR's seven intended files; no unexpected changes introduced.
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7). Reply if you need a human review.
|
Auto-rebase failed — merge conflict — this branch has conflicts with Claude 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 Claude 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 Claude 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 Claude 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: |
|
@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: |
|
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: |
Review — fix requested (cycle 3/3)The automated review identified the following issues. Please address each one: Findings to fixAutomated review — NEEDS HUMAN REVIEWRisk: MEDIUM SummaryPR correctly SHA-pins reusable workflow references and adds gitleaks secret scanning — both positive security changes. However, the PR cannot be merged due to unresolved merge conflicts (mergeStateStatus: DIRTY), and there is a documentation inconsistency where the updated ci-standards.md exception note states claude.yml must use @v1 (not a SHA) due to an Anthropic OIDC constraint, yet this PR SHA-pins claude.yml — if the OIDC constraint is real, this could break the Claude workflow. A minor wrong version comment on the checkout action also needs correction. Findings
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7). Reply if you need a human review. Additional tasks
The review cascade will automatically re-review after new commits are pushed. |
donpetry-bot
dismissed
their stale review
Superseded by automated re-review at 2842908.
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. |
|
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: |
Summary
Addresses all 7 compliance findings for the
.githubrepository from the 2026-04-17 audit (issue #146).SHA Pinning (3 error findings fixed)
claude.yml: pinned reusable workflow reference from@main→ SHA (ae9709f...= v1)agent-shield.yml: pinned reusable workflow reference from@v1tag → SHAdependency-audit.yml: replaced full inline workflow with canonical thin caller stub perstandards/workflows/dependency-audit.ymltemplate — this eliminates the unpinneddtolnay/rust-toolchain@stablereference (logic now lives in the centralized reusable)Secret Scan CI Job (1 error finding fixed)
ci.yml: addedsecret-scanjob usinggitleaks/gitleaks-action@ff98106...(v2.3.9) with full-history checkout perstandards/push-protection.mdtemplateAPI-Applied Settings (3 findings fixed directly)
codeql-default-setup-not-configured) — triggered run #24189208962allow_auto_merge=trueapplied (allow_auto_mergewarning)delete_branch_on_merge=trueapplied (delete_branch_on_mergewarning)Not Actionable
security_and_analysis_unavailable(2 warnings) — GitHub Advanced Security features require an org plan upgrade; not addressable at the workflow levelTest plan
allow_auto_mergeanddelete_branch_on_mergeenabled.githubrepoCloses #146
Generated with Claude Code