Skip to content

[limen GH-organvm-i-theoria-github-451] 🚨 Security Alert: Potential Secrets Detected in Code - #457

Open
4444J99 wants to merge 2 commits into
mainfrom
limen/gh-organvm-i-theoria-github-451-fd3b
Open

[limen GH-organvm-i-theoria-github-451] 🚨 Security Alert: Potential Secrets Detected in Code#457
4444J99 wants to merge 2 commits into
mainfrom
limen/gh-organvm-i-theoria-github-451-fd3b

Conversation

@4444J99

@4444J99 4444J99 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Autonomous limen dispatch of task GH-organvm-i-theoria-github-451.

GitHub issue #451. ## Secret Scanning Alert Our automated secret scanning has detected potential secrets or credentials in the codebase. ### Scan Results - TruffleHog: ✅ Clean - Gitleaks: ⚠️ Found 6862 potential leaks - detect-secrets: ⚠️ Found 43 files with secrets ### Action Required 1. Review the scan results attached to this workflow run 2. Identify false positives and update .gitleaks.toml o

Refs: #451

Produced in an isolated worktree off origin — review before merge.

Summary by Sourcery

Improve secret-scanning workflows to use centralized configuration and baseline files, report only new findings, and align documentation with the updated setup.

Enhancements:

  • Update TruffleHog, Gitleaks, detect-secrets, and video OCR steps to use quoted GITHUB_OUTPUT writes and more robust shell practices.
  • Resolve Gitleaks configuration and detect-secrets baseline from .config/, falling back to legacy locations when present.
  • Change Gitleaks and detect-secrets runs to redact output, exclude common tooling directories, and compute structured counts of findings for workflow outputs and summaries.
  • Refine workflow summaries to distinguish code vs. video secret scanning status and streamline the reported results.
  • Extend Gitleaks configuration to keep default rules enabled and broaden the allowlist for known documentation and governance files.

Documentation:

  • Update secret scanning runbook and guide to reference .config/.gitleaks.toml and .config/.secrets.baseline, describe the new exclusion patterns and baseline behavior, and refresh last-updated metadata.

@sourcery-ai

sourcery-ai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

Updates the secret scanning workflows and documentation to centralize scanner configuration under .config/, improve detection accuracy and reporting, and refine handling of new vs. baseline findings across TruffleHog, Gitleaks, detect-secrets, and video OCR scans.

File-Level Changes

Change Details Files
Enhance GitHub Actions secret scanning workflows to use centralized .config-based scanner configuration, correctly handle new findings vs. baseline, improve output summarization, and harden shell usage.
  • Quote all uses of $GITHUB_OUTPUT and read loops to follow GitHub Actions and shell best practices.
  • Update Gitleaks steps to prefer .config/.gitleaks.toml, fall back to .gitleaks.toml, enable redaction, and emit leak counts plus concise per-finding summaries.
  • Update detect-secrets steps to prefer .config/.secrets.baseline, add exclude patterns for caches and virtualenvs, post-process scan output with jq to compute only new findings relative to the baseline, and expose both file and finding counts as outputs.
  • Upload an additional detect-secrets-new-findings.json artifact from both workflows for easier triage.
  • Refactor summary steps: Safeguard workflow now builds the step summary in a single grouped block; video scanning workflow summary is narrowed to video OCR results only with adjusted messaging.
  • Adjust issue text and runbooks to reference .config/.gitleaks.toml and .config/.secrets.baseline and document the new baseline generation and exclusion behavior.
  • Extend .config/.gitleaks.toml to keep default rules enabled and broaden the allowlist paths to include new docs locations and security governance files.
.github/workflows/safeguard-5-secret-scanning.yml
.github/workflows/scan-for-secrets.yml
docs/runbooks/SECRET_SCANNING_RESOLUTION.md
docs/guides/SECRET_SCANNING_GUIDE.md
.config/.gitleaks.toml
.config/.secrets.baseline

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@4444J99, we couldn't start this review because you've reached your PR review rate limit.

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

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

⌛ 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.

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 refill rate.

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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 168f50a3-4d26-45c6-b664-7f4727c6a698

📥 Commits

Reviewing files that changed from the base of the PR and between 4047628 and f6cabbf.

📒 Files selected for processing (6)
  • .config/.gitleaks.toml
  • .config/.secrets.baseline
  • .github/workflows/safeguard-5-secret-scanning.yml
  • .github/workflows/scan-for-secrets.yml
  • docs/guides/SECRET_SCANNING_GUIDE.md
  • docs/runbooks/SECRET_SCANNING_RESOLUTION.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch limen/gh-organvm-i-theoria-github-451-fd3b

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.

@github-actions

Copy link
Copy Markdown
Contributor

Action Pinning Required

This PR contains GitHub Actions that are not pinned to SHA commits.

Why this matters:
SHA pinning prevents supply chain attacks where a malicious actor could hijack a version tag.

How to fix:

  1. Run python src/automation/scripts/utils/update-action-pins.py
  2. Or manually pin actions using format: action@SHA # ratchet:action@version

Example:

# Before (vulnerable)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

# After (secure)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Hi @4444J99, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

@github-actions github-actions Bot added documentation Improvements or additions to documentation github-actions Related to GitHub Actions workflows configuration Configuration changes labels Jun 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewers Assigned

Reviewers have been automatically assigned based on the CODEOWNERS file.

What's Next:

  • Reviewers will be notified
  • Please respond to any feedback
  • Ensure all CI checks pass
  • Reviews typically completed within 48 hours

Need Help?


Automated reviewer assignment - PR #457

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Large Pull Request Detected

This PR is quite large:

  • 6 files changed
  • 24170 total line changes

Consider:

  • Breaking this into smaller, focused PRs
  • Each PR should address a single concern
  • Smaller PRs are easier to review and less likely to introduce bugs

If this PR must remain large, please provide extra context in the description about why these changes are grouped together.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

💬 Task Catcher Summary

💬 Unresolved discussions - Resolve review threads

📋 Task Overview

Category Count
PR Body Unchecked Tasks 0
PR Body Checked Tasks 0 ✅
Comment Tasks 0
Blocker Items 0 🚨
Suggestions 4 💡
Unresolved Review Threads 11

🎯 Next Steps

  • 💬 Resolve review discussion threads

Options:

  • ✅ Check off tasks as you complete them
  • 📋 Create issues for tasks to handle later: Add create-issues-for-tasks label
  • 🚫 Ignore tasks for merge: Add ignore-task-checks label

Last scanned: 2026-07-19 08:42 UTC
Triggered by: issue_comment

@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Large Pull Request Detected

This PR is quite large:

  • 6 files changed
  • 24170 total line changes

Consider:

  • Breaking this into smaller, focused PRs
  • Each PR should address a single concern
  • Smaller PRs are easier to review and less likely to introduce bugs

If this PR must remain large, please provide extra context in the description about why these changes are grouped together.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 I'm sorry @4444J99, but I was unable to process your request. Please see the logs for more details.

@llamapreview llamapreview 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.

Auto Pull Request Review from LlamaPReview

Large PR Notification

Dear contributor,

Thank you for your substantial contribution to this project. LlamaPReview has detected that this Pull Request contains a large volume of changes, which exceeds our current processing capacity.

Details:

  • PR and related contents total size: Approximately 917,033 characters
  • Current limit: 256,000 characters

Next steps:

  1. Consider breaking this PR into smaller, more focused changes if possible.
  2. For manual review, please reach out to your team members or maintainers.

We appreciate your understanding and commitment to improving this project. Your contributions are valuable, and we want to ensure they receive the attention they deserve.

LlamaPReview is continuously evolving to better serve the community. Share your thoughts on handling large PRs in our GitHub Discussions - your feedback helps us improve and expand our capabilities.

If you have any questions or need assistance, our community and support team are here to help.

Best regards,
LlamaPReview Team

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results

Test Suite Status
Month 1 Core Workflows ✅ Passed
Month 2 Features ✅ Passed
Month 3 Advanced ✅ Passed
Critical Path ✅ Passed

Overall: ✅ All tests passed!

View detailed test reports

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request moves the Gitleaks and detect-secrets configuration files into a .config/ subdirectory, updates the documentation and runbooks to reflect these new paths, and adds file exclusion patterns to the detect-secrets scan command. The review feedback highlights a security risk where enabling useDefault = true alongside weaker custom rules in .gitleaks.toml overrides default Gitleaks rules. Additionally, the reviewer suggests modifying the detect-secrets file exclusion regex to support both forward slashes and backslashes, ensuring cross-platform compatibility for Windows users.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread .config/.gitleaks.toml
Comment on lines +6 to +7
[extend]
useDefault = 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.

security-high high

Enabling useDefault = true while having custom rules with duplicate IDs (such as aws-access-key, github-pat, and private-key defined later in this file) will override the default Gitleaks rules entirely. This is a significant security risk because the custom aws-access-key rule defined in this file only matches AKIA[0-9A-Z]{16}, which is much weaker than Gitleaks' default AWS rule (which also detects ASIA, AROA, AIDA, etc.). Consider removing these duplicate custom rules and using the global [allowlist] instead to avoid reducing scanner coverage.

Comment on lines +77 to +79
detect-secrets scan --all-files --force-use-all-plugins \
--exclude-files '(^|/)(\.git|\.mypy_cache|\.pytest_cache|__pycache__|node_modules|\.venv|venv)/' \
> .config/.secrets.baseline

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 --exclude-files regex uses / as a path separator. On Windows, detect-secrets (which runs on Python) uses the OS-specific path separator (backslash \\), which means this regex will fail to match and exclude these directories on Windows. To make the command cross-platform and robust for developers on all operating systems, the regex should support both / and \\ as path separators.

Suggested change
detect-secrets scan --all-files --force-use-all-plugins \
--exclude-files '(^|/)(\.git|\.mypy_cache|\.pytest_cache|__pycache__|node_modules|\.venv|venv)/' \
> .config/.secrets.baseline
detect-secrets scan --all-files --force-use-all-plugins \\
--exclude-files '(^|[/\])(\\.git|\\.mypy_cache|\\.pytest_cache|__pycache__|node_modules|\\.venv|venv)[/\\]' \\
> .config/.secrets.baseline

Comment on lines +113 to +115
detect-secrets scan --all-files --force-use-all-plugins \
--exclude-files '(^|/)(\.git|\.mypy_cache|\.pytest_cache|__pycache__|node_modules|\.venv|venv)/' \
> .config/.secrets.baseline

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 --exclude-files regex uses / as a path separator. On Windows, detect-secrets (which runs on Python) uses the OS-specific path separator (backslash \\), which means this regex will fail to match and exclude these directories on Windows. To make the command cross-platform and robust for developers on all operating systems, the regex should support both / and \\ as path separators.

Suggested change
detect-secrets scan --all-files --force-use-all-plugins \
--exclude-files '(^|/)(\.git|\.mypy_cache|\.pytest_cache|__pycache__|node_modules|\.venv|venv)/' \
> .config/.secrets.baseline
detect-secrets scan --all-files --force-use-all-plugins \\
--exclude-files '(^|[/\])(\\.git|\\.mypy_cache|\\.pytest_cache|__pycache__|node_modules|\\.venv|venv)[/\\]' \\
> .config/.secrets.baseline

@sourcery-ai sourcery-ai 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.

Hey - I've left some high level feedback:

  • The detect-secrets post-processing (baseline handling, jq diff logic, and DETECT_EXCLUDE regex) is duplicated across both workflows; consider extracting this into a shared script or reusable workflow to keep the logic in one place and reduce the risk of divergence over time.
  • Similarly, the Gitleaks configuration resolution (.config/.gitleaks.toml vs .gitleaks.toml) and invocation flags are duplicated in both workflows; you might want to centralize this into a shared helper to ensure future changes to config handling only need to be made once.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The detect-secrets post-processing (baseline handling, `jq` diff logic, and `DETECT_EXCLUDE` regex) is duplicated across both workflows; consider extracting this into a shared script or reusable workflow to keep the logic in one place and reduce the risk of divergence over time.
- Similarly, the Gitleaks configuration resolution (`.config/.gitleaks.toml` vs `.gitleaks.toml`) and invocation flags are duplicated in both workflows; you might want to centralize this into a shared helper to ensure future changes to config handling only need to be made once.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown

Code Review — PR #457: Secret Scanning Hardening

Overall this is a solid improvement: centralizing config paths, adding --redact, quoting env vars, and the { echo...; } >> "$GITHUB_STEP_SUMMARY" refactor all reduce real risk. A few items worth addressing before merge:


Bugs / Correctness

1. jq length missing error guard in Gitleaks step (scan-for-secrets.yml line ~105, safeguard-5-secret-scanning.yml line ~84)

The TruffleHog step correctly uses:

FINDING_COUNT=$(jq 'length' trufflehog-results.json 2>/dev/null || echo "0")

But the Gitleaks step does:

LEAK_COUNT=$(jq length gitleaks-results.json)

If gitleaks-results.json is empty or malformed (which continue-on-error: true allows), this will produce an empty string and the downstream [ "$LEAK_COUNT" -gt 0 ] test will error. Add 2>/dev/null || echo "0" for consistency.

2. jq baseline-diff filter has O(n²) complexity (both workflow files, detect-secrets step)

($known | index($key) | not)

index() does a linear scan of $known for every finding. With 43+ files in the old baseline this is fine today, but as the baseline grows this degrades. Consider converting $known to an object for O(1) lookup:

([$baseline[0] | findings | {("\(.filename)\u0000\(.type)\u0000\(.hashed_secret)"): true}] | add // {}) as $known_map |
[findings | select(("\(.filename)\u0000\(.type)\u0000\(.hashed_secret)") as $key | $known_map[$key] | not)]

Security

3. --redact on gitleaks ✅ Good addition — prevents actual secret values from appearing in logs. The jq -r '.[] | "\(.RuleID)\t\(.File):\(.StartLine)"' summary output is appropriately safe.

4. useDefault = true in .gitleaks.toml — this enables all built-in rules. The extended allowlist additions (governance docs, security guides) should keep FP noise reasonable, but monitor the first few runs post-merge to validate the alert volume.


Minor Issues

5. DETECT_EXCLUDE regex can be tightened

DETECT_EXCLUDE='(^|/)(\.git|\.mypy_cache|\.pytest_cache|__pycache__|node_modules|\.venv|venv)/'

The bare venv pattern matches any directory path component named exactly venv. Consider anchoring it to require a path separator after: this is already done with the trailing /, so venv/ works. However venv without a dot would also match something like envvenv/ — it won't because the alternation requires (^|/)venv/. This is actually fine; just confirming the pattern is correct.

One gap: .tox/ is commonly generated and not excluded. Worth adding alongside .venv.

6. Duplicate logic across two workflow files

Both scan-for-secrets.yml and safeguard-5-secret-scanning.yml now have nearly identical detect-secrets and gitleaks shell blocks. This is a pre-existing issue, not introduced here, but the PR doubles down on the duplication by updating both. A follow-up to extract this into a composite action or reusable workflow would reduce future drift.


Positive Changes

  • Quoting $GITHUB_OUTPUT and $GITHUB_STEP_SUMMARY — correct and required per ShellCheck
  • read -r video fix in the find loop — prevents backslash interpretation in paths
  • Centralized config lookup with fallback (if .config/.gitleaks.toml; elif .gitleaks.toml) — clean migration path
  • { echo ...; } >> "$GITHUB_STEP_SUMMARY" grouping — eliminates repeated file handle opens
  • Removing .mypy_cache/ and .git/FETCH_HEAD from .secrets.baseline — these were generator-produced false positives that should never have been in the baseline

Summary

Blocking: none
Recommended before merge: items 1 (quick fix) and 2 (optional but worthwhile)
Follow-up: item 6 (composite action extraction)

The baseline cleanup alone (−23,000 lines of mypy cache FPs) makes this PR a net win.

@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: 0861af1e9e

ℹ️ 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 .config/.gitleaks.toml
Comment on lines +6 to +7
[extend]
useDefault = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid overriding the default generic API key rule

Per Gitleaks' configuration docs, rules in an extended config override default rules with the same id; this repo already has a local id = "generic-api-key" rule below that only matches api_key/apikey. Since the workflows now load this .config/.gitleaks.toml instead of falling back to the built-in config, enabling useDefault here causes the pinned v8.18.1 default generic-api-key detector to be replaced, so generic secrets such as TOKEN=..., client_secret=..., or password=... that were previously caught by Gitleaks can be missed unless another scanner happens to catch them.

Useful? React with 👍 / 👎.

@4444J99
4444J99 force-pushed the limen/gh-organvm-i-theoria-github-451-fd3b branch from 0861af1 to ac972ac Compare June 19, 2026 10:29
@github-actions

Copy link
Copy Markdown
Contributor

Action Pinning Required

This PR contains GitHub Actions that are not pinned to SHA commits.

Why this matters:
SHA pinning prevents supply chain attacks where a malicious actor could hijack a version tag.

How to fix:

  1. Run python src/automation/scripts/utils/update-action-pins.py
  2. Or manually pin actions using format: action@SHA # ratchet:action@version

Example:

# Before (vulnerable)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

# After (secure)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Large Pull Request Detected

This PR is quite large:

  • 6 files changed
  • 24172 total line changes

Consider:

  • Breaking this into smaller, focused PRs
  • Each PR should address a single concern
  • Smaller PRs are easier to review and less likely to introduce bugs

If this PR must remain large, please provide extra context in the description about why these changes are grouped together.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results

Test Suite Status
Month 1 Core Workflows ✅ Passed
Month 2 Features ✅ Passed
Month 3 Advanced ✅ Passed
Critical Path ✅ Passed

Overall: ✅ All tests passed!

View detailed test reports

@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results

Test Suite Status
Month 1 Core Workflows ✅ Passed
Month 2 Features ✅ Passed
Month 3 Advanced ✅ Passed
Critical Path ✅ Passed

Overall: ✅ All tests passed!

View detailed test reports

@github-actions

Copy link
Copy Markdown
Contributor

Action Pinning Required

This PR contains GitHub Actions that are not pinned to SHA commits.

Why this matters:
SHA pinning prevents supply chain attacks where a malicious actor could hijack a version tag.

How to fix:

  1. Run python src/automation/scripts/utils/update-action-pins.py
  2. Or manually pin actions using format: action@SHA # ratchet:action@version

Example:

# Before (vulnerable)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

# After (secure)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Large Pull Request Detected

This PR is quite large:

  • 24 files changed
  • 24939 total line changes

Consider:

  • Breaking this into smaller, focused PRs
  • Each PR should address a single concern
  • Smaller PRs are easier to review and less likely to introduce bugs

If this PR must remain large, please provide extra context in the description about why these changes are grouped together.

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Claude Code Review — PR #457

Summary: This PR consolidates secret scanning configuration into .config/, improves shell hygiene, and introduces baseline-diffing logic to report only new findings. The overall direction is correct and meaningful. A few items to address before merge.


✅ Positive Changes

  • Quoted $GITHUB_OUTPUT throughout — eliminates a class of word-splitting/glob issues in shell scripts.
  • .config/ centralization with fallback — clean migration path; legacy locations still work.
  • --redact on Gitleaks — prevents actual secret values appearing in stored artifacts. Right tradeoff.
  • useDefault = true in .config/.gitleaks.toml — important fix; without this, the custom config would silently disable all built-in Gitleaks rules.
  • Tooling dir exclusions (.git, node_modules, .mypy_cache, etc.) — the .secrets.baseline bulk cleanup (~23k lines) removing .mypy_cache and .git/FETCH_HEAD entries is correct since those paths are now excluded from scans.
  • Baseline-diffing for detect-secrets — only alerting on findings absent from the baseline is a major UX improvement and will eliminate noise.
  • Group {} writes to $GITHUB_OUTPUT and $GITHUB_STEP_SUMMARY — cleaner and avoids repeated file opens.
  • read -r video in while loop — correct shell practice.

⚠️ Issues to Address

1. jq baseline comparison: missing 2>/dev/null guard on video OCR step

In the video OCR section (scan-for-secrets.yml, around line 373), the unchanged code:

FINDING_COUNT=$(jq 'length' video-ocr-results.json)

has no 2>/dev/null || echo "0" fallback, unlike the updated scan steps. If video-ocr-results.json is malformed or empty, jq exits non-zero and FINDING_COUNT is unset, which causes the [ "$FINDING_COUNT" -gt 0 ] comparison to fail with a confusing error. The other steps in this PR already apply this pattern — apply it here too:

FINDING_COUNT=$(jq 'length' video-ocr-results.json 2>/dev/null || echo "0")

2. Complex jq filter for new-only findings — recommend a test

The new baseline-diffing jq filter (duplicated identically in both workflow files) is non-trivial:

def findings:
  .results // {} | to_entries[] as $file |
  ($file.value // [])[] |
  {filename: (.filename // $file.key), type, hashed_secret, line_number};
([$baseline[0] | findings | {("\(.filename)\u0000\(.type)\u0000\(.hashed_secret)"): true}] | add // {}) as $known_map |
[findings | select(("\(.filename)\u0000\(.type)\u0000\(.hashed_secret)") as $key | $known_map[$key] | not)]

Logically this is sound — findings is applied to $baseline[0] to build a lookup map, then to . (the new scan output) to filter out known entries. However:

  • If the detect-secrets output schema has a filename key inside the finding object, the .filename // $file.key fallback is correct. But double-check: in standard detect-secrets output, the filename is the key of .results, not a field inside the finding. The .filename field typically isn't present in baseline entries (only in audit output). This filter may always fall back to $file.key, which is fine — but the .filename fallback is dead code and slightly misleading.
  • The filter is duplicated verbatim in both workflow files. Consider extracting it to a shared .config/jq/new-findings.jq script to avoid drift.

3. --baseline flag + output redirect interaction

detect-secrets scan --baseline <file> writes the merged (old + new) baseline to stdout when redirected. The $DETECT_BASELINE file on disk is NOT modified. The jq --slurpfile baseline "$DETECT_BASELINE" therefore reads the pre-merge original — which is the intended behavior. This is correct as implemented, but worth a comment in the workflow explaining the pattern for future maintainers:

# detect-secrets scan --baseline writes merged output to stdout;
# the original $DETECT_BASELINE file is unchanged, so jq can diff against it.

4. Allowlist path additions in .config/.gitleaks.toml

Three new paths were added to the allowlist:

'''docs/guides/SECRETS_SETUP\.md''' → '''docs/guides/security-best-practices\.md'''
'''docs/governance/BRANCH_PROTECTION\.md'''

Verify these files actually contain documented example patterns (not real secrets). The useDefault = true addition means more rules run now, so the allowlist will get exercised.


💬 Minor / Non-blocking

  • The DETECT_EXCLUDE regex (^|/)(\.git|\.mypy_cache|...)/' is defined as a shell variable and passed unquoted to --exclude-files "$DETECT_EXCLUDE". This works correctly — just note that the trailing / in the pattern is load-bearing for directory matching.
  • safeguard-5-secret-scanning.yml summary now correctly uses the scan-for-secrets.yml approach of treating pre-record-scan and post-record-video-scan result statuses separately. The distinction between code scan and video scan in the summary output is an improvement.
  • Documentation (SECRET_SCANNING_GUIDE.md, SECRET_SCANNING_RESOLUTION.md) is consistently updated to reference the new .config/ paths. Last-updated date is refreshed. Good hygiene.

Verdict

Approve with the video OCR jq guard fix (issue #1 above) — that's the only change that could cause a runtime error. The baseline-comparison logic (#2) is correct but benefits from a comment. The rest are quality suggestions.

Reviewed by Claude Code (claude-sonnet-4-6)

@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results

Test Suite Status
Month 1 Core Workflows ✅ Passed
Month 2 Features ✅ Passed
Month 3 Advanced ✅ Passed
Critical Path ✅ Passed

Overall: ✅ All tests passed!

View detailed test reports

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Claude Code Review — PR #457

Overall this is a solid improvement to the secret scanning infrastructure. The core intent — centralize config resolution, report only new findings, and prevent secret values from leaking into logs — is well-executed. Below are findings grouped by priority.


🔴 Issues to fix before merge

1. run: |2 in .github/workflows/reusable/demo-sandbox.yml

-      run: |
+      run: |2

The |2 block scalar (explicit indentation indicator) is a YAML oddity that is almost certainly unintentional. |2 tells the YAML parser the block content is indented by exactly 2 characters from the current level, which can silently corrupt the script if the indentation doesn't match. This should revert to run: |.

2. cat trufflehog-results.json still present in safeguard-5-secret-scanning.yml

The --redact flag was correctly added to Gitleaks, but TruffleHog still dumps raw output:

# safeguard-5-secret-scanning.yml, TruffleHog step
cat trufflehog-results.json

This can print actual credential values to the CI log. Either add TruffleHog's --only-verified / redaction flags, or replace cat with a summary-only output (e.g. jq 'length' + rule IDs only), consistent with what was done for Gitleaks.


🟡 Worth discussing

3. detect-secrets new-findings jq filter — fragile filename matching

The composite key "\(.filename)\u0000\(.type)\u0000\(.hashed_secret)" is a clever approach, but it has a subtle fragility: the filename field in a detect-secrets result entry is typically derived from the top-level JSON key (the file path), not a sub-field on each finding. The fallback .filename // $file.key may not always resolve consistently between the baseline and the fresh scan, especially on paths with unusual characters. Consider adding an explicit test or validation step, or at minimum a comment explaining the assumption.

4. [extend] useDefault = true in .config/.gitleaks.toml

Enabling all built-in Gitleaks rules is the right long-term posture, but it will substantially increase findings on the first run — especially given that the .secrets.baseline was simultaneously pruned by ~23K entries. It is worth confirming that the allowlist paths added in this same commit are sufficient to suppress the expected false positives before merging, or the first post-merge scan run may be very noisy.

5. .secrets.baseline mass removal

The 23K-line removal is intentional (entries now excluded by the DETECT_EXCLUDE pattern or resolved as genuine false positives), but a few removed entries may still be valid to keep:

  • .github/WORKFLOW_GUIDE.md — this path is not covered by the DETECT_EXCLUDE regex, which only excludes tooling directories. The file was in the baseline as a known false positive. Removing it from the baseline while not adding it to the Gitleaks allowlist could cause it to appear as a new finding on the next scan.
  • .github/examples/README.md, .github/examples/*.yml — same concern; the examples/ directory is not excluded.

Consider verifying that all removed entries are covered by either the directory exclusion regex or the Gitleaks allowlist.

6. Docker image tag change in docker-build-push.yml

-          type=sha,prefix={{branch}}-
+          type=sha,prefix=sha-

This removes the branch name from SHA-tagged Docker images. Images that were previously tagged main-abc1234 become sha-abc1234, losing traceability to the source branch. If consumers of this registry rely on that naming convention, this is a breaking change. Worth calling out explicitly in the PR description.


✅ Improvements that look good

  • "$GITHUB_OUTPUT" quoting throughout both workflows — correct shell hygiene.
  • --redact on Gitleaks — prevents actual secret values from appearing in logs; important security improvement.
  • Config resolution fallback (.config/ → legacy root) — clean backward-compatible migration.
  • read -r in the video frame while loop — correct; prevents backslash-escape issues.
  • Block-style { ... } >> "$GITHUB_OUTPUT" writes — cleaner and less error-prone than repeated echo calls.
  • jq 2>/dev/null || echo "0" guards — sensible defensive handling.
  • detect-secrets-new-findings.json artifact upload — good for auditability.
  • Markdown formatting cleanup in .ai/CLAUDE.md — consistent line wrapping and table alignment.

Review generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

Action Pinning Required

This PR contains GitHub Actions that are not pinned to SHA commits.

Why this matters:
SHA pinning prevents supply chain attacks where a malicious actor could hijack a version tag.

How to fix:

  1. Run python src/automation/scripts/utils/update-action-pins.py
  2. Or manually pin actions using format: action@SHA # ratchet:action@version

Example:

# Before (vulnerable)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

# After (secure)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Large Pull Request Detected

This PR is quite large:

  • 26 files changed
  • 24993 total line changes

Consider:

  • Breaking this into smaller, focused PRs
  • Each PR should address a single concern
  • Smaller PRs are easier to review and less likely to introduce bugs

If this PR must remain large, please provide extra context in the description about why these changes are grouped together.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results

Test Suite Status
Month 1 Core Workflows ✅ Passed
Month 2 Features ✅ Passed
Month 3 Advanced ✅ Passed
Critical Path ✅ Passed

Overall: ✅ All tests passed!

View detailed test reports

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Claude Code Review — PR #457: Secret Scanning Improvements

Overall this is a solid security improvement. The changes address real operational pain points: config path centralization, log redaction, tooling-dir exclusions, and baseline diffing to surface only new findings. A few issues deserve attention before merge.


✅ Strengths

  • --redact on Gitleaks — prevents actual secret values from appearing in CI logs. Good practice even when findings are false positives.
  • >> "$GITHUB_OUTPUT" quoting — correct shellcheck fix throughout.
  • [extend] useDefault = true in .config/.gitleaks.toml — re-enables default Gitleaks rules, which is the right posture after establishing a proper allowlist.
  • Tooling-dir exclusions for detect-secrets (.git, .mypy_cache, .pytest_cache, etc.) — directly addresses the root cause of the 23k-line baseline bloat.
  • read -r video fix in the while loop — correct.
  • Grouped { ... } >> "$GITHUB_OUTPUT" blocks — cleaner and avoids partial-write races.
  • Config fallback logic (.config/.gitleaks.toml.gitleaks.toml) — good backwards compatibility.

⚠️ Issues

1. Baseline pruning may cause immediate regressions (Medium)

The PR removes ~23k lines from .config/.secrets.baseline, including entries for:

  • .github/workflows/scan-for-secrets.yml
  • .github/workflows/safeguard-5-secret-scanning.yml
  • .github/workflows/validate-quality.yml
  • .github/examples/ files
  • .github/scheduled-walkthrough-config.yml

The new jq logic identifies new findings by comparing current scan output against $baseline[0]. If these entries are removed from the baseline, those files are still present in the repo and detect-secrets will still flag them — but now they won't be in $known_map, so they'll be reported as new findings on the very next run.

The exclusion regex DETECT_EXCLUDE='(^|/)(\.git|\.mypy_cache|...|venv)/' only covers tooling directories, not the workflow and example files that were pruned from the baseline.

Recommendation: Either keep the workflow/example entries in the baseline (they're known false positives), or add --exclude-files patterns for those paths before removing them.

2. jq new-findings logic has a subtle edge case (Low)

jq --slurpfile baseline "$DETECT_BASELINE" '
  def findings:
    .results // {} | to_entries[] as $file |
    ($file.value // [])[] |
    {filename: (.filename // $file.key), type, hashed_secret, line_number};
  ...
' detect-secrets-results.json

The findings generator is called twice in the same filter. In jq, def findings: is a 0-arg function that re-evaluates its body each time — this is fine. However, detect-secrets scan --baseline produces an updated baseline file (merging new findings into the known set), not just the new findings. The jq then uses --slurpfile baseline "$DETECT_BASELINE" to load the original file on disk. This means the comparison is against the pre-scan baseline, which is the intended behavior. Worth documenting this intent in a comment, since it's non-obvious.

3. Gitleaks allowlist does not cover .mypy_cache (Low)

.config/.gitleaks.toml now includes [extend] useDefault = true (good), but the allowlist.paths doesn't add .mypy_cache or other tooling dirs. Gitleaks has its own exclusion mechanism separate from detect-secrets. If .mypy_cache exists in the repo, Gitleaks may still flag the hex hashes within it.

Recommendation: Add to .config/.gitleaks.toml:

[allowlist]
  paths = [
    # existing entries...
    '''\.mypy_cache/''',
    '''\.pytest_cache/''',
    '''node_modules/''',
  ]

4. detect-secrets scan --baseline semantics (Informational)

detect-secrets scan --baseline <file> writes an updated baseline to stdout that includes both old known findings and new ones. Redirecting this to detect-secrets-results.json and then computing the diff via jq is a valid workaround, but it means detect-secrets-results.json in the artifact represents the updated baseline, not the raw scan. If someone downloads the artifact expecting to see only new findings, they'll need detect-secrets-new-findings.json instead. Both are now uploaded, which is good — just document the distinction.


📋 Minor

  • The .ai/CLAUDE.md and documentation changes are purely formatting (line wrapping, table alignment, ordered list normalization). No concerns.
  • The docs/guides/SECRET_SCANNING_GUIDE.md and docs/runbooks/SECRET_SCANNING_RESOLUTION.md updates correctly reflect the new .config/ paths.

Summary

The core workflow improvements (redaction, config centralization, exclusions, new-findings diffing) are sound. The main risk is issue #1: the massive baseline pruning removes known false positives without a corresponding exclusion mechanism, which will likely cause the next workflow run to re-flag those files as new secrets. Consider either restoring those entries or adding path exclusions before landing.

limen and others added 2 commits June 19, 2026 12:22
limen task GH-organvm-i-theoria-github-451
Format docs/guides/SECRET_SCANNING_GUIDE.md and
docs/runbooks/SECRET_SCANNING_RESOLUTION.md with mdformat (--wrap=80)
to pass the mdformat pre-commit hook.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@4444J99
4444J99 force-pushed the limen/gh-organvm-i-theoria-github-451-fd3b branch from c4b7fff to f6cabbf Compare June 19, 2026 16:23
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Large Pull Request Detected

This PR is quite large:

  • 6 files changed
  • 24173 total line changes

Consider:

  • Breaking this into smaller, focused PRs
  • Each PR should address a single concern
  • Smaller PRs are easier to review and less likely to introduce bugs

If this PR must remain large, please provide extra context in the description about why these changes are grouped together.

@github-actions

Copy link
Copy Markdown
Contributor

Action Pinning Required

This PR contains GitHub Actions that are not pinned to SHA commits.

Why this matters:
SHA pinning prevents supply chain attacks where a malicious actor could hijack a version tag.

How to fix:

  1. Run python src/automation/scripts/utils/update-action-pins.py
  2. Or manually pin actions using format: action@SHA # ratchet:action@version

Example:

# Before (vulnerable)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

# After (secure)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # ratchet:actions/checkout@v6.0.2

@github-actions

Copy link
Copy Markdown
Contributor

Version Control Standards Validation Failed

This pull request does not meet our version control standards.

Common Issues:

  1. Branch Name: Must follow format <lifecycle>/<type>/<component>[/<subcomponent>]

    • Examples:
      • develop/feature/user-authentication
      • production/hotfix/critical-security-fix
      • maintenance/v1.x/security-patches
  2. Commit Messages: Must follow Conventional Commits format

    • Format: <type>(<scope>): <subject>
    • Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
    • Examples:
      • feat(auth): add OAuth2 authentication
      • fix: resolve memory leak
      • docs: update installation guide

Documentation:

Please update your branch name and/or commit messages to follow the standards.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results

Test Suite Status
Month 1 Core Workflows ✅ Passed
Month 2 Features ✅ Passed
Month 3 Advanced ✅ Passed
Critical Path ✅ Passed

Overall: ✅ All tests passed!

View detailed test reports

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Code Review - PR 457: Secret Scanning Improvements

Overview

This PR centralizes secret-scanning configuration to .config/, adds --redact to Gitleaks runs, implements a baseline-diff strategy for detect-secrets (only new findings trigger alerts), excludes tooling directories from scans, and fixes several shell quoting issues. The bulk of the diff is the cleanup of .config/.secrets.baseline (23,527 lines removed).


Positives

  • --redact on Gitleaks: excellent security practice; prevents real secret values from appearing in CI logs.
  • Quoted $GITHUB_OUTPUT / $GITHUB_STEP_SUMMARY: correct and consistent fix across both workflows.
  • while read -r: proper fix; prevents backslash interpretation in the video frame loop.
  • Fallback config resolution (.config/ to root): clean backward-compatible migration path.
  • jq ... 2>/dev/null || echo "0": defensive error handling prevents count comparisons from failing on malformed JSON.
  • Baseline-diff for detect-secrets: reporting only new findings is a meaningful noise reduction.

Issues and Concerns

1. Bug Risk - scan-for-secrets.yml Summary Regression (Medium)

The Summary step in scan-for-secrets.yml has been renamed to "Video Secret Scanning Summary" and the code scan status lines (TruffleHog, Gitleaks, detect-secrets) have been removed from it. But scan-for-secrets.yml handles both code and video scanning. Code scan results will no longer appear in the step summary for this workflow, making it harder to diagnose failures at a glance.

If the intent is to separate summaries by concern, each job should write its own summary section - not silently omit code-scan results.

2. [extend] useDefault = true May Increase Noise (Medium)

Adding useDefault = true re-enables all built-in Gitleaks rules. The original 6,862 findings may partly be from enforcing these rules for the first time. The PR adds some allowlist paths, but there is no confirmation they are sufficient to suppress false positives from the full default ruleset. This could leave the workflow as noisy as before.

Recommend: run gitleaks detect with the new config locally and confirm the finding count is acceptable before merging.

3. jq Baseline-Diff Logic - Silent Failure on Error (Low-Medium)

The fallback pattern in the detect-secrets new-findings computation:

jq ... detect-secrets-results.json > detect-secrets-new-findings.json || echo "[]" > detect-secrets-new-findings.json

If jq fails (malformed input, schema mismatch), the fallback produces an empty array file and the step reports "scan clean." Given continue-on-error: true is already set, a jq failure should at minimum emit a ::warning:: annotation before falling back to avoid silently suppressing all findings.

4. .secrets.baseline Mass Cleanup Needs Justification (Low-Medium)

23,527 lines were removed from the baseline. The PR description does not explain the methodology - were entries manually reviewed and confirmed as false positives, or bulk-deleted? Entries removed from the baseline will now be treated as new findings on the next scan. A brief explanation of how entries were selected for removal would strengthen the audit trail.

5. Dead Code in jq findings Filter (Nitpick)

detect-secrets output never includes a filename field at the item level (the filename is the map key). The .filename // $file.key fallback is therefore always taking the $file.key branch. Consider removing the dead .filename check.

6. DETECT_EXCLUDE Regex - Minor Overmatch Risk (Nitpick)

The unescaped venv alternation in the exclude pattern could inadvertently match a path component like convenience/. Using (^|/)venv/ is more precise.


Shell Quality

All $GITHUB_OUTPUT writes are now properly quoted. The grouped { echo ...; } >> "$GITHUB_OUTPUT" pattern for multi-line writes is idiomatic. The while read -r fix is a genuine correctness improvement.


Documentation

SECRET_SCANNING_GUIDE.md and SECRET_SCANNING_RESOLUTION.md are consistently updated to reference the new .config/ paths. The regeneration commands include --exclude-files, keeping the baseline and workflow behavior in sync.


Summary

Approve with suggestions. The core changes (redact flag, shell quoting, baseline-diff, config fallback) are solid improvements. The two main concerns before merge: (1) the code-scan summary being dropped from scan-for-secrets.yml, and (2) confirming that useDefault = true plus the updated allowlist actually reduces Gitleaks noise to an acceptable level. The baseline cleanup also warrants a brief justification note.

@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: f6cabbf748

ℹ️ 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".


if [ -n "$GITLEAKS_CONFIG" ]; then
echo "Using $GITLEAKS_CONFIG configuration"
gitleaks detect --source . --config "$GITLEAKS_CONFIG" --report-format json --report-path gitleaks-results.json --redact --verbose || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fail closed on Gitleaks config errors

Because this now prefers .config/.gitleaks.toml from the checkout, a PR that also touches a scanned path can make that config invalid or otherwise cause gitleaks detect to exit before writing gitleaks-results.json; the || true here suppresses that failure, and the report-file check below records found_secrets=false. That turns scanner/config errors into a clean Gitleaks result, so fail the step or use a trusted base-branch config when the scan command fails without producing a report.

Useful? React with 👍 / 👎.

{filename: (.filename // $file.key), type, hashed_secret, line_number};
([$baseline[0] | findings | {("\(.filename)\u0000\(.type)\u0000\(.hashed_secret)"): true}] | add // {}) as $known_map |
[findings | select(("\(.filename)\u0000\(.type)\u0000\(.hashed_secret)") as $key | $known_map[$key] | not)]
' detect-secrets-results.json > detect-secrets-new-findings.json || echo "[]" > detect-secrets-new-findings.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fail closed when baseline comparison errors

The fallback here converts any jq/baseline parse error into an empty detect-secrets-new-findings.json. In PR runs where the baseline is read from the checkout, a malformed .config/.secrets.baseline or broken scan output can make the comparison fail and the later count reports found_secrets=false, even if the same PR added a new secret in a scanned path. Please fail closed on comparison errors instead of writing [].

Useful? React with 👍 / 👎.


if [ -n "$GITLEAKS_CONFIG" ]; then
echo "Using $GITLEAKS_CONFIG configuration"
gitleaks detect --source . --config "$GITLEAKS_CONFIG" --report-format json --report-path gitleaks-results.json --redact --verbose || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use a trusted Gitleaks config for PR scans

On pull_request runs the checkout is the PR head, and this command now runs Gitleaks with the PR's own .config/.gitleaks.toml. If a PR adds a secret under a scanned path such as src/** while also relaxing that config (for example by allowlisting the path or disabling a rule), this workflow still runs but Gitleaks evaluates the attacker-controlled policy and can report clean. Load the scanner config from the base branch, or require config changes to be reviewed before applying them to PR scans.

Useful? React with 👍 / 👎.

Comment thread .config/.gitleaks.toml
title = "Gitleaks Configuration for {{ORG_NAME}}/.github"

[extend]
useDefault = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the default private-key rule

With useDefault enabled, Gitleaks extended rules with the same id override built-ins, and this repo defines its own id = "private-key" rule below. That replacement only matches RSA|DSA|EC|OPENSSH|PGP before PRIVATE KEY, so commits containing common PKCS8 or encrypted headers such as -----BEGIN PRIVATE KEY----- or -----BEGIN ENCRYPTED PRIVATE KEY----- no longer trigger Gitleaks even though the default private-key detector covers these cases. Rename the custom rule or broaden it so enabling defaults does not weaken private-key coverage.

Useful? React with 👍 / 👎.

@4444J99

4444J99 commented Jul 19, 2026

Copy link
Copy Markdown
Member Author

Backlog engagement 2026-07-19 — disposition: superseded by latest clean main secret scan.

Verified live state: CONFLICTING/DIRTY with failing title/SHA/version/task checks and very large secret-baseline churn. Current main's latest scan-for-secrets.yml run 29674133254 succeeded with code secret scanning clean, so this old alert branch is superseded.

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

Labels

configuration Configuration changes documentation Improvements or additions to documentation github-actions Related to GitHub Actions workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant