Commit fb0e621
Michael Sakaluk
·
2026-03-10 18:09:01 -0400 EDT
parent f520c4c
fix: validate session name length against Unix socket path limit Session names that cause the socket path to exceed the OS sun_path limit (104 bytes on macOS, 108 on Linux) now produce a clear error message instead of failing silently with exit code 1. - Add max_socket_path_len constant derived from platform sockaddr_un - Add validation in getSocketPath returning error.NameTooLong - Add maxSessionNameLen helper for dynamic limit computation - Add printSessionNameTooLong for user-facing error on stderr - Handle NameTooLong in all command paths (run, attach, kill, history, detach, list) - Move validation before sessionExists in kill/history so the error message is shown even when the socket file doesn't exist - Add 6 unit tests covering boundary conditions and platform constants Fixes: https://github.com/neurosnap/zmx/issues/84
3 files changed,
+135,
-12
+41,
-9
| ... | ... | @@ -128,7 +128,10 @@ pub fn main() !void { | |
| 128 | 128 | .cwd = cwd, | |
| 129 | 129 | .created_at = @intCast(std.time.timestamp()), | |
| 130 | 130 | }; | |
| 131 | - | daemon.socket_path = try socket.getSocketPath(alloc, cfg.socket_dir, sesh); | |
| 131 | + | daemon.socket_path = socket.getSocketPath(alloc, cfg.socket_dir, sesh) catch |err| switch (err) { | |
| 132 | + | error.NameTooLong => return printSessionNameTooLong(sesh, &cfg), | |
| 133 | + | error.OutOfMemory => return err, | |
| 134 | + | }; | |
| 132 | 135 | std.log.info("socket path={s}", .{daemon.socket_path}); | |
| 133 | 136 | return attach(&daemon); | |
| 134 | 137 | } else if (std.mem.eql(u8, cmd, "run") or std.mem.eql(u8, cmd, "r")) { |
| ... | ... | @@ -160,7 +163,10 @@ pub fn main() !void { | |
| 160 | 163 | .is_task_mode = true, | |
| 161 | 164 | .task_command = cmd_args_raw.items, | |
| 162 | 165 | }; | |
| 163 | - | daemon.socket_path = try socket.getSocketPath(alloc, cfg.socket_dir, sesh); | |
| 166 | + | daemon.socket_path = socket.getSocketPath(alloc, cfg.socket_dir, sesh) catch |err| switch (err) { | |
| 167 | + | error.NameTooLong => return printSessionNameTooLong(sesh, &cfg), | |
| 168 | + | error.OutOfMemory => return err, | |
| 169 | + | }; | |
| 164 | 170 | std.log.info("socket path={s}", .{daemon.socket_path}); | |
| 165 | 171 | return run(&daemon, cmd_args_raw.items); | |
| 166 | 172 | } else if (std.mem.eql(u8, cmd, "wait") or std.mem.eql(u8, cmd, "w")) { |
| ... | ... | @@ -711,6 +717,23 @@ fn help() !void { | |
| 711 | 717 | try w.interface.flush(); | |
| 712 | 718 | } | |
| 713 | 719 | ||
| 720 | + | fn printSessionNameTooLong(session_name: []const u8, cfg: *Cfg) void { | |
| 721 | + | var buf: [4096]u8 = undefined; | |
| 722 | + | var w = std.fs.File.stderr().writer(&buf); | |
| 723 | + | if (socket.maxSessionNameLen(cfg.socket_dir)) |max_len| { | |
| 724 | + | w.interface.print( | |
| 725 | + | "error: session name is too long ({d} bytes, max {d} for socket directory \"{s}\")\n", | |
| 726 | + | .{ session_name.len, max_len, cfg.socket_dir }, | |
| 727 | + | ) catch {}; | |
| 728 | + | } else { | |
| 729 | + | w.interface.print( | |
| 730 | + | "error: socket directory path is too long (\"{s}\")\n", | |
| 731 | + | .{cfg.socket_dir}, | |
| 732 | + | ) catch {}; | |
| 733 | + | } | |
| 734 | + | w.interface.flush() catch {}; | |
| 735 | + | } | |
| 736 | + | ||
| 714 | 737 | fn wait(cfg: *Cfg, session_names: std.ArrayList([]const u8)) !void { | |
| 715 | 738 | var gpa = std.heap.GeneralPurposeAllocator(.{}){}; | |
| 716 | 739 | defer _ = gpa.deinit(); |
| ... | ... | @@ -860,7 +883,10 @@ fn detachAll(cfg: *Cfg) !void { | |
| 860 | 883 | var dir = try std.fs.openDirAbsolute(cfg.socket_dir, .{}); | |
| 861 | 884 | defer dir.close(); | |
| 862 | 885 | ||
| 863 | - | const socket_path = try socket.getSocketPath(alloc, cfg.socket_dir, session_name); | |
| 886 | + | const socket_path = socket.getSocketPath(alloc, cfg.socket_dir, session_name) catch |err| switch (err) { | |
| 887 | + | error.NameTooLong => return printSessionNameTooLong(session_name, cfg), | |
| 888 | + | error.OutOfMemory => return err, | |
| 889 | + | }; | |
| 864 | 890 | defer alloc.free(socket_path); | |
| 865 | 891 | const result = ipc.probeSession(alloc, socket_path) catch |err| { | |
| 866 | 892 | std.log.err("session unresponsive: {s}", .{@errorName(err)}); |
| ... | ... | @@ -879,6 +905,12 @@ fn kill(cfg: *Cfg, session_name: []const u8) !void { | |
| 879 | 905 | defer _ = gpa.deinit(); | |
| 880 | 906 | const alloc = gpa.allocator(); | |
| 881 | 907 | ||
| 908 | + | const socket_path = socket.getSocketPath(alloc, cfg.socket_dir, session_name) catch |err| switch (err) { | |
| 909 | + | error.NameTooLong => return printSessionNameTooLong(session_name, cfg), | |
| 910 | + | error.OutOfMemory => return err, | |
| 911 | + | }; | |
| 912 | + | defer alloc.free(socket_path); | |
| 913 | + | ||
| 882 | 914 | var dir = try std.fs.openDirAbsolute(cfg.socket_dir, .{}); | |
| 883 | 915 | defer dir.close(); | |
| 884 | 916 |
| ... | ... | @@ -890,9 +922,6 @@ fn kill(cfg: *Cfg, session_name: []const u8) !void { | |
| 890 | 922 | w.interface.flush() catch {}; | |
| 891 | 923 | return error.SessionNotFound; | |
| 892 | 924 | } | |
| 893 | - | ||
| 894 | - | const socket_path = try socket.getSocketPath(alloc, cfg.socket_dir, session_name); | |
| 895 | - | defer alloc.free(socket_path); | |
| 896 | 925 | const result = ipc.probeSession(alloc, socket_path) catch |err| { | |
| 897 | 926 | std.log.err("session unresponsive: {s}", .{@errorName(err)}); | |
| 898 | 927 | var buf: [4096]u8 = undefined; |
| ... | ... | @@ -923,6 +952,12 @@ fn history(cfg: *Cfg, session_name: []const u8, format: util.HistoryFormat) !voi | |
| 923 | 952 | defer _ = gpa.deinit(); | |
| 924 | 953 | const alloc = gpa.allocator(); | |
| 925 | 954 | ||
| 955 | + | const socket_path = socket.getSocketPath(alloc, cfg.socket_dir, session_name) catch |err| switch (err) { | |
| 956 | + | error.NameTooLong => return printSessionNameTooLong(session_name, cfg), | |
| 957 | + | error.OutOfMemory => return err, | |
| 958 | + | }; | |
| 959 | + | defer alloc.free(socket_path); | |
| 960 | + | ||
| 926 | 961 | var dir = try std.fs.openDirAbsolute(cfg.socket_dir, .{}); | |
| 927 | 962 | defer dir.close(); | |
| 928 | 963 |
| ... | ... | @@ -934,9 +969,6 @@ fn history(cfg: *Cfg, session_name: []const u8, format: util.HistoryFormat) !voi | |
| 934 | 969 | w.interface.flush() catch {}; | |
| 935 | 970 | return error.SessionNotFound; | |
| 936 | 971 | } | |
| 937 | - | ||
| 938 | - | const socket_path = try socket.getSocketPath(alloc, cfg.socket_dir, session_name); | |
| 939 | - | defer alloc.free(socket_path); | |
| 940 | 972 | const result = ipc.probeSession(alloc, socket_path) catch |err| { | |
| 941 | 973 | std.log.err("session unresponsive: {s}", .{@errorName(err)}); | |
| 942 | 974 | if (err == error.ConnectionRefused) socket.cleanupStaleSocket(dir, session_name); |
+90,
-2
| ... | ... | @@ -67,11 +67,99 @@ pub fn createSocket(fname: []const u8) !i32 { | |
| 67 | 67 | return fd; | |
| 68 | 68 | } | |
| 69 | 69 | ||
| 70 | - | pub fn getSocketPath(alloc: std.mem.Allocator, socket_dir: []const u8, session_name: []const u8) ![]const u8 { | |
| 70 | + | /// Maximum number of usable bytes in a Unix domain socket path. | |
| 71 | + | /// Derived from the platform's sockaddr_un.path field, minus 1 for the | |
| 72 | + | /// required null terminator. | |
| 73 | + | pub const max_socket_path_len: usize = @typeInfo( | |
| 74 | + | @TypeOf(@as(posix.sockaddr.un, undefined).path), | |
| 75 | + | ).array.len - 1; | |
| 76 | + | ||
| 77 | + | pub fn getSocketPath(alloc: std.mem.Allocator, socket_dir: []const u8, session_name: []const u8) error{ NameTooLong, OutOfMemory }![]const u8 { | |
| 71 | 78 | const dir = socket_dir; | |
| 72 | - | const fname = try alloc.alloc(u8, dir.len + session_name.len + 1); | |
| 79 | + | const path_len = dir.len + 1 + session_name.len; | |
| 80 | + | if (path_len > max_socket_path_len) return error.NameTooLong; | |
| 81 | + | const fname = try alloc.alloc(u8, path_len); | |
| 73 | 82 | @memcpy(fname[0..dir.len], dir); | |
| 74 | 83 | @memcpy(fname[dir.len .. dir.len + 1], "/"); | |
| 75 | 84 | @memcpy(fname[dir.len + 1 ..], session_name); | |
| 76 | 85 | return fname; | |
| 77 | 86 | } | |
| 87 | + | ||
| 88 | + | /// Returns the maximum session name length for a given socket directory, | |
| 89 | + | /// or null if the socket directory itself is already too long. | |
| 90 | + | pub fn maxSessionNameLen(socket_dir: []const u8) ?usize { | |
| 91 | + | // path = socket_dir + "/" + session_name | |
| 92 | + | const overhead = socket_dir.len + 1; | |
| 93 | + | if (overhead >= max_socket_path_len) return null; | |
| 94 | + | return max_socket_path_len - overhead; | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | test "max_socket_path_len matches platform sockaddr_un" { | |
| 98 | + | const path_field_len = @typeInfo( | |
| 99 | + | @TypeOf(@as(posix.sockaddr.un, undefined).path), | |
| 100 | + | ).array.len; | |
| 101 | + | try std.testing.expectEqual(path_field_len - 1, max_socket_path_len); | |
| 102 | + | try std.testing.expect(max_socket_path_len > 0); | |
| 103 | + | } | |
| 104 | + | ||
| 105 | + | test "getSocketPath succeeds for paths within limit" { | |
| 106 | + | const alloc = std.testing.allocator; | |
| 107 | + | const result = try getSocketPath(alloc, "/tmp/zmx", "mysession"); | |
| 108 | + | defer alloc.free(result); | |
| 109 | + | try std.testing.expectEqualStrings("/tmp/zmx/mysession", result); | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | test "getSocketPath returns NameTooLong when path exceeds limit" { | |
| 113 | + | const alloc = std.testing.allocator; | |
| 114 | + | const dir = [_]u8{'d'} ** (max_socket_path_len - 2); | |
| 115 | + | const dir_slice: []const u8 = &dir; | |
| 116 | + | ||
| 117 | + | const ok = try getSocketPath(alloc, dir_slice, "x"); | |
| 118 | + | defer alloc.free(ok); | |
| 119 | + | try std.testing.expectEqual(max_socket_path_len, ok.len); | |
| 120 | + | ||
| 121 | + | const err = getSocketPath(alloc, dir_slice, "xx"); | |
| 122 | + | try std.testing.expectError(error.NameTooLong, err); | |
| 123 | + | } | |
| 124 | + | ||
| 125 | + | test "getSocketPath returns NameTooLong for empty dir with oversized name" { | |
| 126 | + | const alloc = std.testing.allocator; | |
| 127 | + | const name = [_]u8{'n'} ** (max_socket_path_len); | |
| 128 | + | const name_slice: []const u8 = &name; | |
| 129 | + | const err = getSocketPath(alloc, "", name_slice); | |
| 130 | + | try std.testing.expectError(error.NameTooLong, err); | |
| 131 | + | } | |
| 132 | + | ||
| 133 | + | test "maxSessionNameLen computes correct dynamic limit" { | |
| 134 | + | const short_dir = "/tmp/zmx"; | |
| 135 | + | const short_max = maxSessionNameLen(short_dir).?; | |
| 136 | + | try std.testing.expectEqual(max_socket_path_len - short_dir.len - 1, short_max); | |
| 137 | + | ||
| 138 | + | const full_dir = [_]u8{'f'} ** max_socket_path_len; | |
| 139 | + | const full_dir_slice: []const u8 = &full_dir; | |
| 140 | + | try std.testing.expectEqual(@as(?usize, null), maxSessionNameLen(full_dir_slice)); | |
| 141 | + | ||
| 142 | + | const tight_dir = [_]u8{'t'} ** (max_socket_path_len - 2); | |
| 143 | + | const tight_dir_slice: []const u8 = &tight_dir; | |
| 144 | + | try std.testing.expectEqual(@as(?usize, 1), maxSessionNameLen(tight_dir_slice)); | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | test "getSocketPath boundary: name fills exactly to limit" { | |
| 148 | + | const alloc = std.testing.allocator; | |
| 149 | + | const dir = "/tmp/zmx"; | |
| 150 | + | const max_name_len = maxSessionNameLen(dir).?; | |
| 151 | + | ||
| 152 | + | const name_at_limit = try alloc.alloc(u8, max_name_len); | |
| 153 | + | defer alloc.free(name_at_limit); | |
| 154 | + | @memset(name_at_limit, 'a'); | |
| 155 | + | ||
| 156 | + | const path = try getSocketPath(alloc, dir, name_at_limit); | |
| 157 | + | defer alloc.free(path); | |
| 158 | + | try std.testing.expectEqual(max_socket_path_len, path.len); | |
| 159 | + | ||
| 160 | + | const name_over_limit = try alloc.alloc(u8, max_name_len + 1); | |
| 161 | + | defer alloc.free(name_over_limit); | |
| 162 | + | @memset(name_over_limit, 'b'); | |
| 163 | + | ||
| 164 | + | try std.testing.expectError(error.NameTooLong, getSocketPath(alloc, dir, name_over_limit)); | |
| 165 | + | } |
+4,
-1
| ... | ... | @@ -40,7 +40,10 @@ pub fn get_session_entries(alloc: std.mem.Allocator, socket_dir: []const u8) !st | |
| 40 | 40 | const name = try alloc.dupe(u8, entry.name); | |
| 41 | 41 | errdefer alloc.free(name); | |
| 42 | 42 | ||
| 43 | - | const socket_path = try socket.getSocketPath(alloc, socket_dir, entry.name); | |
| 43 | + | const socket_path = socket.getSocketPath(alloc, socket_dir, entry.name) catch |err| switch (err) { | |
| 44 | + | error.NameTooLong => continue, | |
| 45 | + | error.OutOfMemory => return err, | |
| 46 | + | }; | |
| 44 | 47 | defer alloc.free(socket_path); | |
| 45 | 48 | ||
| 46 | 49 | const result = ipc.probeSession(alloc, socket_path) catch |err| { |