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 --- docs/config.md | 5 +++-- src/builtins.zig | 8 ++++---- src/exec.zig | 8 ++++---- src/host_io.zig | 22 +++++++++++++++++++++- src/ninep/ctl.zig | 6 ++++-- src/pardes.zig | 4 ++++ test/snapshots/shellset.golden | 2 +- 7 files changed, 41 insertions(+), 14 deletions(-) diff --git a/docs/config.md b/docs/config.md index 95f034d7..ebedc4d0 100644 --- a/docs/config.md +++ b/docs/config.md @@ -49,8 +49,9 @@ opens the file. Shell follows the same requested/effective/pending model as Font. `Default shell` is the one used while no `Shell` is set: `$SHELL`, the user's login shell, else `/bin/sh` (also when `$SHELL` names nothing executable); an explicit `Shell` overrides it. `Shell ` is -refused, `Shell: : no executable by that name`, unless it names an -executable (a bare name is looked for in the usual bin directories), and a +refused unless it names an executable file (`Shell: no shell "x" (...)`, or +`Shell: not a shell: /etc is a directory`; a bare name is looked for in the +usual bin directories), as `Tty ` is, and a bare `Shell` goes back to the default. The root ctl reads `Shell `. `Shell effective (last spawn)` is the executable the native host really chose after 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")); diff --git a/test/snapshots/shellset.golden b/test/snapshots/shellset.golden index ae3b9fd0..e2830801 100644 --- a/test/snapshots/shellset.golden +++ b/test/snapshots/shellset.golden @@ -4,7 +4,7 @@ | /tmp/pardes-snap/shellset/cwd/cmd.txt Save Tty Collapse Del | 1 Shell zznosuchshell == snap builtin-ran grid=90x21 cursor=25,3 -|3: 1 Shell zznosuchshell Shell: zznosuchshell: no executable by that name +|3: 1 Shel Shell: no shell "zznosuchshell" (a name on the usual paths, or a path to one) == snap fallback-shell grid=90x21 cursor=49,6 |1: New Tty Find Grep Joincol Delcol New Tty Find Grep Joincol Delcol |2: /tmp/pardes-snap/shellset/cwd/cmd.txt Save /tmp/pardes-snap/shellset/cwd Tty+bash Sav -- cgit v1.3