Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions docs/issues/open/I212.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# I212: 差分のレビューが、組み込みの /security-review が動かなかった回の記録を起点にして、セキュリティレビューを飛ばして OK を出すのを止める

## 関連資料
- GitHub Issue: #419
- 旧 GitHub Issue: #358(停止中の bot `osushi0404-bot` が起票したため GitHub で見えない。2026-11-07 に GitHub App の名義で起票し直した。bot の停止が解けたら #358 は「起票し直した先へ移した」と書いて閉じる)
- 起点: I211(#356・PR #357)の review-gate 2 周目(retro からの起票)。**次のハーネスの作業で最優先**(判定の仕組み `scripts/claude/code-review.sh` に関わる穴)
- 記録: `docs/reviews/I211_code_review_20261029_0155.md`(`OFFICIAL_SECURITY: FAILED`・`FINAL_VERDICT: HIGH`)→ `docs/reviews/I211_code_review_20261029_0204.md`(範囲のファイル 0 本・`FINAL_VERDICT: OK`)。I211 の自律判断の記録(`docs/reviews/closed/I211_review.md` の「[review-gate 2]」)
- 差分の起点の選び方: `scripts/claude/code-review.sh:482-506`(`ledger_records`・`latest_ledger_record`・`review_mode`)
- 手順: `.claude/skills/review-gate/SKILL.md`(「出力に `OFFICIAL_SECURITY: FAILED` がある: …1 回だけ実行し直す」)
- 正: `docs/runbooks/review-rules.md`「検証の全体設計」

## 同梱: I213 の retro から(2026-10-29・GitHub のコメント https://github.com/osushi0404-dev/study-app-multitenant/issues/358#issuecomment-6485976047 と同じ内容)
I213(#359)の review-gate でこのイシューの穴が実際に起き、近い種類の穴が 2 つ見つかった(記録: `docs/reviews/closed/I213_review.md` の「自律判断の記録」の review-gate の行)。3 つとも「差分だけのやり直しが、前の回の確かめの結果を正しく扱わない」穴。計画で、まとめて扱うか、3 を別イシューに切り出すかを決める。
1. **このイシューの穴の実例**: `docs/reviews/closed/I213_code_review_20261029_1009.md` が `OFFICIAL_SECURITY: FAILED` で HIGH → 1 回だけやり直した `..._1020.md` は「差分のレビュー・範囲のファイル 0 本」で、組み込みの /security-review を動かさずに OK。I213 では 1 回目の /security-review の本文(ブランチ全体を見て指摘なし)を根拠に進めた
2. **「指摘なし」の取り違え**: 1 回目の /security-review は実際には動き、「報告対象の脆弱性はありませんでした」と日本語の文章で書いていた。`scripts/claude/code-review.sh` の `official_security_verdict()` は、指摘の配列か決まった「指摘なし」の形しか読まないため「読めていない」と判定した。根本原因は、判定が組み込みのレビューの文章の形に頼っていること。予防策の案: 組み込みのレビューへの依頼に「最後の行に機械判定の 1 行(例: `SECURITY_FINDINGS: 0`)を必ず出す」を足し、判定はその行だけで行う
3. **敵対的レビューの差分の周で、中身の変わらない「実験必須」の項目が未検証になる**: `docs/reviews/closed/I213_adversarial_review_20261029_1032.md` で実験により確認済みだった V2〜V4 が、文言だけの差分を見た `..._1044.md` で実験されず「未検証」→ HIGH → 上限到達でユーザーの延長の判断が要った(中身のファイルはその間に 1 行も変わっていない)。根本原因は、前の周の確認結果を、関係するファイルが変わっていなければ引き継ぐ仕組みが無いこと。予防策の案: `scripts/claude/adversarial-review.sh` で、前の周に「確認済み(実験)」だった項目は、その周の後に記録の場所の外のファイルが変わっていなければ確認済みとして引き継ぎ、引き継いだことを記録に書く(決定論)

## 同梱: I200 の retro から(2026-10-30・GitHub のコメント https://github.com/osushi0404-dev/study-app-multitenant/issues/358#issuecomment-6500741220 と同じ内容)
4. **2 の再発**: I200 の `docs/reviews/I200_code_review_20261029_2240.md`・`..._2357.md` でも、/security-review は動いて「報告する(できる)脆弱性はありません」と書いたのに `OFFICIAL_SECURITY: FAILED` → やり直しは差分なしで /security-review を動かさず OK(I213 と合わせて 3 回)
5. **敵対的レビューの締め切りで実験が 1 本も走らない**(I206 の確かめる 2 点の②): `docs/reviews/I200_adversarial_review_20261029_2318.md` で 3 観点とも 920〜990 秒かかり、実験の案を出した時点で締め切り。「実行していない(締め切り)」2 本・実験必須の項目がすべて未検証で HIGH。根本原因は締め切りがレビュー役と実験役で共有されていること。予防策の案(実験の時間を別に確保する・実験必須の項目は計画で決定論の TC を必須にする 等)は計画で決める。対象: `scripts/claude/adversarial-review.sh`・`.claude/agents/adversarial-reviewer.md`・`docs/runbooks/review-rules.md`

## 同梱: I226 の retro から(2026-11-06・**次のハーネスの作業で最優先**の印つき。#358 は bot の停止で見えず、GitHub へのコメントは停止が解けてから同じ内容で付ける)
6. **G10 の `\b` の後退(I226 の台帳 F1・Medium・SECURITY=yes・再現あり)**: I226 で `scripts/claude/tests/test_i223_gh_app.sh` の G10 の正規表現を `gh auth (status|token)\b` にした。Python の `\w` は和文も含むので、「gh auth tokenを打つ」のように和文がすぐ続く書き方で `\b` が当たらず、G10 が合格になる(変更前の `gh auth status` は `\b` なしで止めていた)。記録: `docs/reviews/closed/I226_code_review_20261106_2139.md` の F1。根本原因: 語の境目を `\b` に任せ、和文が続く形の FG(失敗の注入)を足さなかった。直し方の案: `\b` を `(?![A-Za-z0-9_-])` などの英数字だけの先読みに替え、`g10_fg "和文が続く auth token は不合格" 1 'gh auth tokenを打つ'` を足す。対象: `scripts/claude/tests/test_i223_gh_app.sh`。このイシューの本題(差分のレビューの起点)とは別の小さな直しなので、計画で同じ PR に入れるか決める

## 背景/目的

**背景**: I211 で python の版を 3.12.3 に揃え直した差分(`.python-version`・`pyproject.toml`・`uv.lock`。CI の設定を含むブランチなので高リスク)を review-gate に通した。1 回目は組み込みの `/security-review` が動かず(基盤の失敗)、手順どおり `FINAL_VERDICT: HIGH` になった。手順どおり 1 回だけやり直したところ、スクリプトは失敗した回の記録の `REVIEWED_COMMIT` を差分の起点にし、「起点の後に記録の場所の外は変わっていない(範囲のファイル 0 本)」と判断して、レビュー役も `/security-review` も呼ばずに台帳を引き継ぎ、`FINAL_VERDICT: OK` を出した。`gate-check.sh` もこの記録で通った。I211 では Claude が気づいて `/security-review` を直接実行して補った(指摘なし)。
**目的**: レビューが失敗した回(HIGH・BLOCKER・組み込みのレビューが動かなかった回)の後のやり直しで、その回が見るはずだった差分を必ず見直す。セキュリティレビューを一度も通らない変更が test-gate に進む道を無くす。

### 現状の問題(証拠・再現手順)
- `review_mode()`(`scripts/claude/code-review.sh:494-506`)は、台帳のある最新の記録(`LEDGER_ROUND:` がある記録)の `REVIEWED_COMMIT` が HEAD の祖先なら差分のレビューにする。記録の `VERDICT`/`FINAL_VERDICT`/`OFFICIAL_SECURITY` は見ていない
- 1 回目の記録(`OFFICIAL_SECURITY: FAILED`・HIGH)にも `LEDGER_ROUND:` と `REVIEWED_COMMIT:` が書かれるため、やり直しの起点になる
- 再現(I211): 高リスクのブランチで、組み込みの `/security-review` が動かない回を作る → 記録が commit される → やり直す → 「差分のレビュー(台帳の 3 周目・範囲のファイル 0 本)」「レビュー役は呼ばずに台帳を引き継ぎます」→ `FINAL_VERDICT: OK`

### 根本原因
差分のレビューの起点を「最後にレビューした場所」で選んでいて、「最後に合格した場所」で選んでいない。失敗した回の記録も「見た」ことになり、その回が見るはずだった差分が、やり直しの範囲から外れる。

### 解決方針
- 差分のレビューの起点にするのは、`VERDICT: OK`(台帳の結論が合格)で、高リスクなら組み込みの `/security-review` が動いた(`OFFICIAL_SECURITY: FAILED` が無い)記録だけにする。HIGH・BLOCKER・FAILED の記録は起点にしない(その前の合格した記録から差分を取る。合格した記録が無ければ全体のレビュー)
- 台帳(指摘の番号・状態)の引き継ぎは今までどおり(起点の選び方だけを変える)
- 決定論のテストで固定する(失敗した記録を最新に置いた写しで、差分の範囲が前の合格した記録から取られること。失敗注入: 失敗した記録の後に記録の場所の外の変更が無くても、範囲が 0 本にならない)
- `gate-check.sh` が失敗した回の後の「範囲 0 本・OK」の記録で通らないかも確かめる(必要なら同じ考え方で直す)。詳細は計画で決める

## 実装対象(既知のもの)
| ファイル | 変更内容 |
|---------|---------|
| `scripts/claude/code-review.sh` | `review_mode()`(と `latest_ledger_record` の使い方)で、差分の起点を合格した記録に限る |
| `scripts/claude/gate-check.sh` | 必要なら、失敗した回の後のやり直しの記録の扱いを揃える(未特定・計画で確認) |
| `scripts/claude/tests/`(既存の code-review.sh のテストへの追加か新規) | 失敗した記録を起点にしないことの決定論のテスト(失敗注入つき) |
| `.claude/skills/review-gate/SKILL.md` | 「`OFFICIAL_SECURITY: FAILED` は 1 回だけやり直す」の手順と、起点の選び方の説明を揃える(必要なら) |
| `scripts/claude/code-review.sh`(同梱 2) | 組み込みのレビューへの依頼に機械判定の 1 行を出させ、`official_security_verdict()` はその行だけで判定する(範囲は計画で決める) |
| `scripts/claude/adversarial-review.sh`・`docs/runbooks/review-rules.md`(同梱 3) | 前の周の「確認済み(実験)」の項目を、関係するファイルが変わっていなければ引き継ぐ(計画で同梱か切り出しかを決める) |

## スコープ
### 含む
- 差分のレビューの起点の選び方(失敗した記録を起点にしない)
- それを固定する決定論のテスト
- 関係する手順の記述の整合
### 含まない
- 組み込みの `/security-review` が動かない原因そのもの(基盤側の失敗。再発すれば別途)
- 台帳の振り分け(`--route`)の決まり

## 関連する品質基準
- C8 セキュリティ — 高リスクの変更が組み込みのセキュリティレビューを一度も通らずに進む道を無くす

## 受け入れ条件(Acceptance Criteria)
- [ ] 最新の記録が HIGH・BLOCKER・`OFFICIAL_SECURITY: FAILED` のとき、やり直しの差分のレビューは、その前の合格した記録(無ければ全体)を起点にする
- [ ] その結果、失敗した回の後に記録の場所の外の変更が無くても、失敗した回が見るはずだった差分をレビュー役と(高リスクなら)組み込みの `/security-review` が見る
- [ ] 上の 2 点を、失敗注入つきの決定論のテストで固定している(失敗した記録を起点にする前の形に戻すとテストが落ちる)
- [ ] 失敗が続いた場合(失敗した記録が 2 つ以上続く)も、遡って最後の合格した記録を起点にする(テストで固定)
- [ ] `gate-check.sh` が、失敗した回の後に「範囲 0 本」で出た OK の記録だけでは通さない(失敗した回が見るはずだった差分を見た記録が要る)ことをテストで固定している(計画で `gate-check.sh` を直す必要が無いと分かったら、その理由を計画書に書き、テストだけ足す)
- [ ] 台帳(指摘の番号・状態)は、起点にした記録ではなく最新の台帳のある記録から引き継ぐ(I206 の番号の合流を壊さない。どの記録から引き継ぐかを計画書に明記する)
- [ ] 合格の判定に使う欄(`VERDICT`/`FINAL_VERDICT`/`OFFICIAL_SECURITY`)と、欄が無い古い記録の扱いを計画書に明記する
- [ ] 合格した記録の後のやり直し(今までの差分のレビュー)の動きは変わらない(既存のテストが通る)
- [ ] 既存のハーネステスト(`bash scripts/claude/tests/run-all.sh`)・CI がすべて pass する

## 影響範囲(想定)
- Backend: なし
- Frontend: なし
- DB: なし
- Config/Infra: `scripts/claude/code-review.sh`(判定の仕組み)・必要なら `scripts/claude/gate-check.sh`・`.claude/skills/review-gate/SKILL.md`・ハーネステスト。同梱分を入れるなら `scripts/claude/adversarial-review.sh`・`docs/runbooks/review-rules.md`

## 制約・引き継ぎ情報
- I211 では、`/security-review` を直接実行して補った(指摘なし)。I211 の PR #357 にはこの穴の影響は残っていない
- 差分のレビュー・台帳の仕組みは I197・I202・I205・I206・I207・I208 で作った。台帳の番号の引き継ぎ(I206)を壊さない
- 判定の仕組みの変更なので、計画レビューで高リスクの扱いになる見込み(未定・計画で確認)

## Danger Ops(有/無)
- 無
19 changes: 19 additions & 0 deletions docs/reviews/I212_issue_review_20261029_0226.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# イシュー自己完結度レビュー [I212]

## 判定: 十分
重要セクションに空欄はなく、重大な論理矛盾もありません。「未特定・計画で確認」は意図的な宣言なので、未記入とは扱いません。下記は計画時に詰めるべき助言です。

## 未記入の重要セクション
なし

## 指摘一覧(助言)
| 重大度 | 観点 | 指摘内容 | 該当箇所 |
|--------|------|---------|---------|
| Warning | 解決方針 | 台帳の引き継ぎ元が曖昧です。起点を「前の合格した記録」に戻すと、失敗した回で採番・更新された指摘(番号・状態)を、どの記録から引き継ぐのかが読み取れません。「今までどおり」とあるだけで、起点の記録と台帳の引き継ぎ元が別になる場合の扱いは未記載です。I206 の番号引き継ぎを壊さないという制約と直結します。 | 解決方針 2 項目目/制約 |
| Warning | スコープ/受け入れ条件 | `gate-check.sh` の確認は解決方針と実装対象に出ていますが、受け入れ条件にはありません。「失敗した回の後の範囲 0 本・OK の記録で通らない」ことを検証可能な形で入れないと、確認だけで終わる恐れがあります。 | 解決方針 4 項目目/受け入れ条件 |
| Info | 解決方針 | 「高リスクなら `OFFICIAL_SECURITY` が動いた記録」の判定方法が曖昧です。非高リスクの記録や、古い形式でこのフィールドが無い記録は、合格扱い(フィールドなし=FAILED でない)にするのかを決める必要があります。 | 解決方針 1 項目目 |
| Info | 受け入れ条件 | `VERDICT: OK`(台帳の結論)と `FINAL_VERDICT` の使い分けが混在しています。合格の判定にどちらを使うかを統一すると、テストが書きやすくなります。 | 解決方針 1 項目目/受け入れ条件 1 |
| Info | 根本原因 | 原因は成立しています(起点が最後にレビューした場所で、最後に合格した場所ではない)。ただし、失敗が連続した場合(FAILED が 2 回続くなど)に、遡って最初の合格記録を探すことを受け入れ条件かテストに入れると堅くなります。 | 根本原因/受け入れ条件 3 |

## 次のアクションの推奨
`/grill-me` をスキップして `/plan-issue` に進められます。計画では、上の Warning 2 件(台帳の引き継ぎ元と `gate-check.sh` の受け入れ条件化)を先に決めてください。
Loading