Skip to content

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

Closed
keito4 wants to merge 1 commit into
fix/dangerous-cmd-bound-remaining-patternsfrom
feat/secret-detection-coverage
Closed

feat(security): extend secret detection coverage (GitLab/Doppler/Google)#796
keito4 wants to merge 1 commit into
fix/dangerous-cmd-bound-remaining-patternsfrom
feat/secret-detection-coverage

Conversation

@keito4

@keito4 keito4 commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Why

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

⚠️ このPRは #795 の上に積んだスタックです(base = fix/dangerous-cmd-bound-remaining-patterns)。#795 が main にマージされると base は自動的に main へ retarget されます。

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 を検出するケース追加(トークンは push protection 回避のため実行時生成)
  • hooks-command-safety.test.js: 3 種別のパターン存在アサーション追加
  • behavioral: 実トークン3種を block、変数参照 / doppler run / 通常 commit は allow

Risk

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

🤖 Generated with Claude Code

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

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f471776-50eb-4e25-a565-e8c2366a7c56

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/secret-detection-coverage

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.

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

ℹ️ 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 Support Doppler service-token config segments

For Doppler service tokens, official examples use a shape like DOPPLER_TOKEN='dp.st.prd.xxxx', with an extra config slug segment after dp.st.. This regex requires the secret material to start immediately after dp.st., so real dp.st.<config>.<secret> tokens are not matched; the hook adds the same pattern as well. That leaves common production Doppler tokens undetected despite claiming to cover st tokens.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

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


PR #796 コードレビュー

  • コンテキスト収集
  • 変更ファイルの分析
  • レビューフィードバック作成

全体評価

追加のみの変更で、既存の検出・FP抑制ロジックを変えずに3種別のトークンを揃える方針は正しい。テスト構造(実行時トークン生成でpush protection回避)も適切。ただし Doppler service token のレジェックスに実際の本番トークンを検出しない構造的バグ があり、Codex レビューの指摘通り要修正。


Major: Doppler service token regex がミス

両ファイル共通の問題。

block_inline_secrets.py:47 および security-credential-scan.sh:110:

dp\.(pt|st|sa|ct|scim|audit)\.[A-Za-z0-9]{40,}

Doppler service token の実際のフォーマットは dp.st.<config_slug>.<secret> で、prd/stg などの設定スラグが dp.st. と秘密値の間に挟まります。

dp.st.prd.AbcDef12345678901234567890123456789012345
         ^^^   ← config slug (dot を含む)

現在の正規表現は dp.st. の直後に 40文字以上の英数字を要求するため、prd.xxx...最初のドットで照合が停止し、prd の 3 文字では 40 文字に届かずノーマッチになります。
dp.pt.* / dp.sa.* / dp.ct.* は config slug を持たないため検出可能ですが、最も一般的に使われる dp.st.* が抜け落ちます。

修正案(両ファイル共通):

# block_inline_secrets.py
(r"dp\.(pt|sa|ct|scim|audit)\.[A-Za-z0-9]{40,}|dp\.st\.[A-Za-z0-9_-]+\.[A-Za-z0-9]{40,}", "Doppler token"),
# security-credential-scan.sh
PATTERNS["Doppler Token"]="dp\\.(pt|sa|ct|scim|audit)\\.[a-zA-Z0-9]{40,}|dp\\.st\\.[a-zA-Z0-9_-]+\\.[a-zA-Z0-9]{40,}"

st だけ optional の config slug セグメント [A-Za-z0-9_-]+\. を追加することで、他の型の FP リスクを増やさずに service token を検出できます。

Fix this →


Minor: BATS テストが dp.st.<config>.<secret> 形式を試していない

test/integration/security-scripts.bats:207:

dp="dp.pt.$(printf 'x%.0s' $(seq 40))"

dp.pt.* しか生成していないため、バグのある dp.st.* 形式の不検出が回帰テストで捕捉されません。
修正後は dp.st.prd.$(printf 'x%.0s' $(seq 40)) のような service token 形式も含めると安心です。


Minor: ラベル表記の不統一

block_inline_secrets.py:47"Doppler token" (小文字 t)
security-credential-scan.sh:110"Doppler Token" (大文字 T)

機能上の影響はありませんが、JS テストが 'Doppler token'(小文字)でアサートしているため、scanner 側のラベルが BATS の assert_output --partial "Doppler Token" に依存していて二重管理になっています。どちらかに揃えると一貫性が増します。


問題なし

観点 評価
GitLab PAT regex glpat-[A-Za-z0-9_-]{20} 現行 26 文字形式に一致。{20} の exact match は意図的で FP を抑えている
Google API key AIza[0-9A-Za-z_-]{35} scanner に既存のパターンと一致。hook への追加は多層防御として正しい
re.IGNORECASE フラグ AIza のような混在ケースに対して問題なし
scanner の .claude dir 除外と focused scan の組み合わせ 既存ロジック。変更なし
FP 抑制ロジック 既存ロジックに手を入れていない

判定

Doppler service token regex のバグ(major)が 1 件あるため REQUEST_CHANGES。
修正は 2 行(hook + scanner)+ BATS テスト 1 ケース追加で完結します。

@keito4 keito4 self-assigned this Jun 5, 2026
@keito4
keito4 deleted the branch fix/dangerous-cmd-bound-remaining-patterns June 5, 2026 00:41
@keito4 keito4 closed this Jun 5, 2026
@keito4
keito4 deleted the feat/secret-detection-coverage branch June 5, 2026 00:46
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