Fix verified bugs (#83)

Commit `a27b371`

Ian · 2026-03-08 23:03:14 -0400 EDT

parent ccdc1b9

Fix verified bugs (#83)

* fix: validate session names to prevent path traversal

Session names become filenames under socket_dir. Without validation,
`zmx attach ../foo` would create a socket outside that directory, and
(worse) stale-socket cleanup via unlinkat(dirfd, "../foo") would delete
files outside it. The ZMX_SESSION_PREFIX env var was similarly unsanitized.

Also fixes a minor inefficiency: seshPrefix() was called twice.

* fix(history): don't panic when only flags are provided

`zmx history --vt` panicked on the .? unwrap when no positional session
name was given. Pass empty string instead so getSeshName handles it
(returns error.SessionNameRequired if no ZMX_SESSION_PREFIX is set either).

* fix(wait): don't report vacuous success when no sessions match

total == done was true when both were 0, so `zmx wait foo` returned
exit 0 immediately if no session named foo existed — either because of a
typo, or because of a race where the preceding `zmx run foo` hadn't yet
created its socket. Now wait keeps polling until at least one matching
session is seen.

Also tracks the high-water mark of matched sessions: if the count drops,
a session disappeared (daemon crashed or was killed), so wait errors out
instead of looping forever.

* fix(run): reset task state when receiving a Run command

The daemon's exit-marker scan is gated on task_exit_code == null, but that
field was never reset after the first task completed. A second `zmx run`
on the same session would have its ZMX_TASK_COMPLETED marker ignored,
causing `zmx wait` to immediately return the *first* task's exit code.

Also explicitly set is_task_mode so `zmx run` works against sessions that
were originally created by `zmx attach` (non-task mode).

* fix(run): use CR not LF to submit commands to an already-prompting shell

When `zmx run` sends a command to an existing session, the shell is at
the readline prompt with the PTY in raw mode. readline binds accept-line
to CR (\r), not LF (\n), so a trailing \n just moves the cursor — the
command sits typed but unexecuted.

The first-ever `zmx run` on a fresh session happened to work because
it's sent during shell startup, before readline takes over: the line
discipline is still canonical and ICRNL translates \n. Any subsequent
run silently did nothing, which was masked by the stale-task-state bug
(wait returned the first run's exit code, looking like success).

\r works in both modes: canonical mode translates it via ICRNL, and
raw-mode readline accepts it directly.

2 files changed, +50, -9

M src/main.zig

M src/socket.zig

M src/main.zig

+37, -7

......@@ -83,7 +83,7 @@ pub fn main() !void {
                session_name = arg;
            }
        }
        const sesh = try socket.getSeshName(alloc, session_name.?);
        const sesh = try socket.getSeshName(alloc, session_name orelse "");
        defer alloc.free(sesh);
        return history(&cfg, sesh, format);
    } else if (std.mem.eql(u8, cmd, "attach") or std.mem.eql(u8, cmd, "a")) {

......@@ -567,6 +567,13 @@ const Daemon = struct {
    }

    pub fn handleRun(self: *Daemon, client: *Client, pty_fd: i32, payload: []const u8) !void {
        // Reset task tracking so the new command's exit marker is detected.
        // Without this, a second `zmx run` on the same session is ignored
        // because task_exit_code is still set from the first run.
        self.task_exit_code = null;
        self.task_ended_at = null;
        self.is_task_mode = true;

        if (payload.len > 0) {
            _ = try posix.write(pty_fd, payload);
        }

......@@ -641,6 +648,11 @@ fn wait(cfg: *Cfg, session_names: std.ArrayList([]const u8)) !void {
    var stdout_writer = std.fs.File.stdout().writer(&stdout_buffer);
    const stdout = &stdout_writer.interface;

    // Highest match count seen so far. Lets us distinguish "sessions haven't
    // appeared yet" (keep polling) from "sessions we were tracking
    // disappeared" (fail — daemon crashed or was killed).
    var max_seen: i32 = 0;

    while (true) {
        var sessions = try util.get_session_entries(alloc, cfg.socket_dir);
        var total: i32 = 0;

......@@ -676,7 +688,18 @@ fn wait(cfg: *Cfg, session_names: std.ArrayList([]const u8)) !void {
        }
        sessions.deinit(alloc);

        if (total == done) {
        // Check disappearance BEFORE completion: if one of N sessions
        // crashed and the remaining N-1 happen to be done, total==done
        // would be a false success.
        if (total < max_seen) {
            try stdout.print("error: {d} session(s) disappeared before completing\n", .{max_seen - total});
            try stdout.flush();
            std.process.exit(1);
            return;
        }
        max_seen = total;

        if (total > 0 and total == done) {
            try stdout.print("tasks completed!\n", .{});
            try stdout.flush();
            std.process.exit(agg_exit_code);

......@@ -947,7 +970,11 @@ fn run(daemon: *Daemon, command_args: [][]const u8) !void {
        }

        try cmd_list.appendSlice(alloc, inline_task_marker);
        try cmd_list.append(alloc, '\n');
        // \r, not \n: once the shell is at the readline prompt the PTY is in
        // raw mode; readline's accept-line binds to CR. The first-ever run
        // works with \n only because it arrives during shell startup while
        // the line discipline is still canonical.
        try cmd_list.append(alloc, '\r');

        cmd_to_send = try cmd_list.toOwnedSlice(alloc);
        allocated_cmd = @constCast(cmd_to_send.?);

......@@ -968,13 +995,16 @@ fn run(daemon: *Daemon, command_args: [][]const u8) !void {
            }

            if (stdin_buf.items.len > 0) {
                const needs_newline = stdin_buf.items[stdin_buf.items.len - 1] != '\n';
                if (needs_newline) {
                    try stdin_buf.append(alloc, '\n');
                // Normalize any trailing newline to CR so readline (raw mode)
                // accepts each line.
                if (stdin_buf.items[stdin_buf.items.len - 1] == '\n') {
                    stdin_buf.items[stdin_buf.items.len - 1] = '\r';
                } else {
                    try stdin_buf.append(alloc, '\r');
                }

                try stdin_buf.appendSlice(alloc, stdin_task_marker);
                try stdin_buf.append(alloc, '\n');
                try stdin_buf.append(alloc, '\r');

                cmd_to_send = try alloc.dupe(u8, stdin_buf.items);
                allocated_cmd = @constCast(cmd_to_send.?);

M src/socket.zig

+13, -2

......@@ -7,10 +7,21 @@ pub fn seshPrefix() []const u8 {

pub fn getSeshName(alloc: std.mem.Allocator, sesh: []const u8) ![]const u8 {
    const prefix = seshPrefix();
    if (std.mem.eql(u8, prefix, "") and std.mem.eql(u8, sesh, "")) {
    if (prefix.len == 0 and sesh.len == 0) {
        return error.SessionNameRequired;
    }
    return std.fmt.allocPrint(alloc, "{s}{s}", .{ seshPrefix(), sesh });
    const full = try std.fmt.allocPrint(alloc, "{s}{s}", .{ prefix, sesh });
    // Session names become filenames under socket_dir. Rejecting path
    // separators and dot-dot prevents socket creation and stale-socket
    // deletion from operating outside that directory.
    if (std.mem.indexOfScalar(u8, full, '/') != null or
        std.mem.indexOfScalar(u8, full, 0) != null or
        std.mem.eql(u8, full, ".") or std.mem.eql(u8, full, ".."))
    {
        alloc.free(full);
        return error.InvalidSessionName;
    }
    return full;
}

pub fn sessionConnect(sesh: []const u8) !i32 {

...	...		@@ -567,6 +567,13 @@ const Daemon = struct {
567	567		}
568	568
569	569		pub fn handleRun(self: Daemon, client: Client, pty_fd: i32, payload: []const u8) !void {
	570	+	// Reset task tracking so the new command's exit marker is detected.
	571	+	// Without this, a second `zmx run` on the same session is ignored
	572	+	// because task_exit_code is still set from the first run.
	573	+	self.task_exit_code = null;
	574	+	self.task_ended_at = null;
	575	+	self.is_task_mode = true;
	576	+
570	577		if (payload.len > 0) {
571	578		_ = try posix.write(pty_fd, payload);
572	579		}

...	...		@@ -641,6 +648,11 @@ fn wait(cfg: *Cfg, session_names: std.ArrayList([]const u8)) !void {
641	648		var stdout_writer = std.fs.File.stdout().writer(&stdout_buffer);
642	649		const stdout = &stdout_writer.interface;
643	650
	651	+	// Highest match count seen so far. Lets us distinguish "sessions haven't
	652	+	// appeared yet" (keep polling) from "sessions we were tracking
	653	+	// disappeared" (fail — daemon crashed or was killed).
	654	+	var max_seen: i32 = 0;
	655	+
644	656		while (true) {
645	657		var sessions = try util.get_session_entries(alloc, cfg.socket_dir);
646	658		var total: i32 = 0;

...	...		@@ -676,7 +688,18 @@ fn wait(cfg: *Cfg, session_names: std.ArrayList([]const u8)) !void {
676	688		}
677	689		sessions.deinit(alloc);
678	690
679		-	if (total == done) {
	691	+	// Check disappearance BEFORE completion: if one of N sessions
	692	+	// crashed and the remaining N-1 happen to be done, total==done
	693	+	// would be a false success.
	694	+	if (total < max_seen) {
	695	+	try stdout.print("error: {d} session(s) disappeared before completing\n", .{max_seen - total});
	696	+	try stdout.flush();
	697	+	std.process.exit(1);
	698	+	return;
	699	+	}
	700	+	max_seen = total;
	701	+
	702	+	if (total > 0 and total == done) {
680	703		try stdout.print("tasks completed!\n", .{});
681	704		try stdout.flush();
682	705		std.process.exit(agg_exit_code);

...	...		@@ -947,7 +970,11 @@ fn run(daemon: *Daemon, command_args: [][]const u8) !void {
947	970		}
948	971
949	972		try cmd_list.appendSlice(alloc, inline_task_marker);
950		-	try cmd_list.append(alloc, '\n');
	973	+	// \r, not \n: once the shell is at the readline prompt the PTY is in
	974	+	// raw mode; readline's accept-line binds to CR. The first-ever run
	975	+	// works with \n only because it arrives during shell startup while
	976	+	// the line discipline is still canonical.
	977	+	try cmd_list.append(alloc, '\r');
951	978
952	979		cmd_to_send = try cmd_list.toOwnedSlice(alloc);
953	980		allocated_cmd = @constCast(cmd_to_send.?);

...	...		@@ -968,13 +995,16 @@ fn run(daemon: *Daemon, command_args: [][]const u8) !void {
968	995		}
969	996
970	997		if (stdin_buf.items.len > 0) {
971		-	const needs_newline = stdin_buf.items[stdin_buf.items.len - 1] != '\n';
972		-	if (needs_newline) {
973		-	try stdin_buf.append(alloc, '\n');
	998	+	// Normalize any trailing newline to CR so readline (raw mode)
	999	+	// accepts each line.
	1000	+	if (stdin_buf.items[stdin_buf.items.len - 1] == '\n') {
	1001	+	stdin_buf.items[stdin_buf.items.len - 1] = '\r';
	1002	+	} else {
	1003	+	try stdin_buf.append(alloc, '\r');
974	1004		}
975	1005
976	1006		try stdin_buf.appendSlice(alloc, stdin_task_marker);
977		-	try stdin_buf.append(alloc, '\n');
	1007	+	try stdin_buf.append(alloc, '\r');
978	1008
979	1009		cmd_to_send = try alloc.dupe(u8, stdin_buf.items);
980	1010		allocated_cmd = @constCast(cmd_to_send.?);

...	...		@@ -83,7 +83,7 @@ pub fn main() !void {
83	83		session_name = arg;
84	84		}
85	85		}
86		-	const sesh = try socket.getSeshName(alloc, session_name.?);
	86	+	const sesh = try socket.getSeshName(alloc, session_name orelse "");
87	87		defer alloc.free(sesh);
88	88		return history(&cfg, sesh, format);
89	89		} else if (std.mem.eql(u8, cmd, "attach") or std.mem.eql(u8, cmd, "a")) {

...	...		@@ -7,10 +7,21 @@ pub fn seshPrefix() []const u8 {
7	7
8	8		pub fn getSeshName(alloc: std.mem.Allocator, sesh: []const u8) ![]const u8 {
9	9		const prefix = seshPrefix();
10		-	if (std.mem.eql(u8, prefix, "") and std.mem.eql(u8, sesh, "")) {
	10	+	if (prefix.len == 0 and sesh.len == 0) {
11	11		return error.SessionNameRequired;
12	12		}
13		-	return std.fmt.allocPrint(alloc, "{s}{s}", .{ seshPrefix(), sesh });
	13	+	const full = try std.fmt.allocPrint(alloc, "{s}{s}", .{ prefix, sesh });
	14	+	// Session names become filenames under socket_dir. Rejecting path
	15	+	// separators and dot-dot prevents socket creation and stale-socket
	16	+	// deletion from operating outside that directory.
	17	+	if (std.mem.indexOfScalar(u8, full, '/') != null or
	18	+	std.mem.indexOfScalar(u8, full, 0) != null or
	19	+	std.mem.eql(u8, full, ".") or std.mem.eql(u8, full, ".."))
	20	+	{
	21	+	alloc.free(full);
	22	+	return error.InvalidSessionName;
	23	+	}
	24	+	return full;
14	25		}
15	26
16	27		pub fn sessionConnect(sesh: []const u8) !i32 {

Commit a27b371

Commit `a27b371`