68e7a144
fix: error audit — say what happened, never guess what a lower layer named
a73x 2026-08-10 18:06
Commit message
src/client.zig
| Old | New | ||
|---|---|---|---|
| @@ -56,6 +56,11 @@ const reconnect_grace_ms: i64 = 5000; | |||
| 56 | /// muxd lost" claims one existed and blames the wrong end. Once any frame | 56 | /// muxd lost" claims one existed and blames the wrong end. Once any frame |
| 57 | /// has arrived the old wording is the true one, so it stays. | 57 | /// has arrived the old wording is the true one, so it stays. |
| 58 | /// | 58 | /// |
| 59 | /// Which of those causes it was is not this line's to say: the transport | ||
| 60 | /// command's own stderr passes through untouched and has already named it. | ||
| 61 | /// So the message states only what is certain — the command failed, no | ||
| 62 | /// session ever started — and leaves the cause to the layer that saw it. | ||
| 63 | /// | ||
| 59 | /// `session_epoch` is the signal because it is set from the first snapshot | 64 | /// `session_epoch` is the signal because it is set from the first snapshot |
| 60 | /// and never reset; callers before the first read pass 0, which is what it | 65 | /// and never reset; callers before the first read pass 0, which is what it |
| 61 | /// is at that point by construction. | 66 | /// is at that point by construction. |
| @@ -64,7 +69,7 @@ const reconnect_grace_ms: i64 = 5000; | |||
| 64 | /// has no command to have failed, and saying so there would be its own lie. | 69 | /// has no command to have failed, and saying so there would be its own lie. |
| 65 | fn lostMsg(via: ?[]const u8, session_epoch: u64) []const u8 { | 70 | fn lostMsg(via: ?[]const u8, session_epoch: u64) []const u8 { |
| 66 | if (via != null and session_epoch == 0) | 71 | if (via != null and session_epoch == 0) |
| 67 | return "mux: transport command failed before connecting (is muxd installed on the host?)"; | 72 | return "mux: transport command failed before a session started"; |
| 68 | return "mux: connection to muxd lost"; | 73 | return "mux: connection to muxd lost"; |
| 69 | } | 74 | } |
| 70 | 75 | ||
| @@ -435,7 +440,11 @@ pub fn attach( | |||
| 435 | } else if (via) |cmd| { | 440 | } else if (via) |cmd| { |
| 436 | std.debug.print("mux: cannot start --via command: {s}\n", .{cmd}); | 441 | std.debug.print("mux: cannot start --via command: {s}\n", .{cmd}); |
| 437 | } else { | 442 | } else { |
| 438 | std.debug.print("mux: cannot connect to {s} (is muxd running?)\n", .{sock_path.?}); | 443 | // No "is muxd running?": auto-start checked that moments ago. |
| 444 | // Reaching here means a daemon answered the probe (or was just | ||
| 445 | // spawned) and then vanished before this connect — the path is | ||
| 446 | // the whole of what we know, so the path is all we say. | ||
| 447 | std.debug.print("mux: cannot connect to {s}\n", .{sock_path.?}); | ||
| 439 | } | 448 | } |
| 440 | return 1; | 449 | return 1; |
| 441 | }; | 450 | }; |
| @@ -1900,9 +1909,12 @@ test "parseQuicAddr: no port means 4433, explicit port wins" { | |||
| 1900 | } | 1909 | } |
| 1901 | 1910 | ||
| 1902 | test "lostMsg: only a --via transport that never connected gets the new wording" { | 1911 | test "lostMsg: only a --via transport that never connected gets the new wording" { |
| 1903 | // The case the message exists for: a command that failed to start. | 1912 | // The case the message exists for: a command that failed to start. It |
| 1913 | // names what happened and guesses no cause — ssh's own stderr passes | ||
| 1914 | // through and has already named the real one (host key, DNS, refused, | ||
| 1915 | // no such binary). The lower layer spoke; this line must not talk over it. | ||
| 1904 | try std.testing.expectEqualStrings( | 1916 | try std.testing.expectEqualStrings( |
| 1905 | "mux: transport command failed before connecting (is muxd installed on the host?)", | 1917 | "mux: transport command failed before a session started", |
| 1906 | lostMsg("ssh box muxd proxy", 0), | 1918 | lostMsg("ssh box muxd proxy", 0), |
| 1907 | ); | 1919 | ); |
| 1908 | // Same transport, but a session existed — there WAS a connection, and | 1920 | // Same transport, but a session existed — there WAS a connection, and |
src/main.zig
| Old | New | ||
|---|---|---|---|
| @@ -229,6 +229,24 @@ pub fn main() !u8 { | |||
| 229 | try defaultSockPath(alloc); | 229 | try defaultSockPath(alloc); |
| 230 | defer alloc.free(sock_path); | 230 | defer alloc.free(sock_path); |
| 231 | 231 | ||
| 232 | // sun_path is 108 bytes including the NUL. Checked here, once, before | ||
| 233 | // any command acts: the alternative is a spawned daemon that can never | ||
| 234 | // answer and a 2s timeout story about a path that was doomed at parse. | ||
| 235 | // `--version` and `keygen` are dispatched from the switch below, i.e. | ||
| 236 | // after this point, so they are exempted by name rather than by order — | ||
| 237 | // neither touches the socket, and neither should be refused over it. | ||
| 238 | const uses_socket = switch (o.cmd) { | ||
| 239 | .version, .keygen => false, | ||
| 240 | else => true, | ||
| 241 | }; | ||
| 242 | if (uses_socket and sock_path.len > 107) { | ||
| 243 | std.debug.print( | ||
| 244 | "muxd: socket path too long ({d} bytes, max 107): {s}\n", | ||
| 245 | .{ sock_path.len, sock_path }, | ||
| 246 | ); | ||
| 247 | return 1; | ||
| 248 | } | ||
| 249 | |||
| 232 | switch (o.cmd) { | 250 | switch (o.cmd) { |
| 233 | // The socket path resolved above is unused here and harmless: asking | 251 | // The socket path resolved above is unused here and harmless: asking |
| 234 | // a binary its version must work with no daemon and no runtime dir. | 252 | // a binary its version must work with no daemon and no runtime dir. |
| @@ -371,7 +389,7 @@ fn run(alloc: std.mem.Allocator, o: Opts, sock_path: []const u8) !u8 { | |||
| 371 | return 1; | 389 | return 1; |
| 372 | }, | 390 | }, |
| 373 | error.SockPathNotASocket => { | 391 | error.SockPathNotASocket => { |
| 374 | std.debug.print("muxd: {s} exists and is not a socket\n", .{sock_path}); | 392 | std.debug.print("muxd: {s} exists and is not a socket (move it, or name another with --sock)\n", .{sock_path}); |
| 375 | return 1; | 393 | return 1; |
| 376 | }, | 394 | }, |
| 377 | else => return err, | 395 | else => return err, |
| @@ -397,7 +415,7 @@ pub fn defaultSockPath(alloc: std.mem.Allocator) ![]const u8 { | |||
| 397 | 415 | ||
| 398 | fn dump(alloc: std.mem.Allocator, sock_path: []const u8, vt_mode: bool) !u8 { | 416 | fn dump(alloc: std.mem.Allocator, sock_path: []const u8, vt_mode: bool) !u8 { |
| 399 | const stream = std.net.connectUnixSocket(sock_path) catch { | 417 | const stream = std.net.connectUnixSocket(sock_path) catch { |
| 400 | std.debug.print("muxd dump: cannot connect to {s} (is `muxd run` running?)\n", .{sock_path}); | 418 | std.debug.print("muxd dump: cannot connect to {s} (no daemon; `muxd start` starts one)\n", .{sock_path}); |
| 401 | return 1; | 419 | return 1; |
| 402 | }; | 420 | }; |
| 403 | defer stream.close(); | 421 | defer stream.close(); |
| @@ -415,7 +433,7 @@ fn dump(alloc: std.mem.Allocator, sock_path: []const u8, vt_mode: bool) !u8 { | |||
| 415 | 433 | ||
| 416 | fn stats(alloc: std.mem.Allocator, sock_path: []const u8) !u8 { | 434 | fn stats(alloc: std.mem.Allocator, sock_path: []const u8) !u8 { |
| 417 | const stream = std.net.connectUnixSocket(sock_path) catch { | 435 | const stream = std.net.connectUnixSocket(sock_path) catch { |
| 418 | std.debug.print("muxd stats: cannot connect to {s} (is `muxd run` running?)\n", .{sock_path}); | 436 | std.debug.print("muxd stats: cannot connect to {s} (no daemon; `muxd start` starts one)\n", .{sock_path}); |
| 419 | return 1; | 437 | return 1; |
| 420 | }; | 438 | }; |
| 421 | defer stream.close(); | 439 | defer stream.close(); |
| @@ -533,7 +551,7 @@ fn startCmd(alloc: std.mem.Allocator, sock_path: []const u8, forwarded: []const | |||
| 533 | }; | 551 | }; |
| 534 | if (r == .already_running) { | 552 | if (r == .already_running) { |
| 535 | std.debug.print( | 553 | std.debug.print( |
| 536 | "muxd: already running on {s} (stop it first if you meant different flags)\n", | 554 | "muxd: already running on {s} (`muxd stop` it first if you meant different flags)\n", |
| 537 | .{sock_path}, | 555 | .{sock_path}, |
| 538 | ); | 556 | ); |
| 539 | } | 557 | } |
test/e2e.sh
| Old | New | ||
|---|---|---|---|
| @@ -455,7 +455,7 @@ set +e | |||
| 455 | VRC=$? | 455 | VRC=$? |
| 456 | set -e | 456 | set -e |
| 457 | [ "$VRC" = "1" ] || { echo "e2e FAIL: dead --via exit $VRC, want 1"; exit 1; } | 457 | [ "$VRC" = "1" ] || { echo "e2e FAIL: dead --via exit $VRC, want 1"; exit 1; } |
| 458 | grep -q "transport command failed before connecting" "$OUT.via" || { | 458 | grep -q "transport command failed before a session started" "$OUT.via" || { |
| 459 | echo "e2e FAIL: --via death message:"; cat "$OUT.via"; exit 1; } | 459 | echo "e2e FAIL: --via death message:"; cat "$OUT.via"; exit 1; } |
| 460 | grep -q "connection to muxd lost" "$OUT.via" && { | 460 | grep -q "connection to muxd lost" "$OUT.via" && { |
| 461 | echo "e2e FAIL: the old lie is still printed"; cat "$OUT.via"; exit 1; } | 461 | echo "e2e FAIL: the old lie is still printed"; cat "$OUT.via"; exit 1; } |
| @@ -480,9 +480,16 @@ set -e | |||
| 480 | # above — a transport command that died before carrying a frame — so it now | 480 | # above — a transport command that died before carrying a frame — so it now |
| 481 | # gets the honest message. What this scenario is FOR is the exit code above | 481 | # gets the honest message. What this scenario is FOR is the exit code above |
| 482 | # (1, never 124); the diagnostic is asserted so the exit is not a silent one. | 482 | # (1, never 124); the diagnostic is asserted so the exit is not a silent one. |
| 483 | grep -q "transport command failed before connecting" "$OUT.dead" || { | 483 | grep -q "transport command failed before a session started" "$OUT.dead" || { |
| 484 | echo "e2e FAIL: dead first transport lost its diagnostic; got:"; cat "$OUT.dead"; exit 1; | 484 | echo "e2e FAIL: dead first transport lost its diagnostic; got:"; cat "$OUT.dead"; exit 1; |
| 485 | } | 485 | } |
| 486 | # The can-fail control for the M13 reword: a grep for the new wording passes | ||
| 487 | # just as well if BOTH lines are printed, so the old one must be absent from | ||
| 488 | # the very capture that just satisfied it. The dropped parenthetical guessed | ||
| 489 | # a cause the transport's own stderr had already named. | ||
| 490 | grep -q "is muxd installed on the host" "$OUT.dead" && { | ||
| 491 | echo "e2e FAIL: old lostMsg wording still emitted alongside the new pin" | ||
| 492 | cat "$OUT.dead"; exit 1; } || true | ||
| 486 | rm -f "$OUT.dead" | 493 | rm -f "$OUT.dead" |
| 487 | 494 | ||
| 488 | # --- M7: aborting a reconnect exits cleanly. The client establishes a real | 495 | # --- M7: aborting a reconnect exits cleanly. The client establishes a real |