Fixes from three adversarial reviews, and a destructive one among them

The registry sweep could delete a live socket, anywhere on the filesystem. A
reviewer reproduced it: a socket that is bound but has not reached listen(2)
answers ECONNREFUSED exactly like a dead one -- that window is every server's
startup -- and the sweep then followed the entry's symlink and unlinked
whatever absolute path it named. It now follows a target only into the
directory our own sockets live in and only to a `pardes-9p-*.sock` name, it
re-probes immediately before deleting rather than trusting a probe that is by
then several syscalls old, and a readlink that exactly filled its buffer is
treated as the truncation it is. The test grew a case for an entry whose
target is not ours: the entry goes, the file does not.

Ctrl-V in raw tty mode was a black hole when the yank register was empty --
neither typed nor forwarded -- so vim's visual block, readline's quoted-insert
and every other program's Ctrl-V simply vanished. With nothing to paste the
chord belongs to the program again.

The lone-ESC flush added earlier was dead code. vaxis already returns Escape
for a one-byte 0x1b (`Parser.parseGround` asserts `input.len == 1`), so the
carried byte it waited for can never exist; a reviewer showed a 3 ms gap and a
60 ms gap behaving identically. Removed rather than left to imply a guarantee
it never provided.

A shell whose editor is gone can start one again. Naming a live but
unreachable session made `pardes <file>` exit 1, which let a stale environment
variable lock someone out of their own editor; it falls through to an ordinary
session, as it did before the variable existed.

Also: the macOS ABI check for `pardes_topbar_pane_border_px` had been replaced
by a duplicate of the line above it; `--startup` now fails on a leak the way
every other measurement in that file does, and stops calling its maximum a p95
below twenty samples; the served README and the skill no longer tell you to
write to `data` with `>`, which truncates the whole body before the write
lands; `docs/v9fs.md` described the allocate-on-walk design that was rejected;
and `test/fs.py` keys nesting off `PARDES_PID`, so its forwarding case stops
passing only when the runner happens to be inside a live pardes.

fs-test now reaches its one documented pre-existing failure instead of dying
early. Suite 778/783 with the two known crashes.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

1 files changed, 50 insertions, 9 deletions

diff --git a/src/9p_io.zig b/src/9p_io.zig
index 3d019e0d..7f0134ed 100644
--- a/src/9p_io.zig
+++ b/src/9p_io.zig
@@ -671,7 +671,7 @@ pub const Listener = struct {
             log.warn("registry post skipped: cannot create {s}", .{svc});
             return;
         }
-        sweepRegistry(l.io, svc);
+        sweepRegistry(l.io, svc, xdg);
         var entry_buf: [sun_path_len:0]u8 = undefined;
         const entry = std.fmt.bufPrintSentinel(&entry_buf, "{s}/{s}", .{ svc, name }, 0) catch {
             log.warn("registry post skipped: name too long: {s}", .{name});
@@ -903,7 +903,21 @@ fn probe(path: [:0]const u8) Probe {
 /// a definite refusal counts as gone. The socket a stale entry points
 /// at goes too, but not before a stat agrees it is a socket of ours: a
 /// plain file answers a connect with the same refusal.
-fn sweepRegistry(io: std.Io, svc: [:0]const u8) void {
+/// Is this the kind of path this editor is allowed to delete? A registry
+/// entry is a symlink we wrote, but its target is just bytes on disk that
+/// anyone could have pointed anywhere, so the sweep only ever follows one
+/// into the directory our own sockets live in, and only to a name of the
+/// shape we give them. Everything else gets its entry removed and its target
+/// left strictly alone.
+fn ourSocket(target: []const u8, sockets: []const u8) bool {
+    if (sockets.len == 0 or !std.mem.startsWith(u8, target, sockets)) return false;
+    if (target.len <= sockets.len or target[sockets.len] != '/') return false;
+    const base = target[sockets.len + 1 ..];
+    if (std.mem.indexOfScalar(u8, base, '/') != null) return false;
+    return std.mem.startsWith(u8, base, "pardes-9p-") and std.mem.endsWith(u8, base, ".sock");
+}
+
+fn sweepRegistry(io: std.Io, svc: [:0]const u8, sockets: []const u8) void {
     if (comptime !supported) return;
 
     // The names are staged before anything is unlinked, so the sweep
@@ -941,12 +955,22 @@ fn sweepRegistry(io: std.Io, svc: [:0]const u8) void {
         if (libc.unlink(entry) != 0) continue;
         reaped += 1;
         // A relative target would resolve against this editor's working
-        // directory, which says nothing about what the entry named.
+        // directory, which says nothing about what the entry named, and a
+        // readlink that exactly filled the buffer was truncated, so the path
+        // it produced is some other file's.
         if (state != .stale or n <= 0 or link_buf[0] != '/') continue;
+        if (@as(usize, @intCast(n)) >= link_buf.len) continue;
         var target_buf: [sun_path_len:0]u8 = undefined;
         const target = std.fmt.bufPrintSentinel(&target_buf, "{s}", .{link_buf[0..@intCast(n)]}, 0) catch continue;
+        if (!ourSocket(target, sockets)) continue;
         const t = statNoFollow(target) orelse continue;
-        if (t.mode & 0o170000 == 0o140000 and t.uid == libc.getuid()) _ = libc.unlink(target);
+        if (t.mode & 0o170000 != 0o140000 or t.uid != libc.getuid()) continue;
+        // Ask again, immediately before deleting. The first probe was of the
+        // ENTRY and is by now several syscalls old; a socket that is bound but
+        // has not reached listen(2) yet answers ECONNREFUSED exactly like a
+        // dead one, and that window is every server's startup.
+        if (probe(target) != .stale) continue;
+        _ = libc.unlink(target);
     }
     if (reaped != 0) log.info("reaped {d} stale registry entries under {s}", .{ reaped, svc });
 }
@@ -997,14 +1021,23 @@ test "the registry sweep takes the dead entries and leaves everything else" {
         return error.SkipZigTest;
     if (libc.mkdir(svc, 0o700) != 0) return error.SkipZigTest;
 
-    var paths: [5][sun_path_len:0]u8 = undefined;
-    const live_sock = try std.fmt.bufPrintSentinel(&paths[0], "{s}/live.sock", .{svc}, 0);
-    const dead_sock = try std.fmt.bufPrintSentinel(&paths[1], "{s}/dead.sock", .{svc}, 0);
+    // The sockets live where the real ones do -- beside the registry, not in
+    // it, and named the way a session names them -- because the sweep only
+    // follows an entry to a target of exactly that shape and place.
+    var paths: [6][sun_path_len:0]u8 = undefined;
+    const pid: u32 = @intCast(libc.getpid());
+    const live_sock = try std.fmt.bufPrintSentinel(&paths[0], "{s}/pardes-9p-sweeplive-{d}.sock", .{ base, pid }, 0);
+    const dead_sock = try std.fmt.bufPrintSentinel(&paths[1], "{s}/pardes-9p-sweepdead-{d}.sock", .{ base, pid }, 0);
     const live = try std.fmt.bufPrintSentinel(&paths[2], "{s}/live", .{svc}, 0);
     const dead = try std.fmt.bufPrintSentinel(&paths[3], "{s}/dead", .{svc}, 0);
     const stranger = try std.fmt.bufPrintSentinel(&paths[4], "{s}/stranger", .{svc}, 0);
+    // A dead entry pointing at something that is NOT one of our sockets: the
+    // entry goes, the file it named must not.
+    const outsider_sock = try std.fmt.bufPrintSentinel(&paths[5], "{s}/sweep-outsider-{d}.sock", .{ base, pid }, 0);
+    var outsider_buf: [sun_path_len:0]u8 = undefined;
+    const outsider = try std.fmt.bufPrintSentinel(&outsider_buf, "{s}/outsider", .{svc}, 0);
     defer {
-        for ([_][:0]const u8{ live_sock, dead_sock, live, dead, stranger }) |p| _ = libc.unlink(p);
+        for ([_][:0]const u8{ live_sock, dead_sock, live, dead, stranger, outsider_sock, outsider }) |p| _ = libc.unlink(p);
         _ = libc.rmdir(svc);
     }
 
@@ -1021,6 +1054,12 @@ test "the registry sweep takes the dead entries and leaves everything else" {
 
     try testing.expectEqual(@as(c_int, 0), libc.symlink(live_sock, live));
     try testing.expectEqual(@as(c_int, 0), libc.symlink(dead_sock, dead));
+    // Dead too, but its target is not one of our sockets by name, so the
+    // entry must go and the file it named must survive untouched.
+    const outside = bindSocket(outsider_sock);
+    try testing.expect(outside >= 0);
+    defer _ = libc.close(outside);
+    try testing.expectEqual(@as(c_int, 0), libc.symlink(outsider_sock, outsider));
     // Not a symlink, so not this program's to reason about, even though
     // connecting to it is refused exactly like the dead socket.
     try std.Io.Dir.cwd().writeFile(testing.io, .{ .sub_path = stranger, .data = "" });
@@ -1058,7 +1097,7 @@ test "the registry sweep takes the dead entries and leaves everything else" {
     try testing.expectEqual(Probe.live, probe(live));
     try testing.expectEqual(Probe.stale, probe(dead));
 
-    sweepRegistry(testing.io, svc);
+    sweepRegistry(testing.io, svc, base);
 
     // Promptly, and not "eventually": a blocking probe never comes back
     // at all, so any wall-clock bound at all is the assertion that
@@ -1071,6 +1110,8 @@ test "the registry sweep takes the dead entries and leaves everything else" {
     try testing.expect(statNoFollow(live) != null);
     try testing.expect(statNoFollow(live_sock) != null);
     try testing.expect(statNoFollow(stranger) != null);
+    try testing.expect(statNoFollow(outsider) == null);
+    try testing.expect(statNoFollow(outsider_sock) != null);
     try testing.expectEqual(Probe.live, probe(live));
 }