Skip to content

feat: implement issue #408 — petry-projects — workflow failures detected 2026-05-29 - #477

Merged
don-petry merged 35 commits into
mainfrom
dev-lead/issue-408-20260608-0045
Jun 10, 2026
Merged

don-petry merged 35 commits into
mainfrom
dev-lead/issue-408-20260608-0045

Conversation

@don-petry

Copy link
Copy Markdown
Collaborator

Closes #408

Implemented by dev-lead agent. Please review.

Copilot AI review requested due to automatic review settings June 8, 2026 00:53
@don-petry
don-petry requested a review from a team as a code owner June 8, 2026 00:53
@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@don-petry, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 12 minutes and 37 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 92991af0-f4d3-4cfc-8dcb-1d0ded80bf2b

📥 Commits

Reviewing files that changed from the base of the PR and between 0f4d894 and c8a1457.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • .gitleaks.toml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead/issue-408-20260608-0045

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.

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

Copy link
Copy Markdown

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 introduces a .gitleaks.toml configuration file to define an allowlist for suppressing false positives in git-subtree framework imports and test fixtures. The reviewer suggested anchoring the regular expressions in the paths allowlist with ^ and $ to prevent accidental matches in other directories or files.

Comment thread .gitleaks.toml Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 83b6b0dd76

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .gitleaks.toml Outdated

Copilot AI left a comment

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.

Pull request overview

This PR addresses issue #408 by updating the repository’s secret-scanning setup in the CI workflow, aiming to reduce workflow failures caused by gitleaks false positives and/or action behavior changes.

Changes:

  • Add a repository-level .gitleaks.toml configuration intended to suppress known false positives.
  • Replace gitleaks/gitleaks-action with a pinned, checksum-verified download/execution of the gitleaks CLI in .github/workflows/ci.yml.
  • Tighten CI job permissions by removing security-events: write from the gitleaks job.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.gitleaks.toml Introduces gitleaks configuration intended to allowlist known false positives.
.github/workflows/ci.yml Switches the secret-scan job to a pinned + checksum-verified gitleaks binary download and removes unneeded permissions.

Comment thread .gitleaks.toml Outdated
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — rate-limited (intent: fix-bot-comment)

PR: #477
Please re-trigger manually (re-mention @dev-lead) when the rate limit clears — the original request cannot be reconstructed automatically.

@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:39
@don-petry
don-petry disabled auto-merge June 8, 2026 01:41
@don-petry

Copy link
Copy Markdown
Collaborator Author

Note

@don-petry I received your request but all AI engines are currently rate-limited. Please re-mention @dev-lead when the rate limit clears (estimated: unknown) — I cannot reconstruct the original instruction automatically.

@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:41
@don-petry
don-petry disabled auto-merge June 8, 2026 01:42
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:42
@don-petry
don-petry disabled auto-merge June 8, 2026 01:42
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:42
@don-petry
don-petry disabled auto-merge June 8, 2026 01:43
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:43
@don-petry
don-petry disabled auto-merge June 8, 2026 01:44
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:44
@don-petry
don-petry disabled auto-merge June 8, 2026 01:44
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:44
@don-petry
don-petry disabled auto-merge June 8, 2026 01:45
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:45
@don-petry
don-petry disabled auto-merge June 8, 2026 01:45
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:46
@don-petry
don-petry disabled auto-merge June 8, 2026 01:46
@don-petry
don-petry enabled auto-merge (squash) June 8, 2026 01:46
@don-petry
don-petry disabled auto-merge June 8, 2026 01:47
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 1
  - Quality Gate Passed with 0 new issues and 0 security hotspots — no actionable findings
    in changed files (.github/workflows/ci.yml, .gitleaks.toml); no open review threads
    from this bot to resolve.
Tier 1 blocker (unresolvable via code):
  - review / review: Fine-grained PAT detected for DON_PETRY_BOT_GH_PAT.
    Action required: rotate the secret to a classic PAT with repo, workflow, and read:org
    scopes in the petry-projects/.github-private repository settings.
```

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 1
  - "You have reached your Codex usage limits for code reviews" — this is a
    service quota notification, not a code finding; no file, line, or fix is
    referenced, and no open review threads exist from this bot to resolve.
```
The PR has no blockers. All CI checks pass and no reviews are in `CHANGES_REQUESTED` state.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 1
  - "You have reached your Codex usage limits for code reviews" — usage limit
    notification, not a code finding; no threads to reply to or resolve
```
No open review threads from this bot exist, and the comment carries no actionable code issues. All CI checks are queued with no Tier 1 blockers detected.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 0
Tier 1 blockers (CI failures or CHANGES_REQUESTED reviews): 0
Notes:
- SonarCloud Quality Gate passed with 0 new issues and 0 security hotspots.
- No open review threads from sonarqubecloud[bot] exist on PR #477.
- All CI checks are success or skipped; one check (review / review) is still
  in_progress but is not a failure-class conclusion and therefore not a blocker.
- The changed files (.github/workflows/ci.yml, .gitleaks.toml) contain no
  curl|bash patterns, insecure HTTP downloads, hardcoded credentials, or
  dynamic eval/exec with user-controlled input.
```

@donpetry-bot donpetry-bot left a comment

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.

Automated review — APPROVED ✓

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

Summary

Replaces the gitleaks/gitleaks-action@v3 step in .github/workflows/ci.yml with a SHA256-pinned download of the gitleaks CLI (8.30.1), and adds a .gitleaks.toml config that allowlists known historical test fixtures so the full-history scan no longer fails on them. Net: +68/-7 across 2 files. Addresses fleet-monitor issue #408 (12.5% failure rate on ci.yml).

Linked issue analysis

Issue #408 is the 2026-05-29 fleet-monitor report flagging .github-private/ci.yml with a 12.5% failure rate; the gitleaks-action was a contributor to those failures. Replacing it with a pinned CLI invocation plus a scoped allowlist is a direct, proportionate fix. The "Secret scan (gitleaks)" check is now passing on this PR, confirming the new pipeline works against the full repo history.

Findings

  • Security posture is solid. The CLI binary is pinned by version (8.30.1) AND verified against an explicit SHA256 (551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb) before extraction. Download uses --fail --location --retry 3 --retry-delay 5. The security-events: write permission was correctly dropped since no SARIF is uploaded by the CLI path.
  • .gitleaks.toml is well-scoped. Every [[allowlists]] entry sets condition = "AND" and pairs paths with specific regexes, so only the documented historical placeholder values in the named fixture files are suppressed — a real secret in those same paths would still be flagged. This resolves the earlier advisory-bot P2 ("Require path and regex to match together") and ("Bound framework allowlist to historical findings"). The inline comment explaining condition = "AND" is helpful.
  • One documented follow-up. Advisory-bot flagged that the org push-protection compliance check (secret_scan_ci_job_present in petry-projects/.github/standards/push-protection.md) looks specifically for gitleaks/gitleaks-action; switching to the CLI may cause that downstream compliance audit to mark this repo as missing the secret-scan job. The PR explicitly calls this out in an inline comment pointing to the standard that needs updating. Not a blocker for merging this PR — the secret scan itself works correctly — but a follow-up is needed in the petry-projects/.github repo to teach the compliance check to accept the CLI approach.
  • Trade-off worth noting. Without security-events: write the scan no longer uploads SARIF to the Security tab; findings live in workflow logs only. Reasonable for a CLI-based scan; mentioning here for awareness.
  • No SonarCloud/CodeQL/AgentShield issues. Quality gate passed with 0 new issues.

CI status

All 24 checks green: Lint, ShellCheck, shellcheck, bats, unit-tests, validate-agent-profiles, gh-aw-compile, Compile agentic workflows, Secret scan (gitleaks), Agent Security Scan, CodeQL (actions/python), SonarCloud, AgentShield, Dependency audit detector, dependabot-automerge (skipped as expected). The only non-success is review / review (this run, in progress).


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

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
- "Codex usage limits" comment: not actionable — usage limit notification with no code findings
Files changed: none
Skipped (informational): 1 (usage limit notification)
```

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 0
```
**Reasoning:**
The SonarQube Quality Gate **passed** with zero actionable findings:
- 0 New issues
- 0 Accepted issues
- 0 Security Hotspots
- 0.0% Duplication on New Code
There are also **zero open review threads** from `sonarqubecloud[bot]` to resolve (GraphQL query returned `[]`).
**PR state check — no Tier 1 blockers:**
- No CI checks with `failure`, `timed_out`, `cancelled`, `action_required`, `stale`, or `startup_failure` — all are either `queued`, `skipped` (expected for dependabot-automerge), or `success` (CodeRabbit)
- No reviews with `state = "CHANGES_REQUESTED"` — `donpetry-bot` has already approved
**Diff scan for SonarCloud hotspot patterns** (per instructions, checked even though report shows 0 hotspots):
- `ci.yml` uses HTTPS for the gitleaks download (`https://github.com/...`) — no insecure HTTP
- No `curl | bash` / `wget | bash` — download and execution are separate steps with SHA256 verification in between
- No hardcoded credentials or `eval`/`exec` with user-controlled input
- `.gitleaks.toml` uses `condition = "AND"` throughout — no over-broad allowlisting
No changes required. The PR is clean and ready for merge once the queued CI jobs complete.

@sonarqubecloud

Copy link
Copy Markdown

@donpetry-bot

Copy link
Copy Markdown
Contributor

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: c8a1457c6ecd143ac7f11c77dd0aebbb8dc1be0e
Cascade: triage → audit (triage: haiku 4.5 → deep: sonnet 4.6 + duck: o4-mini → audit: opus 4.7)

Summary

The code-level security of this PR is sound — gitleaks CLI is pinned to v8.30.1 with SHA256 verification, downloaded over HTTPS with --fail, extracted by explicit name, and the redundant security-events: write permission is correctly dropped. The .gitleaks.toml allowlists now use condition="AND" everywhere, which is a real security improvement over the original action's globally-OR'd regexes. However, the org-wide secret_scan_ci_job_present compliance check (verified at scripts/lib/push-protection.sh:234 in petry-projects/.github, which greps for 'uses: gitleaks/gitleaks-action@' or 'zricethezav/gitleaks-action@') will silently start reporting this repo as missing the required secret-scan job, even though the scan runs correctly. The PR author chose to document the gap in an inline workflow comment rather than land the matching standards update, leaving two P2 findings open at the current head.

Findings

  • major: Switching from gitleaks/gitleaks-action to a CLI run: block breaks the pp_check_secret_scan_ci_job compliance check in petry-projects/.github (scripts/lib/push-protection.sh greps specifically for 'uses:[[:space:]]*(gitleaks/gitleaks-action|zricethezav/gitleaks-action)@'). The matching standard at standards/push-protection.md line 420 documents this as an error-severity check. The PR comment at ci.yml:128-132 acknowledges the gap but defers the fix; the standard and check live in a separate repo and were not updated as part of this rollout, so the next weekly audit will incorrectly report this repo as non-compliant. Beyond the false signal, this normalizes failed compliance findings and reduces visibility into future real gaps.
  • major: Codex re-raised P2 'Keep secret scan compliant with audit' against the current head (c8a1457) on 2026-06-08 06:35:40Z after re-reading the org standard. The author's two subsequent responses point at the inline workflow comment and say no code change is needed here, but the advisory's substantive concern — the audit will report a false failure — is not closed by a comment in this repo. Either the action shape should be preserved (e.g., keep a thin gitleaks/gitleaks-action@SHA usage that wraps the CLI, or revert to the action) or the cross-repo standard/audit update must land in the same rollout.
  • minor: gitleaks/gitleaks-action uploads SARIF findings to GitHub Advanced Security; the CLI flow does not. The deep review correctly notes this is the reason the security-events: write permission was previously needed. Detection is unaffected (the job still fails the build on findings), but historical scan results no longer appear in the Security tab for this repo. Recommend adding --report-format sarif --report-path gitleaks.sarif to the CLI invocation and an actions/upload-artifact (or github/codeql-action/upload-sarif) step if Advanced Security visibility is desired.
  • info: Binary fetch is well-constructed: HTTPS to the canonical gitleaks GitHub release URL, curl --fail with retries, version (8.30.1) and SHA256 (551f6fc...70eb) both pinned, sha256sum -c before tar extraction, only the gitleaks binary extracted from the tarball by explicit name, tarball removed after use. No untrusted-input expansion. Removing the security-events: write permission is correct least-privilege when SARIF upload is no longer used.
  • info: All .gitleaks.toml allowlist entries now use condition="AND", binding regexes to specific paths. This is materially safer than the prior structure: without AND, gitleaks defaults to OR, so regexes like 'dummy-test-key', 'test-key', 'password123', and 'BSA-1234567890abcdef' would have suppressed any matching string repo-wide, not just in the fixture files. The fix addresses the Codex P2 from 2026-06-08 on allowlist scoping and is a net improvement over the prior state.
  • info: Trigger surface is safe: the workflow runs on pull_request/push (not pull_request_target), so a hostile PR cannot read secrets via this job. The job uses no ${{ github.event.* }} expressions in shell, no inputs interpolated into the run: block, and no secrets at all under the new approach. No expression-injection or token-exfiltration vector 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.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

petry-projects — workflow failures detected 2026-05-29

3 participants