Skip to content

chore(security): secret scanning skips the vendored gitleaks allowlist - #944

Merged
github-actions[bot] merged 1 commit into
mainfrom
chore/secret-scanning-gitleaks-allowlist
Nov 7, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
chore/secret-scanning-gitleaks-allowlist

Conversation

@cdeust

@cdeust cdeust commented Nov 7, 2026

Copy link
Copy Markdown
Owner

GitHub secret scanning opened 16 « Google API Key » alerts (2026-10-21). All of them point at the allowlist of gitleaks' gcp-api-key rule, the public example keys the rule must ignore, in two files:

  • third_party/gitleaks/v8.30.1/gitleaks.toml, lines 619 to 634. The file is byte-identical to upstream config/gitleaks.toml at tag v8.30.1: its sha256 e163e53b…9caf matches PROVENANCE.json, checked on 2026-11-07.
  • its conversion mcp_server/shared/redaction_rules/gitleaks.json, lines 831 to 846.

They are not leaked secrets. .github/secret_scanning.yml adds both paths to paths-ignore, so that every new vendored copy does not reopen them. A future version under another directory will need its own line, as the file says. The 16 open alerts must still be resolved as « false positive », by hand or through the API, by someone allowed to write them.

The gcp-api-key rule of the vendored gitleaks v8.30.1 rule set allowlists
public example keys. GitHub secret scanning flagged them as 16 'Google API Key'
leaks, in third_party/gitleaks/v8.30.1/gitleaks.toml (byte-identical to
upstream: sha256 e163e53b...9caf, PROVENANCE.json) and its JSON conversion
mcp_server/shared/redaction_rules/gitleaks.json. Both paths are ignored.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-07T09:58:09.425354Z 08f2eb4 PR opened
🔒 Security Review ✅ Completed 2026-11-07T10:00:23.536479Z 08f2eb4 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

Claude finished @cdeust's task in 2m 2s —— View job


Review 1 of 3: done

  • Read the diff and check the vendored file's sha256 against PROVENANCE.json
  • Run the five reviews (quality, security, docs, tests, performance)
  • Post the findings and the verdict (review comment)

I found no Blockers. There are three Notes, none needing action before merge: the PR body gives the wrong JSON line range (it is 821-836, not 831-846), the header comment doesn't mention that push protection is skipped too, and the v8.30.1 entry has no test to keep it in step with a version bump. The 16 open alerts still have to be closed by hand.

Verdict: approve

@claude

claude Bot commented Nov 7, 2026

Copy link
Copy Markdown

Review 1 of 3: PR #944

I found no Blockers. The five reviews (code quality, security, docs accuracy, test coverage, performance) all ran on the one changed file, .github/secret_scanning.yml.

What I checked:

  • Hash: sha256sum third_party/gitleaks/v8.30.1/gitleaks.toml gives e163e53b…329caf, the same value as PROVENANCE.json:6.
  • Alert count: the full pattern AIza[0-9A-Za-z_-]{35} matches 16 times in each of the two ignored files and nowhere else in the repo. That fits the 16 alerts.
  • JSON integrity: gitleaks.json is generated by scripts/vendor_gitleaks_rules.py. tests_py/shared/test_redaction_engine.py:119-122 checks it byte for byte, so a hand edit (an added secret, say) fails CI even though scanning now skips the file.
  • Format and conventions: the file is valid YAML in GitHub's documented schema, both paths exist, and it carries a # source: line. Nothing in CI reads the file and no runtime code path touches it.

Notes, no action before merge:

  • The PR body puts the allowlist at gitleaks.json lines 831-846. It is actually at lines 821-836.
  • paths-ignore also turns off push protection for these two files, not only alerts. The header comment mentions only alerts.
  • The v8.30.1 path will silently stop matching on a version bump. Consider a test next to test_vendored_rule_file_is_the_pinned_upstream_file that checks every third_party/gitleaks/*/gitleaks.toml is listed.

The 16 open alerts still have to be closed as false positives by someone with write access.

Verdict: approve

@github-actions
github-actions Bot merged commit 843fdd3 into main Nov 7, 2026
18 checks passed
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