a73x

6f45c707

fix: askpass binds through serve; retire keeps its no-close shape

a73x   2026-08-31 21:58

Commit message
fix: askpass binds through serve; retire keeps its no-close shape

The prompt socket's bind was the third hand-rolled one — deleteFile, socket,
bind, listen — and its `retire` unlinked by name with no guard at all. The
name is `mux-ask-<pid>.sock`, and a pid comes round again: a client that got
ours back binds the same path, and the old wall's retire on the way out took
its socket, leaving that client's ssh prompting at nothing nobody would read.

`retire` cannot use `Bound.close`, and that is deliberate rather than an
oversight: it runs on a process about to `std.posix.exit` with detached pumps
live, one of which may be inside `declined` on this object. So it calls
`Bound.unlinkIfOurs` — the guard without the close — and `stop` keeps the
close. Spelled the other way the suite panics on the double close, which is
how the shape stays pinned.

New test: two Listeners at the same pid-named path, the first retires, and
the successor's socket is still there AND still answers a connect.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TWxBL1HBULH1ZwTNzzKTja
build.zig
Old New
@@ -220,7 +220,7 @@ const mod_table = [_]ModSpec{
220 // sits here rather than in either front so the CLI wall and the browser 220 // sits here rather than in either front so the CLI wall and the browser
221 // hub resolve a host line the same way. Nothing here WRITES that file — 221 // hub resolve a host line the same way. Nothing here WRITES that file —
222 // `wall_host.recordHost` and `webhub_main` do. 222 // `wall_host.recordHost` and `webhub_main` do.
223 .{ .name = "client", .path = "src/client/client.zig", .link_libc = true, .imports = &.{ "term", "quic", "xdg", "sockpath", "dial", "link" }, .test_imports = &.{"testtmp"}, .quic_tests = true }, 223 .{ .name = "client", .path = "src/client/client.zig", .link_libc = true, .imports = &.{ "term", "quic", "xdg", "sockpath", "serve", "dial", "link" }, .test_imports = &.{"testtmp"}, .quic_tests = true },
224 // ---- the two fronts ---- 224 // ---- the two fronts ----
225 // The browser hub's HTTP/WebSocket decisions: Origin gate, route table, 225 // The browser hub's HTTP/WebSocket decisions: Origin gate, route table,
226 // WS endpoint naming. Assets are injected (the exe root @embedFiles 226 // WS endpoint naming. Assets are injected (the exe root @embedFiles
src/client/askpass.zig
Old New
@@ -8,6 +8,9 @@
8 //! process, and `helperMain` is what `mux askpass` runs. Nothing here paints or 8 //! process, and `helperMain` is what `mux askpass` runs. Nothing here paints or
9 //! spawns, so the whole carriage drives from a test with no ssh and no wall. 9 //! spawns, so the whole carriage drives from a test with no ssh and no wall.
10 const std = @import("std"); 10 const std = @import("std");
11 // `serve_mod` and not `serve`: Listener has its own `serve` method, and
12 // inside the struct the bare name is ambiguous.
13 const serve_mod = @import("serve");
11 14
12 /// Env var naming the socket. The mode word for the helper, too: ssh execs 15 /// Env var naming the socket. The mode word for the helper, too: ssh execs
13 /// its helper with the prompt as argv[1] and nothing else, so there is no 16 /// its helper with the prompt as argv[1] and nothing else, so there is no
@@ -116,7 +119,10 @@ pub const Listener = struct {
116 119
117 alloc: std.mem.Allocator, 120 alloc: std.mem.Allocator,
118 path: []const u8, 121 path: []const u8,
119 fd: std.posix.socket_t, 122 /// The listening socket and the identity of the file it was bound to.
123 /// The id is what keeps `retire` from unlinking a SUCCESSOR's socket:
124 /// the name has this client's pid in it, and a pid comes round again.
125 bound: serve_mod.Bound,
120 /// How `stop` reaches a thread parked in `poll`. Closing the listening 126 /// How `stop` reaches a thread parked in `poll`. Closing the listening
121 /// fd under an accept is not defined to wake it; a byte here is. 127 /// fd under an accept is not defined to wake it; a byte here is.
122 stop_r: std.posix.fd_t, 128 stop_r: std.posix.fd_t,
@@ -148,18 +154,17 @@ pub const Listener = struct {
148 .{ runtime_dir, std.os.linux.getpid() }, 154 .{ runtime_dir, std.os.linux.getpid() },
149 ); 155 );
150 errdefer alloc.free(path); 156 errdefer alloc.free(path);
151 // A client that died without unlinking left a file, and a pid comes 157 // `clobber_own`: a client that died without unlinking left a file,
152 // round again. Nothing else may own this name: it has our pid in it. 158 // and a pid comes round again. Nothing else may own this name — it
153 std.fs.cwd().deleteFile(path) catch {}; 159 // has our pid in it. CLOEXEC because this process spawns the ssh
154 const addr = try std.net.Address.initUnix(path); 160 // that the prompt is FOR, and every command run inside a session
155 const fd = try std.posix.socket( 161 // below it; none of them may hold the socket that answers.
156 std.posix.AF.UNIX, 162 var bound = try serve_mod.bind(path, .{
157 std.posix.SOCK.STREAM | std.posix.SOCK.CLOEXEC, 163 .policy = .clobber_own,
158 0, 164 .backlog = backlog,
159 ); 165 .cloexec = true,
160 errdefer std.posix.close(fd); 166 });
161 try std.posix.bind(fd, &addr.any, addr.getOsSockLen()); 167 errdefer bound.close(path);
162 try std.posix.listen(fd, backlog);
163 const bell = try std.posix.pipe2(.{ .CLOEXEC = true }); 168 const bell = try std.posix.pipe2(.{ .CLOEXEC = true });
164 errdefer { 169 errdefer {
165 std.posix.close(bell[0]); 170 std.posix.close(bell[0]);
@@ -170,7 +175,7 @@ pub const Listener = struct {
170 self.* = .{ 175 self.* = .{
171 .alloc = alloc, 176 .alloc = alloc,
172 .path = path, 177 .path = path,
173 .fd = fd, 178 .bound = bound,
174 .stop_r = bell[0], 179 .stop_r = bell[0],
175 .stop_w = bell[1], 180 .stop_w = bell[1],
176 .wake = wake, 181 .wake = wake,
@@ -185,7 +190,7 @@ pub const Listener = struct {
185 pub fn stop(self: *Listener) void { 190 pub fn stop(self: *Listener) void {
186 self.retire(); 191 self.retire();
187 if (self.thread) |t| t.join(); 192 if (self.thread) |t| t.join();
188 std.posix.close(self.fd); 193 std.posix.close(self.bound.fd);
189 std.posix.close(self.stop_r); 194 std.posix.close(self.stop_r);
190 std.posix.close(self.stop_w); 195 std.posix.close(self.stop_w);
191 self.alloc.free(self.path); 196 self.alloc.free(self.path);
@@ -203,7 +208,11 @@ pub const Listener = struct {
203 self.cv.broadcast(); 208 self.cv.broadcast();
204 self.mu.unlock(); 209 self.mu.unlock();
205 _ = std.posix.write(self.stop_w, "x") catch {}; 210 _ = std.posix.write(self.stop_w, "x") catch {};
206 std.fs.cwd().deleteFile(self.path) catch {}; 211 // The guard without the close: a successor Listener in a process
212 // that got our pid back binds this same name, and unlinking by name
213 // here would take ITS socket and leave that client's ssh prompting
214 // at nothing.
215 self.bound.unlinkIfOurs(self.path);
207 } 216 }
208 217
209 /// The keyboard's side. Copies the pending prompt out and marks it 218 /// The keyboard's side. Copies the pending prompt out and marks it
@@ -265,13 +274,13 @@ pub const Listener = struct {
265 fn acceptLoop(self: *Listener) void { 274 fn acceptLoop(self: *Listener) void {
266 while (true) { 275 while (true) {
267 var pfds = [_]std.posix.pollfd{ 276 var pfds = [_]std.posix.pollfd{
268 .{ .fd = self.fd, .events = std.posix.POLL.IN, .revents = 0 }, 277 .{ .fd = self.bound.fd, .events = std.posix.POLL.IN, .revents = 0 },
269 .{ .fd = self.stop_r, .events = std.posix.POLL.IN, .revents = 0 }, 278 .{ .fd = self.stop_r, .events = std.posix.POLL.IN, .revents = 0 },
270 }; 279 };
271 _ = std.posix.poll(&pfds, -1) catch return; 280 _ = std.posix.poll(&pfds, -1) catch return;
272 if (pfds[1].revents != 0) return; 281 if (pfds[1].revents != 0) return;
273 if (pfds[0].revents & std.posix.POLL.IN == 0) continue; 282 if (pfds[0].revents & std.posix.POLL.IN == 0) continue;
274 const c = std.posix.accept(self.fd, null, null, std.posix.SOCK.CLOEXEC) catch continue; 283 const c = std.posix.accept(self.bound.fd, null, null, std.posix.SOCK.CLOEXEC) catch continue;
275 self.serve(c); 284 self.serve(c);
276 std.posix.close(c); 285 std.posix.close(c);
277 self.mu.lock(); 286 self.mu.lock();
@@ -981,6 +990,44 @@ test "askpass.Listener: the kind ssh named rides the wire, one byte ahead of the
981 } 990 }
982 } 991 }
983 992
993 test "askpass.Listener: retire takes only its own socket, never a successor's at the same pid-named path" {
994 const alloc = std.testing.allocator;
995 var tmp = try testtmp.TmpDir.make();
996 defer tmp.cleanup();
997 var counter: Counter = .{};
998
999 // Pid reuse, in one process: the name is `mux-ask-<pid>.sock` in the
1000 // runtime dir, so a second Listener here IS the client that got our pid
1001 // back. The first one's retire runs as the wall exits, after the second
1002 // has bound — and it must not take the name out from under it.
1003 const old_l = try Listener.start(alloc, tmp.path(), counter.hooks());
1004 const successor = try Listener.start(alloc, tmp.path(), counter.hooks());
1005 try std.testing.expectEqualStrings(old_l.path, successor.path);
1006 // Copied: `stop` frees the Listener and its path, and the last assertion
1007 // is about the name AFTER both are gone.
1008 const name = try alloc.dupe(u8, successor.path);
1009 defer alloc.free(name);
1010
1011 old_l.retire();
1012
1013 // Asked of the filesystem, and then of the socket: a file at the name is
1014 // not enough, because the point is that the ssh this successor spawned
1015 // can still reach the client that will answer it.
1016 const st = try std.posix.fstatat(std.posix.AT.FDCWD, successor.path, 0);
1017 try std.testing.expect(std.posix.S.ISSOCK(st.mode));
1018 const probe = try std.net.connectUnixSocket(successor.path);
1019 probe.close();
1020
1021 // retire closes nothing, so the first Listener's own descriptors are
1022 // still ours to release — `stop` does the join and the closes.
1023 old_l.stop();
1024 successor.stop();
1025 try std.testing.expectError(
1026 error.FileNotFound,
1027 std.posix.fstatat(std.posix.AT.FDCWD, name, 0),
1028 );
1029 }
1030
984 test { 1031 test {
985 std.testing.refAllDeclsRecursive(@This()); 1032 std.testing.refAllDeclsRecursive(@This());
986 } 1033 }