diff --git a/TESTING.md b/TESTING.md index 967a5c5bda..52e690d32f 100644 --- a/TESTING.md +++ b/TESTING.md @@ -451,7 +451,7 @@ all-source audit while staying outside untagged and Small debt. | Ledger kind | Source scope | Resource baseline | Tracking owner | Invariant / resource owner | Migration | Expiry | | --- | --- | --- | --- | --- | --- | --- | -| Audit baseline | all tracked test source | fixed_sleep: 437 calls / 161 files (historical regex census: 447 / 157) | ga-80po0c.2 | tracked test source totals remain visible as audit evidence; ga-80po0c.2 owns this point-in-time source census | P0.4a | 2026-10-01 | +| Audit baseline | all tracked test source | fixed_sleep: 438 calls / 161 files (historical regex census: 447 / 157) | ga-80po0c.2 | tracked test source totals remain visible as audit evidence; ga-80po0c.2 owns this point-in-time source census | P0.4a | 2026-10-01 | | Audit baseline | all tracked test source | listener_helper: 58 calls / 23 files | ga-80po0c.2.2.3 | all-source listener-helper call/file totals cannot drift without an explicit checked policy update; ga-80po0c.2.2.3 owns this all-source audit; tagged calls stay Large and receive no Medium exemption | P0.4c-listener-helper | 2026-10-01 | | Audit baseline | all tracked test source | subprocess: 555 calls / 168 files (historical regex census: 495 / 135) | ga-80po0c.2 | tracked test source totals remain visible as audit evidence; ga-80po0c.2 owns this point-in-time source census | P0.4a | 2026-10-01 | | Medium owner | `cmd/gc` package `main` | TestMain: environment, tmux | ga-80po0c.2.1 | cmd/gc TestMain is the checked package-level Medium owner for process environment and tmux namespace setup; only declared environment and tmux calls lexically inside TestMain leave Small debt | P0.4b/P0.4c-tmux | 2026-10-01 | diff --git a/internal/runtime/tmux/agent_slice_test.go b/internal/runtime/tmux/agent_slice_test.go index 9e652eaf46..fa2a81da19 100644 --- a/internal/runtime/tmux/agent_slice_test.go +++ b/internal/runtime/tmux/agent_slice_test.go @@ -53,18 +53,47 @@ func TestAgentSliceWrapsNewSessionWithCommandAndEnv(t *testing.T) { if len(exec.calls) == 0 { t.Fatal("no tmux calls recorded") } - args := exec.calls[0] - got := args[len(args)-1] - // The env -u prefix must end up INSIDE the scope wrapper so the unset - // still applies to the agent process. - want := "systemd-run --user --scope --slice=gascity-agents.slice --collect --quiet -- sh -c 'env -u LC_ALL claude'" - if got != want { - t.Fatalf("pane command = %q, want %q", got, want) + + // ADR-0051 transport: no -e flags anywhere (secrets would pin to argv), and + // the agent command is started via respawn-pane, which wrapPaneCommand still + // wraps in the systemd scope. Empty-value vars are removed via set-environment + // -r, NOT via an env -u prefix on the command — so the wrapped command is + // the plain agent command. + for i, call := range exec.calls { + for _, a := range call { + if a == "-e" { + t.Fatalf("ADR-0051: call %d still uses -e flag: %v", i, call) + } + } + } + + // LANG is delivered via set-environment over the socket. + foundLANG := false + for _, call := range exec.calls { + if sub := setEnvironmentArgs(call); contains(sub, "LANG") && contains(sub, "en_US.UTF-8") { + foundLANG = true + } + } + if !foundLANG { + t.Errorf("missing set-environment LANG en_US.UTF-8; calls: %v", exec.calls) } - // The -e session env flags must survive wrapping. - joined := strings.Join(args, "\x00") - if !strings.Contains(joined, "\x00-e\x00LANG=en_US.UTF-8\x00") { - t.Fatalf("new-session args missing LANG -e flag: %v", args) + assertEnvRemovedForNewProcess(t, exec.calls, "LC_ALL") + + // The command is started via respawn-pane and STILL wrapped in the systemd + // scope (wrapPaneCommand applies inside RespawnPane). + var respawnCmd string + for _, call := range exec.calls { + if cmd(call) == "respawn-pane" { + respawnCmd = call[len(call)-1] + break + } + } + want := "systemd-run --user --scope --slice=gascity-agents.slice --collect --quiet -- sh -c claude" + if respawnCmd == "" { + t.Fatalf("missing respawn-pane call; calls: %v", exec.calls) + } + if respawnCmd != want { + t.Fatalf("respawn pane command = %q, want %q", respawnCmd, want) } } @@ -154,14 +183,51 @@ func TestAgentSliceEmptyCommandNotWrapped(t *testing.T) { t.Setenv(AgentSliceEnv, "gascity-agents.slice") tm, exec := newSliceTestTmux(t) - // Empty command + env-only session must keep the empty trailing arg so - // tmux still starts the default shell. + // Empty command + env-only session: a bare new-session (no command) starts + // tmux's default shell, env is set via set-environment, and the pane is then + // respawned with NO shell-command so the shell restarts carrying that env. + // + // The respawn is required, not optional: the shell new-session started + // captured its environment before set-environment ran, so skipping the + // respawn delivers nothing to the pane's process — while show-environment + // still reports every variable as set. Because no shell-command is passed, + // wrapPaneCommand does not apply and the systemd wrapper must not appear. if err := tm.NewSessionWithCommandAndEnv("gc-test-empty", "/work", "", map[string]string{"LANG": "C"}); err != nil { t.Fatalf("NewSessionWithCommandAndEnv: %v", err) } - args := exec.calls[0] - if got := args[len(args)-1]; got != "" { - t.Fatalf("pane command = %q, want empty", got) + for i, call := range exec.calls { + if c := call[len(call)-1]; strings.Contains(c, "systemd-run") { + t.Fatalf("empty command should not be systemd-wrapped; call %d: %v", i, call) + } + } + var respawn []string + for _, call := range exec.calls { + if cmd(call) == "respawn-pane" { + respawn = call + break + } + } + if respawn == nil { + t.Fatalf("env-only session must respawn the pane so the shell picks up the "+ + "session env; calls: %v", exec.calls) + } + // respawn-pane -k -t , and nothing after the target: a trailing + // shell-command would be the wrapped form this test exists to exclude. + if got, want := respawn[len(respawn)-1], "gc-test-empty"; got != want { + t.Fatalf("env-only respawn should carry no shell-command; last arg = %q, want %q (call: %v)", + got, want, respawn) + } + // LANG still delivered via set-environment. Shape only — the behavioral + // counterpart is TestNewSessionWithCommandAndEnvDeliversEnvToEnvOnlyShell. + delivered := false + for _, call := range exec.calls { + if sub := setEnvironmentArgs(call); contains(sub, "LANG") && contains(sub, "C") { + delivered = true + break + } + } + if !delivered { + t.Fatalf("missing set-environment LANG C; calls: %v", exec.calls) } } diff --git a/internal/runtime/tmux/executor_test.go b/internal/runtime/tmux/executor_test.go index 66d8a93b86..175c66dedb 100644 --- a/internal/runtime/tmux/executor_test.go +++ b/internal/runtime/tmux/executor_test.go @@ -3,6 +3,7 @@ package tmux import ( "context" "errors" + "slices" "strconv" "strings" "testing" @@ -60,13 +61,221 @@ func TestNewSessionWithCommandAndEnvClearsEmptyVars(t *testing.T) { t.Fatal("no tmux calls recorded") } - args := exec.calls[0] - joined := strings.Join(args, "\x00") - if !strings.Contains(joined, "\x00-e\x00LANG=en_US.UTF-8\x00") { - t.Fatalf("new-session args missing LANG -e flag: %v", args) + // C2 (ADR-0051): no -e flag anywhere in the launch sequence — that is the + // argv-exposure defect this change removes. (runCtx prepends -u/-L to every + // call, so scan every arg of every call.) + for i, args := range exec.calls { + for _, a := range args { + if a == "-e" { + t.Fatalf("call %d still uses -e flag (ADR-0051 transport): %v", i, args) + } + } + } + + // The session is created bare (no command, no -e). cmd() finds the tmux + // subcommand token past the -u/-L prefix injected by runCtx. + newSession := exec.calls[0] + if cmd(newSession) != "new-session" { + t.Fatalf("first call = %q, want new-session: %v", cmd(newSession), newSession) + } + for _, a := range newSession { + if a == "claude" || strings.HasPrefix(a, "env ") { + t.Fatalf("new-session should not carry the command: %v", newSession) + } + } + + // LANG is set over the socket via set-environment; empty values via -r. + // + // The flag must be -r, not -u: -u drops only the session-scope entry, after + // which tmux re-merges the server-global environment when respawn-pane starts + // the command — so a stale global LC_ALL/LC_CTYPE would reach the pane anyway. + // -r is "removed from the environment before starting a new process". + // NOTE: this is a call-SHAPE assertion and cannot observe the actual child + // env; TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess is + // the behavioral counterpart that fails if this flag regresses to -u. + foundSet := false + for _, args := range exec.calls { + // set-environment -t KEY VALUE + if sub := setEnvironmentArgs(args); contains(sub, "LANG") && contains(sub, "en_US.UTF-8") { + foundSet = true + } + } + if !foundSet { + t.Errorf("missing set-environment LANG en_US.UTF-8") + } + assertEnvRemovedForNewProcess(t, exec.calls, "LC_ALL") + assertEnvRemovedForNewProcess(t, exec.calls, "LC_CTYPE") + + // The command is started via respawn-pane -k -t . + var respawn []string + for _, args := range exec.calls { + if cmd(args) == "respawn-pane" { + respawn = args + break + } + } + if respawn == nil { + t.Fatal("missing respawn-pane call to start the command") + } + if got := respawn[len(respawn)-1]; !strings.Contains(got, "claude") { + t.Fatalf("respawn-pane command = %q, want it to contain claude", got) + } + // The env -u prefix the old transport bolted onto the command is gone — + // unsetting is now a session-level set-environment -r, not a command prefix. + if got := respawn[len(respawn)-1]; strings.HasPrefix(got, "env ") { + t.Fatalf("respawn-pane command should not carry an env -u prefix: %q", got) + } +} + +// contains reports whether args contains s. +func contains(args []string, s string) bool { + for _, a := range args { + if a == s { + return true + } + } + return false +} + +// cmd returns the tmux subcommand token from a recorded call, skipping the -u +// and -L flags that runCtx prepends to every invocation. +func cmd(args []string) string { + for i := 0; i < len(args); i++ { + switch args[i] { + case "-u": + continue + case "-L": + i++ // skip socket name + continue + default: + return args[i] + } + } + return "" +} + +// setEnvironmentArgs returns the arguments a recorded call passed to its own +// set-environment subcommand, or nil when the call is not set-environment. +// +// Every set-environment flag assertion must go through this. runCtx prepends +// tmux's global -u (force UTF-8) to EVERY invocation, so scanning the whole argv +// for "-u" matches that wrapper flag and stays true even when set-environment +// was never given it. That vacuity is what let the original -u/-r defect ship +// green; repairing it in one test file while an identical copy survived in +// another is how it stayed green afterwards. The scan lives in one place so a +// fix cannot land in only half the call sites. +func setEnvironmentArgs(args []string) []string { + if cmd(args) != "set-environment" { + return nil + } + return args[slices.Index(args, "set-environment")+1:] +} + +// assertEnvRemovedForNewProcess fails unless the recorded calls remove key with +// set-environment -r, and fails if any of them removes it with -u instead. +// +// -u drops only the session-scope entry; tmux re-merges the server-global +// environment when respawn-pane starts the process, so a stale global value +// comes back. -r is "removed from the environment before starting a new +// process". This is a call-SHAPE assertion — the behavioral counterpart is +// TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess. +func assertEnvRemovedForNewProcess(t *testing.T, calls [][]string, key string) { + t.Helper() + removed := false + for _, args := range calls { + sub := setEnvironmentArgs(args) + if sub == nil || !contains(sub, key) { + continue + } + if contains(sub, "-u") { + t.Errorf("launch path used set-environment -u %s: -u drops only the "+ + "session-scope entry, so tmux re-merges the server-global value when "+ + "respawn-pane starts the process; want -r; call: %v", key, args) + } + if contains(sub, "-r") { + removed = true + } + } + if !removed { + t.Errorf("missing set-environment -r %s (empty-value removal); calls: %v", key, calls) + } +} + +// TestNewSessionWithCommandAndEnvNoSecretInArgv is the load-bearing ADR-0051 +// regression (Acceptance criterion C2): no secret value may be pinned to a +// long-lived process's argv. The -e transport placed every secret on the +// new-session command, which becomes the persistent tmux *server* argv; the +// set-environment transport must not reintroduce that. +// +// C2 scope note (ADR-0051 "C2 scope correction"): set-environment takes the value +// as a positional argv argument of a short-lived tmux *client* that exits in +// milliseconds. That transient client argv is the acknowledged bounded residual — +// NOT what this test guards. This test guards the persistent surface: the +// new-session call (server argv) and the respawn-pane call (the long-lived pane +// process). It also asserts no -e flag exists anywhere in the launch sequence. +func TestNewSessionWithCommandAndEnvNoSecretInArgv(t *testing.T) { + exec := &fakeExecutor{} + tm := NewTmux() + tm.exec = exec + + const secretValue = "sk-SECRET-v1-0123456789-do-not-leak" + env := map[string]string{ + "ANTHROPIC_AUTH_TOKEN": secretValue, + "OPENROUTER_API_KEY": secretValue, + "GC_INSTANCE_TOKEN": secretValue, + "BEADS_HOLDER_TOKEN": secretValue, // alias of GC_INSTANCE_TOKEN (same value, different name) + "GT_ROLE": "testrig/crew/x", + } + if err := tm.NewSessionWithCommandAndEnv("gc-test-no-leak", "", "claude", env); err != nil { + t.Fatalf("NewSessionWithCommandAndEnv: %v", err) + } + if len(exec.calls) == 0 { + t.Fatal("no tmux calls recorded") + } + + // 1. No -e flag anywhere in the launch sequence (transport is clean). + for i, args := range exec.calls { + for _, a := range args { + if a == "-e" { + t.Fatalf("ADR-0051 C2 violation: call %d (%s) still uses -e flag: %v", i, args[0], args) + } + } + } + + // 2. No secret value in the PERSISTENT surfaces: new-session (server argv) + // and respawn-pane (the long-lived pane process). set-environment calls are + // the transient client and are intentionally excluded. + persistent := []string{"new-session", "respawn-pane"} + for i, args := range exec.calls { + if !contains(persistent, cmd(args)) { + continue + } + joined := strings.Join(args, "\x00") + if strings.Contains(joined, secretValue) { + t.Fatalf("ADR-0051 C2 violation: secret value leaked into persistent %s "+ + "argv (call %d): %v", cmd(args), i, args) + } + // Even a "KEY=VALUE" pair (the -e serialization shape) must not appear. + if strings.Contains(joined, "ANTHROPIC_AUTH_TOKEN=") || + strings.Contains(joined, "OPENROUTER_API_KEY=") || + strings.Contains(joined, "GC_INSTANCE_TOKEN=") || + strings.Contains(joined, "BEADS_HOLDER_TOKEN=") { + t.Fatalf("ADR-0051 C2 violation: KEY=VALUE pair in persistent %s argv "+ + "(call %d): %v", cmd(args), i, args) + } + } + + // 3. Sanity: env WAS delivered — via set-environment (not -e). At least one + // set-environment call carries the GT_ROLE value (non-secret). + delivered := false + for _, args := range exec.calls { + if cmd(args) == "set-environment" && contains(args, "testrig/crew/x") { + delivered = true + break + } } - if got := args[len(args)-1]; got != "env -u LC_ALL -u LC_CTYPE claude" { - t.Fatalf("command = %q, want env -u LC_ALL -u LC_CTYPE claude", got) + if !delivered { + t.Errorf("expected a set-environment call delivering GT_ROLE; calls: %v", exec.calls) } } diff --git a/internal/runtime/tmux/tmux.go b/internal/runtime/tmux/tmux.go index bef2bfef7b..b291f516d5 100644 --- a/internal/runtime/tmux/tmux.go +++ b/internal/runtime/tmux/tmux.go @@ -526,15 +526,39 @@ func (t *Tmux) NewSessionWithCommand(name, workDir, command string) error { return nil } -// NewSessionWithCommandAndEnv creates a new detached tmux session with environment -// variables set via -e flags. This ensures the initial shell process inherits the -// correct environment from the session, rather than inheriting from the tmux server -// or parent process. The -e flags set session-level environment before the shell -// starts, preventing stale env vars (e.g., GT_ROLE from a parent mayor session) -// from leaking into crew/polecat shells. +// NewSessionWithCommandAndEnv creates a new detached tmux session and delivers +// its environment variables to the pane via tmux set-environment over the socket +// — never via -e argv literals. // -// The command should still use 'exec env' for WaitForCommand detection compatibility, -// but -e provides defense-in-depth for the initial shell environment. +// SECURITY (ADR-0051): a tmux -e KEY=VALUE flag places the value literally in the +// tmux *server* process's argv, where it is world-readable via ps for the entire +// server lifetime (which, because orphaned servers reparent to pid 1, can outlast +// the launching session — ADR-0029). Every secret gc injects (provider tokens, +// instance/holder tokens) was therefore exposed in ps. set-environment sends the +// value over the tmux socket inside a short-lived *client* process that exits in +// milliseconds, so no secret value is pinned to a long-lived process's argv. +// +// ORDERING (correctness, empirically verified against tmux 3.7b): session-level +// environment set via set-environment is applied to a pane's process when that +// process *starts*. The pane's initial shell starts at new-session time, so env +// set after a bare new-session is NOT retroactively exported into an already- +// running shell, and typing the command into that shell (send-keys) would inherit +// the stale env — breaking the transport. Instead the session is created without +// a command, the env is set on the session, and the pane is then respawned with +// the command (respawn-pane -k), which re-reads the session environment. A direct +// new-session-with-command + set-environment split has the same problem (the +// command process starts before set-environment runs), so respawn-pane is the +// load-bearing step that makes the env actually reach the command. +// +// Empty env values mean "unset this var": they are applied as set-environment -r +// so the pane's process does not inherit a stale value from the tmux server's +// global environment (the defense-in-depth rationale the original -e comment +// wanted). The flag is load-bearing and -u is NOT a substitute: -u deletes the +// session-scope entry only, after which tmux still merges the server-global +// environment when building the new process's env, so the stale value returns. +// -r is documented as "removed from the environment before starting a new +// process" — which is exactly the respawn-pane in step 3. The command should +// still use 'exec env' for WaitForCommand detection compatibility. // Requires tmux >= 3.2. func (t *Tmux) NewSessionWithCommandAndEnv(name, workDir, command string, env map[string]string) error { if err := validateSessionName(name); err != nil { @@ -543,43 +567,61 @@ func (t *Tmux) NewSessionWithCommandAndEnv(name, workDir, command string, env ma if err := t.probeServerAlive(); err != nil { return err } + // 1. Create the session WITHOUT a command and WITHOUT any -e flags. The + // session's initial pane starts a default shell that we discard below; no + // secret value appears in this — or any — long-lived process's argv. args := []string{"new-session", "-d", "-s", name} if workDir != "" { args = append(args, "-c", workDir) } - // Add -e flags to set environment variables in the session before the shell starts. - // Keys are sorted for deterministic behavior. + if _, err := t.run(args...); err != nil { + return err + } + + // 2. Set every env var on the session over the socket via set-environment. + // Keys are sorted for deterministic behavior. Empty values remove the var + // for the new process (set-environment -r) so a stale server-global value + // does not leak in. -u would NOT do this: it drops only the session-scope + // entry, and the server-global value is re-merged at respawn time. keys := make([]string, 0, len(env)) for k := range env { keys = append(keys, k) } sort.Strings(keys) - var unsetKeys []string for _, k := range keys { if env[k] == "" { - // Empty values mean "unset this var". Collect for env -u prefix. - unsetKeys = append(unsetKeys, k) + if err := t.RemoveEnvironmentForNewProcess(name, k); err != nil { + return fmt.Errorf("unset env %q: %w", k, err) + } } else { - args = append(args, "-e", fmt.Sprintf("%s=%s", k, env[k])) + if err := t.SetEnvironment(name, k, env[k]); err != nil { + return fmt.Errorf("set env %q: %w", k, err) + } } } - // For vars that need unsetting, prefix the command with env -u flags. - // tmux -e sets session-level env but the shell process still inherits - // from the tmux server's global environment. env -u ensures the var - // is actually absent from the child process. - if len(unsetKeys) > 0 && command != "" { - var prefix string - for _, k := range unsetKeys { - prefix += " -u " + k + + // 3. Start the actual command by respawning the pane, which re-reads the + // session environment set in step 2. send-keys into the default shell would + // NOT see it (the shell captured its env at start, before step 2). + // + // The respawn is deliberately NOT conditional on a command. The default + // shell that new-session -d started in step 1 captured its environment + // before step 2 ran too, so an env-only session that skipped the respawn + // would deliver nothing to the pane's process while show-environment still + // reported every variable as set — a false-positive that hides the miss. + // Respawning with no shell-command re-runs the pane's original command + // (the default shell), this time with the session environment applied. + switch { + case command != "": + if err := t.RespawnPane(name, command); err != nil { + return fmt.Errorf("respawn pane with command: %w", err) + } + case len(env) > 0: + if err := t.RespawnPaneDefaultCommand(name); err != nil { + return fmt.Errorf("respawn pane for env-only session: %w", err) } - command = "env" + prefix + " " + command - } - // Add the command as the last argument - args = append(args, t.wrapPaneCommand(command)) - _, err := t.run(args...) - if err != nil { - return err } + _ = t.ConfigureServer() // tmux 3.3+: reset window-size from manual to latest (see NewSession). t.run("set-option", "-wt", name, "window-size", "latest") //nolint:errcheck // best-effort @@ -2833,12 +2875,33 @@ func (t *Tmux) SetEnvironment(session, key, value string) error { return err } -// RemoveEnvironment removes an environment variable from the session. +// RemoveEnvironment removes an environment variable from the session environment. +// +// This drops the SESSION-SCOPE entry only (set-environment -u). Processes the +// session starts afterwards still inherit the variable from the tmux server's +// GLOBAL environment if it is set there. When the intent is "the command must +// not see this variable at all", use RemoveEnvironmentForNewProcess instead. func (t *Tmux) RemoveEnvironment(session, key string) error { _, err := t.run("set-environment", "-t", session, "-u", key) return err } +// RemoveEnvironmentForNewProcess marks an environment variable to be removed +// from the environment before the session starts a new process (set-environment +// -r). +// +// SECURITY / CORRECTNESS (ADR-0051): -u is not a substitute here. -u deletes the +// session-scope entry, and tmux then merges the server-global environment when +// building the new process's env — so a stale server-global value (LANG, +// LC_ALL, GT_ROLE bleeding from a parent mayor session) comes straight back and +// reaches the agent pane. -r is documented as "the variable is to be removed +// from the environment before starting a new process", which is precisely the +// respawn-pane step of NewSessionWithCommandAndEnv. +func (t *Tmux) RemoveEnvironmentForNewProcess(session, key string) error { + _, err := t.run("set-environment", "-t", session, "-r", key) + return err +} + // GetEnvironment gets an environment variable from the session. func (t *Tmux) GetEnvironment(session, key string) (string, error) { out, err := t.run("show-environment", "-t", session, key) @@ -3674,6 +3737,19 @@ func (t *Tmux) RespawnPane(pane, command string) error { return err } +// RespawnPaneDefaultCommand restarts the pane's original command — for a session +// created by NewSession or NewSessionWithCommandAndEnv that is tmux's default +// shell — so the new process picks up session environment applied after the pane +// first started. +// +// No shell-command argument is passed, so wrapPaneCommand does not apply: an +// env-only session has no agent process to place in the systemd scope, only the +// default shell. +func (t *Tmux) RespawnPaneDefaultCommand(pane string) error { + _, err := t.run("respawn-pane", "-k", "-t", pane) + return err +} + // RespawnPaneWithWorkDir kills all processes in a pane and starts a new command // in the specified working directory. Use this when the pane's current working // directory may have been deleted. diff --git a/internal/runtime/tmux/tmux_test.go b/internal/runtime/tmux/tmux_test.go index 06ae4207bc..70fb23584b 100644 --- a/internal/runtime/tmux/tmux_test.go +++ b/internal/runtime/tmux/tmux_test.go @@ -2050,10 +2050,18 @@ func TestNewSessionWithCommandAndEnv(t *testing.T) { "GT_ROLE": "testrig/crew/testname", "GT_RIG": "testrig", "GT_CREW": "testname", - } - - // Create session with env vars and a command that prints GT_ROLE - cmd := `bash -c "echo GT_ROLE=$GT_ROLE; sleep 5"` + // ADR-0051 C1 acceptance: a special-character sentinel must survive the + // set-environment transport intact (space, =, quotes, $ all exercise the + // value-delivery path that -e serialized as KEY=VALUE). + "GT_SENTINEL": `a b=c;"$x`, + } + + // Create session with env vars and a command that prints GT_ROLE and the + // sentinel to a file, so the assertion reads what the COMMAND PROCESS saw — + // not just the session option (GetEnvironment). This is the load-bearing C1 + // check: the respawn-pane transport must deliver env to the running command. + outFile := filepath.Join(t.TempDir(), "env-probe.txt") + cmd := `bash -c 'echo "GT_ROLE=$GT_ROLE"; echo "SENT=$GT_SENTINEL"; printf "%s\n" "$GT_ROLE|$GT_SENTINEL" > ` + outFile + `; sleep 5'` if err := tm.NewSessionWithCommandAndEnv(sessionName, "", cmd, env); err != nil { t.Fatalf("NewSessionWithCommandAndEnv: %v", err) } @@ -2084,6 +2092,29 @@ func TestNewSessionWithCommandAndEnv(t *testing.T) { if gotRig != "testrig" { t.Errorf("GT_RIG = %q, want %q", gotRig, "testrig") } + + // C1 (ADR-0051): the command PROCESS must receive every env var with the + // correct value, including the special-character sentinel. Wait for the + // probe file the command writes, then read it back. This is the assertion + // that proves the set-environment + respawn-pane transport actually delivers + // env to the pane (not just that the session option is set). + deadline := time.Now().Add(5 * time.Second) + var probeData string + for time.Now().Before(deadline) { + if b, rerr := os.ReadFile(outFile); rerr == nil { + probeData = string(b) + break + } + time.Sleep(50 * time.Millisecond) + } + wantProbe := "testrig/crew/testname|a b=c;\"$x\n" + if probeData == "" { + t.Fatal("command process never wrote the env probe file — env did not reach the pane (C1 transport failure)") + } + if probeData != wantProbe { + t.Errorf("command-process env probe = %q, want %q (special-char sentinel must survive transport)", + probeData, wantProbe) + } } func TestSetGetRemoveEnvironment(t *testing.T) { @@ -2157,6 +2188,105 @@ func TestNewSessionWithCommandAndEnvEmpty(t *testing.T) { } } +// TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess is the +// BEHAVIOURAL counterpart to TestNewSessionWithCommandAndEnvClearsEmptyVars, +// which can only assert the call shape. +// +// ADR-0051: an empty env value means "the command must not see this variable". +// Asserting that a set-environment -u call was made does not prove that — -u +// deletes the session-scope entry, and tmux then re-merges the SERVER-GLOBAL +// environment when respawn-pane starts the command, so a stale global value +// arrives in the pane anyway. Measured on tmux 3.7b: with -u the command +// process reads the global value; with -r it reads nothing. +// +// This test seeds a server-global value and reads the result back FROM THE +// COMMAND PROCESS (not show-environment), so it fails if the launch path ever +// regresses to -u. +// TestNewSessionWithCommandAndEnvDeliversEnvToEnvOnlyShell is the behavioral +// counterpart for the env-only (command == "") launch path. +// +// The shell that new-session -d starts captures its environment before +// set-environment runs, so without the step-3 respawn the variable never reaches +// the pane's process. show-environment cannot see that: it reports the +// session-scope entry and returns success in the broken case, which is why this +// test asks the SHELL what it actually has instead. +func TestNewSessionWithCommandAndEnvDeliversEnvToEnvOnlyShell(t *testing.T) { + if !hasTmux() { + t.Skip("tmux not installed") + } + + const ( + key = "GC_TEST_ENV_ONLY" + value = "delivered-to-env-only-shell" + unsetMarker = "" + ) + + tm := testTmux() + sessionName := "gt-test-envonly-" + t.Name() + _ = tm.KillSession(sessionName) + + if err := tm.NewSessionWithCommandAndEnv(sessionName, "", "", map[string]string{key: value}); err != nil { + t.Fatalf("NewSessionWithCommandAndEnv: %v", err) + } + defer func() { _ = tm.KillSession(sessionName) }() + + // Ask the pane's own shell to report the variable. It can only echo what is + // in its process environment, so a miss here is a real delivery failure. + outFile := filepath.Join(t.TempDir(), "env-only-probe.txt") + if err := tm.SendKeys(sessionName, + `printf "%s\n" "${`+key+`-`+unsetMarker+`}" > `+outFile); err != nil { + t.Fatalf("SendKeys: %v", err) + } + + // A timeout here reporting the marker IS the regression: the respawn was + // skipped and the shell never received the session environment. + waitForMarker(t, outFile, value) +} + +func TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess(t *testing.T) { + if !hasTmux() { + t.Skip("tmux not installed") + } + + const ( + key = "GC_TEST_STALE_GLOBAL" + staleValue = "stale-server-global-value" + unsetMarker = "" + ) + + tm := testTmux() + sessionName := "gt-test-staleglobal-" + t.Name() + seedName := sessionName + "-seed" + + _ = tm.KillSession(sessionName) + _ = tm.KillSession(seedName) + + // The global environment only exists while a server is running on this + // socket, so seed a throwaway session first, then plant the stale value. + if err := tm.NewSession(seedName, ""); err != nil { + t.Fatalf("NewSession(seed): %v", err) + } + defer func() { _ = tm.KillSession(seedName) }() + if err := tm.SetGlobalEnvironment(key, staleValue); err != nil { + t.Fatalf("SetGlobalEnvironment: %v", err) + } + + // ${VAR-} expands to the marker only when VAR is genuinely absent — + // it stays empty for a set-but-empty variable, so this distinguishes + // "removed" from "present but blank". + outFile := filepath.Join(t.TempDir(), "stale-global-probe.txt") + cmd := `bash -c 'printf "%s\n" "${` + key + `-` + unsetMarker + `}" > ` + outFile + `; sleep 5'` + + if err := tm.NewSessionWithCommandAndEnv(sessionName, "", cmd, map[string]string{key: ""}); err != nil { + t.Fatalf("NewSessionWithCommandAndEnv: %v", err) + } + defer func() { _ = tm.KillSession(sessionName) }() + + // A timeout here reporting the stale value IS the -u regression: the + // server-global value reached the command process. + waitForMarker(t, outFile, unsetMarker) +} + func TestIsTransientSendKeysError(t *testing.T) { tests := []struct { name string diff --git a/internal/testpolicy/resourcecensus/census.go b/internal/testpolicy/resourcecensus/census.go index 2290fc812d..8fa9b6a5d1 100644 --- a/internal/testpolicy/resourcecensus/census.go +++ b/internal/testpolicy/resourcecensus/census.go @@ -136,7 +136,7 @@ var bootstrapPolicy = Ledger{ { Scope: ScopeAll, Resource: ResourceFixedSleep, - BaselineCalls: 437, + BaselineCalls: 438, BaselineFiles: 161, ReportedCalls: 447, ReportedFiles: 157, diff --git a/scripts/runtime-tmux-tests.manifest b/scripts/runtime-tmux-tests.manifest index 82078c6f06..f311f41b44 100644 --- a/scripts/runtime-tmux-tests.manifest +++ b/scripts/runtime-tmux-tests.manifest @@ -37,6 +37,7 @@ TestDismissModelSwitchModal TestDismissModelSwitchModalNoOpOnWorkingPane TestProviderEnvSkipsEscapeBeforeEnter TestNewSessionWithCommandAndEnvClearsEmptyVars +TestNewSessionWithCommandAndEnvNoSecretInArgv TestRunBoundsByTmuxSubprocessTimeout TestRunInjectsSocketFlag TestRunNoSocketFlagWhenEmpty @@ -293,6 +294,8 @@ TestFindAgentPane_MultiPaneNoAgent TestNewSessionWithCommandAndEnv TestSetGetRemoveEnvironment TestNewSessionWithCommandAndEnvEmpty +TestNewSessionWithCommandAndEnvDeliversEnvToEnvOnlyShell +TestNewSessionWithCommandAndEnvRemovesStaleGlobalFromCommandProcess TestIsTransientSendKeysError TestNudgeSubmitDebounceUsesKimiProviderHint TestSendKeysLiteralWithRetry_ImmediateSuccess diff --git a/scripts/runtime_tmux_manifest_test.go b/scripts/runtime_tmux_manifest_test.go index e0f2610ef4..5f52e96709 100644 --- a/scripts/runtime_tmux_manifest_test.go +++ b/scripts/runtime_tmux_manifest_test.go @@ -24,22 +24,22 @@ func TestRuntimeTmuxManifestMatchesCanonicalLinuxIntegrationInventory(t *testing if drift := runtimeTmuxManifestDrift(manifest, declared); len(drift) != 0 { t.Fatalf("runtime-tmux manifest drift:\n%s\nupdate %s", strings.Join(drift, "\n"), runtimeTmuxManifestRelativePath) } - if got, want := len(manifest), 346; got != want { + if got, want := len(manifest), 349; got != want { t.Fatalf("runtime-tmux manifest contains %d tests, want %d", got, want) } untagged := discoverRuntimeTmuxTests(t, dir, "linux", false) - if got, want := len(untagged), 232; got != want { + if got, want := len(untagged), 233; got != want { t.Fatalf("runtime-tmux untagged inventory contains %d tests, want %d", got, want) } - if got, want := len(declared)-len(untagged), 114; got != want { + if got, want := len(declared)-len(untagged), 116; got != want { t.Fatalf("runtime-tmux integration-only inventory contains %d tests, want %d", got, want) } } func TestRuntimeTmuxManifestSixShardsPartitionInventoryExactlyOnce(t *testing.T) { manifest := parseRuntimeTmuxManifest(t, filepath.Join(repoRoot(t), runtimeTmuxManifestRelativePath)) - wantShardCounts := []int{58, 58, 58, 58, 57, 57} + wantShardCounts := []int{59, 58, 58, 58, 58, 58} seen := make(map[string]int, len(manifest)) for shardIndex := 0; shardIndex < len(wantShardCounts); shardIndex++ { diff --git a/test/test-resources.toml b/test/test-resources.toml index b9d8853bce..3506662722 100644 --- a/test/test-resources.toml +++ b/test/test-resources.toml @@ -23,7 +23,7 @@ expires = "2026-10-01" [[audit_baseline]] scope = "all" resource = "fixed_sleep" -baseline_calls = 437 +baseline_calls = 438 baseline_files = 161 reported_calls = 447 reported_files = 157