test: stop aiming I12 transport tests at recycled ports

crates/bugwarden-core/Cargo.toml

@@ -30,5 +30,5 @@ reqwest = { version = "0.13", default-features = false, features = [
 ] }
 
 [dev-dependencies]
-tokio = { version = "1", features = ["macros", "rt-multi-thread"] }
+tokio = { version = "1", features = ["macros", "rt-multi-thread", "time"] }
 wiremock = "0.6"

crates/bugwarden-core/tests/guard_wiremock.rs

@@ -32,6 +32,53 @@ fn client(server: &MockServer) -> BugzillaClient {
     BugzillaClient::new(&server.uri(), false, TEST_USER_AGENT).expect("client must build")
 }
 
+/// Connect-time failure that wiremock's 127.0.0.1 pool cannot serve (#115).
+///
+/// `127.0.0.1:1` is a privileged port: a non-root wiremock listener binds
+/// `127.0.0.1:0` and cannot occupy it. Both Linux and macOS refuse
+/// immediately if nothing is listening (unlike `127.0.0.2`, which is not
+/// aliased on macOS and hangs). A bounded probe refuses to return an
+/// address that accepted or timed out. The URL is built from the probed
+/// socket so the two cannot drift, and the port must stay privileged so
+/// a bind-then-drop of an ephemeral port fails this helper.
+fn refused_base_url() -> String {
+    let addr = std::net::SocketAddr::from(([127, 0, 0, 1], 1));
+    assert!(
+        addr.port() < 1024,
+        "I12 transport tests must use a privileged port; wiremock binds 127.0.0.1:0 (#115)"
+    );
+    match std::net::TcpStream::connect_timeout(&addr, std::time::Duration::from_millis(500)) {
+        Ok(_) => panic!(
+            "{addr} accepted a connection; I12 tests need a refused address \
+             that wiremock's 127.0.0.1:0 pool cannot occupy (#115)"
+        ),
+        Err(e) if e.kind() == std::io::ErrorKind::TimedOut => panic!(
+            "{addr} timed out; refusing to point the 30s client at an address \
+             that would hang the test (#115)"
+        ),
+        Err(_) => format!("http://{addr}"),
+    }
+}
+
+/// Bound on the I12 client calls. Loopback refuse is immediate; a hang
+/// here is a proxy or routing defect, not a 30s client timeout.
+const REFUSED_CONNECT_BUDGET: std::time::Duration = std::time::Duration::from_secs(2);
+
+/// I12: the error must be a real reqwest transport failure, and neither
+/// Display nor Debug may carry the API key. An empty or HTTP-status error
+/// would pass a bare `!contains(KEY)` without exercising sanitization.
+fn assert_key_absent_from_transport_error(err: &anyhow::Error) {
+    let full = format!("{err:#} {err:?}");
+    assert!(
+        full.contains("error sending request"),
+        "expected a reqwest transport error (I12), got: {full}"
+    );
+    assert!(
+        !full.contains(KEY),
+        "API key leaked into transport error: {full}"
+    );
+}
+
 fn bug(id: u64, groups: &[&str], creation_time: &str) -> serde_json::Value {
     json!({
         "id": id,
@@ -408,20 +455,16 @@ async fn api_key_reaches_server_but_never_error_text_i12() {
 
 #[tokio::test]
 async fn api_key_absent_from_transport_error_i12() {
-    // Point at a closed port: reqwest yields a connect error whose URL would
-    // contain the api_key query parameter — sanitize must strip it.
-    let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
-    let addr = listener.local_addr().expect("addr");
-    drop(listener); // free the port so the connection is refused
-    let bz =
-        BugzillaClient::new(&format!("http://{addr}"), false, TEST_USER_AGENT).expect("client");
+    // Point at an address the wiremock pool can never occupy: reqwest
+    // yields a connect error whose URL would contain the api_key query
+    // parameter — sanitize must strip it.
+    let bz = BugzillaClient::new(&refused_base_url(), false, TEST_USER_AGENT).expect("client");
 
-    let err = bz.get_bugs(KEY, &[1], None).await.unwrap_err();
-    let full = format!("{err:#} {err:?}");
-    assert!(
-        !full.contains(KEY),
-        "API key leaked into transport error: {full}"
-    );
+    let err = tokio::time::timeout(REFUSED_CONNECT_BUDGET, bz.get_bugs(KEY, &[1], None))
+        .await
+        .expect("connect to the refused privileged port must not hang")
+        .unwrap_err();
+    assert_key_absent_from_transport_error(&err);
 }
 
 #[tokio::test]
@@ -475,11 +518,8 @@ async fn client_create_bug_posts_the_payload_to_rest_bug() {
     // The payload travels as the POST body, untouched.
     //
     // Select the request by method and path rather than by position (#93).
-    // wiremock hands out mock servers from a process-wide pool of listeners,
-    // and the I12 transport-error tests in this binary deliberately aim a
-    // request at a just-freed ephemeral port; when that port has meanwhile
-    // been taken by a pooled listener, their bodyless GET is recorded here
-    // too. `[0]` would then assert against that request instead.
+    // The recording can contain more than the create POST — `[0]` would
+    // then assert against the wrong request.
     let reqs = server.received_requests().await.expect("recording enabled");
     let posts: Vec<&Request> = reqs
         .iter()
@@ -1138,18 +1178,13 @@ async fn client_whoami_missing_non_string_or_blank_name_is_a_failure() {
 async fn whoami_api_key_absent_from_transport_error_i12() {
     // Same pattern as api_key_absent_from_transport_error_i12: a connect
     // error's URL would carry api_key=... — sanitize must strip it.
-    let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
-    let addr = listener.local_addr().expect("addr");
-    drop(listener); // free the port so the connection is refused
-    let bz =
-        BugzillaClient::new(&format!("http://{addr}"), false, TEST_USER_AGENT).expect("client");
+    let bz = BugzillaClient::new(&refused_base_url(), false, TEST_USER_AGENT).expect("client");
 
-    let err = bz.whoami(KEY).await.unwrap_err();
-    let full = format!("{err:#} {err:?}");
-    assert!(
-        !full.contains(KEY),
-        "API key leaked into whoami transport error: {full}"
-    );
+    let err = tokio::time::timeout(REFUSED_CONNECT_BUDGET, bz.whoami(KEY))
+        .await
+        .expect("connect to the refused privileged port must not hang")
+        .unwrap_err();
+    assert_key_absent_from_transport_error(&err);
 }
 
 // ---------------------------------------------------------------------------
@@ -1218,18 +1253,16 @@ async fn client_valid_login_unusable_shape_is_an_error_never_false() {
 
 #[tokio::test]
 async fn valid_login_api_key_absent_from_transport_error_i12() {
-    let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
-    let addr = listener.local_addr().expect("addr");
-    drop(listener); // free the port so the connection is refused
-    let bz =
-        BugzillaClient::new(&format!("http://{addr}"), false, TEST_USER_AGENT).expect("client");
-
-    let err = bz.valid_login(KEY, "svc@example.com").await.unwrap_err();
-    let full = format!("{err:#} {err:?}");
-    assert!(
-        !full.contains(KEY),
-        "API key leaked into valid_login transport error: {full}"
-    );
+    let bz = BugzillaClient::new(&refused_base_url(), false, TEST_USER_AGENT).expect("client");
+
+    let err = tokio::time::timeout(
+        REFUSED_CONNECT_BUDGET,
+        bz.valid_login(KEY, "svc@example.com"),
+    )
+    .await
+    .expect("connect to the refused privileged port must not hang")
+    .unwrap_err();
+    assert_key_absent_from_transport_error(&err);
 }
 
 #[tokio::test]

crates/bugwarden/tests/common/mod.rs

@@ -0,0 +1,33 @@
+//! Shared helpers for bugwarden integration tests.
+
+/// Connect-time failure that wiremock's 127.0.0.1 pool cannot serve (#115).
+///
+/// `127.0.0.1:1` is a privileged port: a non-root wiremock listener binds
+/// `127.0.0.1:0` and cannot occupy it. Both Linux and macOS refuse
+/// immediately if nothing is listening (unlike `127.0.0.2`, which is not
+/// aliased on macOS and hangs). A bounded probe refuses to return an
+/// address that accepted or timed out. The URL is built from the probed
+/// socket so the two cannot drift, and the port must stay privileged so
+/// a bind-then-drop of an ephemeral port fails this helper.
+pub fn refused_base_url() -> String {
+    let addr = std::net::SocketAddr::from(([127, 0, 0, 1], 1));
+    assert!(
+        addr.port() < 1024,
+        "I12 transport tests must use a privileged port; wiremock binds 127.0.0.1:0 (#115)"
+    );
+    match std::net::TcpStream::connect_timeout(&addr, std::time::Duration::from_millis(500)) {
+        Ok(_) => panic!(
+            "{addr} accepted a connection; I12 tests need a refused address \
+             that wiremock's 127.0.0.1:0 pool cannot occupy (#115)"
+        ),
+        Err(e) if e.kind() == std::io::ErrorKind::TimedOut => panic!(
+            "{addr} timed out; refusing to point the 30s client at an address \
+             that would hang the test (#115)"
+        ),
+        Err(_) => format!("http://{addr}"),
+    }
+}
+
+/// Bound on the I12 client calls. Loopback refuse is immediate; a hang
+/// here is a proxy or routing defect, not a 30s client timeout.
+pub const REFUSED_CONNECT_BUDGET: std::time::Duration = std::time::Duration::from_secs(2);

crates/bugwarden/tests/preflight_wiremock.rs

@@ -19,6 +19,8 @@
 
 use std::sync::Arc;
 
+mod common;
+
 use bugwarden::config::Cli;
 use bugwarden::server::{BugWarden, USER_AGENT};
 use bugwarden_core::client::BugzillaClient;
@@ -262,18 +264,16 @@ async fn preflight_declared_login_transport_error_names_the_endpoint() {
 
 #[tokio::test]
 async fn preflight_transport_error_does_not_leak_the_api_key_i12() {
-    // Point the server at a closed port: the whoami lookup fails at the
-    // transport level, where the unsanitized error would carry the
-    // request URL with api_key=... in it. Nothing in the preflight error
-    // text may contain the key.
-    let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
-    let addr = listener.local_addr().expect("addr");
-    drop(listener); // free the port so every connection is refused
+    // Point the server at an address the wiremock pool can never occupy:
+    // the whoami lookup fails at the transport level, where the
+    // unsanitized error would carry the request URL with api_key=... in
+    // it. Nothing in the preflight error text may contain the key.
+    let base = common::refused_base_url();
 
     let mut cli = Cli::parse_from([
         "bugwarden",
         "--bugzilla-server",
-        &format!("http://{addr}"),
+        &base,
         "--transport",
         "stdio",
         "--api-key",
@@ -284,17 +284,18 @@ async fn preflight_transport_error_does_not_leak_the_api_key_i12() {
     let guard = Arc::new(Guard {
         policy: Policy::from_toml_str(IDENTITY_POLICY).expect("test policy must parse"),
     });
-    let bz = Arc::new(
-        BugzillaClient::new(&format!("http://{addr}"), false, USER_AGENT)
-            .expect("client must build"),
-    );
+    let bz = Arc::new(BugzillaClient::new(&base, false, USER_AGENT).expect("client must build"));
     let server = BugWarden::new(cfg, guard, bz).expect("server must build");
 
-    let err = server
-        .preflight()
+    let err = tokio::time::timeout(common::REFUSED_CONNECT_BUDGET, server.preflight())
         .await
+        .expect("connect to the refused privileged port must not hang")
         .expect_err("an unreachable whoami endpoint must fail preflight");
-    let msg = format!("{err:#}");
+    let msg = format!("{err:#} {err:?}");
+    assert!(
+        msg.contains("error sending request"),
+        "expected a whoami transport error (I12), got: {msg}"
+    );
     assert!(
         !msg.contains("SUPERSECRETKEY123"),
         "API key leaked into a preflight error: {msg}"

crates/bugwarden/tests/tools_wiremock.rs

@@ -21,6 +21,8 @@
 
 use std::sync::Arc;
 
+mod common;
+
 use bugwarden::config::Cli;
 use bugwarden::server::{BugWarden, USER_AGENT, WRITE_TOOLS};
 use bugwarden_core::client::BugzillaClient;
@@ -1642,18 +1644,16 @@ async fn created_by_me_reaches_the_write_gate_add_comment_too() {
 
 #[tokio::test]
 async fn whoami_transport_error_does_not_leak_the_api_key_i12() {
-    // Point the server at a closed port: the whoami lookup (and everything
-    // after it) fails at the transport level, where the unsanitized error
-    // would carry the request URL with api_key=... in it. Nothing the
-    // client sees may contain the key.
-    let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
-    let addr = listener.local_addr().expect("addr");
-    drop(listener); // free the port so every connection is refused
+    // Point the server at an address the wiremock pool can never occupy:
+    // the whoami lookup (and everything after it) fails at the transport
+    // level, where the unsanitized error would carry the request URL with
+    // api_key=... in it. Nothing the client sees may contain the key.
+    let base = common::refused_base_url();
 
     let mut cli = Cli::parse_from([
         "bugwarden",
         "--bugzilla-server",
-        &format!("http://{addr}"),
+        &base,
         "--transport",
         "stdio",
         "--api-key",
@@ -1664,10 +1664,7 @@ async fn whoami_transport_error_does_not_leak_the_api_key_i12() {
     let guard = Arc::new(Guard {
         policy: Policy::from_toml_str(IDENTITY_POLICY).expect("test policy must parse"),
     });
-    let bz = Arc::new(
-        BugzillaClient::new(&format!("http://{addr}"), false, USER_AGENT)
-            .expect("client must build"),
-    );
+    let bz = Arc::new(BugzillaClient::new(&base, false, USER_AGENT).expect("client must build"));
     let server = BugWarden::new(cfg, guard, bz).expect("server must build");
     let (client_io, server_io) = tokio::io::duplex(1 << 16);
     tokio::spawn(async move {
@@ -1680,7 +1677,12 @@ async fn whoami_transport_error_does_not_leak_the_api_key_i12() {
             .await
             .expect("MCP handshake must succeed");
 
-    let result = call(&client, "bug_info", json!({ "bug_ids": [7] })).await;
+    let result = tokio::time::timeout(
+        common::REFUSED_CONNECT_BUDGET,
+        call(&client, "bug_info", json!({ "bug_ids": [7] })),
+    )
+    .await
+    .expect("connect to the refused privileged port must not hang");
     let text = serde_json::to_string(&result).unwrap();
     assert!(
         !text.contains("SUPERSECRETKEY123"),