config: allow commands to specify whether they shell expand or not

This introduces a syntax for `command` and `initial-command` that allows
the user to specify whether it should be run via `/bin/sh -c` or not.
The syntax is a prefix `direct:` or `shell:` prior to the command,
with no prefix implying a default behavior as documented.

Previously, we unconditionally ran commands via `/bin/sh -c`, primarily
to avoid having to do any shell expansion ourselves. We also leaned on
it as a crutch for PATH-expansion but this is an easy problem compared
to shell expansion.

For the principle of least surprise, this worked well for configurations
specified via the config file, and is still the default. However, these
configurations are also set via the `-e` special flag to the CLI, and it
is very much not the principle of least surprise to have the command run via
`/bin/sh -c` in that scenario since a shell has already expanded all the
arguments and given them to us in a nice separated format. But we had no
way to toggle this behavior.

This commit introduces the ability to do this, and changes the defaults
so that `-e` doesn't shell expand. Further, we also do PATH lookups
ourselves for the non-shell expanded case because thats easy (using
execvpe style extensions but implemented as part of the Zig stdlib). We don't
do path expansion (e.g. `~/`) because thats a shell expansion.

So to be clear, there are no two polar opposite behavioes here with
clear semantics:

  1. Direct commands are passed to `execvpe` directly, space separated.
     This will not handle quoted strings, environment variables, path
     expansion (e.g. `~/`), command expansion (e.g. `$()`), etc.

  2. Shell commands are passed to `/bin/sh -c` and will be shell expanded
     as per the shell's rules. This will handle everything that `sh`
     supports.

In doing this work, I also stumbled upon a variety of smaller
improvements that could be made:

  - A number of allocations have been removed from the startup path that
    only existed to add a null terminator to various strings. We now
    have null terminators from the beginning since we are almost always
    on a system that's going to need it anyways.

  - For bash shell integration, we no longer wrap the new bash command
    in a shell since we've formed a full parsed command line.

  - The process of creating the command to execute by termio is now unit
    tested, so we can test the various complex cases particularly on
    macOS of wrapping commands in the login command.

  - `xdg-terminal-exec` on Linux uses the `direct:` method by default
    since it is also assumed to be executed via a shell environment.
This commit is contained in:
Mitchell Hashimoto
2025-04-05 11:45:40 -04:00
parent 6f7977fef1
commit 722d41a359
9 changed files with 901 additions and 283 deletions

View File

@@ -27,7 +27,7 @@ pub const ShellIntegration = struct {
/// bash in particular it may be different.
///
/// The memory is allocated in the arena given to setup.
command: []const u8,
command: config.Command,
};
/// Set up the command execution environment for automatic
@@ -41,7 +41,7 @@ pub const ShellIntegration = struct {
pub fn setup(
alloc_arena: Allocator,
resource_dir: []const u8,
command: []const u8,
command: config.Command,
env: *EnvMap,
force_shell: ?Shell,
features: config.ShellIntegrationFeatures,
@@ -51,14 +51,24 @@ pub fn setup(
.elvish => "elvish",
.fish => "fish",
.zsh => "zsh",
} else exe: {
// The command can include arguments. Look for the first space
// and use the basename of the first part as the command's exe.
const idx = std.mem.indexOfScalar(u8, command, ' ') orelse command.len;
break :exe std.fs.path.basename(command[0..idx]);
} else switch (command) {
.direct => |v| std.fs.path.basename(v[0]),
.shell => |v| exe: {
// Shell strings can include spaces so we want to only
// look up to the space if it exists. No shell that we integrate
// has spaces.
const idx = std.mem.indexOfScalar(u8, v, ' ') orelse v.len;
break :exe std.fs.path.basename(v[0..idx]);
},
};
const result = try setupShell(alloc_arena, resource_dir, command, env, exe);
const result = try setupShell(
alloc_arena,
resource_dir,
command,
env,
exe,
);
// Setup our feature env vars
try setupFeatures(env, features);
@@ -69,7 +79,7 @@ pub fn setup(
fn setupShell(
alloc_arena: Allocator,
resource_dir: []const u8,
command: []const u8,
command: config.Command,
env: *EnvMap,
exe: []const u8,
) !?ShellIntegration {
@@ -83,7 +93,10 @@ fn setupShell(
// we're using Apple's Bash because /bin is non-writable
// on modern macOS due to System Integrity Protection.
if (comptime builtin.target.os.tag.isDarwin()) {
if (std.mem.eql(u8, "/bin/bash", command)) {
if (std.mem.eql(u8, "/bin/bash", switch (command) {
.direct => |v| v[0],
.shell => |v| v,
})) {
return null;
}
}
@@ -104,7 +117,7 @@ fn setupShell(
try setupXdgDataDirs(alloc_arena, resource_dir, env);
return .{
.shell = .elvish,
.command = try alloc_arena.dupe(u8, command),
.command = try command.clone(alloc_arena),
};
}
@@ -112,7 +125,7 @@ fn setupShell(
try setupXdgDataDirs(alloc_arena, resource_dir, env);
return .{
.shell = .fish,
.command = try alloc_arena.dupe(u8, command),
.command = try command.clone(alloc_arena),
};
}
@@ -120,7 +133,7 @@ fn setupShell(
try setupZsh(resource_dir, env);
return .{
.shell = .zsh,
.command = try alloc_arena.dupe(u8, command),
.command = try command.clone(alloc_arena),
};
}
@@ -139,7 +152,14 @@ test "force shell" {
inline for (@typeInfo(Shell).@"enum".fields) |field| {
const shell = @field(Shell, field.name);
const result = try setup(alloc, ".", "sh", &env, shell, .{});
const result = try setup(
alloc,
".",
.{ .shell = "sh" },
&env,
shell,
.{},
);
try testing.expectEqual(shell, result.?.shell);
}
}
@@ -215,25 +235,21 @@ test "setup features" {
/// enables the integration or null if integration failed.
fn setupBash(
alloc: Allocator,
command: []const u8,
command: config.Command,
resource_dir: []const u8,
env: *EnvMap,
) !?[]const u8 {
// Accumulates the arguments that will form the final shell command line.
// We can build this list on the stack because we're just temporarily
// referencing other slices, but we can fall back to heap in extreme cases.
var args_alloc = std.heap.stackFallback(1024, alloc);
var args = try std.ArrayList([]const u8).initCapacity(args_alloc.get(), 2);
) !?config.Command {
var args = try std.ArrayList([:0]const u8).initCapacity(alloc, 2);
defer args.deinit();
// Iterator that yields each argument in the original command line.
// This will allocate once proportionate to the command line length.
var iter = try std.process.ArgIteratorGeneral(.{}).init(alloc, command);
var iter = try command.argIterator(alloc);
defer iter.deinit();
// Start accumulating arguments with the executable and `--posix` mode flag.
if (iter.next()) |exe| {
try args.append(exe);
try args.append(try alloc.dupeZ(u8, exe));
} else return null;
try args.append("--posix");
@@ -267,17 +283,17 @@ fn setupBash(
if (std.mem.indexOfScalar(u8, arg, 'c') != null) {
return null;
}
try args.append(arg);
try args.append(try alloc.dupeZ(u8, arg));
} else if (std.mem.eql(u8, arg, "-") or std.mem.eql(u8, arg, "--")) {
// All remaining arguments should be passed directly to the shell
// command. We shouldn't perform any further option processing.
try args.append(arg);
try args.append(try alloc.dupeZ(u8, arg));
while (iter.next()) |remaining_arg| {
try args.append(remaining_arg);
try args.append(try alloc.dupeZ(u8, remaining_arg));
}
break;
} else {
try args.append(arg);
try args.append(try alloc.dupeZ(u8, arg));
}
}
try env.put("GHOSTTY_BASH_INJECT", inject.slice());
@@ -310,30 +326,36 @@ fn setupBash(
);
try env.put("ENV", integ_dir);
// Join the accumulated arguments to form the final command string.
return try std.mem.join(alloc, " ", args.items);
// Since we built up a command line, we don't need to wrap it in
// ANOTHER shell anymore and can do a direct command.
return .{ .direct = try args.toOwnedSlice() };
}
test "bash" {
const testing = std.testing;
const alloc = testing.allocator;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
var env = EnvMap.init(alloc);
defer env.deinit();
const command = try setupBash(alloc, "bash", ".", &env);
defer if (command) |c| alloc.free(c);
const command = try setupBash(alloc, .{ .shell = "bash" }, ".", &env);
try testing.expectEqualStrings("bash --posix", command.?);
try testing.expectEqual(2, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("./shell-integration/bash/ghostty.bash", env.get("ENV").?);
try testing.expectEqualStrings("1", env.get("GHOSTTY_BASH_INJECT").?);
}
test "bash: unsupported options" {
const testing = std.testing;
const alloc = testing.allocator;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
const cmdlines = [_][]const u8{
const cmdlines = [_][:0]const u8{
"bash --posix",
"bash --rcfile script.sh --posix",
"bash --init-file script.sh --posix",
@@ -345,7 +367,7 @@ test "bash: unsupported options" {
var env = EnvMap.init(alloc);
defer env.deinit();
try testing.expect(try setupBash(alloc, cmdline, ".", &env) == null);
try testing.expect(try setupBash(alloc, .{ .shell = cmdline }, ".", &env) == null);
try testing.expect(env.get("GHOSTTY_BASH_INJECT") == null);
try testing.expect(env.get("GHOSTTY_BASH_RCFILE") == null);
try testing.expect(env.get("GHOSTTY_BASH_UNEXPORT_HISTFILE") == null);
@@ -354,17 +376,20 @@ test "bash: unsupported options" {
test "bash: inject flags" {
const testing = std.testing;
const alloc = testing.allocator;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
// bash --norc
{
var env = EnvMap.init(alloc);
defer env.deinit();
const command = try setupBash(alloc, "bash --norc", ".", &env);
defer if (command) |c| alloc.free(c);
const command = try setupBash(alloc, .{ .shell = "bash --norc" }, ".", &env);
try testing.expectEqualStrings("bash --posix", command.?);
try testing.expectEqual(2, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("1 --norc", env.get("GHOSTTY_BASH_INJECT").?);
}
@@ -373,52 +398,55 @@ test "bash: inject flags" {
var env = EnvMap.init(alloc);
defer env.deinit();
const command = try setupBash(alloc, "bash --noprofile", ".", &env);
defer if (command) |c| alloc.free(c);
const command = try setupBash(alloc, .{ .shell = "bash --noprofile" }, ".", &env);
try testing.expectEqualStrings("bash --posix", command.?);
try testing.expectEqual(2, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("1 --noprofile", env.get("GHOSTTY_BASH_INJECT").?);
}
}
test "bash: rcfile" {
const testing = std.testing;
const alloc = testing.allocator;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
var env = EnvMap.init(alloc);
defer env.deinit();
// bash --rcfile
{
const command = try setupBash(alloc, "bash --rcfile profile.sh", ".", &env);
defer if (command) |c| alloc.free(c);
try testing.expectEqualStrings("bash --posix", command.?);
const command = try setupBash(alloc, .{ .shell = "bash --rcfile profile.sh" }, ".", &env);
try testing.expectEqual(2, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("profile.sh", env.get("GHOSTTY_BASH_RCFILE").?);
}
// bash --init-file
{
const command = try setupBash(alloc, "bash --init-file profile.sh", ".", &env);
defer if (command) |c| alloc.free(c);
try testing.expectEqualStrings("bash --posix", command.?);
const command = try setupBash(alloc, .{ .shell = "bash --init-file profile.sh" }, ".", &env);
try testing.expectEqual(2, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("profile.sh", env.get("GHOSTTY_BASH_RCFILE").?);
}
}
test "bash: HISTFILE" {
const testing = std.testing;
const alloc = testing.allocator;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
// HISTFILE unset
{
var env = EnvMap.init(alloc);
defer env.deinit();
const command = try setupBash(alloc, "bash", ".", &env);
defer if (command) |c| alloc.free(c);
_ = try setupBash(alloc, .{ .shell = "bash" }, ".", &env);
try testing.expect(std.mem.endsWith(u8, env.get("HISTFILE").?, ".bash_history"));
try testing.expectEqualStrings("1", env.get("GHOSTTY_BASH_UNEXPORT_HISTFILE").?);
}
@@ -430,9 +458,7 @@ test "bash: HISTFILE" {
try env.put("HISTFILE", "my_history");
const command = try setupBash(alloc, "bash", ".", &env);
defer if (command) |c| alloc.free(c);
_ = try setupBash(alloc, .{ .shell = "bash" }, ".", &env);
try testing.expectEqualStrings("my_history", env.get("HISTFILE").?);
try testing.expect(env.get("GHOSTTY_BASH_UNEXPORT_HISTFILE") == null);
}
@@ -440,25 +466,35 @@ test "bash: HISTFILE" {
test "bash: additional arguments" {
const testing = std.testing;
const alloc = testing.allocator;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
var env = EnvMap.init(alloc);
defer env.deinit();
// "-" argument separator
{
const command = try setupBash(alloc, "bash - --arg file1 file2", ".", &env);
defer if (command) |c| alloc.free(c);
try testing.expectEqualStrings("bash --posix - --arg file1 file2", command.?);
const command = try setupBash(alloc, .{ .shell = "bash - --arg file1 file2" }, ".", &env);
try testing.expectEqual(6, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("-", command.?.direct[2]);
try testing.expectEqualStrings("--arg", command.?.direct[3]);
try testing.expectEqualStrings("file1", command.?.direct[4]);
try testing.expectEqualStrings("file2", command.?.direct[5]);
}
// "--" argument separator
{
const command = try setupBash(alloc, "bash -- --arg file1 file2", ".", &env);
defer if (command) |c| alloc.free(c);
try testing.expectEqualStrings("bash --posix -- --arg file1 file2", command.?);
const command = try setupBash(alloc, .{ .shell = "bash -- --arg file1 file2" }, ".", &env);
try testing.expectEqual(6, command.?.direct.len);
try testing.expectEqualStrings("bash", command.?.direct[0]);
try testing.expectEqualStrings("--posix", command.?.direct[1]);
try testing.expectEqualStrings("--", command.?.direct[2]);
try testing.expectEqualStrings("--arg", command.?.direct[3]);
try testing.expectEqualStrings("file1", command.?.direct[4]);
try testing.expectEqualStrings("file2", command.?.direct[5]);
}
}