MCP server: replies SDK clients accept, cancelling that works under load, safe captures

.github/workflows/ci.yml

@@ -380,6 +380,41 @@ jobs:
         run: npm run build
         working-directory: apps/netscli-gui
 
+  # Drive `netscli serve` with the official MCP TypeScript SDK client.
+  #
+  # The Rust tests check the server's replies against what the server itself
+  # expects. Every reply carried both `result` and `error` for months with all
+  # of them green, while the SDK most MCP clients are built on dropped those
+  # replies and timed out waiting for `initialize`. This asks the SDK instead.
+  mcp-smoke:
+    name: MCP SDK smoke test
+    needs: [changes, lint]
+    if: needs.changes.outputs.code == 'true'
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+      - name: Install Rust
+        # See the pin note on the lint job above.
+        uses: dtolnay/rust-toolchain@4360b52568e2003a75bf9bc1d59f33a8e3fc893c # stable
+        with:
+          toolchain: "1.96.0"
+      - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
+        with:
+          shared-key: mcp-smoke
+      - name: cargo build -p netscli
+        run: cargo build -p netscli
+      - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
+        with:
+          node-version-file: .nvmrc
+      - name: npm ci
+        run: npm ci --ignore-scripts
+        working-directory: scripts/mcp-smoke
+      - name: Connect, list tools and call one
+        run: node scripts/mcp-smoke/smoke.mjs target/debug/netscli
+
   # Release-config binary build. This catches regressions that only show
   # up under optimizations / lto that debug builds miss, and the artifact
   # upload makes it possible to grab a "tip of main" binary straight from
@@ -499,6 +534,7 @@ jobs:
       - lint
       - test
       - gui-build
+      - mcp-smoke
       - release-build
       - publish-dry-run
     runs-on: ubuntu-latest

CHANGELOG.md

@@ -64,8 +64,80 @@ its heading and collects entries. The date and the link go on with the tag.
   `--version` and gives up after 3 seconds. A program still running at that
   point was left behind. It is now stopped.
 
+- **MCP clients built on the official SDK can connect.** Every reply the MCP
+  server sent carried both a result and an error, one of them empty, which
+  the protocol doesn't allow. The official TypeScript SDK drops a reply like
+  that, so a client built on it waited for the answer to its first message
+  and timed out. Replies now carry one or the other, and CI connects to the
+  server with the SDK whenever the code changes.
+
+- **Cancelling an MCP call works when the server is busy.** When a 17th call
+  arrived with 16 running, the server stopped reading what the client sent
+  until one finished. A cancel, which is how a client frees a slot, went
+  unread, and so did the client closing the connection. The server now
+  keeps reading. Up to 16 more calls wait for a slot, and any past that are
+  refused at once with an error.
+
+- **Cancelling or disconnecting stops an MCP packet capture.** A capture
+  ran on after its call was cancelled, timed out or lost its client, for up
+  to an hour, and gave its slot back straight away, so cancelling and
+  restarting got past the limit of four captures. It now stops, and gives
+  the slot back once it has. Background captures stop when the client goes
+  too. In a test on Windows, a server whose client left mid-capture exited
+  within 0.3 seconds, where before it was still running after 20. A
+  `capture_pcap` call with only `maxPackets` could also run for an hour. It
+  now stops after the 10 seconds the tool advertises.
+
+- **mDNS discovery no longer leaves its listener running.** A service type
+  without its final dot, such as `_http._tcp.local`, or a cancelled MCP
+  call left the listener running in the background, its network sockets
+  open and any searches it had started still repeating, until the program
+  exited. It now always stops.
+
+- **An MCP result too large to send keeps its file path and counts.** A
+  result over 1 MiB that wasn't a plain list was replaced by an error, so a
+  large packet capture lost its file name, packet count and job status along
+  with its packets. Now only its longest list is cut to fit. A background
+  capture also reports its file name as soon as it starts.
+
+- **Smaller MCP server fixes.**
+  - The server answers `ping`, which clients use to check that a server is
+    alive. It answered with an error.
+  - It agrees a protocol version it supports, instead of whatever version
+    the client asked for.
+  - Text from scanned hosts in HTTP headers and mDNS records is cut to 256
+    characters, as banners already were.
+  - `discover_mdns` takes at most 32 service types, and browses each once.
+  - An over-long request line no longer has its tail read as the next
+    message.
+  - A request that reuses the id of one just answered can always be
+    cancelled. A race could leave it impossible to cancel.
+
+- **Smaller scan fixes.**
+  - A port behind a firewall that rejects connections with an ICMP message
+    reads as filtered, as nmap reports it, instead of as an error.
+  - The HTTP probe sends IPv6 targets a valid `Host` header, with the
+    address in brackets. The old one was malformed, and a strict server can
+    refuse it.
+  - A stray `data/oui.json` in the folder netscli ran from no longer
+    replaces or empties the vendor list.
+  - On a network wider than /16, the default /24 is the one around this
+    machine's address on that network. It could come from another interface
+    or fall back to 192.168.1.0/24.
+  - Windows discovery re-checks at most 64 devices at once. Checked all at
+    once, a very long device table could drop devices that were there.
+  - The example in the `netscli-core` README compiles.
+
 ### Security
 
+- **MCP packet captures never overwrite a file or follow a symlink.**
+  `capture_pcap` wrote `capture.pcap` in the server's working directory,
+  often your project, and replaced any file of that name. On Linux and
+  macOS, a `capture.pcap` symlink in a repository could send the capture
+  through to wherever it pointed, as root if the server ran as root.
+  Captures now create a new file and refuse a name that is taken, symlinks
+  included, and without `outputFile` each capture gets a new name.
+
 - **DNS lookups no longer go to Cloudflare behind your back.** When your own
   DNS servers could not answer a lookup, including when a name simply had no
   records of the type asked for, NetsCLI asked Cloudflare's public resolver

crates/netscli-core/README.md

@@ -39,7 +39,7 @@ async fn main() -> anyhow::Result<()> {
 
     let scanner = PortScanner::new(100); // max concurrent connections
     let target = "192.168.1.1".parse()?;
-    let results = scanner.scan_host(target, ports, 500).await; // timeout_ms
+    let results = scanner.scan_host(target, ports, 500).await?; // timeout_ms
 
     for result in results {
         // `PortResult` carries the port, not the host — the host is the
@@ -56,6 +56,8 @@ async fn main() -> anyhow::Result<()> {
 |---------|---------|---------|
 | `db`    | off     | Enables the `Database` type and SQLite persistence via sqlx + chrono. Pulls ~90 extra transitive crates, so library consumers who don't need scan history should leave this off. |
 | `pcap`  | off     | Enables `PcapEngine` packet capture. Needs libpcap/Npcap at runtime. |
+| `mdns`  | off     | Enables `MdnsEngine` for mDNS/DNS-SD (Bonjour) discovery. Pure Rust, no system dependency. |
+| `ts`    | off     | Generates TypeScript types for the desktop app. Build tooling only. |
 
 The `netscli` binary crate enables `db` by default (it uses `Database`
 for scan history). Library consumers who only want the scan / DNS /

crates/netscli-core/src/arp.rs

@@ -48,13 +48,25 @@ impl NetworkManager {
 pub(crate) async fn still_present(candidates: &[Ipv4Addr], wait: Duration) -> HashSet<Ipv4Addr> {
     #[cfg(target_os = "windows")]
     {
-        let checks = candidates.iter().map(|&ip| async move {
+        use futures::stream::StreamExt;
+
+        // Lookups in flight at once. All of them used to start together, and
+        // each `wait` was counted from then, so with more candidates than
+        // tokio's blocking pool has threads, lookups spent their time queued
+        // and real neighbours were dropped. Now `wait` starts when a lookup
+        // does.
+        const AT_ONCE: usize = 64;
+        // By value: a closure over `&Ipv4Addr` makes the stream's future not
+        // `Send` for every lifetime, which `tokio::spawn` in discovery needs.
+        let checks = candidates.iter().copied().map(|ip| async move {
             // A lookup that outlives `wait` keeps its blocking thread until
             // Windows gives up on it, about 4 s; its answer is just ignored.
             let asked = tokio::task::spawn_blocking(move || platform::answers_arp(ip));
             matches!(tokio::time::timeout(wait, asked).await, Ok(Ok(true))).then_some(ip)
         });
-        futures::future::join_all(checks)
+        futures::stream::iter(checks)
+            .buffer_unordered(AT_ONCE)
+            .collect::<Vec<_>>()
             .await
             .into_iter()
             .flatten()

crates/netscli-core/src/arp/platform/windows_resolve.rs

@@ -17,16 +17,18 @@ use windows_sys::Win32::NetworkManagement::IpHelper::SendARP;
 
 /// The MAC `ip` answers ARP with, or `None` if nothing answered. Blocking.
 pub(super) fn resolve(ip: Ipv4Addr) -> Option<[u8; 6]> {
-    let mut mac = [0u8; 8];
-    let mut len = mac.len() as u32;
+    // Two ULONGs, as SendARP documents, which also gives the buffer a ULONG's
+    // alignment. A byte array has none.
+    let mut mac = [0u32; 2];
+    let mut len = std::mem::size_of_val(&mac) as u32;
     // SendARP takes the address in network byte order as a u32.
     let dest = u32::from_ne_bytes(ip.octets());
     // SAFETY: an 8-byte buffer and its length; SendARP writes at most `len`.
     let status = unsafe { SendARP(dest, 0, mac.as_mut_ptr().cast(), &mut len) };
     if status != 0 || len != 6 {
         return None;
     }
-    let mut out = [0u8; 6];
-    out.copy_from_slice(&mac[..6]);
-    Some(out)
+    // The bytes as SendARP wrote them to memory.
+    let [low, high] = mac.map(u32::to_ne_bytes);
+    Some([low[0], low[1], low[2], low[3], high[0], high[1]])
 }

crates/netscli-core/src/arp/platform/windows_table.rs

@@ -17,8 +17,8 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 use mac_address::MacAddress;
 use windows_sys::Win32::Foundation::NO_ERROR;
 use windows_sys::Win32::NetworkManagement::IpHelper::{
-    FreeMibTable, GetIpNetTable2, GetUnicastIpAddressTable, MIB_IPNET_TABLE2,
-    MIB_UNICASTIPADDRESS_TABLE,
+    FreeMibTable, GetIpNetTable2, GetUnicastIpAddressTable, MIB_IPNET_ROW2, MIB_IPNET_TABLE2,
+    MIB_UNICASTIPADDRESS_ROW, MIB_UNICASTIPADDRESS_TABLE,
 };
 use windows_sys::Win32::Networking::WinSock::{
     NlnsDelay, NlnsPermanent, NlnsProbe, NlnsReachable, NlnsStale, AF_INET, AF_INET6,
@@ -39,8 +39,12 @@ pub(super) fn read() -> Result<Vec<ArpEntry>> {
         return Err(Error::Network(format!("GetIpNetTable2 failed: {status}")));
     }
     // SAFETY: on success `table` points at NumEntries rows; freed below.
+    // `Table` is declared as one row and the rest follow it, so the pointer
+    // is taken from `table` itself. `.Table.as_ptr()` borrowed the
+    // one-row array first, which covers only the first row.
     let rows = unsafe {
-        std::slice::from_raw_parts((*table).Table.as_ptr(), (*table).NumEntries as usize)
+        let first = std::ptr::addr_of!((*table).Table).cast::<MIB_IPNET_ROW2>();
+        std::slice::from_raw_parts(first, (*table).NumEntries as usize)
     };
     let mut entries = Vec::new();
     for row in rows {
@@ -97,8 +101,10 @@ fn interface_ipv4_by_index() -> HashMap<u32, Ipv4Addr> {
         return map;
     }
     // SAFETY: on success `table` points at NumEntries rows; freed below.
+    // The pointer comes from `table`, as in `read` above.
     let rows = unsafe {
-        std::slice::from_raw_parts((*table).Table.as_ptr(), (*table).NumEntries as usize)
+        let first = std::ptr::addr_of!((*table).Table).cast::<MIB_UNICASTIPADDRESS_ROW>();
+        std::slice::from_raw_parts(first, (*table).NumEntries as usize)
     };
     for row in rows {
         if let Some(IpAddr::V4(ip)) = sockaddr_ip(&row.Address) {

crates/netscli-core/src/common/network.rs

@@ -4,8 +4,15 @@ use ipnetwork::IpNetwork;
 
 use super::constants::DEFAULT_SUBNET;
 
-#[cfg(not(windows))]
 pub fn detect_default_ipv4_subnet() -> Option<Ipv4Net> {
+    // `.trunc()` drops the host bits, so this is the network address.
+    default_interface_ipv4().map(|net| net.trunc())
+}
+
+/// This host's IPv4 address on the preferred interface, with that
+/// network's prefix length.
+#[cfg(not(windows))]
+fn default_interface_ipv4() -> Option<Ipv4Net> {
     // Prefer non-tunnel interfaces (vpn/wireguard adapters often sort first
     // on laptops, and scanning the VPN subnet is almost never what the user
     // wants). We iterate ALL qualifying interfaces and return the first one
@@ -21,9 +28,7 @@ pub fn detect_default_ipv4_subnet() -> Option<Ipv4Net> {
         for ip in iface.ips {
             if let IpNetwork::V4(v4) = ip {
                 if let Ok(net) = Ipv4Net::new(v4.ip(), v4.prefix()) {
-                    // `.trunc()` drops any host bits so we return the network
-                    // address regardless of whether the interface IP is a host.
-                    return Some(net.trunc());
+                    return Some(net);
                 }
             }
         }
@@ -46,9 +51,12 @@ fn interface_preference_rank(name: &str) -> u8 {
     }
 }
 
+/// This host's IPv4 address on the first adapter that is up with a
+/// network-shaped prefix, with that network's prefix length.
 #[cfg(windows)]
-pub fn detect_default_ipv4_subnet() -> Option<Ipv4Net> {
+fn default_interface_ipv4() -> Option<Ipv4Net> {
     use ipconfig::{IfType, OperStatus};
+    use std::net::IpAddr;
 
     let adapters = ipconfig::get_adapters().ok()?;
     for adapter in adapters {
@@ -59,7 +67,17 @@ pub fn detect_default_ipv4_subnet() -> Option<Ipv4Net> {
             continue;
         }
         if let Some(net) = pick_ipv4_subnet_from_prefixes(adapter.prefixes()) {
-            return Some(net);
+            // The prefix list gives the network. This host's own address on
+            // it is among the adapter's addresses.
+            let host = adapter
+                .ip_addresses()
+                .iter()
+                .find_map(|ip| match ip {
+                    IpAddr::V4(v4) if net.contains(v4) => Some(*v4),
+                    _ => None,
+                })
+                .unwrap_or(net.network());
+            return Ipv4Net::new(host, net.prefix_len()).ok();
         }
     }
 
@@ -146,9 +164,10 @@ pub fn detect_default_ipv4_addr() -> Option<std::net::Ipv4Addr> {
 const NARROWED_DEFAULT_PREFIX: u8 = 24;
 
 pub fn default_ipv4_subnet_string() -> String {
-    let Some(net) = detect_default_ipv4_subnet() else {
+    let Some(interface) = default_interface_ipv4() else {
         return DEFAULT_SUBNET.to_string();
     };
+    let net = interface.trunc();
 
     // An interface legitimately carrying a /8 -- a 10.0.0.0/8 corporate
     // network, or Docker -- produced a default that the /16 cap then
@@ -158,11 +177,13 @@ pub fn default_ipv4_subnet_string() -> String {
     if net.prefix_len() >= 16 {
         return format!("{}/{}", net.network(), net.prefix_len());
     }
-    match detect_default_ipv4_addr()
-        .and_then(|addr| Ipv4Net::new(addr, NARROWED_DEFAULT_PREFIX).ok())
-    {
-        Some(narrowed) => format!("{}/{}", narrowed.network(), narrowed.prefix_len()),
-        None => DEFAULT_SUBNET.to_string(),
+    // Around this host's address on that same interface. The address used to
+    // come from `detect_default_ipv4_addr`, which picks an interface by a
+    // different rule: the first one up, which may be a tunnel, or have no
+    // IPv4 address and fall back to 192.168.1.0/24.
+    match Ipv4Net::new(interface.addr(), NARROWED_DEFAULT_PREFIX) {
+        Ok(narrowed) => format!("{}/{}", narrowed.network(), narrowed.prefix_len()),
+        Err(_) => DEFAULT_SUBNET.to_string(),
     }
 }