From a79038b5ea85227c3d203704df99c27fb92258a1 Mon Sep 17 00:00:00 2001 From: Shawn Hartsock Date: Mon, 13 Jul 2026 17:11:37 -0400 Subject: [PATCH 1/2] test(net-proxy): serialize fenced_child on net_test_lock() (#207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fenced_child_reaches_allowed_via_proxy_denied_refused_direct_kernel_blocked was the only loopback-exercising test in the net_proxy module that did not take the module's net_test_lock() serialization guard. It is also the heaviest net test — it spawns a loopback origin, a proxy, and a real curl child — so running it concurrently with a sibling proxy test raced on loopback and flaked CI (observed as "Empty reply from server" on the allow-leg in PR #195's macOS CI). Acquire net_test_lock() as the test's first statement, matching every sibling loopback test (same lock, same RAII scope held through the whole test), so it can no longer race siblings on loopback. Co-Authored-By: Claude Opus 4.8 --- agent-bridle-tool-shell/src/net_proxy.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/agent-bridle-tool-shell/src/net_proxy.rs b/agent-bridle-tool-shell/src/net_proxy.rs index 1acc3f0..0022473 100644 --- a/agent-bridle-tool-shell/src/net_proxy.rs +++ b/agent-bridle-tool-shell/src/net_proxy.rs @@ -1326,6 +1326,10 @@ mod tests { best_available_sandbox, loopback_fenced_caveats, seatbelt_is_supported, Caveats, SandboxPolicy, Scope, }; + // Serialize with sibling loopback tests (issue #207): this is the + // heaviest net test (origin + proxy + real curl child) and must not + // race siblings on loopback. Held for the whole test via RAII. + let _serial = net_test_lock(); if !seatbelt_is_supported() { eprintln!("skipping: /usr/bin/sandbox-exec unavailable"); return; From 6a2b864fbfda2bcfdcdbf8d7db302418de6d0ee8 Mon Sep 17 00:00:00 2001 From: Shawn Hartsock Date: Wed, 15 Jul 2026 08:03:53 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(net-proxy):=20restore=20net=5Ftest=5Flo?= =?UTF-8?q?ck()=20=E2=80=94=20removed=20in=20#216,=20so=20the=20#207=20fix?= =?UTF-8?q?=20could=20not=20compile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one-line fix from #207 called net_test_lock(), but the #216 in-memory migration (17e7d7b) had removed that helper together with most real-socket tests. The call sits behind cfg(target_os = "macos", feature = "macos-seatbelt"), so the Linux dev host never compiled it and macOS CI failed with E0425 (cannot find function net_test_lock). Restore the helper — the same poison-immune static-Mutex idiom #155 introduced — and take it in all three remaining real-socket tests (proxy_env…, fenced_child…, dropping_the_handle…), so the serialization domain is complete and the helper is used (not dead code under -D warnings) on every CI platform. Verified on an M4 mac: CI-parity clippy green; full workspace suite green under BRIDLE_REQUIRE_SEATBELT=1 with the fenced_child kernel proof genuinely running (no self-skip); tool-shell suite 5/5 repeat runs green; --no-default-features workspace tests green. Co-Authored-By: Claude Fable 5 --- agent-bridle-tool-shell/src/net_proxy.rs | 34 ++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/agent-bridle-tool-shell/src/net_proxy.rs b/agent-bridle-tool-shell/src/net_proxy.rs index 0022473..f22d3c1 100644 --- a/agent-bridle-tool-shell/src/net_proxy.rs +++ b/agent-bridle-tool-shell/src/net_proxy.rs @@ -1275,10 +1275,31 @@ mod tests { ); } + /// Serialize the tests that touch REAL loopback sockets (#155, #207). They + /// share the host's loopback and ephemeral-port space, so concurrent + /// siblings can interfere — e.g. a port released by one test can be + /// re-bound by a sibling before the first is done probing it, and the + /// probe then observes the sibling's live listener. One shared lock + /// removes that interference class wholesale; the motivating (never + /// locally reproduced) flake was `Empty reply from server` on the + /// fenced_child test in PR #195's macOS CI. The in-memory tests above + /// (#216) need no lock — only the real-socket tests below take it. + /// Process-local, which suffices because `cargo test` runs test binaries + /// sequentially; a per-test-process runner (e.g. nextest) would NOT be + /// covered, nor would the loopback binds in other crates' test binaries. + /// Poison is ignored so a panicking test does not cascade-fail the rest. + fn net_test_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(|e| e.into_inner()) + } + #[test] fn proxy_env_points_at_the_bound_loopback_addr() { - // One of two remaining REAL-socket tests: `proxy_env` is derived from the - // actually-bound loopback address, so this exercises the real `bind`. + // A REAL-socket test: `proxy_env` is derived from the actually-bound + // loopback address, so this exercises the real `bind` — and its live + // accept-loop listener must not interleave with the port probes of the + // sibling real-socket tests. + let _serial = net_test_lock(); let proxy = start_null(["x".to_string()], Arc::new(StdResolver)).unwrap(); let env = proxy.proxy_env(); let url = format!("http://127.0.0.1:{}", proxy.addr().port()); @@ -1405,12 +1426,15 @@ mod tests { drop(proxy); } - /// The second (and only other) REAL-socket test: it must exercise the actual + /// The other always-on REAL-socket test: it must exercise the actual /// `TcpListener` bind + accept loop + `Drop` teardown, which no in-memory - /// fake can. Self-contained (its own ephemeral port, no shared origin), so it - /// does not need the old cross-test serialization lock. + /// fake can. It probes a just-released ephemeral port, so it serializes on + /// `net_test_lock()` — a concurrent sibling could re-bind that port between + /// the drop and the probe, and the probe would then hit the sibling's live + /// listener (#207). #[test] fn dropping_the_handle_stops_the_listener() { + let _serial = net_test_lock(); let proxy = start_null(["x".to_string()], Arc::new(StdResolver)).unwrap(); let addr = proxy.addr(); drop(proxy);