feat: SAML デコーダに共有用マスク出力を追加 - #755
Conversation
issue #745 の機能候補「共有用マスク出力(secret-scrubber との連携)」の 設計をブレインストーミングで確定。構造ベースマスク+scrubber 併用方針。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
構造ベース(NameID/AttributeValue)+secret-scrubber 併用(HIGH_ENTROPY 除外)で PII・機密を除去。値ベース一貫トークンで相関を保つ。陽性/陰性対照テスト付き。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
Deploying devtools with
|
| Latest commit: |
7e618d6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d407c1ba.devtools-d9w.pages.dev |
| Branch Preview URL: | https://claude-issue-745-m0e3kx.devtools-d9w.pages.dev |
🖼️ Visual Regression Test 結果
|
fumtas1k
left a comment
There was a problem hiding this comment.
レビュー: SAML デコーダ共有用マスク出力(#755)
セキュリティ / フロントエンド / ロジック / アーキテクチャ / テストの各観点で確認しました。全体として設計・実装・テストの質が高く、CI も全 green(test / e2e / VRT 74 件 / build)。ローカルで saml-mask ユニット 11 件の pass も再現できました。
良い点
- セキュリティ: 全処理がブラウザ内完結、
dangerouslySetInnerHTML不使用、DOMParserは外部エンティティ非解決で XXE 安全。HIGH_ENTROPYを除外してds:SignatureValue/ds:X509Certificate(公開情報)の over-mask を避け、それを裏付ける陰性対照テストまで用意しているのは堅実。マスクの限界(完全な匿名化を保証しない)を UI・docs・SPEC に明記し過信を防ぐ設計も良い。 - アーキテクチャ:
ns.tsへの名前空間定数切り出しは parse/mask の SoT 化として自然で、リファクタと機能追加をコミット単位で分離できている。maskSamlXmlは純関数+パース失敗時フォールバックで責務が明快。 - ロジック: 値ベース一貫トークン化(同一値→同一トークン)で NameID とメール属性の相関をマスク後も追える点はトラブルシュート実務に即しており良い判断。フェーズ1(構造)+フェーズ2(scrubber 残余救済)の二段構えと、
[REDACTED:PII_n]が有効カテゴリの正規表現に再マッチしない冪等性の考慮も spec で押さえられている。 - フロントエンド:
ToggleGroup/StatusBadge/CopyButtonの既存共通コンポーネント再利用、semantic class のみ使用、Clear 時のxmlViewリセット、E2E のwaitForReactHydration呼び出し(#750 対策)まで規約準拠。
指摘(対応推奨)
- [テスト品質] LogoutRequest の陽性対照が空振り(
saml-mask.test.ts:62にインラインコメント)
expect(xml).not.toContain('taro@example.com')だが、フィクスチャの NameID はtaro.yamada@example.comで、taro@example.comは入力に存在しません。マスクが何もしなくても通る vacuous なアサーションで、test-gates の「機構を無効化したら fail する陽性対照」を満たしていません。実在値taro.yamada@example.comで検証するよう修正を推奨。
任意(低優先・ブロッカーではない)
- [効率] マスクの eager 計算:
masked/maskedXmlは[ok]依存でデコード成功のたびに実行されます。detailsは初期折りたたみ・初期rawのため、ユーザーがマスク表示を一度も開かなくてもscrubTextが全 XML を走査します。通常サイズの SAML では無視できますが、xmlView === 'masked'で遅延評価にすれば大きな入力での無駄な計算を避けられます。 - [網羅性] 属性値の PII: フェーズ1 は要素テキスト(NameID / AttributeValue)のみが対象のため、XML 属性に載る PII(例:
SubjectConfirmationData@Addressの IP、独自スキーマの属性に入った日本語氏名)はフェーズ2 のパターン依存で漏れる余地があります。既に「完全な匿名化を保証しない・共有前に目視確認」と明記済みなので許容範囲ですが、認識として。 - [nit] E2E ロケータ:
page.locator('pre').last()はgetByRole/getByText/getByLabel方針(ui-conventions §3.3)からの逸脱です。<pre>に role が無いための妥協は plan にも注記済みで許容できますが、対象 details 内にスコープを絞る書き方だとより堅牢です。
結論: 指摘1(陽性対照の空振り)だけ本 PR で直すと test-gates の趣旨に沿います。他は任意対応で問題ありません。機能・型・CI いずれも問題は見当たりませんでした。
🤖 Generated with Claude Code
Generated by Claude Code
|
|
||
| it('LogoutRequest の NameID もマスクされる', () => { | ||
| const { xml, piiCount } = maskSamlXml(LOGOUT_REQUEST_XML); | ||
| expect(xml).not.toContain('taro@example.com'); |
There was a problem hiding this comment.
このアサーションは空振り(vacuous)しています。フィクスチャ LOGOUT_REQUEST_XML の NameID は taro.yamada@example.com で、taro@example.com という部分文字列は入力に存在しません。そのため not.toContain('taro@example.com') は マスクが何もしなくても通り、NameID が実際に除去されたことを実証できていません(陽性対照として機能していない)。除去の裏付けは下の piiCount >= 1 のみに依存しています。
test-gates の「機構を無効化したら fail する陽性対照」を満たすには、入力に実在する値で検証してください:
expect(xml).not.toContain('taro.yamada@example.com');
expect(xml).toContain('[REDACTED:PII_');
expect(piiCount).toBeGreaterThanOrEqual(1);Generated by Claude Code
フィクスチャに存在しない taro@example.com での not.toContain は空振り していたため、実在値 taro.yamada@example.com とトークン出現で検証する 真の陽性対照に修正(レビュー指摘・test-gates 準拠)。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzXQvK6mSGKEtAHRki75Ds
|
レビューありがとうございます。指摘1(LogoutRequest 陽性対照の空振り)を修正しました(7e618d6)。
任意項目(低優先・ブロッカーではない旨了解)の扱い:
据え置き分のうちご希望のものがあれば別 issue 化して対応します。 Generated by Claude Code |
fumtas1k
left a comment
There was a problem hiding this comment.
再レビュー: 指摘1 修正確認 ✅
7e618d6 を確認しました。指摘1(LogoutRequest 陽性対照の空振り)は解消されています。
expect(xml).not.toContain('taro.yamada@example.com'); // 実在値で検証(空振り解消)
expect(xml).toContain('[REDACTED:PII_'); // 除去→トークン化を実証
expect(piiCount).toBeGreaterThanOrEqual(1);- フィクスチャ
LOGOUT_REQUEST_XMLの NameID 実在値taro.yamada@example.comを対象にしたため、マスクを無効化すれば fail する真の陽性対照になりました(test-gates 準拠)。 - ローカルで
saml-maskユニット 11 件 pass を再確認。CI も test / VRT green(e2e 実行中)。
残りの任意項目(#2 eager 計算 / #3 属性値 PII / #4 E2E ロケータ)はブロッカーではなく、本 PR で対応しなくても問題ありません。マージ可能な状態と判断します。LGTM 👍
🤖 Generated with Claude Code
Generated by Claude Code
概要
issue #745「SAMLデコーダ第2版」の残機能候補のうち 共有用マスク出力(secret-scrubber との連携) を実装しました。デコード結果 XML から社員 PII(NameID・属性値)と機密文字列を除去した「共有用マスク XML」を生成し、既存の整形 XML 表示ブロック内でトグル切替・コピーできます。SSO トラブルシュートで「PII を伏せた状態なら同僚・チケットに共有したい」というニーズに応え、ツールの存在意義(PII を外部サービスに貼れない現場向け)を出力面から完成させます。
docs/superpowers/specs/2026-07-20-saml-mask-output-design.mddocs/superpowers/plans/2026-07-20-saml-mask-output.md残る第2版候補(XMLDSig 署名検証・EncryptedAssertion 復号)は本 PR スコープ外で、別 PR とします(issue #745 は引き続きオープン)。
マスク戦略(構造ベース+scrubber 併用)
saml:NameID/saml:AttributeValueのテキストを値ベース一貫トークン[REDACTED:PII_n]に置換。同一値は同一トークンにすることで NameID とメール属性の相関を保つ。パターンでは拾えない日本語氏名(displayNameの「山田 太郎」等)も構造的に確実に除去。secret-scrubberのscrubTextをHIGH_ENTROPYカテゴリ除外で適用し、URL クエリ埋め込みメール等の構造で拾えない残余を救済。HIGH_ENTROPYを除外するのはds:SignatureValue/ds:X509Certificateの base64(非 PII・公開情報)を over-mask しないため。判断理由の詳細は
docs/decisions.md [125]に記載。変更点
src/utils/saml/ns.ts(新規): 名前空間定数を切り出し parse/mask で共有src/utils/saml/mask.ts(新規):maskSamlXml(xml): SamlMaskResultsrc/components/tools/SamlDecoder.tsx: 整形 XML ブロックにToggleGroup(生 XML / マスク XML)・件数バッジ・注記を追加docs/tools.md/docs/decisions.md/SPEC.md)更新テスト
test-gates 準拠でマスク(除去機構)の陽性対照を必須化。
E2E は整形 XML details 内のトグル切替(PII トークン化・件数バッジ・コピー対象切替)を追加。
検証結果
npm run format:checknpm run testnode_modules/.bin/astro checknpm run lintnpm run buildnpm run test:e2e -- saml-decoder補足
detailsは初期折りたたみのため既存/tools/saml-decoderの VRT baseline への影響は無い見込み。Closes #745 の一部(共有用マスク出力)。
🤖 Generated with Claude Code
Generated by Claude Code