diff --git a/build.zig b/build.zig index b97cceb..a779d63 100644 --- a/build.zig +++ b/build.zig @@ -180,17 +180,21 @@ pub fn build(b: *std.Build) void { palette_tests.root_module.addImport("ghostty-vt", ghostty.module("ghostty-vt")); test_step.dependOn(&b.addRunArtifact(palette_tests).step); - // herdr-core's frame format is its own root: the bytes of the wire - // protocol, testable with no core to talk to. The connection on top of it + // herdr-core's client is its own root: the frame format (the bytes of the + // wire protocol) and the pure rules about what a terminal's environment + // keeps, both testable with no core to talk to. The connection itself // wants a socket and a main loop, and is checked by running it. - const hc_frame_tests = b.addTest(.{ + const hc_tests = b.addTest(.{ .root_module = b.createModule(.{ - .root_source_file = b.path("src/hc/frame.zig"), + .root_source_file = b.path("src/hc.zig"), .target = target, .optimize = optimize, + .link_libc = true, }), }); - test_step.dependOn(&b.addRunArtifact(hc_frame_tests).step); + hc_tests.root_module.addImport("glib", gobject.module("glib2")); + hc_tests.root_module.addImport("glibunix", gobject.module("glibunix2")); + test_step.dependOn(&b.addRunArtifact(hc_tests).step); // And a root for the settings file itself. `Settings.zig` reaches GLib for // where the file lives, but reading and writing its *contents* is pure diff --git a/src/Session.zig b/src/Session.zig index 9aa9298..60e8916 100644 --- a/src/Session.zig +++ b/src/Session.zig @@ -678,7 +678,8 @@ fn reattach(self: *Session, address: []const u8) !Remote { } /// The environment a herdr-core shell is started with: this process's, as the -/// local pty would hand on, on top of the core's own. +/// local pty would hand on — and only that, since `hc.run` clears whatever the +/// core's own environment would otherwise add. /// /// Passed whole rather than left to the core, whose environment is whatever /// the session that first started it had — not necessarily the one this app diff --git a/src/hc.zig b/src/hc.zig index 77c7a4d..92d30a2 100644 --- a/src/hc.zig +++ b/src/hc.zig @@ -152,7 +152,8 @@ pub const Attached = struct { pub const RunOptions = struct { argv: []const []const u8, cwd: ?[]const u8 = null, - /// `NAME=value`, set on top of the core's own environment. + /// `NAME=value`: the program's whole environment. Not merely added to the + /// core's — see `run`. env: []const []const u8 = &.{}, }; @@ -174,6 +175,9 @@ const Core = struct { next_id: u64 = 1, + /// The core's process, asked for once per connection — see `coreEnviron`. + pid: ?i32 = null, + streams: std.AutoHashMapUnmanaged(u32, Stream) = .empty, /// Set while `dispatch` runs, which must not nest. @@ -651,13 +655,40 @@ fn writeAddress(address: []const u8) Error![]u8 { /// is forgotten straight away instead of piling up in `hc ls --all`. It has /// no parent: a pane is nobody's child, and with none the core types no notes /// about it into anything. +/// +/// The program gets exactly `opts.env`, the way a child of this process would. +/// The core only *adds* what it is given to its own environment, and its own is +/// whatever the shell that happened to start it had: start it from a Claude +/// Code session and every terminal it runs inherits that session's markers +/// (and so every `claude` in them believes it is a subagent), start it from a +/// nested compositor and they all inherit its `SWAYSOCK`. So the program is +/// started through `env -u`, unsetting each variable the core has and `opts` +/// does not — names only, so no value of the core's ends up in `hc ls`. pub fn run(alloc: std.mem.Allocator, opts: RunOptions) Error!Started { const self = connection() orelse return error.Unavailable; + const environ = coreEnviron(self); + defer if (environ) |e| glib.free(e.ptr); + + var argv: std.ArrayListUnmanaged([]const u8) = .empty; + defer argv.deinit(gpa); + if (environ) |e| { + var it = std.mem.splitScalar(u8, e, 0); + while (it.next()) |entry| { + const eq = std.mem.indexOfScalar(u8, entry, '=') orelse continue; + const name = entry[0..eq]; + if (name.len == 0 or coreOwned(name) or setIn(name, opts.env)) continue; + try argv.appendSlice(gpa, &.{ "-u", name }); + } + if (argv.items.len > 0) try argv.insert(gpa, 0, envBinary()); + } + try argv.appendSlice(gpa, opts.argv); + var p: Json = undefined; p.init(); defer p.deinit(); - writeRunParams(&p.j, opts) catch return error.OutOfMemory; + writeRunParams(&p.j, .{ .argv = argv.items, .cwd = opts.cwd, .env = opts.env }) catch + return error.OutOfMemory; const reply = try call(self, "terminal.run", p.written()); defer reply.deinit(); @@ -669,6 +700,55 @@ pub fn run(alloc: std.mem.Allocator, opts: RunOptions) Error!Started { }; } +/// The core's own environment, NUL-separated, to be freed with `glib.free`. +/// Null when it can't be read, which leaves programs with the core's leftovers +/// as before rather than failing to start them. +/// +/// Read out of procfs because the API has no call for it. The core always runs +/// on this machine here — nothing in this app starts terminals `on` another. +fn coreEnviron(self: *Core) ?[]u8 { + const pid = self.pid orelse blk: { + const reply = call(self, "server.status", "{}") catch return null; + defer reply.deinit(); + const pid: i32 = std.math.cast(i32, intField(result(reply), "pid") orelse return null) orelse + return null; + self.pid = pid; + break :blk pid; + }; + + var path_buf: [64]u8 = undefined; + const path = std.fmt.bufPrintSentinel(&path_buf, "/proc/{d}/environ", .{pid}, 0) catch return null; + var contents: [*]u8 = undefined; + var length: usize = 0; + if (glib.fileGetContents(path.ptr, &contents, &length, null) == 0) { + std.log.warn("herdr-core: could not read the core's environment; terminals inherit it", .{}); + return null; + } + return contents[0..length]; +} + +/// Variables the core sets for each terminal itself, which must survive `env -u` +/// even when its own environment happens to hold them too. +fn coreOwned(name: []const u8) bool { + return std.mem.startsWith(u8, name, "HC_") or + std.mem.eql(u8, name, "TERM_PROGRAM") or + std.mem.eql(u8, name, "JOURNAL_STREAM"); +} + +/// Whether `env` (`NAME=value` entries) sets `name`. +fn setIn(name: []const u8, env: []const []const u8) bool { + for (env) |entry| { + if (entry.len > name.len and entry[name.len] == '=' and std.mem.startsWith(u8, entry, name)) return true; + } + return false; +} + +/// `env` by path where it has one, so finding it doesn't depend on the PATH of +/// the very environment it is there to correct. +fn envBinary() []const u8 { + return if (c.access("/usr/bin/env", X_OK) == 0) "/usr/bin/env" else "env"; +} + fn writeRunParams(j: *std.json.Stringify, opts: RunOptions) !void { try j.beginObject(); try j.objectField("argv"); @@ -822,6 +902,21 @@ pub fn detach(channel: u32) void { sendFrame(self, channel, .close, &.{}) catch {}; } +test "a variable counts as set only by its exact name" { + const env = [_][]const u8{ "PATH=/bin", "CLAUDE=1", "EMPTY=" }; + try std.testing.expect(setIn("PATH", &env)); + try std.testing.expect(setIn("EMPTY", &env)); + try std.testing.expect(!setIn("PAT", &env)); + try std.testing.expect(!setIn("CLAUDECODE", &env)); +} + +test "the core's own per-terminal variables are never unset" { + try std.testing.expect(coreOwned("HC_TERMINAL_ID")); + try std.testing.expect(coreOwned("TERM_PROGRAM")); + try std.testing.expect(!coreOwned("CLAUDE_CODE_CHILD_SESSION")); + try std.testing.expect(!coreOwned("SWAYSOCK")); +} + test { _ = frame; }