docs(PLAN56): 機密ストアの置き場をアカウントグループごとに分ける要求仕様と設計 (#182) - #183
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | kiro | REQUEST_CHANGES
設計は要求仕様と概ね整合しているが、公開コマンド仕様の網羅性に 2 点抜けがある。
issues/PLAN56_secret-group-paths-design.md:412（決定 8,env backend test）: 対象範囲・受け入れ条件・テスト設計の 3 つに対応が無く、未テストで出荷され得る。issues/PLAN56_secret-group-paths.md:120（受け入れ条件）: 決定 7 の up 中断挙動に対応する受け入れ条件が無い。
詳細はインラインコメント参照。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | agy | REQUEST_CHANGES
PR #183 の PLAN56 要求仕様（issues/PLAN56_secret-group-paths.md）および設計書（issues/PLAN56_secret-group-paths-design.md）について、既存コードベースとの整合性・セキュリティ境界・CLI の挙動仕様を中心にレビューを実施しました。
仕様書・設計書ともに OpenBao KV v2 パスの階層構造やグループ解決ロジックが綿密に定義されていますが、境界条件における判定仕様の曖昧さや、既存 CLI 実装との整合性、テスト設計の不足について以下の 5 点（Major 3件、Minor 2件）の修正を提案します。
指摘事項サマリ
- [major / specification]
--group指定時のプロジェクトグループ判定（決定 6）におけるエイリアス解決の適用可否が曖昧 (issues/PLAN56_secret-group-paths-design.mdL232)- L232-233 に「比較は読み替える前のグループ名で行う」とある一方、「
defaultとnyleは同じ置き場を指す」と記載されています。declared_groupがdefaultのプロジェクトで--group nyle -pを実行した際、読み替え前比較で拒否されるのか、同じ置き場として許可されるのか判定基準を明記してください。
- L232-233 に「比較は読み替える前のグループ名で行う」とある一方、「
- [major / specification]
env getにおける--group指定時のプロジェクト設定探索仕様が未規定 (issues/PLAN56_secret-group-paths-design.mdL220)- L220 の構文に
[-p]が記載されていますがenv getに-pオプションは存在しません。プロジェクト配下でdevbase env get --group kkg KEYを実行した際、自動フォールバックでプロジェクト設定（_project_env）も探索対象とするのか、スキップ/拒絶するのか探索規則を明記してください。
- L220 の構文に
- [major / consistency] 受け入れ条件 5 の操作前提（
$DEVBASE_ROOTで実行）と-pの結果条件が矛盾 (issues/PLAN56_secret-group-paths.mdL145)- 受け入れ条件 5 は「
$DEVBASE_ROOTで」実行する前提ですが、結果欄に-pの判定が書かれています。現行実装上$DEVBASE_ROOTでの-pはエラー終了するため、プロジェクト配下での検証を明示的に分けるか補足を追記し、設計書 L433 のテスト設計に決定 6 のテスト項目を追加してください。
- 受け入れ条件 5 は「
- [minor / validation]
--group-alias引数検証におけるglobal/projects拒絶の記述が不足 (issues/PLAN56_secret-group-paths-design.mdL248)- 受け入れ条件 9 で求められる
default=globalやprojects=nyleの拒絶について、resolve_account_groupはこれらを弾かないため、L248 にFROMとTOの両方でglobal/projectsを拒絶する旨を明記してください。
- 受け入れ条件 9 で求められる
- [minor / design-gap]
_ensure_env_filesが起動する子プロセスenv initの実行時 CWD はコード上すでに$DEVBASE_ROOTで確定 (issues/PLAN56_secret-group-paths-design.mdL451)- 現行実装（
container.pyL1469）でcwd=str(devbase_root)とハードコードされているため未確認事項を解消し、親プロセスの実行時プロジェクトのグループを子プロセスに正しく伝播させる設計を確定させてください。
- 現行実装（
各指摘の詳細は inline コメントをご確認ください。
…6〜18） Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
cross-review round 1 の指摘 7 件（major 4 / minor 3）を 41509fc で反映しました。主な変更: 決定 10（子プロセスの env init へ --group を渡す）を追加、-p と --group の比較を読み替え後の名前に統一、受け入れ条件 5 を 5 / 5a に分割し 16〜18 を追加。 |
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | kiro | REQUEST_CHANGES
設計・要求文書としての整合性は概ね取れているが、PR 本文の件数と実体のずれが 1 件ある。
- [major / 正確性] PLAN56_secret-group-paths.md / PLAN56_secret-group-paths-design.md（PR 本文の件数）: PR 本文は「受け入れ条件 15 件」「決定 9 件」と述べるが、spec の受け入れ条件は 1〜18 + 5a の 19 項目、design の決定は 1〜10 の 10 件。加えて PR 本文の『決めたこと』一覧（
pr-body-decisions.sh同期）は決定 1〜9 までで決定 10 が欠落している。本文には『手で書き換えない（sync が作る）』とあるため、sync を再実行して件数と決定一覧を実体（決定 10 を含む）へ合わせてほしい。（差分外＝PR 本文のため body に記載）
その他 2 件（受け入れ条件の番号非連続、テスト設計表の決定 1 重複）はインラインに記載。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | codex | REQUEST_CHANGES
グループ分離の要求を満たすため、起動対象の解決順序、export/import の対象選別、scale の不一致検査を設計へ反映してください。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | kiro | COMMENT
設計・仕様は内部で概ね整合しており、決定の根拠と反証（決定 5 の「グループを常に入れる案を採らない」理由など）も追えます。round 2 で追記した受け入れ条件・決定の反映で、番号と表の整合が 2 か所崩れているので、そこだけ直すと参照追跡が保てます。インラインで 2 件指摘しました。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
cross-review round 2 の指摘 7 件（major 3 / minor 4、重複を含む）を bedce9a で反映しました。決定 11（dispatch 前の注入を切替先で解決）・決定 12（export / import を対象のグループへ限定）を追加し、食い違いの検査を scale にも広げました。 |
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 3 | codex | REQUEST_CHANGES
修正が必要な点はインラインの 2 件です。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 3 | agy | REQUEST_CHANGES
PR #183 の round 2 反映コミット（bedce9a）について、追加された決定・検査ロジックおよび CLI 契約の整合性を agy の観点でレビューしました。
グループ食い違い検査における副作用発生タイミングの問題（Major 1 件）および CLI オプション名の誤記（Minor 1 件）について、以下の 2 点の修正を提案します。
指摘事項サマリ
- [major / 設計] scale / up におけるグループ食い違い検査の配置位置 (
issues/PLAN56_secret-group-paths-design.md:292)_check_group_consistencyを「構成生成前 / 機密注入前」に行うと、scaleでは先行するwrite_scaleでproject.local.ymlが書き換わり、upでは先行するボリューム作成やスナップショット作成の副作用が発生した後に終了してしまいます。副作用の発生前（cmd_scale冒頭、cmd_upの_run_pre_up_checks等）で検査する設計に修正してください。
- [minor / CLI契約] env import のエラー案内におけるオプション名誤記 (
issues/PLAN56_secret-group-paths-design.md:465)- 案内文の
--exclude-projectsは、実際の CLI オプション定義（lib/devbase/cli.pyL536）では単数形の--exclude-projectです。ユーザーが未定義オプションエラーに遭遇するのを防ぐため、--exclude-projectに修正してください。
- 案内文の
各指摘の具体的な修正内容は各 inline コメントをご確認ください。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
cross-review round 3 の指摘 4 件（major 2 / minor 2）を fa86c1b で反映しました。判定は収束でしたが major を含む修正を入れたため、もう 1 ラウンド回して確かめます。 |
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 4 | kiro | APPROVE
設計・要求とも内部整合が取れており、コードとの主要な前提（_openbao_from_dict が未知キーを無視／version != 1 を拒否／OpenBaoBackend._seen の鍵が SecretRef／cache.entry_path・entry_key のパスとキー形式／export・import の既存 --exclude-project）を worktree の実装で確認した。設計 Pull Request として承認できる。修正アクションは以下 1 点のみ。
- [minor / 整合性]
issues/PLAN56_secret-group-paths-design.md:373以降の「決定の記録」は決定 1〜12（12 件）を持つが、PR 本文の自動同期セクション「## 決めたこと」は決定 1〜9 までしか載っておらず、決定 10（_ensure_env_filesの--group伝播）・決定 11（dispatch 前注入の切替先解決）・決定 12（export/importのグループ絞り込み）と本文サマリの「決定 9 件」が古い。pr-body-decisions.sh syncを再実行して本文を再生成し、本文だけ読むレビュアーが 12 決定すべてを把握できるようにしてほしい。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 4 | agy | REQUEST_CHANGES
機密ストアのアカウントグループ別置き場（PLAN56）の要求仕様および設計について確認しました。起動時の副作用抑止順序、移行時のテナント境界保護、設定バリデーションの整合性に関して 5 件の修正提案（major 3 件、minor 2 件）があります。インラインコメントの各項目について対応をご検討ください。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 4 | kiro | REQUEST_CHANGES
設計文書内で up のグループ食い違い検査の位置が三箇所で食い違っています（構成要素表・配列図は _run_deploy_pipeline、入出力契約表は _run_pre_up_checks）。実コードでは _auto_snapshot とボリューム作成が _run_deploy_pipeline より前に走るため、図どおり実装すると決定 7 の「副作用より前に止める」保証が崩れます。検査位置を _run_pre_up_checks（自動スナップショットより前）に統一してください。詳細はインライン参照。
そのほか PR 本文の同期漏れ（差分外のため body に記載）:
- issues/PLAN56_secret-group-paths.md:1 (PR body) — 『決めたこと』が設計文書と非同期。設計は決定 1〜12 だが PR body は決定 9 で終端し 10〜12 が欠落（決定 10/11/12 は受け入れ条件 18/7/13 から参照される）。
pr-body-decisions.sh syncを再実行。あわせて PR body 冒頭『受け入れ条件 15 件』は実際 19 件（1〜18 + 5a）。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
cross-review round 4 の指摘 7 件（major 4 / minor 3）を反映しました。kiro は申告件数と投稿数の食い違いで判定が中断扱いになりましたが、投稿された指摘は取り込んで対応済みです。確認のためもう 1 ラウンド回します。 |
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 5 | kiro | REQUEST_CHANGES
設計文書 (issues/PLAN56_secret-group-paths-design.md) 本体は内部整合が取れている。指摘は PR body の自動生成サマリが設計文書と食い違っている点のみ。差分の行ではなく PR 説明文なので body に記す。
- [major / 整合性] PR body「決めたこと」/ Summary「決定 9 件」が設計文書の 決定 12 件と食い違う。 設計文書には
### 決定 1:〜### 決定 12:が 12 個あるが（issues/PLAN56_secret-group-paths-design.md:379,389,399,409,416,…,455 ほか）、PR body の「決めたこと」は 決定 1〜9 だけを列挙し、Summary も「決定 9 件」と書く。欠けている 決定 10（子プロセスenv initへの--group引き渡し）・決定 11（名前指定ライフサイクル操作の dispatch 前注入の切替先解決）・決定 12（export/importのグループ制限）は注入と入出力の振る舞いを変える判断であり、この設計 PR の承認可否に直接効く。「決めたこと」節は<!-- … pr-body-decisions.sh sync が作る。手で書き換えない -->とある自動生成節なので、pr-body-decisions.sh syncを再実行して 12 件へ更新し、Summary の「決定 9 件」も合わせて直す。 - [minor / 整合性] PR body Summary「受け入れ条件 15 件」が要求仕様の 19 件と食い違う。
issues/PLAN56_secret-group-paths.mdの- [ ] N.は 1〜18 に 5a を加えた 19 項目（`grep '^- [ ] [0-9]' で 19 一致）。Summary の「受け入れ条件 15 件」を 19 件へ直す（非機能 4 項目・前提 7 件は一致）。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 5 | codex | REQUEST_CHANGES
同期状態をグループごとの保存先に対応させる設計の追加が必要です。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 5 | kiro | APPROVE
設計・要求とも整合しており、決定1の版番号引き上げの根拠（_from_dict が version != 1 を拒み、_openbao_from_dict が未知キーを無視する）もコードで確認できた。以下は軽微な整合性のみ。
- PLAN56_secret-group-paths-design.md:24 のインライン参照。
- PR body の「決めたこと」節が設計文書の現状と乖離している。設計文書の見出しには決定 1〜12 があるが、PR body には決定 1〜9 しか載っておらず、かつ決定 7 の文言が旧版（
upのみ、scaleを含まない）のまま。body はpr-body-decisions.sh syncが生成すると明記されているので、sync を再実行して決定 10/11/12 と更新後の決定 7 を反映すること。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 6 | codex | REQUEST_CHANGES
受け入れ条件の再現手順に修正が必要です（minor 1 件）。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 6 | agy | REQUEST_CHANGES
概要
PLAN56 の要求仕様書（PLAN56_secret-group-paths.md）および設計文書（PLAN56_secret-group-paths-design.md）について、round 1〜5 の改訂内容（決定 10〜13 の追加、整合性検査、事前注入、export/import でのグループ分離など）を反映した最新状態を精査しました。
全体としてエッジケースや既存実装との整合性が非常に緻密に詰められていますが、round 5 で追加された決定 13（SourcesManager のグループ別ハッシュファイル保持）に伴い、.gitignore の除外設定および doctor 点検の変更範囲の考慮漏れが 1 点確認されたため、設計文書への反映を要請します。
指摘事項（要対応 1 件）
- [major / specification] 決定 13 で導入された
.env.sources.<g>.ymlの.gitignore除外およびenv doctor点検漏れ（インラインコメント参照）layout: groupで生成される$DEVBASE_ROOT/.env.sources.<g>.ymlが現行の.gitignore（.env.sources.ymlのみ一致）から漏れ、Git 未追跡ファイルとして残ってしまうリスクがあります。構成要素および対象範囲に.gitignoreの更新を追加してください。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 7 | kiro | APPROVE
設計文書と要求仕様を精査し、実コード（_ensure_env_files の cwd=$DEVBASE_ROOT・cmd_scale の write_scale 先行・_load_secret_env の cwd ベース注入・resolve_account_group の検証）と照合した。決定 1〜13・受け入れ条件 1〜18・パス/キャッシュ対応表・4 参照の往復数はドキュメント内および実装前提と整合しており、round 6 までの指摘が反映済み。新たに修正を要する不整合・矛盾は検出できなかったため承認する（残る不確定は「未確認のまま残ること」として明示済み）。
takemi-ohama
left a comment
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 7 | agy | APPROVE
PR #183 の設計文書（issues/PLAN56_secret-group-paths-design.md）および要求仕様書（issues/PLAN56_secret-group-paths.md）を agy の観点（共通整合性、ドキュメント妥当性、認証・認可・機密情報保護、テナント分離境界）からレビューしました。
総括
本設計は、OpenBao の KV v2 シークレットエンジンにおいてアカウントグループ別のパス分離を導入し、テナント境界（グループ権限）を厳格に保護するためのアーキテクチャが極めて論理的かつ網羅的に設計されています。
特に以下の点が優れています:
- テナント分離と循環依存の排除:
declared_groupを環境変数や機密ストアからではなく非機密のenvファイルから静的に決定することで、機密漏洩や権限昇格、循環解決を防止（決定 3）。 - 副作用前の厳格なフェイルセーフ:
up/scaleにおいてボリュームのグループ（環境変数）と機密のグループ（ファイル）が食い違った場合、コンテナ起動やスナップショット作成、scale書き換えなどの副作用の直前に早期停止（決定 7、受け入れ条件 16）。 - 境界外アクセスの防止:
exportでの他グループプロジェクト自動除外、importでの他グループプロジェクト混入時の完全拒否（決定 12）、migrate --to ageでの他グループ共通参照への要求抑止、env backend testでの対象外グループの安全なスキップ（決定 8）。 - キャッシュおよびメタデータの分離:
index.jsonキーやキャッシュパスへのグループ名組込み（決定 14）、グループ別の.env.sources.<g>.ymlのハッシュ管理と Git 除外（決定 13）。 - 受け入れ条件 1〜18 とテスト設計の 1:1 対応: 全てのシナリオに対する検証方針が明記されている。
軽微な表記揺れおよび I/O 契約の境界条件に関する minor な改善点 2 件をインラインコメントにて指摘していますが、設計の根幹は承認できる水準に達しています。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Summary
#182 の設計 Pull Request。実装は含まない（この PR のマージ後に実装用の作業ツリーで行う）。
issues/PLAN56_secret-group-paths.md（受け入れ条件 19 件（1〜18 と 5a）、非機能 4 項目、前提 7 件）issues/PLAN56_secret-group-paths-design.md（機能 8 件、構成要素 17 件、テスト設計 21 行、未確認 3 件）issues/PLAN56_secret-group-paths-decisions.md（決定 13 件。設計文書が 500 行を超えたため分けた）standard（envコマンドのオプションとsecrets/backend.ymlの形が変わり、注入の振る舞いが変わる）docs/specifications/secret-backend.mddefaultは置き場のパスだけ読み替える・全グループ共通の置き場を持たない・ファイル backend は分けない）operation）、carmo-cdk のグループ単位のポリシー（carmo-cdk へ起票）決めたこと
issues/PLAN56_secret-group-paths-decisions.mdbackend.ymlのversion: 2にするSecretRefのフィールドとして持つenvファイルだけから決め、プロセスの環境変数を見ないdefaultの読み替えはgroup_aliasesで置き場の上だけ行うversion: 1とファイル backend では参照のグループを常に空にする-pと違うグループの--groupは拒むlayout: groupのupとscaleはボリュームと機密のグループの食い違いで止めるenv backend testは対象のグループに属するプロジェクトだけを調べる_ensure_env_filesは子プロセスのenv initへ--groupを渡すexportは対象のグループのプロジェクトだけを集め、importは別グループのプロジェクトで止めるenv syncの同期済みハッシュは置き場のグループごとに持つTest plan
設計の段階で確かめたこと:
DEVBASE_ACCOUNT_GROUPの解決経路（volume/manager.pyresolve_account_groupはプロセスの環境変数を読む。bin/devbaseは実行時のディレクトリのenvだけを source する）SecretRefの生成箇所（grep -rn "SecretRef.for_global\|SecretRef.for_project" lib）と、OpenBaoBackend._seenの鍵がSecretRefであることdevbase/data/team/*の読み取り、devbase/data/users/{{identity.entity.name}}/*の読み書き）がグループ別のパスを含むこと_openbao_from_dictが未知のキーを無視し、_from_dictがversion1 以外を拒むこと（決定 1 の根拠）_ensure_env_filesの子プロセスはcwd=devbase_root（決定 10）、cmd_scaleは_run_deploy_pipelineを通らない（決定 7）、cmd_upは_run_pre_up_checks→_auto_snapshotの順、SourcesManagerは.env.sources.yml1 つ（決定 13）、env import/exportの除外は--exclude-project🤖 Generated with Claude Code