a73x

ddbd08e2

feat: -A with no agent is refused, not kept silently

a73x   2026-08-21 08:33

Commit message
feat: -A with no agent is refused, not kept silently

The offer is a declaration, not a capability: a client that typed -A but
has no agent behind it offered anyway, refused every dial the session
made without a word, and — being an offerer — could out-rank a second -A
client that WOULD have answered. The user met it three layers down as
'permission denied (publickey)' from a git remote.

Dial the socket before attaching and exit 2 if nothing answers. Dial
rather than read the variable: a path left behind by a dead agent is the
ordinary case, and it is the one a set-ness check would pass.

Found by running it: the branch's own e2e had no leg for a client with no
agent, because the suite's agent legs always start one first.
README.md
Old New
@@ -178,6 +178,13 @@ and every client would share one identity — but it does mean local sessions
178 need `mux -A --sock PATH` (or `mux -A` on the auto-started daemon) where 178 need `mux -A --sock PATH` (or `mux -A` on the auto-started daemon) where
179 they previously needed nothing. 179 they previously needed nothing.
180 180
181 `-A` with no agent running is a usage error, not a silent no-op: `mux`
182 dials `$SSH_AUTH_SOCK` before it attaches and exits 2 if nothing answers,
183 including the common case of a variable left behind by an agent that has
184 died. Without that check the flag was kept silently — the offer is a
185 declaration, not a capability — and the first sign was `permission denied
186 (publickey)` from a git remote inside the session.
187
181 `-A` goes on the attach form (`mux -A HOST`, `mux -A quic://HOST`, 188 `-A` goes on the attach form (`mux -A HOST`, `mux -A quic://HOST`,
182 `mux -A --sock PATH`), not on `mux wall`, which refuses a flag where a 189 `mux -A --sock PATH`), not on `mux wall`, which refuses a flag where a
183 target belongs. Sibling tiles grown from an `-A` attach by chord (`Ctrl-\ c`, 190 target belongs. Sibling tiles grown from an `-A` attach by chord (`Ctrl-\ c`,
build.zig
Old New
@@ -287,7 +287,7 @@ const mod_table = [_]ModSpec{
287 // daemon downstream has to notice and refuse. Layer 5 since `mux wall` 287 // daemon downstream has to notice and refuse. Layer 5 since `mux wall`
288 // pulled in wallview (layer 4); wall rides along for the no-arg wall 288 // pulled in wallview (layer 4); wall rides along for the no-arg wall
289 // (the state file the browser hub builds). 289 // (the state file the browser hub builds).
290 .{ .name = "mux", .path = "src/mux_main.zig", .layer = 5, .link_libc = true, .imports = &.{ "client", "protocol", "xdg", "spawn", "handoff", "sockpath", "wallview", "wall" }, .quic_tests = true }, 290 .{ .name = "mux", .path = "src/mux_main.zig", .layer = 5, .link_libc = true, .imports = &.{ "client", "protocol", "xdg", "spawn", "handoff", "sockpath", "wallview", "wall" }, .test_imports = &.{"testtmp"}, .quic_tests = true },
291 }; 291 };
292 292
293 /// Comptime row lookup. Every hand-written module name in this file goes 293 /// Comptime row lookup. Every hand-written module name in this file goes
@@ -636,10 +636,11 @@ pub fn build(b: *std.Build) void {
636 mux_mod.addImport("build_options", version_opts.createModule()); 636 mux_mod.addImport("build_options", version_opts.createModule());
637 exe_mod.addImport("build_options", version_opts.createModule()); 637 exe_mod.addImport("build_options", version_opts.createModule());
638 webhub_main_mod.addImport("build_options", version_opts.createModule()); 638 webhub_main_mod.addImport("build_options", version_opts.createModule());
639 // exe is the one hand-wired row that owns a test twin, and build_options 639 // A row with test_imports gets a SEPARATE module for its test twin, and
640 // is outside the table's jurisdiction — so the twin needs it by hand, or 640 // build_options is outside the table's jurisdiction — so every such twin
641 // main.zig's tests lose the version their argument parser prints. 641 // needs it by hand, or the argument parsers lose the version they print.
642 test_mods[comptime idxOf("exe")].addImport("build_options", version_opts.createModule()); 642 test_mods[comptime idxOf("exe")].addImport("build_options", version_opts.createModule());
643 test_mods[comptime idxOf("mux")].addImport("build_options", version_opts.createModule());
643 644
644 const exe = b.addExecutable(.{ .name = "muxd", .root_module = exe_mod }); 645 const exe = b.addExecutable(.{ .name = "muxd", .root_module = exe_mod });
645 // Zig 0.15's self-hosted x86_64 linker can't handle the .sframe 646 // Zig 0.15's self-hosted x86_64 linker can't handle the .sframe
src/mux_main.zig
Old New
@@ -20,6 +20,7 @@ const handoff = @import("handoff");
20 const sockpath = @import("sockpath"); 20 const sockpath = @import("sockpath");
21 const wallview = @import("wallview"); 21 const wallview = @import("wallview");
22 const wall = @import("wall"); 22 const wall = @import("wall");
23 const TmpDir = @import("testtmp").TmpDir;
23 24
24 const usage = 25 const usage =
25 \\usage: mux [HOST | --sock PATH | --via CMD | quic://HOST[:PORT]] 26 \\usage: mux [HOST | --sock PATH | --via CMD | quic://HOST[:PORT]]
@@ -77,6 +78,17 @@ const ParseResult = union(enum) {
77 usage_error, 78 usage_error,
78 }; 79 };
79 80
81 /// Whether an ssh-agent is actually there to forward. A dial and a close:
82 /// the agent is a unix socket on this box, so the question has a cheap
83 /// definite answer and nothing downstream has to guess from a variable
84 /// being merely set — a stale `SSH_AUTH_SOCK` left by a dead agent is the
85 /// ordinary case, not an exotic one.
86 fn agentReachable(path: []const u8) bool {
87 const fd = client.connectAgent(path) orelse return false;
88 std.posix.close(fd);
89 return true;
90 }
91
80 /// The environment variable consulted when `--key` is absent. Named rather 92 /// The environment variable consulted when `--key` is absent. Named rather
81 /// than inlined because the parse cannot read it — the parse stays pure so 93 /// than inlined because the parse cannot read it — the parse stays pure so
82 /// it stays testable — and `main` has to use exactly the same name. 94 /// it stays testable — and `main` has to use exactly the same name.
@@ -237,6 +249,38 @@ pub fn main() !u8 {
237 return wallMain(alloc, args[2..]); 249 return wallMain(alloc, args[2..]);
238 250
239 const parsed = parseArgs(args, std.posix.getenv(key_env)); 251 const parsed = parseArgs(args, std.posix.getenv(key_env));
252
253 // `-A` is a promise, and a client with no agent behind it cannot keep
254 // one. Left to attach, it offers anyway — the offer is a declaration,
255 // not a capability — so every dial the session makes is refused in
256 // silence, and on a session with a second `-A` client it can out-rank
257 // one that WOULD have answered. Refusing here says so once, at the
258 // altitude the flag was typed at, instead of surfacing three layers
259 // down as `permission denied (publickey)` from a git remote.
260 const wants_agent = switch (parsed) {
261 .host => |h| h.agent,
262 .quic => |q| q.agent,
263 .attach => |at| at.agent,
264 else => false,
265 };
266 if (wants_agent) {
267 const sock = std.posix.getenv(proto.agent_sock_env) orelse "";
268 if (!agentReachable(sock)) {
269 if (sock.len == 0) {
270 std.debug.print(
271 "mux: -A: " ++ proto.agent_sock_env ++ " is not set — no ssh-agent to forward\n",
272 .{},
273 );
274 } else {
275 std.debug.print(
276 "mux: -A: no ssh-agent answering at {s}\n",
277 .{sock},
278 );
279 }
280 return 2;
281 }
282 }
283
240 switch (parsed) { 284 switch (parsed) {
241 .version => { 285 .version => {
242 var vbuf: [64]u8 = undefined; 286 var vbuf: [64]u8 = undefined;
@@ -810,3 +854,25 @@ test "insideThisSession: only the exact socket-and-session pair is the loop" {
810 test { 854 test {
811 std.testing.refAllDeclsRecursive(@This()); 855 std.testing.refAllDeclsRecursive(@This());
812 } 856 }
857
858 test "agentReachable: a live socket answers, a stale or unset path does not" {
859 // The three states a user is actually in: an agent running, a variable
860 // pointing at an agent that has died, and no variable at all. Only the
861 // first may attach with `-A`.
862 var tmp = try TmpDir.make();
863 defer tmp.cleanup();
864 var buf: [128]u8 = undefined;
865 const sock = try std.fmt.bufPrintZ(&buf, "{s}/agent.sock", .{tmp.path()});
866 const addr = try std.net.Address.initUnix(sock);
867 var listener = try addr.listen(.{});
868
869 try std.testing.expect(agentReachable(sock));
870
871 // The stale case, which is why this dials rather than reading the
872 // variable: the path is still spelled, the socket file may even still
873 // be there, and nothing is listening.
874 listener.deinit();
875 std.fs.deleteFileAbsolute(sock) catch {};
876 try std.testing.expect(!agentReachable(sock));
877 try std.testing.expect(!agentReachable(""));
878 }
test/e2e.sh
Old New
@@ -7040,6 +7040,56 @@ set -e
7040 cat "$OUT.agt.log"; exit 1; } 7040 cat "$OUT.agt.log"; exit 1; }
7041 ok "agent forwarding: ssh-add -l in the session lists the client's key" 7041 ok "agent forwarding: ssh-add -l in the session lists the client's key"
7042 7042
7043 # --- ...and `-A` with nothing behind it is refused before the attach ------
7044 #
7045 # The flag is a promise the client cannot keep with no agent running, and it
7046 # used to be kept silently: the offer is a declaration rather than a
7047 # capability, so every dial the session made was refused without a word and
7048 # the user met it three layers down as `permission denied (publickey)` from
7049 # a git remote. Refused at usage-error altitude now, exit 2, before the
7050 # transport is touched.
7051 #
7052 # Both spellings of "no agent", because reading the variable would only
7053 # catch the first: unset, and set to a path nothing is listening on — the
7054 # ordinary leftover of an agent that has died.
7055 set +e
7056 env -u SSH_AUTH_SOCK "$MUX" -A --sock "$SOCK48" > "$OUT.anoag" 2>&1
7057 RC=$?
7058 set -e
7059 [ "$RC" -eq 2 ] || {
7060 echo "e2e FAIL: agent: -A with no SSH_AUTH_SOCK exited $RC, want 2:"
7061 cat "$OUT.anoag"; exit 1; }
7062 grep -q "is not set" "$OUT.anoag" || {
7063 echo "e2e FAIL: agent: -A with no SSH_AUTH_SOCK refused for another reason:"
7064 cat "$OUT.anoag"; exit 1; }
7065
7066 set +e
7067 SSH_AUTH_SOCK="$OUT.dead-agent.sock" "$MUX" -A --sock "$SOCK48" > "$OUT.anoag2" 2>&1
7068 RC=$?
7069 set -e
7070 [ "$RC" -eq 2 ] || {
7071 echo "e2e FAIL: agent: -A at a dead agent path exited $RC, want 2:"
7072 cat "$OUT.anoag2"; exit 1; }
7073 grep -q "no ssh-agent answering at" "$OUT.anoag2" || {
7074 echo "e2e FAIL: agent: -A at a dead agent path refused for another reason:"
7075 cat "$OUT.anoag2"; exit 1; }
7076
7077 # The control: the SAME command with the SAME live agent the leg above used
7078 # gets PAST the preflight, so the two refusals are about the agent and not
7079 # about the flag being rejected outright. Bounded and its exit ignored —
7080 # past the preflight it goes on to attach for real, which without a
7081 # terminal is neither a pass nor a failure, only slow.
7082 set +e
7083 SSH_AUTH_SOCK="$AGENT48" timeout 5 "$MUX" -A --sock "$SOCK48" > "$OUT.anoag3" 2>&1
7084 set -e
7085 # `if`, not `grep && {...}`: under `set -e` an AND-list ending in a failed
7086 # grep takes the whole suite down, and here a failed grep is the PASS.
7087 if grep -q "no ssh-agent" "$OUT.anoag3"; then
7088 echo "e2e FAIL: agent: a REACHABLE agent was still refused by the preflight:"
7089 cat "$OUT.anoag3"; exit 1
7090 fi
7091 ok "agent forwarding: -A with no agent is a usage error, not a silent no-op"
7092
7043 # --- ...and a session nobody offered an agent to refuses, fast ------------- 7093 # --- ...and a session nobody offered an agent to refuses, fast -------------
7044 # 7094 #
7045 # A fresh daemon on the freed path, and the spawn is load-bearing: the leg 7095 # A fresh daemon on the freed path, and the spawn is load-bearing: the leg
@@ -7398,9 +7448,12 @@ DPID=""
7398 # comparison speaks to neither. The 58th is the two-agent flip, and no 7448 # comparison speaks to neither. The 58th is the two-agent flip, and no
7399 # convergence point because its subject is which of two clients' terminals 7449 # convergence point because its subject is which of two clients' terminals
7400 # a fingerprint was answered FOR — a difference the two grids do not carry, 7450 # a fingerprint was answered FOR — a difference the two grids do not carry,
7401 # since both replicate the same session and hold both answers alike. 7451 # since both replicate the same session and hold both answers alike. The
7402 [ "$OK_COUNT" = "58" ] || { 7452 # 59th is the `-A` preflight, and no convergence point because it never
7403 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 58 —" 7453 # attaches: what it asserts on is an exit code and a message, before any
7454 # transport exists to converge.
7455 [ "$OK_COUNT" = "59" ] || {
7456 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 59 —"
7404 echo " a scenario was added (update the pin) or silently lost" 7457 echo " a scenario was added (update the pin) or silently lost"
7405 exit 1 7458 exit 1
7406 } 7459 }
@@ -7408,4 +7461,4 @@ DPID=""
7408 echo "e2e FAIL: $CONV_COUNT convergence points ran, the pin says 35" 7461 echo "e2e FAIL: $CONV_COUNT convergence points ran, the pin says 35"
7409 exit 1 7462 exit 1
7410 } 7463 }
7411 echo "e2e OK (58 scenarios, 35 convergence points)" 7464 echo "e2e OK (59 scenarios, 35 convergence points)"