[Nic-dorman]

Reviews 7 and 8 (on 5263208): accepted as known risks, no code change. Review 7 found that the Linux rule letting a directory above the data directory be group-writable, when the group is the user's own primary group and there's no ACL, doesn't prove that no other account shares the group. Review 8 found that macOS ACLs aren't read. Both were already listed under "Not covered". On 2026-11-04 Nic accepted everything in that section as known risks (see the "Accepted" paragraph there). This code runs only for local devnets (ant-devnet, and code that embeds Devnet, such as ant-core's devnet), never for ant-node, and what it protects is throwaway test data on a developer's machine. Refusing every group-writable directory would stop the default devnet starting on stock Ubuntu, whose umask (0002) makes ~/.local/share group-writable, until the developer changed its permissions.

Round 7 (6f59853, then a merge of main, 5263208), for review: 9423105..6f59853. Hermes round 6 found that resolving the parent's links once doesn't keep RunDir::path() still: another user who can write to a directory above the run directory can swap it for a link to another tree, and the nodes' writes follow. It also found that the sweep checked a run directory's name only before removing the empty directory at the end, so one moved after it was found finished was still emptied where it went. Both are fixed and tested, and undoing any part of either fix makes its test fail. On Unix a run now refuses to start in a parent that another user could change, or below one (see "Parent check" below). That also means no one but the user or root can move a run directory while the sweep deletes it. Deletion now checks the name before removing anything, and the sweep leaves run directories other users own. On Windows the held handles already kept every directory above the run directory from being renamed; a test now shows it. The new other-user cases run through sudo in the Linux CI step that runs the mount tests. 5263208 merges main, which had moved on, its CI consolidation (V2-1811) among other changes. The only conflict was ci.yml. There, the lifecycle test now runs in main's loop over every integration target, skipped on Windows, where it compiles to nothing. The sudo tests keep a Linux step of their own in the test job. On scope: the group rule trusts the members of the user's own primary group, and macOS ACLs aren't read (see "Not covered"). The Security Audit failure is inherited (RUSTSEC-2026-0387, V2-1743) and isn't a required check: only linear-link and pr-template are. One correction to an earlier commit for the squash message: 3e3b32d's message says the marker records the directory's device; it records only its file number, and the device is compared with the parent's at the time.

ant-devnet now stops cleanly on SIGTERM, and by default each run gets a fresh data directory. Fixes #261 and the root cause of WithAutonomi/ant-sdk#73. It also fixes something worse that turned up on the way: a graceful stop of a default devnet deleted the whole ant data root.

SIGTERM

ant-devnet waited only for Ctrl+C. A SIGTERM kept its default action and killed the process outright. That means:

• the devnet was never shut down;
• Anvil was orphaned, still holding its port;
• the data directory was left behind.

SIGTERM comes from kill, from service managers, from ant dev stop, and from ant dev start's own wait timeout. Interrupted runs piled up until ant dev start hung for good (#73).

What changes:

• Handlers before anything starts: SIGINT, SIGTERM and SIGHUP (Ctrl+C and Ctrl+Break on Windows), in stop_requested().
• Stop during EVM setup: the setup is dropped, and the Anvil it started goes with it.
• Stop during startup: the start is cancelled, not dropped, through the devnet's own token (new Devnet::cancellation_token()).
  • start returns at its next safe point: between node spawns, or in its readiness and stabilization waits.
  • By then every node it started is registered, so shutdown stops them all before it removes the directory.
  • Dropping the start future part-way, as I first did, left an unregistered node writing into the directory being removed. That leaked the directory in about 1 run in 18.
• Failed start: the nodes that did start are shut down too, including one whose startup failed once its P2P node was running (its WebRTC Direct port taken, say): a node is registered as soon as its P2P node runs.
• Order on stop: Anvil is stopped after the nodes, which verify payments against it.

The data root

• DevnetConfig::default().data_dir was default_root_dir(): the whole ant data root, ~/.local/share/ant on Linux.
• cleanup_data_dir defaults to true, so every graceful stop ran remove_dir_all on it.
• On Linux and macOS that is the same directory as ant-core's config::data_dir(). Ctrl+C on a default devnet therefore deleted:
  • the client's payment caches, which are what resuming a paid upload relies on;
  • its manifest upload history;
  • its peer cache;
  • any real node's data under nodes/.

Run directories

A devnet deletes only a directory it created and marked as its own (new src/devnet/run_dir.rs):

• Exclusive creation: each run creates a fresh {pid}-{random} directory inside DevnetConfig::data_dir / --data-dir, with mkdir (not create_dir_all), so it never adopts something that was already there. It claims what it then opens only if that is empty and, on Unix, owned by the run's own user (geteuid), so a directory another user puts in place of the new one in between is left as it is; a claim that fails removes only the files it created, and then the directory only if it is empty. The default parent is {data root}/devnets (new DEVNETS_SUBDIR).
• --data-dir is now the parent. It, and anything else in it, is never deleted. Before, --data-dir ~/Documents plus Ctrl+C ran remove_dir_all(~/Documents).
• Lock, then marker: the run takes an exclusive lock on .ant-devnet-run.lock for as long as it lasts, then writes .ant-devnet-run.json (run id, the file numbers of the directory and of the lock file, pid, start time, version, keep), atomically via rename. Both names are exported as LOCK_FILE and MARKER_FILE.
• Shutdown deletes the run directory through the handle it opened when it created it. It first checks that the name still refers to that directory and that the marker still names this run; if not (moved, replaced, re-marked), it deletes nothing.
• Sweep: a new run first removes run directories that earlier runs left behind (SIGKILL, crash). It only touches a directory that, on Unix, belongs to the user, whose marker names that very directory (its inode, or file index on Windows, so a copied marker doesn't count), that is on the parent's filesystem and, on Unix, its mount (so not a bind mount of a run directory kept elsewhere), that isn't kept (--no-cleanup sets keep), and whose lock is free, taken on the very lock file the marker names: a run's lock stays held under any name its file is moved to, so a fresh file put in its place doesn't count. The OS releases a lock however its process ends, and only a run's own process ever takes it, so a free lock means the run is over; a reused PID can't fool it. Anything without such a marker is never touched. The marker keeps no device number, since btrfs and NFS can change it on remount, which would strand leftovers after a reboot. Downstream tools (ant dev stop) can follow the same rule.
• Deletion order: first a check that the name still refers to the directory it has open (for the sweep as for shutdown), then the data, then the marker, then the lock, then the empty directory. A deletion that stops part-way (killed, or a file that won't go) leaves the marker and a free lock, so the next sweep finishes it.
• Mounts: on Unix, deletion doesn't cross into another mount inside the run directory. On Linux it tells one by mount id (statx), which catches a bind mount of a directory on the same filesystem too. Where the kernel can't give one (before Linux 5.8, or a sandbox that refuses statx), it deletes nothing, so run directories stay there until removed by hand. macOS has no bind mounts, so there the device tells mounts apart. Deletion stops at a mount, leaving it, what's in it, and the marker. (On Windows a mounted folder is a reparse point, which open_run refuses and remove_dir_all removes without entering.)
• Shutdown waits for writers: as node.rs does on a node's shutdown, the routing loop is aborted and awaited, then the request handlers it started, and then the storage writes they started (ChunkStore::wait_idle), get 5 seconds each to finish. If anything is still writing then, or a node's P2P shutdown fails, the run directory is left, locked until the process exits, and the next run removes it.
• Dropped without shutdown: a Devnet dropped without shutdown() leaves its nodes running (their tasks hold them), so it now keeps the run directory locked until the process exits; no sweep deletes it under them, and the next run after that removes it, as after a SIGKILL.
• Unix: every step after the parent is opened goes through a handle on the parent or the run directory (mkdirat/openat/unlinkat), and deletion walks the tree through O_NOFOLLOW handles. Retargeting a symlink above the run directory mid-run can't redirect it; a link inside is removed, never entered.
• Parent check (Unix): a run refuses to start (PermissionDenied, naming the directory) unless the parent and every directory above it belong to the user or root and no one else can write to them, or they're sticky, as /tmp is, which lets no one rename what they don't own. Whoever could write to one could swap a directory below it for a link, redirecting the nodes' writes, or move a run directory while it's being deleted. On Linux, a directory its group can write to passes if the group is the user's own primary group and it has no ACL (an ACL shows in the group bits only as its mask). That keeps the default parent working on Ubuntu, whose umask makes ~/.local/share group-writable under user private groups. On macOS, where every user's group is staff, it doesn't pass. The default parent and temporary directories under /tmp pass, as do macOS's under /var/folders (the macOS CI job's tests run in them).
• RunDir::path(), and with it config().data_dir and the manifest's data_dir, has the links in the parent's path resolved, so retargeting one mid-run doesn't redirect what the nodes write either. On Windows it's given without the \\?\ prefix where the path means the same without it. On macOS this shows /var/... as /private/var/....
• Windows has no *at calls in the standard library. There, links in the parent's path are resolved once (canonicalize, re-checked once the parent is held), and the parent and the run directory are held open without FILE_SHARE_DELETE. While they are, Windows refuses to rename or remove them or any directory above them, so the paths built on them keep naming the same directories; std::fs::remove_dir_all doesn't follow links inside the tree. Identity is the volume serial number and file index, through winapi-util.
• Devnet::config().data_dir still names the run directory after new(), so the manifest's data_dir and ant-core's LocalDevnet see what they did before.

Not covered

Who the parent check trusts: root, and the user's own processes. On Linux it also trusts the members of the user's primary group, for a directory that group can write to: that's just the user under user private groups, but not where users share a primary group. It reads owners, permission bits and, on Linux, ACLs. It doesn't read macOS ACLs, which no call reaches without unsafe code. Only the user or root can move a run directory while the sweep is deleting it, after its name has been checked.

Accepted (Nic, 2026-11-04, after reviews 7 and 8): every limitation in this section is a known, accepted risk. This code runs only for local devnets, never for ant-node, and it protects throwaway test data on a developer's machine. None of these limitations lets another account do anything on a normal single-user dev machine; each needs an unusual multi-user setup, or a process already running as the developer.

• Shared primary group (Linux): needs a second account in the developer's primary group. The rule stays so that the default data directory works on stock Ubuntu with no setup.
• macOS ACLs: needs an ACL, on a directory above the data directory, that lets another user add or remove its entries. Reading ACLs would take unsafe code or a new macOS-only dependency.
• A process running as the devnet's own user: no check can stop it, and it could delete the user's files anyway.
• Windows: there's no parent check, so the parent's permissions decide who else could slip in a directory of their own, as in the item above. The default parent, under the user's %APPDATA%, is private to the user.
• SIGKILL: the run directory stays until the next run sweeps it, and Anvil is left running. The fix belongs in evmlib.

A process running as the devnet's own user can put an empty directory of its own in place of a new run directory, between its creation and its opening; the run then claims it, and later deletes what that process put in it. No check can stop that, since Unix has no call that creates a directory and opens it in one step, and such a process could as well delete whatever the run keeps. Another user can't, on Unix, since the run refuses a directory it doesn't own. On Windows, which has no owner check without unsafe code, the parent's permissions decide who could.

If ant-devnet is SIGKILLed, its run directory stays until the next run sweeps it, and Anvil is still orphaned, since no handler can run. The fix would be PR_SET_PDEATHSIG on the Anvil child, but Anvil is spawned inside evmlib/alloy, so that's a follow-up there.

Linear issue

Closes V2-1777

Also closes #261 (the GitHub issue). Root cause of WithAutonomi/ant-sdk#73.

Risk tier

• T0 — docs / tooling / CI / pure UX-output. Repo CI only.
• T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
• T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
• T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Proposed T0: only the devnet tooling (ant-devnet, ant_node::devnet) changes. There's no change to node behavior, the wire protocol, storage format, payments or upgrades.

Compatibility

• Wire: none.
• Storage: none for nodes. A devnet's data now lives in {parent}/{pid}-{random}, parent {data root}/devnets by default. Tools that looked for devnet nodes under {data root}/nodes or {--data-dir}/nodes should read data_dir from the manifest.
• CLI: --data-dir now names the directory the run directory is created in. On Unix it must be one that only the user and root can change, and so must every directory above it: ant-devnet refuses to start otherwise, where before it started in, say, a world-writable shared directory. The only caller I found, ant-client's browser-tests/start-devnet.mjs, passes a fresh mkdtemp directory and removes it itself, so it is unaffected; ant dev start and ant-core use the default.
• API: additive: Devnet::cancellation_token(), DEVNETS_SUBDIR, LOCK_FILE, MARKER_FILE. DevnetConfig::data_dir is now the parent (default {data root}/devnets); after Devnet::new, config().data_dir is the run directory, the way base_port: 0 is resolved there too. A Devnet dropped without shutdown() keeps its run directory, and its lock, until the process exits (before, dropping deleted nothing either).

Semver impact

• breaking
• feature
• fix

Proposed feature; flip to breaking if you count --data-dir's new meaning (no known caller is affected).

Test evidence

Round 7 (Linux dev box, Ubuntu 24.04, umask 0002 with user private groups; and Windows 11 for the Windows code):

• clippy on rustc 1.99.1: cargo +stable clippy --all-targets --all-features -- -D warnings clean. cargo fmt --check and cargo doc with -D warnings clean.
• Unit tests: cargo test --lib --features test-utils devnet:: passes, 31 tests plus 6 ignored, which need sudo. New:
  • a parent whose parent directory, or which itself, anyone can write to (0777) is refused and gets nothing in it; on macOS, also one below a group-writable directory;
  • a parent in or below a sticky directory anyone can write to (1777) starts; on Linux, so does one below a directory the user's own group can write to (0775);
  • Linux: a parent below a directory with an ACL granting another user write (written as the kernel's system.posix_acl_access value) is refused;
  • a finished run moved out of its name once the sweep has found it (through a test hook between the two) keeps its data and marker where it went. On Windows the move fails, since the sweep holds it, and the run is swept.
• Through sudo (the CI step "ant-devnet run directories against real mounts and other users", Linux, now 6 tests):
  • as nobody: in a root-owned 0777 directory, nobody can move a directory the user made, and a run refuses to start below it. In a root-owned 1777 one, a run starts, nobody can't move the directory it's below, and a write through RunDir::path() lands in the run directory;
  • a parent below a directory nobody owns, or below one group 65534 can write to, is refused;
  • a finished run's directory that nobody owns, and that the user could empty (a+rwX), isn't swept.
• Windows: run_dir.rs on its own on Windows 11: all 16 tests pass, clippy clean. The run directory, the parent, the directory above that, and the temporary directory above all of them can't be renamed while the run lasts.
• Mutation-checked: each of these makes the matching tests fail:
  • no parent check (both refusal unit tests and both refusal sudo tests fail);
  • ignoring that another user owns a directory above;
  • not exempting sticky directories (every test under /tmp fails);
  • letting world-writable directories pass;
  • no exception for the user's own group;
  • ignoring ACLs;
  • letting any group pass;
  • the sweep taking another user's run;
  • deleting without checking the name first. The moved-run test fails, and so does the shutdown-side replacement test, since remove now relies on that check too.
• Lifecycle: tests/devnet_lifecycle.rs passes (3 tests), now with --features test-utils, as main's loop runs it.
• CI (run 41815472929, on the merge): every job passes but the inherited Security Audit, CI Gate included. On macOS every devnet test runs under /var/folders and passes. On Windows the loop skips devnet_lifecycle, and the sudo step on Linux reports 6 passed.

Round 6:

• clippy on rustc 1.99.1: cargo +stable clippy --all-targets --all-features -- -D warnings clean. cargo fmt --check clean.
• Unit tests: cargo test --lib --features test-utils devnet:: passes, 27 tests plus 3 ignored, which need sudo:
  • real mounts (new CI step "ant-devnet run directories against real mounts", Linux, cargo test ... with_sudo_ -- --ignored, with the same ran-no-tests guard as the other harness steps): a tmpfs, and a bind mount of a directory on the same filesystem, inside a run directory, are not crossed, and what they show survives; a bind mount of a finished run's directory, kept elsewhere, put in the parent under its name, matches in everything but its mount and is not swept;
  • another user's directory: an empty directory root owns, open to anyone (0777, so the run could write in it), put in place of a new run directory before it is opened, is not claimed.
• The retargeting tests (symlink on Unix, junction on Windows) now also write through RunDir::path() after the retarget, and check that the file lands in the original directory, not the decoy. The junction test's paths carry &, %...%, ^, (, ), !, ; and =.
• Windows: run_dir.rs on its own on Windows 11: all 15 tests pass, clippy clean.
• Mutation-checked: each of these makes the matching tests fail: the sweep ignoring the candidate's own mount; mounts told apart by device only (both mount tests fail); no owner check at creation; Unix path() keeping the caller's links; Windows path() keeping the junction; and the junction helper's old cmd /C call (the metacharacter paths break it).
  • Not tested: deleting nothing where Linux gives no mount id, since every kernel here gives one.
  • Lifecycle guard: a probe test that SIGKILLs ant-devnet (Anvil orphaned and still listening) and then fails leaves no Anvil running. It also shows that the group still exists once ant-devnet is reaped, held by its leader. Without the group kill, Anvil survives. The id-reuse race itself can't be staged. tests/devnet_lifecycle.rs passes (3 tests).

Round 5:

• clippy on rustc 1.99.1 (CI's stable): cargo +stable clippy --all-targets --all-features -- -D warnings clean. cargo fmt --check and cargo doc with -D warnings clean.
• Unit tests: cargo test --lib devnet:: passes, 27 tests. New:
  • a live run whose lock file is moved aside and replaced with a free one is not swept (Hermes's case); a marker naming another lock file is not swept;
  • a directory put in place of a new run directory before the run opens it (through a test hook between the two steps) is neither claimed nor emptied, with or without a lock file in it;
  • deletion stops at a directory inside the run that counts as a mount point (through a test hook), leaving it and the marker; and the check itself tells a directory on another mount (/proc on Linux, /dev on macOS) from one on the temp directory's;
  • a chunk PUT sent from one devnet node to another and held inside the store (ChunkStore::test_put_gate) at shutdown keeps the run directory, locked, so the next devnet leaves it too;
  • a devnet whose first node's WebRTC Direct port is taken fails to start, and shutdown still stops that node's P2P node and removes the run directory.
• Real mounts (dev box, with sudo; not in CI, which can't mount): a tmpfs mount and a bind mount of a directory on the same btrfs filesystem, each inside a run directory: removal fails and leaves what is in both. With the check removed, the tmpfs is emptied. With the check comparing devices only, the tmpfs is left but the bind mount is emptied, deleting the file in the directory it shows: that is what the mount id is for.
• Windows: run_dir.rs built and tested on its own on Windows 11: all 15 tests pass, clippy clean.
• Mutation-checked, on Linux and, for the run-directory changes, on Windows too. Each of these makes the matching tests fail: the sweep ignoring the lock file's identity; creation claiming a directory that isn't empty; round 4's creation (no emptiness check, recursive cleanup); deletion crossing mounts; shutdown removing the directory though a node didn't stop; shutdown not waiting at all (neither the handlers nor the storage writes); and a node registered only once fully started. Waiting for either the handlers or the storage writes alone keeps the held PUT's directory, so the PUT test can't tell those two apart. Not tested: keeping the directory when a node's P2P shutdown fails, since nothing in a test makes saorsa-core's shutdown fail; it takes the same path as the held PUT.
• Lifecycle: tests/devnet_lifecycle.rs passes (3 tests).

Round 4:

• clippy on rustc 1.99.1 (CI's stable): cargo +stable clippy --all-targets --all-features -- -D warnings clean. On 1.95, nothing in the changed files (the same 6 existing lints elsewhere). cargo fmt --check and cargo doc with -D warnings clean.
• Unit tests: cargo test --lib devnet:: passes, 21 tests. New:
  • a replacement for a moved run directory, carrying copies of its marker and lock, is refused and nothing is deleted (Unix; Hermes's case);
  • the sweep ignores a copy of a finished run's marker in another directory;
  • a removal stopped part-way by an injected failure keeps the marker and lock, and the next run finishes it;
  • a run kept locked is not swept; a started devnet dropped without shutdown() is not swept by the next one;
  • the negative sweep cases now each use a marker that is valid but for its one difference, with a control that is swept.
• Windows: run_dir.rs built and tested on its own on Windows 11 (rustc 1.97, same clippy lints as the crate): all 13 tests pass, clippy clean. Windows-only tests: the run directory and the parent can't be moved while the run lasts, and retargeting a junction above the run mid-run doesn't redirect removal, even onto a decoy carrying the run's marker.
• Mutation-checked: each of these makes exactly the matching tests fail: removal skipping the same-directory check; the sweep ignoring the file number; deleting the marker before the data; releasing the lock instead of keeping it; Devnet::drop releasing it; and on Windows, not resolving the parent's links.
• Lifecycle guard: a probe test that SIGKILLs ant-devnet (Anvil orphaned and still listening) and then fails leaves no Anvil running. With the process-group kill removed, the orphaned Anvil survives. tests/devnet_lifecycle.rs passes (3 tests).

Rounds 1–3:
Run on a Linux dev box (Ubuntu 24.04, rustc 1.95):

• fmt and doc: cargo fmt --all -- --check and cargo doc --all-features --no-deps both clean.
• clippy: cargo clippy --all-targets --all-features raises nothing in the changed files. That box's 1.95 toolchain flags 6 existing lints in src/replication/pointer.rs and src/storage/chunk_store.rs, which this PR doesn't touch; CI's toolchain passes them on main.
• Unit tests: cargo test --lib devnet:: passes, 16 tests, among them:
  • removal deletes the run directory and nothing beside it, and refuses a directory whose marker names another run;
  • the next run sweeps a finished run, but never a live one (same process, other handle), a kept one, or any directory without a matching marker (no marker, wrong name shape, marker naming another pid, corrupt or oversized marker, no lock);
  • Unix: a parent behind a dangling symlink fails closed and creates nothing (Hermes's round 2 case); a link named like a finished run is removed, never followed; retargeting a symlink above the run mid-run doesn't redirect removal, even onto a decoy carrying the run's own marker (Hermes's round 2 case);
  • two devnets in one parent each remove only their own directory; --no-cleanup directories survive later sweeps.
  • Mutation-checked: path-based removal, following links when opening a run, ignoring the lock, ignoring keep, and skipping the marker check on removal each make exactly the matching test fail.
  • The Windows (path-based) implementation was also wired in on Linux temporarily: it builds warning-free and passes every test except the Unix-only symlink-retarget one, as documented above.
• New integration test, tests/devnet_lifecycle.rs: unix only, and a new CI step with the same ran-no-tests guard as the other harness steps. It runs the real binary with HOME and XDG_DATA_HOME in a temp dir:
  • SIGTERM while running (minimal preset + Anvil): exit 0, data directory removed, Anvil's port freed;
  • SIGTERM during startup (small preset, 1 s spawn delay, --data-dir holding a sentinel): exit 0, run directory removed, the sentinel and --data-dir kept, no manifest written;
  • SIGKILL leaves the run directory; the next run removes it before creating its own, and leaves an unrelated sibling directory alone.
  • The Drop guard sends SIGTERM first (ant-devnet stops Anvil itself), then SIGKILL to ant-devnet's process group, which Anvil joins (round 4; it was pkill -P before SIGKILL).
  • tests/webrtc_direct_devnet.rs (all features), which sets data_dir, still passes.
• Mid-startup stress: 30 runs that send SIGTERM the moment the first node directory appears: all 30 exited 0 and left nothing behind.

New dependency

rustix (already a dependency) gains its process feature, for geteuid and getegid. No crate is added.

winapi-util 0.1, Windows only, for a directory handle's volume serial number and file index. It adds no crate to the build: it was already in Cargo.lock (through walkdir, build and dev only), and what it uses, windows-sys 0.48, already ships in the Windows build through directories. Cargo.lock changes by one line (ant-node's own dependency list).

ADR

n/a

Mitigation / rollback

Revert. No data migrates: run directories are throwaway, and the next run (or deleting devnets/ by hand) clears any that are left.

🤖 Generated with Claude Code

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes requested (1 confirmed blocker)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Coverage: 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). The seats disagreed: sol-1 found changes required; glm-1 approved but also noted the lexical-path guard risk. I independently verified the deletion path described below, so this blocks approval.

Changes required

• src/devnet.rs:386 — contains_data_root compares raw Path components. A --data-dir such as <ant-data-root>/.. is not recognized as an ancestor by this lexical check, but remove_dir_all resolves it to the parent and deletes the ant data root and other contents there. I reproduced the same remove_dir_all behavior in an isolated temporary directory. Canonicalize/normalize the target and protected root before deletion, fail closed if safety cannot be established, and add tests for .., relative paths, and symlink aliases.

CI and verification

• CI passes for tests, builds, clippy, format, docs, and storage checks. Security Audit fails on crypto-bigint 0.5.5 / RUSTSEC-2026-0387; Cargo.lock is byte-identical at base and head, so this is inherited.
• The local cargo test --locked --test devnet_lifecycle -- --test-threads=1 run exceeded the tool timeout; I could not verify a local run.

No merge action taken.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes requested (2 confirmed data-loss paths; 1 test-cleanup issue)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Coverage: 2/2 seats reviewed (sol-1 gpt-6.1-sol xhigh; glm-1 z-ai/glm-5.3). They disagreed: sol-1 found changes required; glm-1 approved. I reproduced both filesystem bypasses below in isolated temporary directories, so approval is blocked.

Changes required

• src/devnet.rs:1302-1310 — resolve treats every NotFound as a missing path component. A dangling symlink also produces NotFound, so the code backs up past it and appends the symlink name lexically. In a probe, a protected path /tmp/alias/ant with /tmp/alias -> /tmp/victim/not-yet resolved lexically; covers(/tmp/victim, /tmp/alias/ant) returned false, and the cleanup removed an unrelated sentinel under /tmp/victim. Detect and resolve dangling symlinks, or fail closed when an existing component cannot be resolved; add a regression test that asserts cleanup preserves the ancestor contents.
• src/devnet.rs:582-589 — the guard and remove_dir_all resolve the configured path in separate operations. An intermediate symlink can be retargeted after may_remove_data_root approves the path but before the async deletion runs. I reproduced the sequence in a temp directory: check a path through a symlink to a safe directory, retarget it to the protected root, then the same remove_dir_all path deletes the protected sentinel. Make validation and deletion operate on the same directory identity, using handle-relative/no-follow removal or an equivalent race-safe method; add a regression test for ancestor-symlink replacement.

Follow-up: test cleanup

• tests/devnet_lifecycle.rs:552-557 — on a test failure, Drop SIGKILLs only ant-devnet. The running test also starts Anvil; killing the parent prevents its Testnet/AnvilInstance destructor from running, so Anvil can remain orphaned. Clean up the process group or otherwise reap descendants on failure.

No merge action taken.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes Requested 🔴 (2 blocking findings, 2 warnings)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Author: @Nic-dorman
Files changed: 6 (+1,372 / -26)
Reviewed head: 7ee4dd77d6c3ab7e5db9ae2be6c202f4250eb2e6 against 02c8682466df5c0c6723f32c46223a572f020ed4

The prior .. escape and Unix dangling-symlink protections are improved. The full current diff still has these data-safety gaps. The two scoped reviews covered the run-directory and lifecycle changes with two seats each. I confirmed the blocking paths against the current source; the seats split on whether the Devnet drop path blocks.

🔴 Critical

• src/devnet/run_dir.rs:151-165 — removal can delete a replacement directory. RunDir stores the parent and run name, but not the original run-directory handle. remove() reopens the current entry by name and accepts a plain-text marker with the same run_id. If the original directory is renamed and a replacement is created under that name with the marker copied, cleanup accepts and deletes the replacement, including unrelated files. I reproduced this sequence. Keep and verify the original directory identity through deletion; add a replacement-with-copied-marker test. The non-Unix branch has a related path-retarget race: it validates a path, drops the validated handle, then calls remove_dir_all on that path (:455-483). The path-based race is source-confirmed; I reproduced the same check-then-reopen sequence on Linux, not on a Windows host. Use identity-safe deletion there too.

• src/devnet.rs:1295-1301 and src/devnet/run_dir.rs:245-258 — dropping a started Devnet releases its run lock while node tasks can continue. Devnet::drop cancels the token and aborts the health monitor, but it does not stop the protocol task. Dropping that task's Tokio JoinHandle detaches it; the task retains the P2P node. A later Devnet::new with the same parent sees the now-free lock and can sweep the still-active run directory. This is a public library path even though ant-devnet's normal CLI path awaits shutdown. Keep the run lease until all node writers stop, or make the sweep preserve current-process runs; test dropping a started instance followed by constructing another.

⚠️ Warnings

• src/devnet/run_dir.rs:376-397 — interrupted cleanup can strand data. Cleanup removes the marker and lock along with the node data in directory enumeration order. If the process is killed or a later removal fails after those metadata entries are removed, later sweeps cannot verify or lock the partial directory and will leave it behind. Remove node contents first and delete ownership metadata last; add a fault-injection test.

• tests/devnet_lifecycle.rs:38-46 — failure cleanup can leave Anvil running. The cleanup guard returns once the parent process has exited. If the test then fails because the Anvil port is still open, the guard no longer kills Anvil. Track and clean the child independently (or use a process group), including after the parent exits.

✅ Looks Good

• The new per-run data root avoids deleting the shared ant data directory.
• Signal handling and cancel-aware startup are covered by the current CI test suite.

Verification

• Current-head CI build, tests (Linux/macOS/Windows), formatting, Clippy, MSRV, storage, WebRTC Direct, documentation, and repository checks pass.
• The separate Security Audit check fails; it is not part of this code verdict, but remains relevant to merge decisions. Cargo.lock is unchanged in this PR.
• git diff --check passes. The local targeted run-directory test did not complete: its process received SIGTERM during dependency compilation, so I do not count it as a local test result.
• The new Windows signal branch compiled in CI; the lifecycle integration test is Unix-only, so I did not verify Windows signal handling at runtime.

---

Reviewed by Hermes Agent

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes requested 🔴 (3 blocking data-safety/lifecycle findings; 1 warning)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Reviewed head: 3e3b32da2af9e5c95939314c304e7f2995c68ab5 against 02c8682466df5c0c6723f32c46223a572f020ed4
Files changed: 8 (+1,845 / -26)

Review coverage: Scope A (src/devnet/run_dir.rs): 2/2 seats reviewed (sol-1 / gpt-6.1-sol xhigh; glm-1 / z-ai/glm-5.3). They disagreed: sol-1 requested changes and glm-1 approved. I confirmed the live-lock replacement path below. Scope B: 1/2 seats reviewed; sol-1 returned a substantive review. glm-1 timed out, and its single narrowed retry ended with ConnectionResetError. No team consensus.

🔴 Critical

• src/devnet/run_dir.rs:326-345 — sweeping can remove a live run after its lock file is replaced. finished_run verifies the marker against the run-directory inode, then opens LOCK_FILE by name and accepts any free lock. It does not verify that this is the lock file held by the active run. On Unix, renaming the locked file and creating a new file at that name lets the sweeper acquire the replacement while the original lock remains held; the sweeper can then delete the active run's data. I reproduced the lock behavior with Rust File::try_lock. Record the lock-file identity and verify the opened handle before treating the run as finished; add a regression test that replaces the lock path during a live run.

• src/devnet/run_dir.rs:113-160 and 416-425 — creation can claim or delete a replacement directory. The code creates a name, then opens it in a separate operation. If another process can change the parent namespace between those calls, open_run may open a different directory. If claim then fails (for example, because that directory already has a lock file), the error path recursively removes every entry without proving ownership or holding that directory's lock. In a shared writable parent this can delete pre-existing contents. Make the creation namespace private or otherwise prove directory identity, and never recursively clean a directory after an unverified claim failure. Add a deterministic replacement test.

• src/devnet.rs:578-617 — shutdown can remove data while writers remain active. protocol_task.abort() stops the event dispatcher, but its tokio::spawn request handlers are detached and can continue through storage.put. Shutdown does not track or await them. It also logs p2p.shutdown() failures and removes the run directory anyway. A related startup path starts P2P before a fallible WebRTC listener bind, but does not add the node to self.nodes until later (src/devnet.rs:1091-1154, 1195); if that bind fails, the subsequent shutdown cannot stop that node before cleanup. Track and drain request tasks, stop partially started nodes on every error path, and retain the run lock/data unless all writers have stopped. Add delayed-PUT and occupied-WebRTC-port regression tests.

⚠️ Warning

• src/devnet/run_dir.rs:524-541 — Unix cleanup descends through nested mount points. The root run directory is checked against its parent device, but recursive deletion does not check mount identity for descendants. A mount or bind mount under a run directory can have its contents removed before removing the mount point fails. Refuse to cross mount boundaries with a platform-appropriate check, or explicitly define and test this cleanup behavior.

Verification

• git diff --check passed. The isolated Rust lock probe confirmed that a replacement lock can be acquired while the original lock remains held.
• Current-head CI passes for Linux, macOS and Windows tests/builds, Clippy, format, Rust 1.95 MSRV, docs, storage filesystems and WebRTC Direct.
• Security Audit still fails on crypto-bigint 0.5.5 / RUSTSEC-2026-0387. That version is also in the base lockfile; the lockfile change here adds the winapi-util dependency edge. This is a separate security gate, not a code-review finding.
• I did not run the Cargo test suite locally at this head.

No merge action taken.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes Requested 🔴 (3 data-safety/lifecycle findings; 2 test-harness safety findings)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Reviewed head: 58965c4e4a8f7365783c46d01a2b87c46b94f65a
Base: 02c8682466df5c0c6723f32c46223a572f020ed4
Files changed: 8 (+2,290 / -45)

Review coverage: 2/2 seats reviewed in each of 3 complete scopes (run-directory production, run-directory tests, and all other changes): sol-1 (gpt-6.1-sol, xhigh) and glm-1 (z-ai/glm-5.3). Six substantive opinions. The seats raised different findings; there was no consensus. I checked the material objections against the current source and tests.

🔴 Critical

• src/devnet/run_dir.rs:374-390, 613-655 — cleanup can still cross a same-filesystem bind mount and delete data outside the run. finished_run compares only the candidate and parent devices. For descendants, mount_of returns id: None when Linux statx is unavailable or returns EPERM; two same-device bind mounts then compare equal, so remove_entry descends into them. The candidate run directory itself is also not checked against the parent's mount: remove_all_but treats that candidate mount as its root boundary and can delete its backing directory's contents. Compare the candidate mount with the held parent before cleanup, and fail closed or use a reliable fallback when mount identity is unavailable. Add tests for a same-device nested bind mount and a candidate-root mount.

• src/devnet/run_dir.rs:139-181 — creation can claim an empty replacement directory. is_empty() does not prove that the opened directory is the one create_run made. Another process can move the new directory and place an empty directory at its name before open_run; this passes the check, receives this run's marker and lock, and is later recursively removed. The regression test at 1146-1178 covers only replacements containing a sentinel. Prove ownership with a race-safe creation scheme or a private parent, and add an empty-replacement test that inserts a sentinel after the claim and verifies it survives.

⚠️ Warnings

• src/devnet/run_dir.rs:185-198 — RunDir::path() keeps the caller's symlink/junction alias. The run's internal operations use the held directory, but path() returns parent.path.join(name). If an ancestor link is retargeted, callers such as Devnet that write through this path are redirected while cleanup remains anchored to the original directory. Store and return a canonical stable path, or use handle-relative writes; extend the retarget test to check the path used for node data, not only RunDir::remove().

• tests/devnet_lifecycle.rs:36-57, 87-91 — the test can signal an unrelated process group. wait_for_exit() calls Child::try_wait(), which reaps the process. Drop later unconditionally sends SIGKILL to the saved numeric process-group ID. If that group has become empty and the ID is reused before Drop, the signal can hit an unrelated group. Keep the group leader unreaped until group cleanup, or otherwise make cleanup safe against ID reuse.

• src/devnet/run_dir.rs:1425-1438 — junction paths are passed through cmd /C without cmd-safe quoting. Command::arg applies Windows argument quoting, not cmd.exe metacharacter escaping. A temporary path containing shell syntax such as & can change the command parsed by cmd. Create junctions through a native API/helper, or safely construct and quote the complete command; add a metacharacter-path test.

Verification

• git diff --check passed; the local checkout matches the reviewed head and is clean.
• GitHub build, formatting, Clippy, documentation, MSRV, storage-filesystem, and WebRTC checks passed. The Linux and macOS ant-devnet stop signals and run directories steps passed; the Windows step is skipped by the workflow. The Linux/macOS/Windows test jobs were still running their end-to-end stage at review time.
• Security Audit failed on the inherited crypto-bigint 0.5.5 advisory. As requested, this is not an approval blocker; it remains a separate merge blocker.
• I did not run the Cargo test suite locally at this head.

No merge action taken.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes Requested 🔴

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Reviewed head: 94231055f17aecdc69648982108f68b763c90eab
Base: 02c8682466df5c0c6723f32c46223a572f020ed4
Files changed: 8

I reviewed the complete base-to-head diff in three scopes. Both seats completed all three scopes. Their opinions differed; I checked the material findings against the current source. I also reproduced the path-redirection behavior described below.

🔴 Changes required

• src/devnet/run_dir.rs:211-215, 502-521; src/devnet.rs:523-529 — canonicalizing the parent once does not make the path stable. Devnet::new stores RunDir::path() as the node data path. If another user can rename a canonical ancestor of the configured parent, they can replace it with a symlink to a different tree after creation. Later path-based node writes then land in that tree, while the held directory descriptor still refers to the original run. The ownership check on the run directory does not protect its ancestors. I reproduced a write through the saved canonical path landing in a replacement tree after its ancestor was moved and replaced by a symlink. Reject parents with untrusted, renameable ancestry or keep writes anchored to the held directory; add a regression that replaces a canonical ancestor after run creation and checks that writes cannot be redirected.

• src/devnet/run_dir.rs:321-335, 364-366 — sweeping can erase a run directory after it has been moved out of its name. finished_run passes its open directory handle to delete; delete recursively removes the contents before checking that the parent entry still names that directory. The same_dir check at line 334 protects only the final empty-directory removal. A writer to a shared parent can move the eligible directory between the check and deletion, and the sweep will still empty it through the held handle. Make the parent namespace safe from untrusted renames, or make the transition into destructive cleanup race-safe; add a deterministic move-after-eligibility test.

Verification

• git diff --check passed.
• All reported build and test checks passed on Linux, macOS, and Windows, including the test and end-to-end stages.
• Security Audit failed. Per the request, I did not treat that as an approval blocker; it remains a merge gate and is not the reason for this verdict.

No merge action taken.

[MaidSafe-Hermes]

Canonicalizing the parent removes links only at open time; it cannot stop another user with rename rights in a canonical ancestor from replacing that ancestor later. Devnet::new persists this path for node writes. I reproduced writes following the replacement into the new tree. Please reject untrusted/renameable ancestry or keep writes descriptor-relative, and test an ancestor swap after run creation.

[MaidSafe-Hermes]

The sweep calls delete without checking same_dir before this recursive removal; the only same_dir check is after contents are removed, at line 334. An actor able to change the parent namespace can move an eligible directory after finished_run checks it, while this held handle still empties it outside its run name. Secure the parent namespace or make the destructive transition race-safe, and add a move-after-eligibility regression.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes requested (1 confirmed data-safety blocker)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Reviewed head: 5263208259b4b7159a1f44d0a1faed32f8390a79
Base: e61446b896b6cbf50bbd7aa39af385ee11dfd16f
Review coverage: 2/2 seats reviewed the Unix parent-permission guard (sol-1, gpt-6.1-sol xhigh; glm-1, z-ai/glm-5.3). The GLM seat first failed with ConnectionResetError; its one narrowed retry returned a substantive CHANGES_REQUIRED review. Both seats identified the shared-effective-GID gap. This is agreement between two model families, not team consensus.

Changes required

• src/devnet/run_dir.rs:736-744 — group-writable parents are accepted based only on the process's effective GID. On Linux, group_writers returns Ok(None) when st_gid == getegid() and the directory has no POSIX ACL. It does not establish that the group has no other members. On a system where another user shares that GID, that user can write to the accepted group-writable parent and rename or replace the run-directory entry. Later node writes through RunDir::path() can then be redirected, defeating the new data-protection guarantee. The added acceptance test explicitly blesses this path. Reject group-writable ancestors unless group exclusivity is verified reliably; add a regression with another user sharing the accepted GID and change the acceptance test.

Verification

• git diff --check passed.
• Current GitHub CI checks passed, including Linux, macOS, Windows, MSRV and CI Gate. Security Audit failed and remains a separate merge gate; it is not the reason for this code verdict.
• The local cargo test --locked --lib devnet::run_dir::tests -- --test-threads=1 did not complete within the tool timeout, so I do not count it as a local test result.

No merge action taken.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Changes requested (1 confirmed macOS data-safety blocker)

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Reviewed head: 5263208259b4b7159a1f44d0a1faed32f8390a79
Base: e61446b896b6cbf50bbd7aa39af385ee11dfd16f
Coverage: 2/2 seats reviewed the Unix parent-permission guard (sol-1, gpt-6.1-sol xhigh; glm-1, z-ai/glm-5.3). Both identified the macOS ACL limitation as an unverified risk; I verified the relevant ACL permissions against Apple’s File System Programming Guide. The Linux own-primary-group exception from review 7 is an owner-accepted risk and is not a blocker for this verdict.

Changes required

• src/devnet/run_dir.rs:698-748 — macOS ACLs can grant another user the ability to replace a run-directory entry, but the parent check inspects only ownership and BSD mode bits. On non-Linux Unix, group_writers does not inspect ACLs. macOS ACLs include directory rights such as add subdirectory and delete child, which are separate from BSD mode bits. If an ACL grants those rights on the configured parent or an ancestor, no_one_else_can_change accepts it; that user can move or replace a run-directory entry, redirecting later writes through RunDir::path(). Add a macOS ACL-aware check that rejects ACEs allowing another user to create/remove directory entries, with a regression test; otherwise narrow the guarantee and record explicit owner acceptance of this risk.

Verification

• The exact PR head is unchanged. git diff --check passes.
• Current required PR checks (linear-link, pr-template) and CI Gate pass. Security Audit remains failed, as disclosed; it is not treated as an approval blocker here.
• The Linux primary-group exception is excluded from this verdict as requested.

No merge action taken.

[MaidSafe-Hermes]

Code Review Summary

Verdict: Approved

PR: #263 — fix(ant-devnet): stop cleanly on SIGTERM and use a per-run data dir by default
Reviewed head: 5263208259b4b7159a1f44d0a1faed32f8390a79
Base: e61446b896b6cbf50bbd7aa39af385ee11dfd16f

The author’s 2026-11-04 update explicitly accepts the documented limitations in the PR body’s “Not covered” section, including the Linux own-primary-group and macOS ACL cases raised in reviews 7 and 8. I therefore do not treat those documented risks as blockers. I found no other unresolved blocker outside that accepted scope. The earlier run-directory race findings are addressed by the round-7 changes and their regression coverage.

Review coverage: The two review seats (sol-1, gpt-6.1-sol xhigh; glm-1, z-ai/glm-5.3) reviewed the latest parent-permission guard at this exact head. Earlier review rounds covered the unchanged lifecycle and run-directory implementation. The current source and CI evidence support approval.

Verification

• git diff --check passed; the local checkout is at the reviewed head and clean.
• Current linear-link, pr-template, and CI Gate checks pass; the Linux, macOS, Windows, MSRV, storage, and release-build jobs pass.
• Security Audit remains failed on the disclosed inherited RUSTSEC-2026-0387 issue. Per the PR’s documented check requirements, it is not a required check and does not block this approval.

Approval is not a merge action.