Skip to content

[limen RESOLVE-organvm-i-theoria-.github-453] resolve .github#453 (BLOCKED) - #470

Open
4444J99 wants to merge 1 commit into
mainfrom
limen/resolve-organvm-i-theoria-.github-453-e20e
Open

[limen RESOLVE-organvm-i-theoria-.github-453] resolve .github#453 (BLOCKED)#470
4444J99 wants to merge 1 commit into
mainfrom
limen/resolve-organvm-i-theoria-.github-453-e20e

Conversation

@4444J99

@4444J99 4444J99 commented Jun 19, 2026

Copy link
Copy Markdown
Member

Autonomous limen dispatch of task RESOLVE-organvm-i-theoria-.github-453.

Resolve blocked PR #453 ('[limen LIMEN-086] Secret Scanning Alert triage: 6862 potenti'), state=BLOCKED. Branch=limen/limen-086-ba70, base=main. In the worktree: git fetch origin limen/limen-086-ba70 main; git checkout -B limen/limen-086-ba70 origin/limen/limen-086-ba70; git rebase origin/main then address the failing checks / unresolved review threads; run the build/tests; git push --force-with-lease origin limen/limen-086-ba70. If the branch is unrecoverable, instead REBUILD the same feature cleanly off origin/main as a fresh PR. Goal: make it mergeable.

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

Summary by CodeRabbit

  • Chores

    • Reorganized Gitleaks and detect-secrets configuration files to a centralized .config/ directory for improved organization.
    • Updated secret-scanning workflows to reference the new configuration paths with backward compatibility fallback support.
  • Documentation

    • Updated security guides and best practices documentation to reflect the new configuration file locations and procedures.

limen task RESOLVE-organvm-i-theoria-.github-453
@4444J99
4444J99 enabled auto-merge June 19, 2026 22:44

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

Sorry @4444J99, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@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

🔍 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 #470

@github-actions

Copy link
Copy Markdown
Contributor

🔗 Broken Links Detected

Full Github Actions output

Please fix the broken links before merging.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jun 19, 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

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

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Moves Gitleaks and detect-secrets configuration files from the repository root into a .config/ subdirectory. Adds [extend] useDefault = true to the Gitleaks config, renames GITLEAKS_CONFIG to GITLEAKS_CONFIG_PATH across workflows, upgrades scan logic to track exit codes and use detect-secrets-hook over git ls-files, and updates all documentation references.

Changes

Secret Scanning Config Migration and Scan Logic Overhaul

Layer / File(s) Summary
Config file location, Gitleaks extension, and trigger path filters
.config/.gitleaks.toml, .github/workflows/safeguard-5-secret-scanning.yml, .github/workflows/scan-for-secrets.yml, .github/workflows/security-scan.yml
Adds [extend] useDefault = true to .config/.gitleaks.toml; renames GITLEAKS_CONFIG to GITLEAKS_CONFIG_PATH in safeguard-5 and scan-for-secrets; expands push/pull_request trigger path filters in all workflows to include .config/.gitleaks.toml and .config/.secrets.baseline.
Gitleaks scan logic: GITLEAKS_EXIT tracking and conditional config
.github/workflows/safeguard-5-secret-scanning.yml, .github/workflows/scan-for-secrets.yml, .github/workflows/security-scan.yml
Refactors gitleaks detect to conditionally pass --config only when the file exists, captures exit status into GITLEAKS_EXIT, handles missing or empty result files using GITLEAKS_EXIT to set found_secrets, and switches to --redact without the prior =100 form.
detect-secrets-hook migration, baseline fallback, and JSON shape handling
.github/workflows/reusable/security-scanning.yml, .github/workflows/safeguard-5-secret-scanning.yml, .github/workflows/scan-for-secrets.yml
Adds secrets-baseline reusable workflow input with default .config/.secrets.baseline and a legacy root-baseline fallback; switches scan execution to detect-secrets-hook over git ls-files with a shared BASELINE_EXCLUDE regex; updates SECRET_COUNT derivation to handle both object and array JSON shapes; extends artifact upload to include .secrets.baseline.
Documentation updates
docs/RESOLUTION_SUMMARY.txt, docs/guides/security-best-practices.md, docs/reference/SECURITY_ADVANCED.md, docs/workflows/REUSABLE_WORKFLOWS.md
Replaces all .secrets.baseline and root-level gitleaks.toml references with .config/.secrets.baseline and .config/.gitleaks.toml across update/audit commands, pre-commit hook args, and workflow input tables.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • organvm-i-theoria/.github#85: Introduced the safeguard-5-secret-scanning.yml workflow with Gitleaks and detect-secrets integration that this PR directly refactors with the new config path, exit-code tracking, and detect-secrets-hook logic.

Suggested labels

security, github-actions, configuration, documentation

Poem

🐇 Hopping through the .config/ burrow,
Where secrets hide in paths less narrow,
GITLEAKS_EXIT tracked with care,
No stray baseline left to snare —
The rabbit seals each scanning gap,
With JSON shapes all tucked and wrapped! 🔒

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title references a blocked PR (#453) and includes a ticket identifier, but does not clearly describe the actual changes made (config reorganization for secret scanning). Revise the title to clearly describe the primary change, such as 'Reorganize secret scanning configuration to .config directory' or similar, focusing on what the changeset accomplishes rather than the issue reference.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch limen/resolve-organvm-i-theoria-.github-453-e20e

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 github-actions Bot added github-actions Related to GitHub Actions workflows configuration Configuration changes security Security vulnerability or security-related issue size/m and removed documentation Improvements or additions to documentation github-actions Related to GitHub Actions workflows configuration Configuration changes security Security vulnerability or security-related issue labels Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 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 0 💡
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

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

@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 and updates their references across the documentation. The review feedback highlights that moving .gitleaks.toml may cause local Gitleaks runs to miss the configuration, and points out documentation errors regarding the usage of the detect-secrets --update flag and potential shell redirection failures if the .config directory does not exist.

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 +8 to +9
[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.

medium

Moving .gitleaks.toml to .config/.gitleaks.toml means that local Gitleaks executions (such as gitleaks protect or a local pre-commit hook) will not automatically detect this configuration file, as Gitleaks defaults to looking in the repository root. This can cause local runs to fail on false positives that are allowed in this configuration.\n\nIf a Gitleaks pre-commit hook is used, ensure it is configured with args: [\"--config\", \".config/.gitleaks.toml\"] to maintain consistency between local and CI environments.


# Update baseline after reviewing findings
detect-secrets scan --update .secrets.baseline
detect-secrets scan --update .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

In detect-secrets (v1.4.0+), the --update flag is a boolean flag and does not take the baseline file as a direct argument. Passing the file directly to --update can cause it to be treated as a positional path to scan, leading to unexpected behavior. Use the --baseline flag to specify the file to update.

Suggested change
detect-secrets scan --update .config/.secrets.baseline
detect-secrets scan --baseline .config/.secrets.baseline --update

```bash
# After resolving findings, update baseline
detect-secrets scan --update .secrets.baseline
detect-secrets scan --update .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

In detect-secrets (v1.4.0+), the --update flag is a boolean flag and does not take the baseline file as a direct argument. Passing the file directly to --update can cause it to be treated as a positional path to scan, leading to unexpected behavior. Use the --baseline flag to specify the file to update.

Suggested change
detect-secrets scan --update .config/.secrets.baseline
detect-secrets scan --baseline .config/.secrets.baseline --update


```bash
detect-secrets scan > .secrets.baseline
detect-secrets scan > .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

If the .config directory does not exist in a fresh checkout or a new repository, the shell redirection > will fail with a No such file or directory error. Prepend mkdir -p .config to ensure the directory exists before writing the baseline file.

Suggested change
detect-secrets scan > .config/.secrets.baseline
mkdir -p .config && detect-secrets scan > .config/.secrets.baseline

@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 #470 — Resolve Blocked Secret Scanning PR #453

Overview: This autonomous limen dispatch consolidates secret-scanning config files under .config/, fixes a critical Gitleaks env-var naming collision, switches detect-secrets to the hook-style scanner for proper new-secret detection, and improves exit-code handling throughout. The direction is correct and the core fixes are sound.


✅ Strengths

  • GITLEAKS_CONFIGGITLEAKS_CONFIG_PATH rename is the right fix. Gitleaks natively reads GITLEAKS_CONFIG as its own env var, so the old name silently overrode config discovery rather than being a neutral shell variable.
  • env -u GITLEAKS_CONFIG guard defensively unsets any ambient env var when running without a config file. Good defensive practice.
  • detect-secrets scan --baselinedetect-secrets-hook is architecturally correct: detect-secrets-hook exits non-zero on new secrets and is designed for this exact use-case, unlike scan --baseline which just writes a new scan file.
  • Exit-code capture (GITLEAKS_EXIT=$? / || GITLEAKS_EXIT=$?) replaces the || true swallow-all pattern. This surfaces real failures instead of silently passing.
  • [extend] useDefault = true in .config/.gitleaks.toml preserves built-in detector coverage rather than starting from scratch.
  • Workflow self-triggers: Adding the workflow file itself to its own on.push.paths / on.pull_request.paths is good hygiene.

⚠️ Issues to Address

1. --redact vs --redact=100 — verify gitleaks version compatibility

All three workflow files change --redact=100 to --redact. In gitleaks ≥ v8.x, --redact is a boolean flag that fully redacts; in earlier versions the flag did not exist in this form. Confirm the installed gitleaks version accepts bare --redact without a value to avoid silent failures.

2. Inconsistent detect-secrets-hook jq parsing between workflows

  • safeguard-5-secret-scanning.yml uses: jq 'if type == "object" then length elif type == "array" then length else 1 end'
  • scan-for-secrets.yml (no-baseline path) uses: jq '.results | to_entries | length'
  • security-scanning.yml (reusable) uses the same if type == "object"... form as safeguard

detect-secrets-hook v1.5.0 writes its output to stdout in a specific JSON format. The if type == "object" then length query will return the number of top-level keys (files), not the number of individual secrets — this could produce misleading counts. All three workflows should agree on a single, verified jq expression.

3. Legacy baseline fallback is only in the reusable workflow

security-scanning.yml has:

if [ ! -f "$SECRET_BASELINE" ] && [ "$SECRET_BASELINE" = ".config/.secrets.baseline" ] && [ -f ".secrets.baseline" ]; then
  SECRET_BASELINE=".secrets.baseline"
fi

scan-for-secrets.yml and safeguard-5-secret-scanning.yml have no equivalent fallback. Callers using those workflows directly and relying on a root-level .secrets.baseline will silently fall through to the no-baseline path. Either add the fallback to all three or document that only the reusable workflow supports legacy paths.

4. --json flag on detect-secrets-hook needs verification

Both safeguard-5-secret-scanning.yml and security-scanning.yml call:

xargs -0 detect-secrets-hook --baseline "$..." --exclude-files "$..." --json > detect-secrets-results.json

detect-secrets-hook v1.5.0 does not expose a --json flag; JSON output goes to stdout by default when secrets are found and the flag may be silently ignored or cause an error. Verify this works against the pinned detect-secrets==1.5.0.

5. RESULTS_COUNT=0 never re-read in safeguard-5-secret-scanning.yml

if git ls-files -z | xargs -0 detect-secrets-hook ... > detect-secrets-results.json; then
  [ -s detect-secrets-results.json ] || echo "{}" > detect-secrets-results.json
  RESULTS_COUNT=0                # ← set here
else
  ...
  RESULTS_COUNT=$(jq ...)
fi

if [ -f "detect-secrets-results.json" ]; then
  if [ "$RESULTS_COUNT" -gt 0 ]; then  # ← only meaningful in the else branch

When the hook exits 0 (no new secrets), RESULTS_COUNT=0 is correct. But the outer if [ -f ... ] block then re-uses RESULTS_COUNT without re-evaluating it from the file, so the results-count reporting is only accurate for the failure path.


💡 Minor Notes

  • Branch name (limen/resolve-...) does not match the project's <lifecycle>/<type>/<component> convention — the CI bot flagged this. For autonomous limen branches this may be acceptable by convention, but worth confirming.
  • Action pinning bot flag: The bot flagged missing SHA pins, but this PR doesn't add new uses: references. The pre-existing unresolved pins in these workflows should be tracked separately rather than blocking this fix.
  • docs/RESOLUTION_SUMMARY.txt: Path corrections here are accurate. Consider whether this file should be a persisted artifact in the repo long-term.

Summary

The core security improvements (env-var naming, exit-code handling, detect-secrets-hook switch) are well-motivated and correct in direction. Before merging, address the --json flag verification, align jq parsing expressions across all three workflows, and add the legacy baseline fallback to the non-reusable workflows for consistency.

Reviewed by Claude (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.

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

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Request Changes

This PR modernizes secret scanning configurations by centralizing files to .config/ and improving workflow patterns. However, it introduces a critical P1 bug in the detect-secrets-hook output parsing that will cause false positive secret counts, undermining the scan's reliability. A P2 maintainability gap exists with inconsistent backward-compatibility fallback across workflows.

📄 Documentation Diagram

This diagram documents the updated secret scanning workflow with centralized configuration and backward compatibility.

sequenceDiagram
    participant E as Event (push/PR)
    participant W as Workflow
    participant C as Config Files
    participant G as Gitleaks
    participant D as detect-secrets
    participant A as Artifacts

    E->>W: Trigger (code change or schedule)
    W->>C: Check config paths
    alt Config in .config/
        C-->>W: Use .config/.gitleaks.toml, .config/.secrets.baseline
    else Legacy root config
        C-->>W: Fallback to .secrets.baseline
    end
    note over W: PR #35;470: Centralized config under .config/<br/>with backward-compat fallback
    W->>G: Run Gitleaks scan
    G-->>W: gitleaks-report.json
    W->>D: Run detect-secrets scan
    alt Baseline exists
        D->>D: Use detect-secrets-hook (new)<br/>only on tracked files
    else No baseline
        D->>D: Use detect-secrets scan --all-files
    end
    D-->>W: detect-secrets-results.json
    W->>A: Upload artifacts
Loading

🌟 Strengths

  • Solid architectural decision to centralize configuration files under .config/.
  • The reusable workflow includes a thoughtful backward-compatibility fallback for legacy .secrets.baseline paths.

⚡ Key Risks & Improvements (P1)

  • .github/workflows/scan-for-secrets.yml: The new detect-secrets-hook --json output parsing uses a flawed jq filter that always returns count 1, causing false positives. This same bug exists in safeguard-5-secret-scanning.yml and reusable/security-scanning.yml — a systemic issue that must be fixed across all three workflows.

💡 Suggestions (P2)

  • .github/workflows/scan-for-secrets.yml: Lacks the legacy fallback logic that the reusable workflow provides for the baseline file path. safeguard-5-secret-scanning.yml has the same gap. Add the same fallback to ensure repositories with root .secrets.baseline don't silently switch to no-baseline scanning.

📈 Risk Diagram

This diagram illustrates the critical risk in the detect-secrets output parsing and the inconsistent backward-compatibility fallback across workflows.

sequenceDiagram
    participant W as Workflow
    participant D as detect-secrets
    participant H as Hook (detect-secrets-hook)
    participant J as jq parse
    participant O as Output

    note over W: Risk: P1 - jq filter always returns 1
    W->>H: Run detect-secrets-hook --json
    H-->>W: Output JSON {"secrets_found": [...]}
    W->>J: jq 'type=="object" then length...'
    note over J: R1(P1): jq returns number of keys (always 1)<br/>even if secrets_found is empty -> false positive
    J-->>W: SECRET_COUNT=1
    W->>O: Report found_secrets=true (incorrect)

    note over W: Risk: P2 - missing fallback in non-reusable workflows
    alt Reusable workflow
        W->>W: Check .config/.secrets.baseline -> fallback to .secrets.baseline
    else scan-for-secrets / safeguard-5
        W->>W: Use DETECT_SECRETS_BASELINE directly<br/>without fallback
        note over W: R2(P2): No fallback -> silently scan without baseline<br/>if .config/.secrets.baseline missing
    end
Loading

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

Comment on lines +141 to +147
if git ls-files -z | xargs -0 detect-secrets-hook --baseline "$DETECT_SECRETS_BASELINE" --exclude-files "$BASELINE_EXCLUDE" --json > detect-secrets-results.json; then
[ -s detect-secrets-results.json ] || echo "{}" > detect-secrets-results.json
SECRET_COUNT=0
else
[ -s detect-secrets-results.json ] || echo "{}" > detect-secrets-results.json
SECRET_COUNT=$(jq 'if type == "object" then length elif type == "array" then length else 1 end' detect-secrets-results.json 2>/dev/null || echo "1")
fi

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 | Confidence: High

The PR replaces detect-secrets scan (which outputs JSON with a .results array) with detect-secrets-hook --json (which outputs a JSON object with a secrets_found key, e.g., {"secrets_found": [...]}). The introduced jq filter if type == "object" then length elif type == "array" then length else 1 end returns the number of top-level keys for an object (which is always 1). This means even when secrets_found is an empty array, length yields 1, causing the workflow to incorrectly report a secret count of 1 – a false positive. At worst, if a real secret exists, the count will still be 1, which might be mistaken for no new secrets (false negative impact depends on later comparison). The same incorrect filter is applied identically in safeguard-5-secret-scanning.yml and reusable/security-scanning.yml, so all three workflows are affected. This breaks the core purpose of the secret-scanning step.

Suggested change
if git ls-files -z | xargs -0 detect-secrets-hook --baseline "$DETECT_SECRETS_BASELINE" --exclude-files "$BASELINE_EXCLUDE" --json > detect-secrets-results.json; then
[ -s detect-secrets-results.json ] || echo "{}" > detect-secrets-results.json
SECRET_COUNT=0
else
[ -s detect-secrets-results.json ] || echo "{}" > detect-secrets-results.json
SECRET_COUNT=$(jq 'if type == "object" then length elif type == "array" then length else 1 end' detect-secrets-results.json 2>/dev/null || echo "1")
fi
# For detect-secrets-hook output, extract the correct field:
SECRET_COUNT=$(jq '.secrets_found | length' detect-secrets-results.json 2>/dev/null || echo "0")
# Ensure empty file yields 0:
[ -s detect-secrets-results.json ] && true || echo '{"secrets_found": []}' > detect-secrets-results.json

Evidence: method:detect-secrets step in scan-for-secrets.yml, path:.github/workflows/safeguard-5-secret-scanning.yml, path:.github/workflows/reusable/security-scanning.yml

echo "Using $DETECT_SECRETS_BASELINE for comparison"
detect-secrets scan --all-files --force-use-all-plugins --baseline "$DETECT_SECRETS_BASELINE" --exclude-files '(^|/)\.config/\.secrets\.baseline$' > detect-secrets-results.json || true
echo "Using $DETECT_SECRETS_BASELINE for new-secret comparison"
if git ls-files -z | xargs -0 detect-secrets-hook --baseline "$DETECT_SECRETS_BASELINE" --exclude-files "$BASELINE_EXCLUDE" --json > detect-secrets-results.json; then

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 | Confidence: Medium

Two non-obvious behavioral changes are introduced without documentation:

  1. Scan coverage reduction: The baseline-detected secrets scan now uses git ls-files -z | xargs detect-secrets-hook, which only scans tracked files. The prior detect-secrets scan --all-files scanned all files in the working tree (including untracked). In CI this is low risk, but local runs or workspace dirtiness could miss secrets. The reusable/security-scanning.yml retains a fallback to detect-secrets scan --all-files when no baseline exists, creating an inconsistency.

  2. No legacy fallback in non-reusable workflows: The reusable security-scanning.yml has a backward-compatibility fallback to ".secrets.baseline" when the new default .config/.secrets.baseline is missing. The scan-for-secrets.yml and safeguard-5-secret-scanning.yml workflows directly reference DETECT_SECRETS_BASELINE (which is .config/.secrets.baseline) without any fallback. Repositories that still have only the root baseline will silently switch to scanning without a baseline, causing spurious findings.

These divergences from the explicit “backward compatibility” goal should be harmonized across all workflows.

Code Suggestion:

# Add the same legacy fallback logic used in reusable/security-scanning.yml to the other two workflows:
# scan-for-secrets.yml, safeguard-5-secret-scanning.yml
SECRET_BASELINE="$DETECT_SECRETS_BASELINE"
if [ ! -f "$SECRET_BASELINE" ] && [ -f ".secrets.baseline" ]; then
  SECRET_BASELINE=".secrets.baseline"
  echo "Using legacy root baseline at $SECRET_BASELINE."
fi

Evidence: path:.github/workflows/safeguard-5-secret-scanning.yml, path:.github/workflows/reusable/security-scanning.yml, search:GITLEAKS_CONFIG_PATH

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/reusable/security-scanning.yml:
- Line 143: The workflow input `inputs.secrets-baseline` is being directly
interpolated into a shell script at line 143 with the SECRET_BASELINE variable
assignment, which creates a shell injection vulnerability if the input contains
malicious shell metacharacters. Move the `${{ inputs.secrets-baseline }}` input
value into an environment variable outside of the shell script context to
prevent shell interpretation of special characters. Set the environment variable
in the job or step level environment section before it is used in any shell
commands, so that the variable value is safely treated as a literal string
rather than executable shell code.

In @.github/workflows/scan-for-secrets.yml:
- Around line 149-151: The non-baseline scan path uses echo "0" as the fallback
value when jq fails, creating a fail-open behavior where a failed scan
incorrectly reports zero secrets, while the baseline-enabled path correctly uses
echo "1" for fail-closed behavior. In the SECRET_COUNT assignment where jq
attempts to parse detect-secrets-results.json with the '.results | to_entries |
length' filter, change the fallback from echo "0" to echo "1" to ensure the
workflow fails safely if detect-secrets crashes or produces invalid JSON. Apply
the same correction to the equivalent line in safeguard-5-secret-scanning.yml.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cfe069aa-56eb-42fe-a0be-171383783e90

📥 Commits

Reviewing files that changed from the base of the PR and between efff71c and 742f32f.

📒 Files selected for processing (9)
  • .config/.gitleaks.toml
  • .github/workflows/reusable/security-scanning.yml
  • .github/workflows/safeguard-5-secret-scanning.yml
  • .github/workflows/scan-for-secrets.yml
  • .github/workflows/security-scan.yml
  • docs/RESOLUTION_SUMMARY.txt
  • docs/guides/security-best-practices.md
  • docs/reference/SECURITY_ADVANCED.md
  • docs/workflows/REUSABLE_WORKFLOWS.md

run: |
pip install detect-secrets==1.5.0
SECRET_BASELINE=".config/.secrets.baseline"
SECRET_BASELINE="${{ inputs.secrets-baseline }}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find callers and how they pass `secrets-baseline` into the reusable workflow
rg -nC4 'uses:\s*\.\/\.github\/workflows\/reusable\/security-scanning\.yml|secrets-baseline:' .github/workflows

Repository: organvm-i-theoria/.github

Length of output: 229


🏁 Script executed:

# First, let's view the actual file to confirm the issue
cat -n .github/workflows/reusable/security-scanning.yml | sed -n '135,155p'

Repository: organvm-i-theoria/.github

Length of output: 1255


🏁 Script executed:

# Search for workflow files that use the reusable security-scanning workflow
rg -l "security-scanning\.yml" .github/workflows

Repository: organvm-i-theoria/.github

Length of output: 51


🏁 Script executed:

# Search for how secrets-baseline is passed
rg -nC3 "secrets-baseline" .github/workflows

Repository: organvm-i-theoria/.github

Length of output: 1380


🏁 Script executed:

# Search for any uses of the reusable workflow with different pattern
rg "security-scanning" .github/workflows --type yaml

Repository: organvm-i-theoria/.github

Length of output: 51


🏁 Script executed:

# Check git history to see if this is an active workflow
git log --oneline -n 5 -- .github/workflows/reusable/security-scanning.yml 2>/dev/null || echo "No git history available"

Repository: organvm-i-theoria/.github

Length of output: 107


🏁 Script executed:

# Look for all workflow files that might call this
find .github/workflows -name "*.yml" -o -name "*.yaml" | head -20

Repository: organvm-i-theoria/.github

Length of output: 889


🏁 Script executed:

# Search more broadly for any reference to the reusable security-scanning workflow
rg "reusable/security-scanning" .github/ || echo "No matches found"

Repository: organvm-i-theoria/.github

Length of output: 205


🏁 Script executed:

# Check the workflow file for any comments about status or why it might be blocked
head -50 .github/workflows/reusable/security-scanning.yml

Repository: organvm-i-theoria/.github

Length of output: 1573


🏁 Script executed:

# Check if there are any open issues or references to this workflow being blocked
rg -i "blocked|security-scanning|scan-type" .github/workflows/reusable/security-scanning.yml | head -20

Repository: organvm-i-theoria/.github

Length of output: 776


Move workflow input to environment variable to prevent shell injection.

Line 143 assigns ${{ inputs.secrets-baseline }} directly in a shell script. If a caller passes a malicious value containing shell metacharacters (e.g., "; command; #), it will execute arbitrary code on the runner. This is a known GitHub Actions anti-pattern.

The reusable workflow is used by the workflow template at .github/workflow-templates/security-scanning.yml, making this a live security issue.

Suggested fix
     - name: Run secret scan with detect-secrets
       if: inputs.scan-type == 'secrets' || inputs.scan-type == 'all'
+      env:
+        SECRET_BASELINE: ${{ inputs.secrets-baseline }}
       run: |
         pip install detect-secrets==1.5.0
-        SECRET_BASELINE="${{ inputs.secrets-baseline }}"
         BASELINE_EXCLUDE='(^|/)(\.config/)?\.secrets\.baseline$'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
SECRET_BASELINE="${{ inputs.secrets-baseline }}"
- name: Run secret scan with detect-secrets
if: inputs.scan-type == 'secrets' || inputs.scan-type == 'all'
env:
SECRET_BASELINE: ${{ inputs.secrets-baseline }}
run: |
pip install detect-secrets==1.5.0
BASELINE_EXCLUDE='(^|/)(\.config/)?\.secrets\.baseline$'
🧰 Tools
🪛 zizmor (1.25.2)

[error] 143-143: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/reusable/security-scanning.yml at line 143, The workflow
input `inputs.secrets-baseline` is being directly interpolated into a shell
script at line 143 with the SECRET_BASELINE variable assignment, which creates a
shell injection vulnerability if the input contains malicious shell
metacharacters. Move the `${{ inputs.secrets-baseline }}` input value into an
environment variable outside of the shell script context to prevent shell
interpretation of special characters. Set the environment variable in the job or
step level environment section before it is used in any shell commands, so that
the variable value is safely treated as a literal string rather than executable
shell code.

Source: Linters/SAST tools

Comment on lines 149 to +151
echo "No detect-secrets baseline found at $DETECT_SECRETS_BASELINE; scanning without baseline"
detect-secrets scan --all-files --force-use-all-plugins --exclude-files '(^|/)\.config/\.secrets\.baseline$' > detect-secrets-results.json || true
detect-secrets scan --all-files --force-use-all-plugins --exclude-files "$BASELINE_EXCLUDE" > detect-secrets-results.json || true
SECRET_COUNT=$(jq '.results | to_entries | length' detect-secrets-results.json 2>/dev/null || echo "0")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify similar fail-open patterns across workflow files
rg -nC3 'detect-secrets scan --all-files.*\|\| true|SECRET_COUNT=.*echo "0"|RESULTS_COUNT=.*echo "0"' .github/workflows

Repository: organvm-i-theoria/.github

Length of output: 2434


🏁 Script executed:

# Check the context around the problematic lines in scan-for-secrets.yml
sed -n '140,165p' .github/workflows/scan-for-secrets.yml | cat -n

Repository: organvm-i-theoria/.github

Length of output: 1837


🏁 Script executed:

# Check how SECRET_COUNT is used after line 151
rg -n 'SECRET_COUNT|RESULTS_COUNT' .github/workflows/scan-for-secrets.yml .github/workflows/safeguard-5-secret-scanning.yml .github/workflows/reusable/security-scanning.yml

Repository: organvm-i-theoria/.github

Length of output: 1547


🏁 Script executed:

# Look for heredoc patterns in these workflow files that might consume the "0" value
rg -n '<<' .github/workflows/scan-for-secrets.yml .github/workflows/safeguard-5-secret-scanning.yml .github/workflows/reusable/security-scanning.yml

Repository: organvm-i-theoria/.github

Length of output: 416


Change echo "0" to echo "1" for fail-closed behavior when detect-secrets scanning fails without a baseline.

When the baseline-disabled scan path fails (line 150: || true), the jq fallback incorrectly returns "0", marking the scan as clean. In contrast, the baseline-enabled path correctly returns "1" on failure (line 146). This inconsistency creates a fail-open condition: if detect-secrets crashes or produces invalid JSON, the workflow reports SECRET_COUNT=0 and incorrectly exits as clean.

The fix aligns the non-baseline path with the baseline-enabled behavior:

Suggested fix
          detect-secrets scan --all-files --force-use-all-plugins --exclude-files "$BASELINE_EXCLUDE" > detect-secrets-results.json || true
-         SECRET_COUNT=$(jq '.results | to_entries | length' detect-secrets-results.json 2>/dev/null || echo "0")
+         SECRET_COUNT=$(jq '.results | to_entries | length' detect-secrets-results.json 2>/dev/null || echo "1")

This same pattern needs correction in .github/workflows/safeguard-5-secret-scanning.yml line 129.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
echo "No detect-secrets baseline found at $DETECT_SECRETS_BASELINE; scanning without baseline"
detect-secrets scan --all-files --force-use-all-plugins --exclude-files '(^|/)\.config/\.secrets\.baseline$' > detect-secrets-results.json || true
detect-secrets scan --all-files --force-use-all-plugins --exclude-files "$BASELINE_EXCLUDE" > detect-secrets-results.json || true
SECRET_COUNT=$(jq '.results | to_entries | length' detect-secrets-results.json 2>/dev/null || echo "0")
echo "No detect-secrets baseline found at $DETECT_SECRETS_BASELINE; scanning without baseline"
detect-secrets scan --all-files --force-use-all-plugins --exclude-files "$BASELINE_EXCLUDE" > detect-secrets-results.json || true
SECRET_COUNT=$(jq '.results | to_entries | length' detect-secrets-results.json 2>/dev/null || echo "1")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/scan-for-secrets.yml around lines 149 - 151, The
non-baseline scan path uses echo "0" as the fallback value when jq fails,
creating a fail-open behavior where a failed scan incorrectly reports zero
secrets, while the baseline-enabled path correctly uses echo "1" for fail-closed
behavior. In the SECRET_COUNT assignment where jq attempts to parse
detect-secrets-results.json with the '.results | to_entries | length' filter,
change the fallback from echo "0" to echo "1" to ensure the workflow fails
safely if detect-secrets crashes or produces invalid JSON. Apply the same
correction to the equivalent line in safeguard-5-secret-scanning.yml.

@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: 742f32fa10

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

echo "Using $DETECT_SECRETS_BASELINE for comparison"
detect-secrets scan --all-files --force-use-all-plugins --baseline "$DETECT_SECRETS_BASELINE" --exclude-files '(^|/)\.config/\.secrets\.baseline$' > detect-secrets-results.json || true
echo "Using $DETECT_SECRETS_BASELINE for new-secret comparison"
if git ls-files -z | xargs -0 detect-secrets-hook --baseline "$DETECT_SECRETS_BASELINE" --exclude-files "$BASELINE_EXCLUDE" --json > detect-secrets-results.json; then

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 Distinguish baseline-update exits from new secrets

In PRs that only move an existing allowlisted finding, detect-secrets-hook can exit 3 after updating baseline line numbers, not because it found a new secret; in Yelp/detect-secrets v1.5.0 pre_commit_hook.py it prints “The baseline file was updated” and then returns 3. This branch treats every nonzero hook exit as found_secrets=true, so this workflow and the copied safeguard logic can raise or fail a secret alert when the only required action is refreshing .config/.secrets.baseline; capture the exit code and handle 3 separately from actual new-secret failures.

Useful? React with 👍 / 👎.

run: |
pip install detect-secrets==1.5.0
SECRET_BASELINE=".config/.secrets.baseline"
SECRET_BASELINE="${{ inputs.secrets-baseline }}"

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 Move the workflow input into an environment variable

When a caller supplies secrets-baseline from any untrusted expression, GitHub renders it into this run script before bash parses it, so a value containing command substitution or a closing quote is executed while setting SECRET_BASELINE. Other inputs in this workflow are passed through env; do the same for this new input and keep the shell using "$SECRET_BASELINE".

Useful? React with 👍 / 👎.

Comment thread .config/.gitleaks.toml
# Keep the built-in detector coverage, then layer repository allowlists and
# organization-specific rules on top.
[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 shadowing default Gitleaks detectors

With this new [extend] block enabled, the local rules later in this file now collide with default rule IDs such as generic-api-key and private-key; Gitleaks' documented/confirmed extend behavior is that a duplicate rule in the extending config replaces the default (see gitleaks#1384's private-key example). That means the default generic detector, which covers names like password/token/credential/secret, is not actually kept here because the local generic-api-key rule only matches api_key/apikey, so these workflows can miss secrets the intended built-in coverage would have caught; use unique IDs for additive rules or move allowlists onto the defaults without redeclaring the same IDs.

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: MERGEABLE/BLOCKED with 11 failures plus a cancelled auto-enable job. It edits secret-scanning workflows/docs, but current main's Scan for Secrets in Videos and Code run 29674133254 is green. Any remaining security hardening should be rebuilt from current main, not merged from this red resolver branch.

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.

1 participant