Skip to content

#673 [tests][mutants] 変異試験が防御を外したまま kill(-1) などを実行し、利用者の全プロセスを止める - #678

Merged
Kewton merged 6 commits into
developfrom
feature/issue-673-tests-mutants-kill-1
Oct 14, 2026
Merged

Kewton merged 6 commits into
developfrom
feature/issue-673-tests-mutants-kill-1

Conversation

@Kewton

@Kewton Kewton commented Oct 14, 2026

Copy link
Copy Markdown
Owner

Refs #673

概要

2026-10-14 に、macOS で cargo mutants が process group の防御(signalable_process_group -> Some(1))を変異させ、変異した試験の binary が kill(-1, SIGTERM) を実行しました。その結果、利用者のプロセスがすべて止まりました。この PR は、その再発を防ぎます。

変更の要約

  • .cargo/mutants.toml(新規):通常の設定で、signalable_process_group と group_is_signalable を exclude_re で除外します。
  • signal の最後の防御:signal を送る関数の中で、kill の直前に、変異の対象にならない assert! を直接書きました。helper には包んでいません。
    • 対象:src/bounded_process.rs、src/bin/gui_server/trial_process.rs、src/planner/runner/acceptance.rs、src/preflight.rs。
    • preflight は、assert! の前で pid <= 1 と自分の pid を bail! で断ります。
  • 再帰削除:dir_fd::strict_descendant_real_path(root, target) -> Option<PathBuf> を足しました。削除先が root の真の子孫かを確かめ、確かめた実パスだけを remove_dir_all に渡します。
    • 対象:session_paths、model_probe、pack::supply、os_sandbox。
  • 危険な命令の一覧:docs/dev/dangerous-commands.md(新規)を足しました。
    • tests/issue673_dangerous_command_inventory.rs(新規)は、src・tests・build.rs・scripts の signal と再帰削除を grep で拾います。一覧と食い違えば失敗します。
  • harness 文書 docs/dev/parallel-dev-harness.md に §3a(mutants の規則)を追加しました。
    • signal・削除・子の session に触る変異は、防御の有無にかかわらず常に隔離環境で流します。
    • Docker の条件:uid 1000、--network none、mount・--privileged・--pid=host・Docker socket なし、木は build の context か docker cp で渡す。
    • --timeout は残ったプロセスを回収しないので、run の後はコンテナごと捨てます(docker rm -f)。
    • 流す前に cargo mutants --list を全件照合します。
  • tests/issue502_os_sandbox_write.rs:pager の試験の HOME と Git の設定を、利用者のものから隔離しました。

利用者に見える挙動の変化

  • preflight:lsof がポートの持ち主として pid 0・1・自分の pid を返した場合、利用者が kill を選んでも signal は送りません。refusing to signal … の Err で返ります。以前は kill を呼んでいました(pid 1 では EPERM などの Err)。
    • 例:launchd や systemd の socket activation で pid 1 が返る場合。
  • 変異のない木では、危険な値は assert! に届く前に Err・bail!・false で返ります。panic はしません。
  • harness 文書 docs/dev/parallel-dev-harness.md に §3a を追加しています(開発の手順の変更です)。

検証

  • 15 ゲート:commandmate verify passed 15/15。HEAD 1f082ada で、最後の commit の後、clean です。
  • cargo deny:ok。
  • Linux の確認:cargo-zigbuild clippy --all-targets --target x86_64-unknown-linux-gnu exit 0。
  • Cargo.lock:差なし。
  • Docker の安全確認(変異なし。uid 1000、--network none、mount なし):
    • 危険な値の試験(dangerous_value_*)、番兵の試験、gui の試験はすべて pass しました。
    • この image(Node 18、network なし、gui/out の build なし)でだけ失敗する 8 本は skip しました。base の 739c441c でも同じ 8 本が失敗します。host の verify では pass しています。
    • 試験の外に置いた canary(同じ UID、別の session)は生き残りました。
  • 二点測定(Docker):新しい dangerous_value_ の試験は、修正前のコードで FAILED(assert! で panic、signal の送信は 0 件)、修正後で pass です。
  • mutants は Docker の中だけで流しました。全組で strace を取り、kill(-1|0|1, …) は 0 件、canary は生存しました。
run 組 本数 caught missed timeout unviable
L-56 A1 signalable_process_group(除外なしの確認用の設定) 7 4 0 3 0
L-56 A2 group_is_signalable(除外なしの確認用の設定) 5 5 0 0 0
L-56 A3 configure_child_session 3 2 1 0 0
L-56 B 変えた行 33 10 14 2 7
L-57 (a) preflight の変えた行 3 3 0 0 0
L-57 (b) B の missed の測り直し(integration の試験を追加) 7 3 3 1 0
  • 補足:
    • A1 の timeout 3 本(Some(0)・Some(1)・Some(-1))は、kill の手前の assert! で panic しました。
    • A3 の missed(delete -)は、setsid() == 1 が真にならないので等価です。
    • missed は、どれも signal を送らない、削除しない、または待ちが変わる向きです。宛先や削除先は広がりません。

レビュー

  • H-35(Claude_Sub):blocker B1。harness §3a が「until the last, non-mutatable defense is in place」で隔離を限っていた点です。「常に隔離環境で流す」に直しました。
    • あわせて、§5 の文書の誤りと、preflight の Err から panic への変化(F6)も直しました。
  • H-36(fix-only の再確認):blocker 0。

後続の論点(この PR では直していません)

  • F1:os_sandbox::install_for_test の root が、削除先と独立に決まっていません(試験だけで使う経路)。
  • F3 の残り:一覧の照合の単位(関数名と件数)と、tests/**/*.sh の読み込み。
  • F4:確かめてから削除するまでの間に、祖先のパスを symlink へ差し替える競合(TOCTOU)。
  • session_paths::rollback_unstarted の NotFound の判定の missed 3 本:試験を強化します。
  • macOS の dedicated account で mutants を流す場合の、片付けの方法。

🤖 Generated with Claude Code

Kewton and others added 6 commits October 15, 2026 01:49
Add `.cargo/mutants.toml` excluding `signalable_process_group` and
`group_is_signalable`, add last, non-mutatable `assert!` defenses in the
functions that call `kill`, guard the product `remove_dir_all` targets with an
independent real-path strict-descendant check, add the dangerous-command
inventory test and doc, and document the mutants rules in the dev harness.

判断: 子の session を変える変異の回収の仕組みは入れず、leader の隔離環境での確認に委ねた
読み替え: `is_strict_descendant` は最終成分が symlink の場合も拒否する(内側を指す symlink を含む)
本文に無い指摘: pager 試験の `git` は利用者の HOME を継承していた。bounded child の HOME を隔離 HOME に上書きした

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Make the recursive-delete guard return the checked real path
(`strict_descendant_real_path -> Option<PathBuf>`) and have each caller delete
only that returned value, so the checked value is the value removed. Widen the
dangerous-command inventory to read tracked `scripts/**/*.sh`, `scripts/**/*.py`,
and `tests/**/*.py` for external `kill`/`pkill`/`killall`, `rm -rf`/`rm -r `,
`os.kill(`/`os.killpg(`, and `shutil.rmtree(`.

判断: F2(install_for_test の run_tmp_root)は変更していない。独立の root にすると src/lib.rs の sigint_finalizer 試験が失敗し、その修正も呼び出し側の署名変更も契約外のファイルを要するため

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
State in the recursive-delete section that the independent-root guarantee
covers the production `install` path, and that the test-only `install_for_test`
seam derives its root from the delete target, so making it independent needs
files outside this Issue's scope.

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Add `dangerous_value_terminate_process_group_by_pid_refuses_init_and_caller_group`
(bounded_process) and
`dangerous_value_send_group_signal_refuses_init_caller_and_negative_groups`
(trial_process). Both skip the selector and pass 0/1/-1 to the `kill` wrappers, so
a mutated selector reaches the non-mutatable `assert!` in front of `kill` instead
of failing on the selector's own assertion (Issue #673 対処の方向 4). Document the
two tests and their Docker commands.

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
… Err

Harness §3a: isolate signal/delete/child-session mutations unconditionally, spell
out the Docker conditions (non-root uid 1000, --network none, no mounts, no
privileged/pid=host/socket), and require a disposable container because
cargo-mutants' --timeout does not reap a TERM-ignoring child.

dangerous-commands §5: correct the scope (no host run only under mutation;
unmutated CI/verify runs them and returns before the syscall; a new
dangerous_value_ test is confirmed once in the container first) and add the new
preflight test.

preflight.rs: SystemPortControl::kill returns Err via bail! for pid 0/1 and the
caller's own pid before the last-defense asserts (which stay). Add the
container-only dangerous_value_system_port_control_kill_refuses_init_zero_and_own_pid.
The comment said the test runs only in the isolated container; it now matches
docs/dev/dangerous-commands.md §5: under a mutation it runs only in the isolated
container, and on an unmutated tree it also runs in host CI and verify.
@Kewton
Kewton merged commit 3d4144d into develop Oct 14, 2026
5 checks passed
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