From 98e94b86a7c38e2bf4d66e57144fc9329906bb3b Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Tue, 29 Sep 2026 03:37:13 -0300 Subject: Tty and Shell refuse a directory: not a shell A directory passes access(X_OK), so Tty /etc made a pane whose shell exited 127 and Shell /etc was taken. The shell lookup now refuses a directory, and both say "not a shell: /etc is a directory" (Tty and Shell share one refusal, host_io.Shell.refusal). shellset's golden takes the shared wording (re-recorded by name). Co-Authored-By: Claude Opus 5.5 --- src/builtins.zig | 8 ++++---- src/exec.zig | 8 ++++---- src/host_io.zig | 22 +++++++++++++++++++++- src/ninep/ctl.zig | 6 ++++-- src/pardes.zig | 4 ++++ 5 files changed, 37 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/builtins.zig b/src/builtins.zig index f7dd543a..87a4d022 100644 --- a/src/builtins.zig +++ b/src/builtins.zig @@ -1020,10 +1020,10 @@ pub const Tty = struct { // A shell that is not there is said, where the host would quietly // start its fallback in its place. if (comptime pardes.hosted) if (arg.len > 0) { - var buf: [std.fs.max_path_bytes]u8 = undefined; - if (@import("host_io.zig").Shell.find(arg, &buf) == null) { - var said: [320]u8 = undefined; - return c.p.reportFailure(c.id, std.fmt.bufPrint(&said, "Tty: no shell \"{s}\" (a name on the usual paths, or a path to one)", .{arg[0..@min(arg.len, 200)]}) catch "Tty: no such shell"); + var why: [320]u8 = undefined; + if (@import("host_io.zig").Shell.refusal(arg[0..@min(arg.len, 200)], &why)) |refused| { + var said: [340]u8 = undefined; + return c.p.reportFailure(c.id, std.fmt.bufPrint(&said, "Tty: {s}", .{refused}) catch "Tty: no such shell"); } }; const pane = exec.spawnTty(c.p, c.id) orelse return; diff --git a/src/exec.zig b/src/exec.zig index 963c8763..ddb89d33 100644 --- a/src/exec.zig +++ b/src/exec.zig @@ -776,10 +776,10 @@ pub fn applySettingBuiltin(p: *Pardes, setting: config.Runtime.Setting, arg: ?[] return; } if (comptime pardes.hosted) { - var buf: [std.fs.max_path_bytes]u8 = undefined; - if (@import("host_io.zig").Shell.find(want, &buf) == null) { - var text: [320]u8 = undefined; - return p.reportFailure(p.active, std.fmt.bufPrint(&text, "Shell: {s}: no executable by that name", .{want[0..@min(want.len, 255)]}) catch "Shell: no such executable"); + var why: [320]u8 = undefined; + if (@import("host_io.zig").Shell.refusal(want[0..@min(want.len, 200)], &why)) |refused| { + var text: [340]u8 = undefined; + return p.reportFailure(p.active, std.fmt.bufPrint(&text, "Shell: {s}", .{refused}) catch "Shell: no such shell"); } } if (!p.settings.apply(setting, want) and p.announce) p.reportFailure(p.active, "Shell: does not take that value"); diff --git a/src/host_io.zig b/src/host_io.zig index f83c7d47..10a85d8f 100644 --- a/src/host_io.zig +++ b/src/host_io.zig @@ -834,7 +834,7 @@ pub const Shell = struct { @memcpy(buf[0..bin.len], bin); buf[bin.len] = 0; const p: [*:0]const u8 = @ptrCast(buf); - return if (libc.access(p, X_OK) == 0) p else null; + return if (libc.access(p, X_OK) == 0 and !isDirectory(p)) p else null; } for (bin_dirs) |dir| { if (dir.len + bin.len + 1 > buf.len) continue; @@ -847,6 +847,26 @@ pub const Shell = struct { return null; } + /// A directory passes access(X_OK) (it can be searched): no shell. + pub fn isDirectory(path: [*:0]const u8) bool { + const d = libc.opendir(path) orelse return false; + _ = libc.closedir(d); + return true; + } + + /// Why `bin` is no shell, for the words that name one (Tty, Shell); + /// null when it is one. + pub fn refusal(bin: []const u8, said: []u8) ?[]const u8 { + var buf: [std.fs.max_path_bytes]u8 = undefined; + if (find(bin, &buf) != null) return null; + if (std.mem.indexOfScalar(u8, bin, '/') != null and bin.len < buf.len) { + @memcpy(buf[0..bin.len], bin); + buf[bin.len] = 0; + if (isDirectory(@ptrCast(&buf))) return std.fmt.bufPrint(said, "not a shell: {s} is a directory", .{bin}) catch "not a shell: a directory"; + } + return std.fmt.bufPrint(said, "no shell \"{s}\" (a name on the usual paths, or a path to one)", .{bin}) catch "no such shell"; + } + fn fallback(buf: *[std.fs.max_path_bytes]u8) [*:0]const u8 { for (fallbacks) |f| { @memcpy(buf[0..f.len], f); diff --git a/src/ninep/ctl.zig b/src/ninep/ctl.zig index 26a18a12..dd2d9ed0 100644 --- a/src/ninep/ctl.zig +++ b/src/ninep/ctl.zig @@ -1428,8 +1428,10 @@ test "Shell refuses a path that is no executable, and bare it goes back to the d const root_ctl = @intFromEnum(tree.TopFile.ctl); const refused = wr(p, root_ctl, "Shell /nonexistent/zzsh\n"); try testing.expectEqual(Status.err, refused.reply.status); - try testing.expect(std.mem.indexOf(u8, refused.reply.ename, "Shell: /nonexistent/zzsh: no executable by that name") != null); - try testing.expect(th.logHas(p, "/nonexistent/zzsh: no executable")); + try testing.expect(std.mem.indexOf(u8, refused.reply.ename, "Shell: no shell \"/nonexistent/zzsh\"") != null); + try testing.expect(th.logHas(p, "no shell \"/nonexistent/zzsh\"")); + try testing.expectEqual(E.IO, wr(p, root_ctl, "Shell /etc\n").errno()); + try testing.expect(th.logHas(p, "Shell: not a shell: /etc is a directory")); try testing.expectEqualStrings("", p.settings.shell.requested.get()); try testing.expectEqual(Status.ok, wr(p, root_ctl, "Shell /bin/sh\n").reply.status); try testing.expectEqualStrings("/bin/sh", p.settings.shell.requested.get()); diff --git a/src/pardes.zig b/src/pardes.zig index 9297c829..72fd4d46 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -1807,6 +1807,10 @@ test "Tty+fish, one word a tag can hold, opens a terminal on that shell" { try std.testing.expect(std.mem.indexOf(u8, caller.msg[0..caller.msg_len], "no shell \"/nonexistent\"") != null); try std.testing.expect(p.executeBuiltinLine(0, "Tty fsh-not-a-shell")); try std.testing.expect(caller.shell == null); + // A directory passes access(X_OK); it is no shell either. + try std.testing.expect(p.executeBuiltinLine(0, "Tty /etc")); + try std.testing.expect(p.active == before); + try std.testing.expect(std.mem.indexOf(u8, caller.msg[0..caller.msg_len], "Tty: not a shell: /etc is a directory") != null); // Only a word that says so splits at `+`: `Dump+x.zon` is no Dump. try std.testing.expect(!p.executeBuiltinLine(0, "Dump+x.zon")); try std.testing.expect(!p.executeBuiltinLine(0, "Msg+hello")); -- cgit v1.3