Skip to content

feat(security): extend secret detection coverage (GitLab/Doppler/Google) - #797

Merged
keito4 merged 1 commit into
mainfrom
feat/secret-detection-coverage-v2
Jun 5, 2026
Merged

feat(security): extend secret detection coverage (GitLab/Doppler/Google)#797
keito4 merged 1 commit into
mainfrom
feat/secret-detection-coverage-v2

Conversation

@keito4

@keito4 keito4 commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Why

secret 検出の対象トークン種別に漏れがあった。scanner と inline-secret フックの両層に、これまで未対応だった GitLab / Doppler / Google のトークンを追加し、「混入を入口で止め(hook)、混入済みを scan で検出」する多層防御を揃える。

#796#795 のスタックとして作成していたが、#795 がレビュー修正込みで先行マージされ base ブランチが消えたため、最新 main の上に cherry-pick し直した再作成版。#795 の quote-strip 修正は保持している。)

What

scanner (security-credential-scan.sh)

  • PATTERNSGitLab PAT (glpat-…) と Doppler Token (dp.<type>.…) を追加し、両者を CRITICAL に分類

hook (block_inline_secrets.py)

  • SECRET_PATTERNSGoogle API key (AIza…) / GitLab PAT / Doppler Token を追加(scanner に既存だった Google を inline 側にも揃える)

tests

  • security-scripts.bats: .claude 設定内の GitLab/Doppler を検出するケース追加(トークンは GitHub push protection 回避のため実行時生成)
  • hooks-command-safety.test.js: 3 種別のパターン存在アサーション追加
  • behavioral: 実トークン3種を block、変数参照 / doppler run / 通常 commit は allow

Risk

低。検出種別の追加のみ。既存の検出・FP 抑制・quote-strip ロジックは無変更。jest 116 / bats 18 green。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Security
    • Enhanced detection of inline credentials to identify Google API keys, GitLab personal access tokens, and Doppler tokens.
    • Critical-level alerts now triggered for detected GitLab and Doppler credentials to strengthen secret protection.

scanner と inline-secret フックの両層に、これまで未対応だったトークン種別を
追加。検出は多層(混入を入口で止め、混入済みを scan で検出)で揃える。

## scanner (security-credential-scan.sh)
- PATTERNS に GitLab PAT (glpat-…) と Doppler Token (dp.<type>.…) を追加
- 両者を CRITICAL 重大度に分類

## hook (block_inline_secrets.py)
- SECRET_PATTERNS に Google API key (AIza…) / GitLab PAT / Doppler Token を追加
  (scanner には既にあった Google を inline 側にも揃える)

## tests
- security-scripts.bats: .claude 設定内の GitLab/Doppler を検出するケース追加
- hooks-command-safety.test.js: 3 種別のパターン存在アサーション追加
- behavioral 確認: 実トークン3種を block、変数参照/`doppler run`/通常commit は allow

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR expands credential detection coverage by adding patterns for Google API keys, GitLab personal access tokens, and Doppler tokens to both the inline-secret hook and the file-scanning script. GitLab and Doppler findings are classified as CRITICAL severity. Unit and integration tests validate the new patterns.

Changes

Credential Detection Expansion

Layer / File(s) Summary
Hook credential pattern detection
.claude/hooks/block_inline_secrets.py, test/hooks-command-safety.test.js
SECRET_PATTERNS is extended with regex entries for Google API keys (AIza...), GitLab PAT (glpat-...), and Doppler tokens (dp...). Unit tests validate the presence of each pattern marker in the loaded hook script.
Scanner pattern detection and severity mapping
script/security-credential-scan.sh, test/integration/security-scripts.bats
File scanner adds GitLab PAT and Doppler Token entries to the PATTERNS map. The record_finding logic classifies both as CRITICAL severity. Integration test verifies the scanner detects both token types with critical severity in configuration files.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • keito4/config#791: Both PRs extend the same credential-detection pipeline with additional secret patterns in the hook and scanner.

Suggested labels

feature


🐰 Three new guardians join the watch,
Google keys and tokens caught,
GitLab whispers, Doppler too,
Critical findings shine bright and true! 🔐✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change—extending secret detection coverage for three credential types (GitLab, Doppler, Google).
Description check ✅ Passed The PR description covers all required template sections with substantial detail: Why (uncovered secret types), What (specific changes across scanner/hook/tests), and related context.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/secret-detection-coverage-v2

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

test/hooks-command-safety.test.js

Oops! Something went wrong! :(

ESLint: 10.4.1

ReferenceError: describe is not defined
at Object. (/test/eslint-config.test.js:4:1)
at Module._compile (node:internal/modules/cjs/loader:1830:14)
at Object..js (node:internal/modules/cjs/loader:1961:10)
at Module.load (node:internal/modules/cjs/loader:1553:32)
at Module._load (node:internal/modules/cjs/loader:1355:12)
at wrapModuleLoad (node:internal/modules/cjs/loader:255:19)
at loadCJSModuleWithModuleLoad (node:internal/modules/esm/translators:326:3)
at ModuleWrap. (node:internal/modules/esm/translators:231:7)
at ModuleJob.run (node:internal/modules/esm/module_job:437:25)
at async node:internal/modules/esm/loader:639:26


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.

@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Claude finished @keito4's task in 3m 4s —— View job


PR #797 コードレビュー

  • コンテキスト収集
  • 変更ファイルの読み込み・diff 確認
  • レビュー実施
  • コメント更新

総評

Approve(minor 観点のみ)。変更は検出種別の追加のみで、既存ロジックへの影響はなし。パターンの精度・重大度分類・テスト戦略いずれも適切。以下、観点別の詳細。


Security ✅

観点 評価
Google API key パターン AIza[0-9A-Za-z_-]{35} — 公式フォーマットに合致。Gemini も同プレフィックスのため一石二鳥
GitLab PAT パターン glpat-[A-Za-z0-9_-]{20} — 公式フォーマット通り
Doppler token パターン dp\.(pt|st|sa|ct|scim|audit)\.[A-Za-z0-9]{40,} — 全トークン種別を網羅
重大度分類 GitLab / Doppler ともに CRITICAL に正しく分類(security-credential-scan.sh:226-229
hook ↔ scanner 整合 Google はすでに scanner に存在し hook 側に追加、GitLab/Doppler は両側に追加と対称性が正しい

Minor — GitLab PAT の文字数が厳密一致({20}

script/security-credential-scan.sh:109.claude/hooks/block_inline_secrets.py の両方が {20} を使用。
GitLab は過去にトークン長を変更した実績があり、将来的に長いトークンが出た場合に検出漏れの恐れ。{20,} への変更でより堅牢になる。

-PATTERNS["GitLab PAT"]="glpat-[a-zA-Z0-9_-]{20}"
+PATTERNS["GitLab PAT"]="glpat-[a-zA-Z0-9_-]{20,}"

Fix this →


Quality ✅

テスト戦略の評価

テスト 評価
hooks-command-safety.test.js:145-153 存在アサーション(toContain)として3パターンを追加。シンプルで明確
security-scripts.bats:202-224 printf 'x%.0s' $(seq 20) で実行時トークン生成 — GitHub push protection 回避として賢い手法
scanner の CRITICAL 検出を JSON で検証 jq -e '.critical_count >= 2' で2件以上を確認

Minor — フック単体の振る舞いテストが不在

bats テストは scanner の動作を検証しているが、hook(block_inline_secrets.py)自体が新3パターンを実際にブロックするかを確認するテストがない。PR説明では「behavioral: 実トークン3種を block」と記載があるが、テストファイル上では確認できず。

JS テストは content.toContain(...) だけのため、パターンが存在しても re.IGNORECASE 起因の誤動作等は検出できない。


Performance ✅

パターン追加による処理コストは無視できるレベル。ループ毎に re.search を1回呼ぶだけで計算量に変化なし。


AI Residuals ✅

テスト内にハードコードされた実トークンの混入なし(runtime 生成で適切に回避)。TODO/FIXME もなし。


その他観察事項

: CI ランナー上の作業ツリーで block_inline_secrets.py に unstaged な変更(PR の3行が削除された状態)が存在する。これはランナー環境の artifact と判断し、committed 状態(a2ab5ed)を基準にレビューを実施した。マージ前に git stash 等で確認推奨。


まとめ

観点 結果
Security ✅ パターン正確、重大度分類も適切
Performance ✅ 影響なし
Quality ⚠️ GitLab {20}{20,} の minor 提案あり
Accessibility N/A
AI Residuals ✅ 問題なし

Verdict: APPROVE(GitLab PAT の {20,} 変更は推奨だがブロッカーではない)

@keito4 keito4 self-assigned this Jun 5, 2026

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

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

PATTERNS["Linear API Key"]="lin_api_[a-zA-Z0-9]{43}"
PATTERNS["Sentry DSN"]="https://[a-f0-9]{32}@[a-z0-9]+\\.ingest\\.sentry\\.io/[0-9]+"
PATTERNS["GitLab PAT"]="glpat-[a-zA-Z0-9_-]{20}"
PATTERNS["Doppler Token"]="dp\\.(pt|st|sa|ct|scim|audit)\\.[a-zA-Z0-9]{40,}"

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 Match Doppler service tokens' config segment

Doppler service tokens include the config name between dp.st and the random token material (the official service-token docs show values shaped like dp.st.prd.xxxx), so this pattern only catches personal-style dp.pt.<secret> tokens and misses the documented production/CI form dp.st.<config>.<secret>. That leaves leaked service tokens in .claude/settings*.json or other scanned files unreported even though this change marks Doppler tokens as critical.

Useful? React with 👍 / 👎.

(r"lin_api_[A-Za-z0-9]{43}", "Linear API key"),
(r"AIza[0-9A-Za-z_-]{35}", "Google API key"),
(r"glpat-[A-Za-z0-9_-]{20}", "GitLab personal access token"),
(r"dp\.(pt|st|sa|ct|scim|audit)\.[A-Za-z0-9]{40,}", "Doppler token"),

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 Match inline Doppler service tokens' config segment

For inline command blocking, this has the same service-token gap: Doppler service tokens are shaped like dp.st.<config>.<secret> (for example the docs use dp.st.prd.xxxx), but the regex requires the secret material immediately after dp.st.. Inline commands such as export DOPPLER_TOKEN='dp.st.prd....' can therefore be approved and persisted to settings even though service tokens are the documented production path.

Useful? React with 👍 / 👎.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
script/security-credential-scan.sh (1)

219-230: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Promote Google API key findings to CRITICAL for strict-mode enforcement.

Line 83 detects "Google API Key", but Lines 219-230 never promote it to CRITICAL. With --strict, a repo containing only a Google key can still exit 0, which undercuts the security gate.

🔐 Suggested fix
   if [[ "$pattern_name" == *"AWS"* ]] || \
+     [[ "$pattern_name" == *"Google API Key"* ]] || \
      [[ "$pattern_name" == *"GitHub Token"* ]] || \
      [[ "$pattern_name" == *"Private Key"* ]] || \
      [[ "$pattern_name" == *"OpenAI"* ]] || \

As per coding guidelines, **/*.{js,ts,jsx,tsx,py,java,go,rb,php,json,lock,txt,yaml,yml}: Fail on critical security vulnerabilities during Security Code Analysis.

🤖 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 `@script/security-credential-scan.sh` around lines 219 - 230, The check that
sets severity="CRITICAL" for discovered secrets omits the "Google API Key"
pattern, so when pattern_name equals "Google API Key" it isn't promoted to
CRITICAL and strict mode can miss-fail; update the condition that tests
pattern_name (the long OR chain that currently checks "AWS", "GitHub Token",
"Private Key", "OpenAI", "Anthropic", "Stripe", "Supabase", "Slack Token",
"GitLab", "Doppler", "Database URL") to also include [[ "$pattern_name" ==
*"Google API Key"* ]] so that the "Google API Key" finding sets
severity="CRITICAL" (the variable severity used later to determine exit
behavior).
🤖 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 `@test/hooks-command-safety.test.js`:
- Around line 153-155: The test 'should detect Doppler tokens' currently only
asserts the label text via expect(content).toContain('Doppler token'); update it
to assert the actual Doppler regex marker instead of just the label by changing
the assertion on the content variable to match the regex pattern (use
expect(content).toMatch(...) with the Doppler detection regex or the exact
marker string used in the scanner). Locate the test named "should detect Doppler
tokens" and replace the toContain check on content with a toMatch against the
canonical Doppler regex/marker used by the hook scanner so the test fails if the
regex is weakened.

---

Outside diff comments:
In `@script/security-credential-scan.sh`:
- Around line 219-230: The check that sets severity="CRITICAL" for discovered
secrets omits the "Google API Key" pattern, so when pattern_name equals "Google
API Key" it isn't promoted to CRITICAL and strict mode can miss-fail; update the
condition that tests pattern_name (the long OR chain that currently checks
"AWS", "GitHub Token", "Private Key", "OpenAI", "Anthropic", "Stripe",
"Supabase", "Slack Token", "GitLab", "Doppler", "Database URL") to also include
[[ "$pattern_name" == *"Google API Key"* ]] so that the "Google API Key" finding
sets severity="CRITICAL" (the variable severity used later to determine exit
behavior).
🪄 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: 7c2e3c42-0e46-4c2b-b1ba-bd180d2084d7

📥 Commits

Reviewing files that changed from the base of the PR and between 212b106 and a2ab5ed.

📒 Files selected for processing (4)
  • .claude/hooks/block_inline_secrets.py
  • script/security-credential-scan.sh
  • test/hooks-command-safety.test.js
  • test/integration/security-scripts.bats

Comment on lines +153 to +155
test('should detect Doppler tokens', () => {
expect(content).toContain('Doppler token');
});

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 | 🟡 Minor | ⚡ Quick win

Assert the Doppler regex marker, not only the label text.

Line 154 validates "Doppler token" only. If the regex is accidentally weakened but the label stays, this test still passes.

💡 Suggested test hardening
 test('should detect Doppler tokens', () => {
-  expect(content).toContain('Doppler token');
+  expect(content).toContain('dp\\.(pt|st|sa|ct|scim|audit)\\.');
 });
🤖 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 `@test/hooks-command-safety.test.js` around lines 153 - 155, The test 'should
detect Doppler tokens' currently only asserts the label text via
expect(content).toContain('Doppler token'); update it to assert the actual
Doppler regex marker instead of just the label by changing the assertion on the
content variable to match the regex pattern (use expect(content).toMatch(...)
with the Doppler detection regex or the exact marker string used in the
scanner). Locate the test named "should detect Doppler tokens" and replace the
toContain check on content with a toMatch against the canonical Doppler
regex/marker used by the hook scanner so the test fails if the regex is
weakened.

@keito4
keito4 merged commit 441cf14 into main Jun 5, 2026
23 checks passed
@keito4
keito4 deleted the feat/secret-detection-coverage-v2 branch June 5, 2026 01:00
@claude claude Bot mentioned this pull request Jun 5, 2026
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.115.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions github-actions Bot added the released リリース済み label Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released リリース済み

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant