Skip to content

fix: コードレビュー指摘の修正(堅牢性・前方互換・セキュリティ)#333

Merged
kkyosuke merged 5 commits into
mainfrom
review
Jun 25, 2026
Merged

fix: コードレビュー指摘の修正(堅牢性・前方互換・セキュリティ)#333
kkyosuke merged 5 commits into
mainfrom
review

Conversation

@kkyosuke

@kkyosuke kkyosuke commented Jun 24, 2026

Copy link
Copy Markdown
Owner

目的

最新 main を複数観点(アーキテクチャ/並行性/セキュリティ/エラー処理・堅牢性/イディオム)でレビューし、見つかった指摘のうち安全かつテスト可能で明確に正しいものを修正する。

変更内容

1. git::primary_worktree の panic を回避

git worktree list --porcelain が exit 0 で worktree 行を 1 つも返さない場合に .expect()プロセスを panicさせていた。状態同期のホットパス(workspace_state::sync / load)で TUI 全体を巻き込むため、純粋ヘルパー primary_of に分離してエラーを返す。

2. UpdateHandle / InstallHandle の poison 回復

両ハンドルの lock() が mutex poison 時に .expect() で panic していた。raw / alternate-screen モード中に毎フレーム読まれるため、端末を壊したまま UI をクラッシュさせる。兄弟ハンドル(SessionsRefreshHandle・terminal pool / monitor)と同じ never-crash-on-poison 方針に統一。

3. on-disk 構造体の version#[serde(default)]

WorkspacesFile / SettingsFile / StateFile / LocalSettingsFileversion が必須で、欠落ファイル(手編集・破損・将来形式)でファイル全体の load が失敗していた。他の on-disk 型が保つ前方互換と矛盾するため #[serde(default)] を付与。書き出す形式は不変。

4. clone URL のトランスポートを allowlist 化(セキュリティ・RCE 防止)

git clone "ext::sh -c <cmd>" は ext リモートヘルパー経由で任意コマンドを実行する。RepoUrl::parse がスキームを検証しておらず ext::... が素通りしていた。usagi init <url> はスクリプト・エージェントから到達しうる入口のため修正。

  • domain: RepoUrl::parse にトランスポート allowlist を追加。https/http/ssh/git スキームと scp 形式(git@host:path)・ローカルパスのみ許可。remote helper(ext::/fd::)と非許可スキーム(file://)は UnsupportedTransport で拒否。ext::…://… のスキーム偽装も弾く。
  • infrastructure(多層防御): git cloneGIT_ALLOW_PROTOCOL=https:http:ssh:git:file を設定。RepoUrl を経由しない将来の呼び出しでも git 自身が ext 等を拒否(file はローカルクローン用に維持)。
  • ドキュメント: 03-commands/01-cli.md に許可トランスポートを明記。

テスト・確認方法

  • cargo fmt / cargo clippy --all-targets -- -D warnings / cargo test すべて通過(1686 + 統合テスト)。
  • 各変更に回帰テストを追加しカバレッジ 100% を維持。

フォローアップ候補(本 PR 対象外)

  • git_capture の失敗潰し(P3): git の一時失敗(index.lock 競合等)が「クリーン / main デフォルト」として永続状態に混入する問題。想定外の失敗を ErrorLog 記録しつつ degrade する分岐をカバレッジ 100% で到達させるテストが難しいため、設計を見直す別 PR とする。
  • issue_store / memory_store の構造的重複の共通化(P5): ジェネリックな MarkdownStore<E> 抽出。規模が大きいため別 PR とする。

🤖 Generated with Claude Code

kkyosuke and others added 4 commits June 25, 2026 07:54
多観点レビューで検出した不具合を修正する。

- Storage(workspaces.json) にプロセス間ロックを追加し、add/remove/touch の
  read-modify-write を直列化(更新の取りこぼし=lost update を防止)。
- session create/remove のロック順を是正。reconcile を store ロック内で実行する
  reconcile_locked に分離し、create→build→record をひとつのロックで保護して、
  並行する create がまだ未記録の worktree を stray として誤削除する競合を防ぐ。
- JSON enum の未知値が settings.json / state.json 全体の読み込みを失敗させる
  前方互換ギャップを修正。domain::serde_fallback::or_default で該当フィールドを
  既定値に縮退させる(AgentCli/Theme/SessionActionUi/Sidebar/BranchSource/
  BranchStatus)。frontmatter 側の「未知キーは無視」と挙動を揃える。
- PTY リサイズの順序を修正。parser ロック内で master.resize と set_size を
  原子的に行い、reflow 出力が旧サイズのグリッドで解釈される一過性の表示崩れを防ぐ。
- 埋め込みターミナルのマウスホイール判定を pane_cell に統一し、ペインの上下
  (タブ列・最終行より下)でも誤スクロールしていた問題を修正。
- MemoryStore::write_locked にも単一コンポーネント検証を適用し、read/remove と
  防御を対称化(パストラバーサルの多層防御)。
- TUI の tasks/sessions_refresh のロックを poison 時にクラッシュさせず復帰する
  方針(terminal_pool と同一)に統一。
- ドキュメント整合: domain の外部クレート依存(serde/chrono)を実態に合わせて
  規約を明確化、モジュール表に agent_feature.rs / serde_fallback.rs / cli feature を
  追記、state.json/settings.json の前方互換・workspaces.json のロックを記載。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
コードレビューで見つかった堅牢性の指摘のうち、安全かつテスト可能な3点を修正。

- git::primary_worktree: `git worktree list` が成功しつつ worktree 行を
  返さない場合に `.expect()` で panic していた。状態同期のホットパスで
  プロセスを落とすため、エラーを返すよう変更(純粋ヘルパー primary_of に分離)。
- UpdateHandle / InstallHandle の lock(): poison 時に `.expect()` で panic し、
  raw mode のまま TUI を巻き込んでクラッシュしていた。兄弟ハンドル
  (SessionsRefreshHandle 等)と同じく poison を回復するよう統一。
- on-disk 構造体(WorkspacesFile / SettingsFile / StateFile /
  LocalSettingsFile)の version に #[serde(default)] を付与。version 欠落
  ファイルでファイル全体の load が失敗する前方互換の穴を塞ぐ。

各変更に回帰テストを追加(カバレッジ維持)。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`git clone "ext::sh -c <cmd>"` は ext リモートヘルパー経由で任意コマンドを
実行する(RCE)。`RepoUrl::parse` がスキームを検証していなかったため、
`:` か `/` を含む文字列(`ext::...` も該当)が素通りしていた。
`usagi init <url>` はスクリプト・エージェントから到達しうる入口のため修正する。

- domain: RepoUrl::parse にトランスポート allowlist を追加。許可は
  https / http / ssh / git スキームと scp 形式(git@host:path)・ローカルパスのみ。
  remote helper(ext:: / fd:: 等)と非許可スキーム(file:// 等)は
  UnsupportedTransport で拒否。`ext::...://...` のスキーム偽装も弾く。
- infrastructure: 多層防御として git clone に
  GIT_ALLOW_PROTOCOL=https:http:ssh:git:file を設定。RepoUrl を経由しない
  将来の呼び出しでも git 自身が ext 等を拒否する(file はローカルクローン用に維持)。
- ドキュメント(03-commands/01-cli.md)に許可トランスポートを明記。

allowlist の各分岐と clone の git レベル拒否に回帰テストを追加。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kkyosuke kkyosuke changed the title fix: コードレビュー指摘の修正(panic 回避・poison 回復・前方互換) fix: コードレビュー指摘の修正(堅牢性・前方互換・セキュリティ) Jun 24, 2026
# Conflicts:
#	src/infrastructure/storage.rs
#	src/infrastructure/workspace_store.rs
@github-actions

Copy link
Copy Markdown

📊 Test Coverage

🚀 いまのカバレッジ (Lines): 100.00% — この調子でいこう!

🎉✨ パーフェクト!全ファイル Lines カバレッジ 100% を達成しました 🏆🐰

@kkyosuke kkyosuke merged commit cf9b67f into main Jun 25, 2026
4 checks passed
@kkyosuke kkyosuke deleted the review branch June 25, 2026 00:06
@kkyosuke kkyosuke mentioned this pull request Jun 25, 2026
kkyosuke added a commit that referenced this pull request Jun 25, 2026
## 目的

`1.2.0` → `1.2.1` のリリース。`Cargo.toml` の version を上げ、マージ時に
`auto-release.yml` がタグ `v1.2.1` とリリースを自動生成する。

## 変更内容

- `Cargo.toml` / `Cargo.lock` の version を `1.2.0` → `1.2.1` に更新。

### このリリースに含まれる変更(v1.2.0 以降)

- fix: コードレビュー指摘の修正(堅牢性・前方互換・セキュリティ) (#333)
- feat(tui): 在席で agent を指定して起動できるようにする (#334)
- refactor(session): 表示ラベル解決を presentation へ寄せ、new の Mode キーを統合する (#332)
- refactor(infra,usecase): version envelope と設定セッターのボイラープレートを削減する (#330)
- refactor(mcp): issue/memory の filter Args を flatten で一本化する (#329)
- refactor(agent): program 名を AgentCli::command に一本化する (#328)
- feat(tui): コマンド引数(サブコマンド・オプション)も Tab 補完する (#331)
- refactor(tui): widgets から rabbit アセットを分離する (#323)
- fix: コードレビュー指摘の修正(並行性・前方互換・堅牢性) (#327)
- feat(error-log): 静かに握りつぶされる本物の失敗をログに記録 (#325)
- fix: アップデート告知と更新後メッセージの文言を変更 (#324)

## テスト・確認方法

- version のみの変更。`release-build-check.yml` が 4 プラットフォームのリリースビルドを検証する。
- マージ後、`auto-release.yml` → `release.yml` でタグ `v1.2.1` と GitHub Release
が自動生成される。

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant