fix: UAT-driven fixes — neli ACK parsing, MCP error codes, disconnect log

Three bugs found during 2026-03-20 UAT session:

1. neli 0.7 ACK parsing: recv_netlink_ack matched NlPayload::Payload
   which never matches for NLMSG_ERROR — neli 0.7 parses these into
   NlPayload::Ack/Err variants. OS route add/remove silently failed.

2. MCP error codes: all daemon errors (peer not found, invalid CIDR)
   were wrapped as internal_error (-32603). Now uses invalid_params
   (-32602) since these are user-input validation failures.

3. API-initiated peer disconnect produced no log line. Added info-level
   log in do_disconnect so the event appears in the ring buffer.

Also: route display uses "via" instead of Unicode arrow, stale
subcommand references removed from help text.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: maxholman

crates/cli/src/bin/wallhack.rs

@@ -7,8 +7,7 @@
 //! Invoked as `wallhack` (no args, slim build): starts daemon engine directly.
 //! Invoked as `wallhack --connect HOST [...]`: daemon in auto-negotiated mode.
 //! Invoked as `wallhack --listen ADDR [...]`: daemon in auto-negotiated mode.
-//! Invoked as `wallhack --role ROLE [...]`: daemon with fixed role hint.
-//! Invoked as `wallhack entry/exit/relay [...]`: daemon with explicit role override.
+//! Invoked as `wallhack --role ROLE [...]`: daemon with fixed role override.
 //! Invoked as `wallhackd` (or `wallhack daemon`): daemon engine directly.
 //! Invoked as `wallhackctl`: IPC control client only; fails if daemon not running.
 //! Invoked as `wallhack <control-subcommand>`: IPC control client.

crates/cli/src/daemon_cli.rs

@@ -24,8 +24,8 @@ use wallhackd::{
 
 /// Network pivoting and tunneling tool.
 ///
-/// Auto-negotiates role from `--connect` / `--listen` flags. Use a subcommand
-/// (`entry`, `exit`, `relay`) to override with an explicit role.
+/// Auto-negotiates role from `--connect` / `--listen` flags. Use `--role` to
+/// override with an explicit role.
 #[allow(clippy::struct_excessive_bools)] // Independent CLI flags, not related state
 #[derive(FromArgs, Debug, Clone)]
 pub struct WallhackCli {

crates/core/src/control/handler.rs

@@ -363,6 +363,7 @@ impl Handler {
     fn do_disconnect(&self, id: &str) {
         self.peers.send_disconnect(id, "disconnected by API");
         let _ = self.peers.unregister(id);
+        tracing::info!("Peer disconnected: {id} (via API)");
     }
 }
 

crates/daemon/src/netlink.rs

@@ -144,10 +144,10 @@ pub(crate) fn add_os_route(cidr: &str, dev: &str) -> Result<(), String> {
 
 /// Receive and check the Netlink ACK/error response.
 ///
-/// `NLMSG_ERROR` (type 2) carries a 4-byte `i32` error code at the start of its
-/// payload. Error 0 = success (pure ACK), negative = errno.
-/// `-3` (ESRCH) after route delete and `-17` (EEXIST) after route add are
-/// treated as success (idempotent operations).
+/// neli 0.7 parses `NLMSG_ERROR` into `NlPayload::Ack` (error=0) or
+/// `NlPayload::Err` (error<0). We check the error code and treat
+/// `-3` (ESRCH, already gone) and `-17` (EEXIST, already present) as
+/// success for idempotent route operations.
 fn recv_netlink_ack(socket: &mut NlSocketHandle, op: &str) -> Result<(), String> {
     let (mut iter, _groups) = socket
         .recv::<u16, neli::types::Buffer>()
@@ -158,28 +158,20 @@ fn recv_netlink_ack(socket: &mut NlSocketHandle, op: &str) -> Result<(), String>
     };
     let msg = msg_result.map_err(|e| format!("Netlink recv error: {e}"))?;
 
-    // NLMSG_ERROR = 2
-    if *msg.nl_type() == 2 {
-        if let NlPayload::Payload(buf) = msg.nl_payload() {
-            let bytes: &[u8] = buf.as_ref();
-            if bytes.len() >= 4 {
-                let error = i32::from_ne_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]);
-                // 0 = success, -3 = ESRCH (already gone), -17 = EEXIST (already present)
-                if error == 0 || error == -3 || error == -17 {
-                    Ok(())
-                } else {
-                    let err_msg = format!("Netlink error: {error}");
-                    tracing::warn!("Failed to {op}: {err_msg}");
-                    Err(err_msg)
-                }
+    match msg.nl_payload() {
+        NlPayload::Ack(_) | NlPayload::Empty => Ok(()),
+        NlPayload::Err(err) => {
+            let code = *err.error();
+            // -3 = ESRCH (route already gone), -17 = EEXIST (route already present)
+            if code == -3 || code == -17 {
+                Ok(())
             } else {
-                Err("Netlink ACK payload too short".into())
+                let err_msg = format!("Netlink error: {code}");
+                tracing::warn!("Failed to {op}: {err_msg}");
+                Err(err_msg)
             }
-        } else {
-            Err("Unexpected payload in ACK".into())
         }
-    } else {
-        Err(format!("Unexpected message type: {}", msg.nl_type()))
+        _ => Err(format!("Unexpected Netlink response for {op}")),
     }
 }
 

crates/mcp/src/convert.rs

@@ -67,7 +67,7 @@ pub fn format_response(resp: &ManagementResponse) -> Result<String, String> {
             let mut out = String::new();
             for route in &r.routes {
                 let tag = if route.auto_managed { " (auto)" } else { "" };
-                let _ = writeln!(out, "{} → {}{tag}", route.cidr, route.peer);
+                let _ = writeln!(out, "{} via {}{tag}", route.cidr, route.peer);
             }
             Ok(out)
         }

crates/mcp/src/tools.rs

@@ -64,7 +64,7 @@ async fn ipc_call(request: management_request::Request) -> Result<String, rmcp::
         .await
         .map_err(|e| rmcp::ErrorData::internal_error(e.to_string(), None))?;
 
-    convert::format_response(&resp).map_err(|msg| rmcp::ErrorData::internal_error(msg, None))
+    convert::format_response(&resp).map_err(|msg| rmcp::ErrorData::invalid_params(msg, None))
 }
 
 #[rmcp::tool_router(vis = "pub")]

uat/2026-03-20.md

@@ -0,0 +1,89 @@
+# UAT Report — 2026-03-20
+
+## Session Summary
+- **Range**: Full 27-VM cyberrange (6 networks, perimeter→office→datacenter→management→vault)
+- **Persona**: CTF Player transitioning to Pentester critique
+- **Objective**: Validate logs feature (v0.13.0), tunnel connectivity, auto-routing, and MCP tool UX
+- **Outcome**: Logs feature works end-to-end. Single-hop pivot to office network confirmed. Found 2 bugs (neli ACK parsing, missing disconnect log) and 1 MCP error code issue.
+
+## Pontoon MCP — Findings
+
+### Tool Completeness
+Sufficient for range lifecycle and VM interaction. `range_up`, `vm_exec`, `vm_cp`, `vm_exec_bg`, `vm_tail` covered all needs.
+
+### UX & Discoverability
+Tool names are clear and consistent. `vm_port_probe` runs from the host, not inside the VM — this caused initial confusion (probed office network from host, got "closed" because the host isn't routed through the tunnel). Not a bug, but worth documenting.
+
+### Specific Issues
+| Severity | Finding | Suggested Fix |
+|----------|---------|---------------|
+| 🟢 Suggestion | `vm_port_probe` description doesn't clarify it runs from the host perspective, not inside the VM | Add "(from the host)" to description |
+
+## Wallhack MCP — Findings
+
+### Tool Completeness
+All management operations covered: info, peers, routes, stats, logs, connect, listen, disconnect, route_add, route_del, hint_set, hint_set_auto, shutdown. The new `logs` tool fills the observability gap — can now diagnose peer connections, route installation, and errors without SSH access.
+
+### Logging Quality
+- **Startup**: clear and informative (version, capabilities, listen addr, PSK warning)
+- **Peer lifecycle**: connect events logged with name, addr, TUN interface, route count
+- **Missing**: peer disconnect events not logged at info level — only connects appear in the ring buffer
+- **Verbosity**: right level for info — no data-plane noise, just lifecycle and errors
+
+### Terminology Consistency
+No issues found in this session. Role names (entry/exit), peer/route terminology all consistent.
+
+### UX & Workflow
+- Auto-routing on connect is excellent — office network was immediately reachable after gateway-perimeter connected
+- Route warning when adding a CIDR not advertised by the peer is helpful and actionable
+- `hint_set` tool name is confusing for first-time users — "hint" is protocol jargon. The REPL uses `role` which is much clearer. MCP should match.
+
+### Error Messages
+- "peer not found: nonexistent" — clear message, but wrapped as MCP internal_error (-32603) instead of invalid_params (-32602). All daemon `ErrorResponse` messages are mapped to internal_error regardless of cause.
+
+### Specific Issues
+| Severity | Finding | Suggested Fix |
+|----------|---------|---------------|
+| 🔴 Critical | neli 0.7 ACK parsing broken — `recv_netlink_ack` returns "Unexpected payload in ACK" for route add/remove OS operations. Kernel routes silently fail to install for manually added routes. Auto-routes may also be affected. | Fix the raw buffer parsing in `recv_netlink_ack` — the 0.7 `Buffer` payload may include the full nlmsghdr, not just the error code. Compare with neli 0.6 behavior. |
+| 🟡 Minor | Peer disconnect events not captured in log ring buffer — connects show "Peer connected: ..." but disconnects produce no corresponding info-level log line | Add `info!("Peer disconnected: name={name}")` in the disconnect path |
+| 🟡 Minor | MCP error codes: all daemon errors mapped to `-32603` (internal_error) regardless of cause. "peer not found" and "invalid CIDR" should be `-32602` (invalid_params) | Map `ErrorResponse` codes to appropriate MCP error types in `convert::format_response` |
+| 🟡 Minor | `hint_set` / `hint_set_auto` MCP tool names don't match REPL (`role` command). Per interface parity, MCP should expose `role` not `hint` | Rename MCP tools or add `role` as the primary name |
+| 🟢 Suggestion | `logs` output could include timestamps for each line (currently just `level: message`) | Prepend compact timestamp to each buffered line |
+
+## Cross-Cutting Observations
+
+- **Hot-patching workflow works but is manual** — kill old process, cp binary, restart with correct flags. The initrd binary is v0.11.0 while we're testing v0.13.0. A `pontoon build` + `range_up` cycle would be cleaner but slower.
+- **CLI changed between initrd (subcommand: `wallhack entry`) and current (flag: `wallhack --role entry`)** — the clusterfuck merge changed the CLI surface. Help text says "Use a subcommand" but they're actually flags now. This will confuse users who read old docs.
+- **The logs feature delivered exactly what was needed** — during this session, `logs` was the primary diagnostic tool for understanding peer connections, route installation, and error conditions. Without it, debugging would have required SSH + reading stderr.
+
+## Session Transcript (Condensed)
+
+▶ Brought range up (27 VMs)
+  → All VMs ready. Wallhack daemon auto-started on attacker (entry) and gateway-perimeter (exit).
+
+▶ `wallhack info` via MCP
+  → Connected to attacker daemon (v0.11.0 — stale initrd binary)
+
+▶ `wallhack logs` on stale binary
+  → "empty request" — logs feature doesn't exist in v0.11.0. Expected.
+
+▶ Hot-patched attacker and gateway-perimeter with v0.13.0 binary
+  → Restarted both daemons with `--role entry/exit` flags
+
+▶ `wallhack logs` on v0.13.0
+  → Returns startup lines, peer connect events, route installation. Feature works.
+
+▶ Curled intranet (10.99.2.80) from attacker through tunnel
+  → Page returned with DB creds. Full tunnel chain working.
+
+▶ `route_add` with nonexistent peer
+  → "peer not found: nonexistent" — clear message, wrong MCP error code (-32603 vs -32602)
+
+▶ `route_add` for CIDR not advertised by peer
+  → "OK" with warning about unreachable destination. Excellent UX.
+  → Bug: OS route operations fail with "Unexpected payload in ACK" (neli 0.7 parsing)
+
+▶ Disconnected web-filter peer
+  → Disconnect succeeded, peer removed from list, but no log line captured for the event
+
+▶ Tunnel still functional after disconnect — office network still reachable