From 8063e5bfea357ef6d6f69c025f1bae435f894087 Mon Sep 17 00:00:00 2001 From: aloekun Date: Sat, 20 Jun 2026 03:15:54 +0900 Subject: [PATCH 1/4] =?UTF-8?q?docs(todo):=20PR=20#214=20post-merge-feedba?= =?UTF-8?q?ck=20T3-1=20=E6=8E=A1=E7=94=A8=20(=E9=A0=86=E4=BD=8D=20215)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #214 (refactor PR B partial) post-merge-feedback report の T3-1 を採用: `~/.claude/rules/common/coding-style.md` に "Defensive State Reset in State Machines" section を追記する todo entry を docs/todo-summary.md + docs/todo10.md に追加。 由来: PR #214 round 2 で CR Major #4 fix として `finalize_initial_review_park` 内 `read_state()` 後の `state.pr`/`state.repo`/`state.started_at` 無条件上書きが land。同型 `head_commit`/`review_recheck_count` reset と合わせて 5 field の確立 された defensive pattern を future reviewer が "redundant" と誤判定して 削除しないよう global rule で防御する。 Severity Medium / Frequency Medium / Effort S / Adoption Risk None で ユーザー承認 (2026-06-20)。pre-push:simplicity + pre-push:security の 独立 2 ソース検出。実装時は `feedback_global_config_backup` 適用必須。 --- docs/todo-summary.md | 1 + docs/todo10.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/docs/todo-summary.md b/docs/todo-summary.md index d336193a..94bc57e3 100644 --- a/docs/todo-summary.md +++ b/docs/todo-summary.md @@ -90,6 +90,7 @@ | 212 | 🔧 Tier 2 | **`hooks-pre-tool-validate` に PowerShell dispatch + `powershell-destructive-write-block` preset 追加 (PR #213 post-merge-feedback feedback-T1-1 + session 派生統合)** | todo10.md | M | なし (PR #213 で PowerShell スクリプトの連鎖失敗 `IndexOf -1 → Substring 例外 → null content → WriteAllText で空 byte 書込` で main.rs 2369 行を 0 byte 化した事故、memory `feedback_no_powershell_inplace_edit` の機械化、現行 dispatch table の盲点 (PowerShell 未対応) を埋めて `WriteAllText`/`Out-File`/`Set-Content` の destructive write を PreToolUse で block、`__*` scratch prefix は exception で allow、session 派生提案と analyzer feedback-T1-1 の独立収束で高い妥当性。**Tier 列との不整合補足**: analyzer feedback report の `Tier 1: Hooks/Linter 改善` カテゴリは project の Tier 1 (🚀 high-impact urgent) ではなく、memory `feedback_tier_classification` の re-classification rule に従い実体 (mechanical enforcement = Rust 新規実装) に基づき project の Tier 2 (🔧 tooling improvements) に再分類している) | | 213 | 🔧 Tier 2 | **`parse_coderabbit_status` の API ordering テスト追加 (PR #213 post-merge-feedback T2-1 採用)** | todo10.md | S | なし (PR #213 takt-fix iter 2 で `.last()` → `.first()` の semantic bug が修正された実観測、GitHub statuses API の reverse-chronological 返却 implicit assumption を test fixture で固定、doc comment + assert message で ordering semantics を明示、refactor 時の意味喪失を防ぐ defensive test、memory `feedback_test_dry_antipattern` 適用で独立 setup) | | 214 | 🔧 Tier 2 | **`parse_actionable_comments` の境界条件テスト追加 (PR #213 post-merge-feedback T2-2 採用)** | todo10.md | S | なし (PR #213 takt-fix iter 2 で `t > push_time` → `t >= push_time` の境界 bug が修正された実観測、`submitted_at == push_time` 完全一致ケースで inclusive 比較を保証、既存 rule⑦ `no-time-field-strict-greater` が変数名抽出後を catch 不可なため test による second defense layer、`actionable_includes_at_exact` + `actionable_excludes_before` の 2 件で境界の inclusive/exclusive を pin) | +| 215 | 💎 Tier 3 | **`~/.claude/rules/common/coding-style.md` に「Defensive State Reset in State Machines」section 追加 (PR #214 post-merge-feedback T3-1 採用)** | todo10.md | S | なし (PR #214 round 2 で `finalize_initial_review_park` の `state.pr` / `state.repo` / `state.started_at` を `read_state()` 後に無条件上書きする pattern が CR Major #4 の fix として land、CR Major #1 (`head_commit`) + CR Major #2 (`review_recheck_count`) と同型の defensive reset が既に 3 field 適用済 = `review_recheck.rs` 内で複数 instance あり、将来の reviewer が「redundant」と誤判定して削除すると prior cycle の stale state が混入して silent bug 化、global rules への docs 追記で派生プロジェクト (techbook-ledger / auto-review-fix-vc) に自動波及、simplicity-review LLM が同 file を読むため "enforced via review" として機能し memory `feedback_no_unenforced_rules` 例外を満たす、`feedback_global_config_backup` 適用必須) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で ADR-032 の前提 + rate-limit + convergence cost 削減を進める → Tier 3 で ADR-032 を land + ドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。 diff --git a/docs/todo10.md b/docs/todo10.md index b6f96340..b0f4f1cd 100644 --- a/docs/todo10.md +++ b/docs/todo10.md @@ -622,6 +622,48 @@ ADR-039 (Experimental Feature 標準パターン) は「behavior の妥当性が --- +### `~/.claude/rules/common/coding-style.md` に「Defensive State Reset in State Machines」section 追加 (PR #214 post-merge-feedback T3-1 採用) + +> **動機**: PR #214 round 2 で CR Major #4 (`既存 state 再利用時も現在の push 情報に更新してください`) の fix として `finalize_initial_review_park` 内で `read_state()` 後に `state.pr` / `state.repo` / `state.started_at` を `ctx` 値で **無条件上書き** する pattern を land した。この pattern は同 function 内の既存 reset と同型 (CR Major #1 fix で `head_commit` 上書き、CR Major #2 fix で `review_recheck_count = 0`) で、現時点で 3 field に適用済の確立された defensive pattern。 +> +> ただしこの「無条件上書き」は新規 reader / reviewer から見ると一見「冗長 (= `unwrap_or_else(|| ::new(...))` で既に同値を設定済だから不要)」に見える危険性がある。同型コードを future PR で reviewer (人間 / AI 両方) が「redundant resets は削除すべき」と誤判定して削除した場合、prior cycle の stale state (古い PR 番号 / repo / 開始時刻) が混入する silent bug を導入するリスクが顕在化する。 +> +> **本タスクの位置づけ**: PR #214 post-merge-feedback Tier 3 #1 採用 (Severity Medium / Frequency Medium / Effort S / Adoption Risk None、2026-06-20 ユーザー承認)。analyzer rationale: 「PR #214 の `review_recheck.rs` で positive pattern として land (lines 185–187)。同型コード (`review_recheck_count`, `head_commit` 上書き) との一貫性がある確立されたパターン。Frequency Medium = cli-pr-monitor には複数の state machine があり再発確実。Effort S、Adoption Risk None → ✅ 採用候補と判定」。pre-push:simplicity + pre-push:security の独立 2 ソース検出。 +> +> **参照**: `.claude/feedback-reports/214.md` Tier 3 #1、[src/cli-pr-monitor/src/stages/poll/review_recheck.rs:177-193](../src/cli-pr-monitor/src/stages/poll/review_recheck.rs#L177-L193) (現行 `finalize_initial_review_park` の defensive reset block)、memory `feedback_no_unenforced_rules.md` (enforcement 要件)、memory `feedback_global_config_backup.md` (snapshot 必須)。 +> +> **実行優先度**: 💎 **Tier 3** — Effort S。global rules への docs 追記 ~30 行で完結、`feedback_global_config_backup` snapshot を忘れない。 + +#### 設計決定 (案) + +- **追加先**: `~/.claude/rules/common/coding-style.md` の末尾 (`## Code Quality Checklist` の直前) または `## Error Handling` 直後に新 section「Defensive State Reset in State Machines」を追加 +- **rule 内容**: 「State machine 内で `read_state()` / `load_state()` 等の persisted state を再利用する場合、`new()` で設定される identity field と同等の **無条件上書き reset** を `read_state()` 後に明示的に書く。これは『冗長』に見えるが、prior cycle の stale state (古い PR 番号 / repo / session ID 等) が再利用 path で混入する silent bug を防ぐ defensive pattern。reviewer (人間 / AI) は redundant 削除を提案しないこと」 +- **anti-pattern 警告**: `let state = read_state().unwrap_or_else(|| State::new(id, repo, time));` だけで identity field を `ctx` で上書きしないと、prior cycle の値が残留する +- **good pattern 例**: PR #214 `review_recheck.rs:177-193` を inline cite (`state.pr` / `state.repo` / `state.started_at` / `state.review_recheck_count` / `state.head_commit` の 5 field reset) +- **由来 cite**: PR #214 の CR Major #4 が「既存 state 再利用時も現在の push 情報に更新してください」として独立検出した実証 +- **enforcement layer**: 機械 lint は困難 (`read_state` pattern の構文認識 + identity field 列挙が必要) だが、simplicity-review LLM が `coding-style.md` を読むため "enforced via review" として機能、memory `feedback_no_unenforced_rules` 例外を満たす +- **派生プロジェクト波及**: `~/.claude/rules/common/` 配下のため techbook-ledger / auto-review-fix-vc に自動 + +#### 作業計画 + +- [ ] `~/.claude/` snapshot 取得 (memory `feedback_global_config_backup` per) +- [ ] `~/.claude/rules/common/coding-style.md` に新 section「Defensive State Reset in State Machines」追記 (anti-pattern + good pattern + PR #214 由来 cite、約 30 行) +- [ ] markdownlint clean +- [ ] 本エントリ削除 + todo-summary.md 行削除 + +#### 完了基準 + +- `~/.claude/rules/common/coding-style.md` に「Defensive State Reset in State Machines」section が追加される +- 派生プロジェクト (techbook-ledger / auto-review-fix-vc) に global rule として自動波及 +- 由来 cite (PR #214 CR Major #4 + `review_recheck.rs:177-193`) で reviewer / Claude が rule 背景を理解可能 +- simplicity-review LLM が future PR で同型 `read_state()` を含む state machine 編集を review する際、本 section の anti-pattern 警告を参照可能 + +#### 詰まっている箇所 + +なし。Effort S、global rules への docs 追記のみ、`feedback_global_config_backup` snapshot を忘れない。 + +--- + ## 既知課題 (記録のみ、本セッションで未対応) (現時点で本ファイルへの既知課題は無し。docs/todo9.md 末尾を参照。) From de3bee33df7fdbc565b41edc574867b49a24522c Mon Sep 17 00:00:00 2001 From: aloekun Date: Sun, 21 Jun 2026 19:30:31 +0900 Subject: [PATCH 2/4] =?UTF-8?q?test(check-ci-coderabbit):=20API=20ordering?= =?UTF-8?q?=20+=20=E5=A2=83=E7=95=8C=E6=9D=A1=E4=BB=B6=E3=83=86=E3=82=B9?= =?UTF-8?q?=E3=83=88=E8=BF=BD=E5=8A=A0=20(=E9=A0=86=E4=BD=8D=20213,=20214)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 順位 213 (PR #213 post-merge-feedback T2-1 採用): parse_coderabbit_status の `.first()` semantics を test 名 + doc comment + assert message で explicit 化。GitHub statuses API の reverse-chronological 返却 implicit assumption を test fixture で 固定し、refactor 時の意味喪失を機械的に検出する。 順位 214 (PR #213 post-merge-feedback T2-2 採用): parse_actionable_comments の `submitted_at >= push_time` inclusive 比較を境界で固定する 2 test を追加: - actionable_includes_review_at_exact_push_time: `==` 境界の inclusive 比較を保証 - actionable_excludes_review_before_push_time: 配列 latest 位置に sentinel (Actionable: 99, 11:00) を置き、rfind が time filter で 正しく除外することを確認 (filter 壊れた場合 99 が返り test 落ちる) 既存 rule⑦ `no-time-field-strict-greater` は変数名抽出後を catch 不可なため、test による second defense layer として機能。 memory `feedback_test_dry_antipattern.md` 適用で独立 setup。 検証: - cargo test -p check-ci-coderabbit: 96 passed (= 93 + 3 new) - 既存 actionable_filters_by_time との相補的 boundary 検証 --- src/check-ci-coderabbit/src/parsers.rs | 61 ++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/check-ci-coderabbit/src/parsers.rs b/src/check-ci-coderabbit/src/parsers.rs index 2ba56ccf..5942dd9f 100644 --- a/src/check-ci-coderabbit/src/parsers.rs +++ b/src/check-ci-coderabbit/src/parsers.rs @@ -302,6 +302,28 @@ mod tests { assert_eq!(parse_coderabbit_status(json), "success"); } + /// 順位 213 (PR #213 post-merge-feedback T2-1 採用): GitHub commit statuses API は + /// reverse-chronological 返却 (新しい→古い順) であり、`parse_coderabbit_status` は + /// `.first()` で「最新 state」を取得する。この semantics を test 名 + doc comment + + /// assert message で explicit に固定し、refactor 時 (例: `.first()` → `.last()`) に + /// 意図しない意味変更を機械的に検出する。 + /// + /// 由来: PR #213 takt-fix iter 2 で `.last()` (= 最古 state を選んでいた) → `.first()` + /// の semantic fix が行われたが、元 production code (main.rs 時代) のコメントが + /// 「最新エントリ」と書きながら `.last()` を使っていた drift 事例。test 表現で固定する。 + #[test] + fn cr_status_reverse_chronological_picks_first() { + let json = r#"[ + {"context": "CodeRabbit", "state": "success"}, + {"context": "CodeRabbit", "state": "pending"} + ]"#; + assert_eq!( + parse_coderabbit_status(json), + "success", + "GitHub statuses API は reverse-chronological 返却のため、配列先頭 (`.first()`) が最新 state。`.last()` (古い state) を返すと CR レビュー終了を見逃す" + ); + } + #[test] fn walkthrough_clean_detected_when_marker_present_with_header() { let json = r#"[ @@ -466,6 +488,45 @@ mod tests { ); } + /// 順位 214 (PR #213 post-merge-feedback T2-2 採用): `parse_actionable_comments` の + /// `submitted_at >= push_time` inclusive 比較を境界で固定する。`>` (exclusive) に + /// 戻された際にこの test が落ちる構造で、direct push 直後の CR review が同時刻 + /// イベントとして報告される edge case の retest 取りこぼしを機械的に防ぐ。 + /// + /// 由来: PR #213 takt-fix iter 2 で `t > push_time` → `t >= push_time` の境界 fix。 + /// 既存 rule⑦ `no-time-field-strict-greater` は `submitted_at` 名直接使用 ケースのみ + /// catch し、変数名 `t` に抽出された後は static lint 不能。test による second defense layer。 + #[test] + fn actionable_includes_review_at_exact_push_time() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, "body": "Actionable comments posted: 3", "submitted_at": "2026-04-01T12:00:00Z"} + ]"#; + assert_eq!( + parse_actionable_comments(json, "2026-04-01T12:00:00Z"), + Some(3), + "submitted_at == push_time の review は inclusive 比較で含むべき (`>=` が `>` に戻されると取りこぼす)" + ); + } + + /// 順位 214 (negative sentinel): 配列 latest 位置に「push 以前の sentinel 99」を + /// 置き、time filter が壊れた場合 `rfind` がそれを先に返す構造にする。time filter が + /// 正しく動くと sentinel は除外され、配列前方の `>=` 適合 review (= 5) が選ばれる。 + /// 既存 `actionable_filters_by_time` は単一 review のみで test するが、本 test は + /// 「複数 review 中で boundary 適合のみを選ぶ」exclusion 挙動を sentinel pre-populate + /// で固定する (memory `feedback_test_dry_antipattern.md` 適用、独立 setup)。 + #[test] + fn actionable_excludes_review_before_push_time() { + let json = r#"[ + {"user": {"login": "coderabbitai[bot]"}, "body": "Actionable comments posted: 5", "submitted_at": "2026-04-01T12:30:00Z"}, + {"user": {"login": "coderabbitai[bot]"}, "body": "Actionable comments posted: 99", "submitted_at": "2026-04-01T11:00:00Z"} + ]"#; + assert_eq!( + parse_actionable_comments(json, "2026-04-01T12:00:00Z"), + Some(5), + "submitted_at < push_time の sentinel review (Actionable: 99) は time filter で除外され、push_time 以降の review (Actionable: 5) が選ばれるべき。99 が返れば time filter が機能していない" + ); + } + #[test] fn extract_count_from_body() { assert_eq!( From 5bcd5a424ff9ea3697ca2a257044fe816b16c3a9 Mon Sep 17 00:00:00 2001 From: aloekun Date: Sun, 21 Jun 2026 19:31:55 +0900 Subject: [PATCH 3/4] =?UTF-8?q?feat(hooks-pre-tool-validate):=20PowerShell?= =?UTF-8?q?=20dispatch=20+=20powershell-destructive-write-block=20preset?= =?UTF-8?q?=20=E8=BF=BD=E5=8A=A0=20(=E9=A0=86=E4=BD=8D=20212)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #213 (refactor PR A) 作業中に発生した PowerShell スクリプトでの `check-ci-coderabbit/src/main.rs` (2369 行) 0 byte 化事故の機械強制 対策。memory `feedback_no_powershell_inplace_edit` の codify と併用する mechanical defense layer として preset と dispatch を追加した。 設計: - 新 preset `powershell-destructive-write-block` (5 BlockedPattern) - `[System.IO.File]::WriteAllText|WriteAllBytes|WriteAllLines` (事故直接因) - `Out-File` (redirect 系 cmdlet) - `Set-Content -Value` (cmdlet 版書込、-Value flag 付きのみ false positive 回避) - すべての pattern に `(?i)` flag を付与 (PowerShell の .NET method 名 + cmdlet 名は case-insensitive) - Exception を path 位置に scope (= command 全体ではなく cmdlet/method 直後の 最初の引数のみ): - WriteAllText/Bytes/Lines: `\(\s*['"]__` で open paren 直後限定 - Out-File: `Out-File\s+(?:-FilePath\s+)?['"]?__` (unquoted も allow) - Set-Content: `Set-Content\s+(?:-(?:Literal)?Path\s+)?['"]?__` - Set-Content main pattern は `[^|]*-Value` で param 順非依存 (`-Value` が `-Path` より先に来る reversed order でも block) - dispatch table に `PowerShell` arm 追加 + `handle_powershell_tool` 関数 (handle_bash_tool と同形で `tool_input.command` を build_blocked_patterns 全 preset で照合) - `default_preset_names` に登録 = Config::default() で default-on 追加 test 19 件 (= 11 base + 8 edge case): base 11 件: - positive (5): WriteAllText / WriteAllBytes / WriteAllLines / Out-File / Set-Content -Value の destructive write block - negative scratch (2): `__*` prefix path への write は allow - negative legit (3): Get-ChildItem / Where-Object / Set-Content 単体 (false positive 回避) - default-on (1) edge case 8 件 (pre-push-review Warning 4 件に対応、ADR-043 fail-closed): - W-002 SEC: lowercase `.NET` method 名 (`writealltext`) でも block - W-001 SEC: `__` 文字列が value 位置にあるとき block (path 位置に scope) - W-001 SEC: ReadAllText で `__` を読んでも WriteAllText 先が prod なら block - W-001 SIMP: Set-Content の reversed parameter order (`-Value` 先) でも block - W-001 SIMP: Set-Content の reversed order でも scratch path なら allow - W-002 SIMP: unquoted scratch path (`Out-File __output.txt`) は allow - value 位置の `__` を path と誤認しない (Set-Content 版) 検証: - cargo test -p hooks-pre-tool-validate: 221 passed (= 202 既存 + 19 new) - cargo clippy -p hooks-pre-tool-validate -- -D warnings: clean - cargo test --workspace: 全テスト pass (regression なし) Note: `src/hooks-pre-tool-validate/src/main.rs` は本 PR 後 2900 行になり file_length lint (800 行ガイドライン) を超過するが、本 PR は順位 212 の mechanical defense 追加に scope を限定し、ファイル分割 refactor は別 PR (PR #213/#214 と同 pattern) で対応する。 --- src/hooks-pre-tool-validate/src/main.rs | 280 ++++++++++++++++++++++++ 1 file changed, 280 insertions(+) diff --git a/src/hooks-pre-tool-validate/src/main.rs b/src/hooks-pre-tool-validate/src/main.rs index d8cf4129..b9bdc51e 100644 --- a/src/hooks-pre-tool-validate/src/main.rs +++ b/src/hooks-pre-tool-validate/src/main.rs @@ -475,6 +475,92 @@ jj split -m "" # commit 分離 }] } +/// プリセット: powershell-destructive-write-block (PowerShell からの破壊的ファイル書込防止) +/// +/// 順位 212 (PR #213 post-merge-feedback feedback-T1-1 + session 派生統合採用): +/// PR #213 (refactor PR A) 作業中、PowerShell スクリプトで `check-ci-coderabbit/src/main.rs` +/// (2369 行) を **0 byte に消去** する事故が発生した。連鎖失敗の構造: +/// ① `IndexOf` の検索 marker に CRLF (`` `r`n ``) を埋め込んだが file は LF only +/// ② `IndexOf` が `-1` を返すも `Substring(0, -1)` 直接呼び出し → `MethodInvocationException` +/// ③ PowerShell default `$ErrorActionPreference = Continue` で script 続行 +/// ④ `$newContent = $null` のまま `[System.IO.File]::WriteAllText($path, $null)` → 空ファイル +/// +/// memory `feedback_no_powershell_inplace_edit.md` で人間 / AI 規範として codify 済だが、 +/// memory は揮発する懸念があり mechanical defense として本 preset を併設する。 +/// +/// 検出対象 (5 patterns): +/// - `[System.IO.File]::WriteAllText(` (今回事故の直接因) +/// - `[System.IO.File]::WriteAllBytes(` +/// - `[System.IO.File]::WriteAllLines(` +/// - `Out-File` (redirect 系 cmdlet) +/// - `Set-Content -Value` (cmdlet 版書き込み) +/// +/// Exception: 文字列リテラル中に `__` prefix (scratch ファイル規約) を含む場合は allow。 +/// 例: `[System.IO.File]::WriteAllText("__dump.json", ...)` は scratch ファイル明示のため通す。 +fn preset_powershell_destructive_write() -> Vec { + vec![ + BlockedPattern { + pattern: Regex::new(r"(?i)\[System\.IO\.File\]::WriteAllText\s*\(").unwrap(), + exception: Some( + Regex::new(r#"(?i)\[System\.IO\.File\]::WriteAllText\s*\(\s*['"]__"#).unwrap(), + ), + message: POWERSHELL_DESTRUCTIVE_WRITE_MSG, + }, + BlockedPattern { + pattern: Regex::new(r"(?i)\[System\.IO\.File\]::WriteAllBytes\s*\(").unwrap(), + exception: Some( + Regex::new(r#"(?i)\[System\.IO\.File\]::WriteAllBytes\s*\(\s*['"]__"#).unwrap(), + ), + message: POWERSHELL_DESTRUCTIVE_WRITE_MSG, + }, + BlockedPattern { + pattern: Regex::new(r"(?i)\[System\.IO\.File\]::WriteAllLines\s*\(").unwrap(), + exception: Some( + Regex::new(r#"(?i)\[System\.IO\.File\]::WriteAllLines\s*\(\s*['"]__"#).unwrap(), + ), + message: POWERSHELL_DESTRUCTIVE_WRITE_MSG, + }, + BlockedPattern { + pattern: Regex::new(r"(?i)\bOut-File\b").unwrap(), + exception: Some( + Regex::new(r#"(?i)\bOut-File\b\s+(?:-FilePath\s+)?['"]?__"#).unwrap(), + ), + message: POWERSHELL_DESTRUCTIVE_WRITE_MSG, + }, + BlockedPattern { + pattern: Regex::new(r"(?i)\bSet-Content\b[^|]*-Value").unwrap(), + exception: Some( + Regex::new( + r#"(?i)\bSet-Content\b\s+(?:-(?:Literal)?Path\s+)?['"]?__"#, + ) + .unwrap(), + ), + message: POWERSHELL_DESTRUCTIVE_WRITE_MSG, + }, + ] +} + +const POWERSHELL_DESTRUCTIVE_WRITE_MSG: &str = r#"**PowerShell からの破壊的ファイル書込がブロックされました** + +PowerShell の `[System.IO.File]::WriteAllText` / `WriteAllBytes` / `WriteAllLines` / `Out-File` / +`Set-Content -Value` は **null / 空文字列を渡された場合に対象ファイルを 0 byte に消去** します。 + +過去事故 (PR #213): `IndexOf` で marker を探したが文字コードズレで -1 が返り、 +`Substring(0, -1)` が例外を投げて `$newContent = $null` のまま `WriteAllText` を実行、 +src/check-ci-coderabbit/src/main.rs (2369 行) を消失させた。 + +**代替方法 (安全な書込手段):** +- 単純な置換: `Edit` tool で old_string / new_string 指定 +- 新規ファイル作成: `Write` tool で content 指定 +- in-place 編集が必要なら: `Bash` tool で `sed -i 's/PATTERN/REPL/' file` +- jj の lifecycle に乗せたいなら: 編集後に `jj describe` で commit + +**例外 (allow される case):** +- `__*` prefix の scratch ファイル ('__dump.json' 等) への書込は規約 (CLAUDE.md § Scratch / 一時ファイル命名規約) + により VCS 管理外のため許可 + +設計判断 (順位 212): memory `feedback_no_powershell_inplace_edit` の機械強制層。"#; + /// プリセット: secret-detection (AWS / OpenAI / GitHub / Anthropic 等の hardcoded secret 検出) /// /// 順位 146 (PR #200 follow-up、`~/.claude/rules/common/security.md` § Secret Management 移管): @@ -547,6 +633,7 @@ fn default_preset_names() -> Vec { "jj-push-guard".to_string(), "electron".to_string(), "secret-detection".to_string(), + "powershell-destructive-write-block".to_string(), ] } @@ -564,6 +651,7 @@ fn resolve_preset_or_custom(name: &str) -> Vec { "polling-anti-pattern" => preset_polling_anti_pattern(), "exe-help-block" => preset_exe_help_block(), "electron" => preset_electron(), + "powershell-destructive-write-block" => preset_powershell_destructive_write(), custom => custom_regex_pattern(custom), } } @@ -1098,10 +1186,31 @@ fn main() -> ExitCode { match tool_name.as_str() { "Bash" => handle_bash_tool(&config, &tool_input), "Write" | "Edit" | "Replace" => handle_write_edit_tool(&config, &tool_input), + "PowerShell" => handle_powershell_tool(&config, &tool_input), _ => ExitCode::SUCCESS, } } +/// 順位 212: PowerShell tool 用ハンドラ。`handle_bash_tool` と同形で +/// `tool_input.command` を `build_blocked_patterns` の全 preset で照合する。 +/// +/// PowerShell preset (`powershell-destructive-write-block`) は default-on のため +/// `WriteAllText` / `WriteAllBytes` / `WriteAllLines` / `Out-File` / `Set-Content -Value` を +/// block する。Bash と共通の guard (git push / jj immutable / electron 等) も +/// 同時に適用される (PowerShell 上で git push 等を叩く case もカバー)。 +fn handle_powershell_tool(config: &Config, tool_input: &ToolInput) -> ExitCode { + let command = tool_input.command.clone().unwrap_or_default(); + if command.trim().is_empty() { + return ExitCode::SUCCESS; + } + let patterns = build_blocked_patterns(config); + if let Some(message) = validate_command(&command, &patterns) { + let _ = io::stderr().write_all(message.as_bytes()); + return ExitCode::from(2); + } + ExitCode::SUCCESS +} + #[cfg(test)] mod tests { use super::*; @@ -1406,6 +1515,177 @@ mod tests { ); } + const PS_DESTRUCTIVE: &[&str] = &["powershell-destructive-write-block"]; + + #[test] + fn powershell_blocks_writealltext_to_production_file() { + assert!(is_blocked_with( + r#"[System.IO.File]::WriteAllText("src/main.rs", $content)"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_blocks_writeallbytes_to_production_file() { + assert!(is_blocked_with( + r#"[System.IO.File]::WriteAllBytes("data.bin", $bytes)"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_blocks_writealllines_to_production_file() { + assert!(is_blocked_with( + r#"[System.IO.File]::WriteAllLines("log.txt", $lines)"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_blocks_out_file_redirect() { + assert!(is_blocked_with( + "Get-Process | Out-File processes.txt", + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_blocks_set_content_with_value() { + assert!(is_blocked_with( + r#"Set-Content -Path src/config.toml -Value $newConfig"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_allows_writealltext_when_path_argument_is_scratch_prefix() { + assert!(!is_blocked_with( + r#"[System.IO.File]::WriteAllText("__dump.json", $json)"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_allows_out_file_when_path_argument_is_scratch_prefix() { + assert!(!is_blocked_with( + r#"Get-Process | Out-File "__processes.log""#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_allows_get_childitem_read_only_cmdlet() { + assert!(!is_blocked_with("Get-ChildItem ./src", PS_DESTRUCTIVE)); + } + + #[test] + fn powershell_allows_where_object_pipeline_read_only() { + assert!(!is_blocked_with( + r#"Get-Process | Where-Object {$_.CPU -gt 100}"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_allows_set_content_bare_invocation_without_value_flag_to_avoid_false_positive() { + assert!(!is_blocked_with("Set-Content", PS_DESTRUCTIVE)); + } + + #[test] + fn powershell_destructive_write_is_default_on_in_default_config() { + let patterns = build_blocked_patterns(&Config::default()); + assert!( + validate_command( + r#"[System.IO.File]::WriteAllText("foo.rs", $null)"#, + &patterns + ) + .is_some(), + "default fallback should include powershell-destructive-write-block (順位 212 で default-on)" + ); + } + + #[test] + fn powershell_blocks_writealltext_with_lowercase_dot_net_method_name() { + assert!( + is_blocked_with( + r#"[system.io.file]::writealltext("src/main.rs", $content)"#, + PS_DESTRUCTIVE + ), + "PowerShell の .NET method 名は case-insensitive のため lowercase でも block すべき (W-002 SEC fix)" + ); + } + + #[test] + fn powershell_blocks_writeallbytes_with_lowercase_dot_net_method_name() { + assert!(is_blocked_with( + r#"[system.io.file]::writeallbytes("data.bin", $bytes)"#, + PS_DESTRUCTIVE + )); + } + + #[test] + fn powershell_blocks_writealltext_when_scratch_string_is_in_value_position_not_path() { + assert!( + is_blocked_with( + r#"[System.IO.File]::WriteAllText("prod.rs", "__placeholder content")"#, + PS_DESTRUCTIVE + ), + "path が prod.rs で value が \"__...\" の場合は block すべき (W-001 SEC fix: exception は path 位置にのみ scope)" + ); + } + + #[test] + fn powershell_blocks_writealltext_when_command_reads_scratch_but_writes_to_production() { + assert!( + is_blocked_with( + r#"$x = [System.IO.File]::ReadAllText("__scratch.txt"); [System.IO.File]::WriteAllText("prod.rs", $null)"#, + PS_DESTRUCTIVE + ), + "ReadAllText から __ scratch を読んでも、WriteAllText が prod.rs に書くなら block (W-001 SEC fix: exception scope)" + ); + } + + #[test] + fn powershell_blocks_set_content_with_value_in_reversed_parameter_order() { + assert!( + is_blocked_with( + "Set-Content -Value $x -Path src/main.rs", + PS_DESTRUCTIVE + ), + "Set-Content は -Value と -Path の順序非依存で block すべき (W-001 SIMP fix)" + ); + } + + #[test] + fn powershell_allows_set_content_with_scratch_path_in_reversed_parameter_order() { + assert!( + !is_blocked_with( + r#"Set-Content "__file.txt" -Value $x"#, + PS_DESTRUCTIVE + ), + "scratch path で positional argument 指定は allow" + ); + } + + #[test] + fn powershell_allows_out_file_with_unquoted_scratch_path() { + assert!( + !is_blocked_with("Out-File __output.txt", PS_DESTRUCTIVE), + "unquoted な __ prefix scratch path も allow (W-002 SIMP fix)" + ); + } + + #[test] + fn powershell_blocks_set_content_when_scratch_string_in_value_position_not_path() { + assert!( + is_blocked_with( + r#"Set-Content -Path "prod.rs" -Value "__placeholder""#, + PS_DESTRUCTIVE + ), + "path が prod.rs で value が \"__...\" の場合は block すべき (Set-Content 版 W-001 SEC)" + ); + } + #[test] fn is_secret_detection_enabled_returns_true_when_listed_in_blocked_patterns() { let config = Config { From f264ece11694bc123a069af183a70bede3e698c2 Mon Sep 17 00:00:00 2001 From: aloekun Date: Mon, 22 Jun 2026 01:31:23 +0900 Subject: [PATCH 4/4] =?UTF-8?q?fix(hooks-pre-tool-validate):=20Set-Content?= =?UTF-8?q?=20exception=20=E3=82=92=E9=A0=86=E5=BA=8F=E9=9D=9E=E4=BE=9D?= =?UTF-8?q?=E5=AD=98=E3=81=AB=E5=AF=BE=E5=BF=9C=20(PR=20#215=20CR=20Minor?= =?UTF-8?q?=20#1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #215 CR review が指摘した Minor finding を対応: `Set-Content` の exception regex が main pattern と order-independence を共有していなかったため、`Set-Content -Value $x -Path "__file"` のような reversed parameter order での scratch path 指定が false positive で block されていた。 修正: 旧 exception: `(?i)\bSet-Content\b\s+(?:-(?:Literal)?Path\s+)?['"]?__` → `-Path` が Set-Content 直後にある順序のみカバー 新 exception: `(?i)\bSet-Content\b(?:\s+['"]?__|[^|]*\s-(?:Literal)?Path\s+['"]?__)` → 2-alternative: - alt 1: `Set-Content` 直後の positional `__` path - alt 2: `Set-Content` 以降のいずれかの位置の `-Path`/`-LiteralPath` 後の `__` path CR の修正案は `(?=...)` lookahead を使っていたが、Rust 標準 `regex` crate は lookahead 非対応 (compile failure) のため、同 semantics を 2-alt 構成で lookahead なしに実装。main pattern `[^|]*-Value` が既に `-Value` 存在を 保証しているため、exception 側に lookahead-based verification は不要。 test 追加: - `Set-Content -Value $x -Path "__file.txt"` (reversed order) → allow - `Set-Content -Value $x -LiteralPath "__file.txt"` (LiteralPath 変種) → allow - 既存の positional case (`Set-Content "__file.txt" -Value $x`) は維持 検証: - cargo test -p hooks-pre-tool-validate: 221 passed - cargo clippy -p hooks-pre-tool-validate -- -D warnings: clean 由来: PR #215 CR Minor #1 (discussion_r3448719154)、ADR-043 fail-closed 原則による security gate と false-positive 抑制の両立。 --- src/hooks-pre-tool-validate/src/main.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/hooks-pre-tool-validate/src/main.rs b/src/hooks-pre-tool-validate/src/main.rs index b9bdc51e..0c87fbd3 100644 --- a/src/hooks-pre-tool-validate/src/main.rs +++ b/src/hooks-pre-tool-validate/src/main.rs @@ -531,7 +531,7 @@ fn preset_powershell_destructive_write() -> Vec { pattern: Regex::new(r"(?i)\bSet-Content\b[^|]*-Value").unwrap(), exception: Some( Regex::new( - r#"(?i)\bSet-Content\b\s+(?:-(?:Literal)?Path\s+)?['"]?__"#, + r#"(?i)\bSet-Content\b(?:\s+['"]?__|[^|]*\s-(?:Literal)?Path\s+['"]?__)"#, ) .unwrap(), ), @@ -1665,6 +1665,20 @@ mod tests { ), "scratch path で positional argument 指定は allow" ); + assert!( + !is_blocked_with( + r#"Set-Content -Value $x -Path "__file.txt""#, + PS_DESTRUCTIVE + ), + "-Value が -Path より先でも scratch path は allow (PR #215 CR Minor #1 fix: exception の order-independence を main pattern と整合)" + ); + assert!( + !is_blocked_with( + r#"Set-Content -Value $x -LiteralPath "__file.txt""#, + PS_DESTRUCTIVE + ), + "-Value 先行 + -LiteralPath でも scratch path は allow" + ); } #[test]