fix(gcp): 使われない GCP 鍵を dev の列挙から外す - #135
Merged
Merged
Conversation
GCP_AUTH_MODE=adc が止めるのは「鍵をファイルへ書き出すこと」だけで、環境変数 としての配布は止まらない。GCP_CREDENTIALS_BASE64__* と後方互換キーは生成 compose の environment に名前だけの列挙として残るため、Compose が値を解決して コンテナへ渡す。アカウントグループを分けても、他社のコンテナから env で サービスアカウント鍵の中身が読める状態が続いていた。 entrypoint の devbase_setup_gcp_credentials が読むのはアクティブプロファイルの 鍵 1 本だけなので、それ以外は dev の列挙から外して値ごと渡さない。後方互換キー GOOGLE_APPLICATION_CREDENTIALS_BASE64 は、鍵モードでアクティブプロファイルの鍵 が無いとき**だけ**供給源になるため、その場合に限って残す。ここを一律に外すと プロファイル別キーへ未移行のプロジェクトが鍵を受け取れず壊れる。 除外の判定は gcp_auth へ集約し、os.environ と「生成 compose へ列挙する名前」の 両方を候補にする。runtime.inject の呼び出し順に依存させないため。 除外は dev だけに効かせる方針を踏襲する。共通機密から鍵を受け取っていた非 dev サービス (独自に鍵を持つ batch 等) の列挙は絞らない。 Closes #134 Refs #133 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMEpP29QkUDLwrsyYjpnDL
除去対象として鍵ファイルのパス 2 変数しか書かれておらず、実装が GCP_CREDENTIALS_BASE64__* と後方互換キーも外すようになった点が抜けていた。 「adc が止めるのは鍵をファイルへ書き出すことだけで、base64 変数の配布は 止まらない」という前提と、3 変数それぞれの可否を表で示す。後方互換キーだけが 条件付きで残る理由 (アクティブプロファイルの鍵が無いときの供給源) も添える。 アクティブプロファイルの鍵が無く GCP_AUTH_MODE も未宣言だと後方互換キーが key モードを引き起こして渡り続ける点は、踏みやすいので Note / Warning にした。 Refs #134 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMEpP29QkUDLwrsyYjpnDL
takemi-ohama
commented
Sep 1, 2026
takemi-ohama
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 cross-review | round 1 | codex | REQUEST_CHANGES
ADC時および元Composeの直書き経路で、未使用のGCP秘密鍵がdevコンテナへ残るため、除外処理を全経路へ適用してください。
takemi-ohama
commented
Sep 1, 2026
takemi-ohama
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 cross-review | round 1 | gemini | APPROVE
修正提案はありません。
cross-review の指摘 2 件への対応。 1. adc でアクティブプロファイルの鍵が残っていた。entrypoint の devbase_setup_gcp_credentials は adc だと creds_b64 を読む前に return する ので、adc のコンテナは鍵の実体を 1 本も必要としない。アクティブ分だけ残すと 「鍵を使わない」と宣言したコンテナの env から秘密鍵が読めてしまう。 既存テスト test_adc_drops_the_key_only_variables の期待値を 「渡す」から「渡さない」へ変更した (理由は docstring に記載)。 2. 除外が列挙にしか効かず、compose.yml の dev へ直書きされた鍵が生成物に 残っていた。_drop_env_names へ同じ除外集合を渡し、auth_mode のガードを外す。 key モードでは dev_excluded_env_names が鍵パス 2 変数を返さないため、 「鍵モードでは直書きのパスを尊重する」既存の挙動は保たれる。 あわせて、除外集合の候補が機密の列挙とホストの環境変数しか見ておらず、 compose.yml へ直書きされた別プロファイルの鍵を外し損ねる穴も塞いだ (_service_env_names を追加し、3 か所すべてを候補にする)。 ドキュメントの表はモード別の 2 列にし、adc で全て渡さないことと、直書きにも 除外が効くことを書いた。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NMEpP29QkUDLwrsyYjpnDL
takemi-ohama
commented
Sep 1, 2026
takemi-ohama
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 cross-review | round 2 | codex | APPROVE
修正提案はありません。
takemi-ohama
commented
Sep 1, 2026
takemi-ohama
left a comment
Contributor
Author
There was a problem hiding this comment.
🤖 cross-review | round 2 | gemini | APPROVE
ロジックは安全かつ適切で、テストの網羅性も高いため問題ありません。別プロファイルの鍵漏洩を確実に防ぐ実装になっています。
This was referenced Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #134
背景
GCP_AUTH_MODE=adcが止めるのは鍵をファイルへ書き出すことだけで、環境変数としての配布は止まりません。GCP_CREDENTIALS_BASE64__*とGOOGLE_APPLICATION_CREDENTIALS_BASE64は生成 compose のenvironmentに名前だけの列挙として残るため、Compose が値を解決してコンテナへ渡します。#133 でアカウントグループを分離した後も、他社のコンテナから
envで nyle のサービスアカウント鍵の中身が読める状態が続いていました。変更内容
gcp_auth.dev_excluded_env_names()を追加し、dev の列挙から外す名前をここへ集約しました。GOOGLE_APPLICATION_CREDENTIALS/BIGQUERY_KEY_FILEadcのとき外す(従来どおり)DefaultCredentialsErrorを招くGCP_CREDENTIALS_BASE64__<非アクティブ>GOOGLE_APPLICATION_CREDENTIALS_BASE643 行目が要点です。entrypoint は
${!var:-${GOOGLE_APPLICATION_CREDENTIALS_BASE64:-}}でフォールバックするため、一律に外すとプロファイル別キーへ未移行のプロジェクトが鍵を受け取れず壊れます。判定には
os.environと「生成 compose へ列挙する名前」の両方を候補にしています。runtime.injectの呼び出し順に依存させないためです。除外を dev だけに効かせる方針は踏襲しました。共通機密から鍵を受け取っていた非 dev サービス(独自に鍵を持つ batch 等)の列挙は絞りません。
この PR だけでは閉じないケース
アクティブプロファイルの鍵が無く、
GCP_AUTH_MODE=adcも宣言していない構成では、後方互換キーがhas_service_account_key()のフォールバックを通って鍵モードを引き起こし、そのまま残ります。これは #133 で with-ai-dev が踏んだ経路そのものです。この場合は供給源なので外せません。プロジェクト側の
GCP_AUTH_MODE=adc宣言と組み合わせて初めて他社の鍵が渡らなくなります。挙動を明示するためtest_legacy_key_still_drives_auto_key_modeとしてテストに残しました。テストプラン
pytest tests/全件: 1714 passed(既存テストの期待値は 1 つも変更していません)ruff check --select=E9,F63,F7,F82 lib(CI と同じ): All checks passedtests/env/test_gcp_auth.pyに追加(非アクティブ鍵の抽出、後方互換キーの 3 条件、空文字の扱い、重複排除)tests/volume/test_compose_gcp_auth.pyに追加projects/*/envを使って生成 compose を作り、dev のenvironmentを確認実環境での生成結果:
GCP_ACTIVE_PROFILEwith-ai-devwithGCP_AUTH_MODE=adcのみ)project-trygroup-prdkkgGCP_AUTH_MODE=adcのみ)carmo-aidefault)GCP_CREDENTIALS_BASE64__default/GOOGLE_APPLICATION_CREDENTIALS/BIGQUERY_KEY_FILE(従来どおり)carmo-aiではGOOGLE_APPLICATION_CREDENTIALS_BASE64も外れます。アクティブプロファイルの鍵が揃っておりフォールバックが発生しないためで、entrypoint の挙動は変わりません。devbase upでコンテナを再作成し、docker exec <c> env | grep BASE64に他社の鍵が出ないことを確認(マージ後に実施)関連
defaultのままで認証情報が共有されていた件(発見元)補足: ローテーションについて
#134 で「配られた鍵の失効・再発行が必要か」を確認事項に挙げていましたが、利用者が 1 名で同一のためローテーションは不要と判断いただきました。
🤖 Generated with Claude Code
https://claude.ai/code/session_01NMEpP29QkUDLwrsyYjpnDL