From 15d59bff2024b33f99876e58fbd598439b9d28f6 Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:53:31 -0500 Subject: [PATCH 1/2] fix(tests): select native Git Bash for Codex adapter deadlines --- plugin/codex_unix_session_end_test.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/plugin/codex_unix_session_end_test.go b/plugin/codex_unix_session_end_test.go index 5162ca2bc..949a29825 100644 --- a/plugin/codex_unix_session_end_test.go +++ b/plugin/codex_unix_session_end_test.go @@ -196,12 +196,17 @@ func codexTestBash(t *testing.T) string { func codexTestBashCandidate(gitPath string) (string, error) { gitDir := filepath.Dir(gitPath) - // git.exe can live in cmd, bin, or mingw64/bin depending on installation. + // Use the actual shell, not Git's bin launcher: the launcher adds a process + // outside the adapter's HTTP deadline. Match the trusted fixture/CI shell. + // git.exe can live in cmd, bin, usr/bin, or mingw64/bin. for _, candidate := range []string{ filepath.Join(gitDir, "bash.exe"), - filepath.Join(gitDir, "..", "bin", "bash.exe"), - filepath.Join(gitDir, "..", "..", "bin", "bash.exe"), + filepath.Join(gitDir, "..", "usr", "bin", "bash.exe"), + filepath.Join(gitDir, "..", "..", "usr", "bin", "bash.exe"), } { + if !strings.HasSuffix(strings.ToLower(filepath.ToSlash(filepath.Clean(candidate))), "/usr/bin/bash.exe") { + continue + } if info, err := os.Stat(candidate); err == nil && !info.IsDir() { return filepath.Clean(candidate), nil } @@ -214,10 +219,12 @@ func TestCodexTestBashCandidates(t *testing.T) { name, gitDir, bashPath string directory, missing bool }{ - {name: "cmd sibling bin", gitDir: "cmd", bashPath: "bin/bash.exe"}, - {name: "bin colocated", gitDir: "bin", bashPath: "bin/bash.exe"}, - {name: "mingw64 sibling bin", gitDir: "mingw64/bin", bashPath: "bin/bash.exe"}, - {name: "reject directory", gitDir: "cmd", bashPath: "bin/bash.exe", directory: true, missing: true}, + {name: "cmd sibling usr bin", gitDir: "cmd", bashPath: "usr/bin/bash.exe"}, + {name: "bin sibling usr bin", gitDir: "bin", bashPath: "usr/bin/bash.exe"}, + {name: "mingw64 sibling usr bin", gitDir: "mingw64/bin", bashPath: "usr/bin/bash.exe"}, + {name: "usr bin colocated", gitDir: "usr/bin", bashPath: "usr/bin/bash.exe"}, + {name: "reject bin launcher", gitDir: "cmd", bashPath: "bin/bash.exe", missing: true}, + {name: "reject directory", gitDir: "cmd", bashPath: "usr/bin/bash.exe", directory: true, missing: true}, {name: "no candidate", gitDir: "cmd", missing: true}, } { t.Run(tc.name, func(t *testing.T) { From b4a7a1ec191896656719fd01c8a45abdb1f3b852 Mon Sep 17 00:00:00 2001 From: Daniel Rosales <111561081+dnlrsls@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:47:15 -0500 Subject: [PATCH 2/2] perf(tests): batch trusted Codex fixture tool checks --- plugin/codex_handoff_guard_test.go | 87 ++++++++++++++++++++++++++++ plugin/codex_session_handoff_test.go | 20 ++++++- 2 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 plugin/codex_handoff_guard_test.go diff --git a/plugin/codex_handoff_guard_test.go b/plugin/codex_handoff_guard_test.go new file mode 100644 index 000000000..8852ce728 --- /dev/null +++ b/plugin/codex_handoff_guard_test.go @@ -0,0 +1,87 @@ +package plugin_test + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +func TestCodexHandoffToolGuard(t *testing.T) { + tools := []string{"engram", "curl", "cat", "dirname", "jq", "date", "touch", "bash"} + cases := []struct{ name, remove, inject string }{ + {name: "owned files use one validation subprocess"}, + {name: "function shadow", inject: "jq() { :; }\n"}, + {name: "alias shadow", inject: "shopt -s expand_aliases\nalias jq=true\n"}, + {name: "wrong resolved path", inject: `command() { + if [ "$1" = -v ] && [ "$2" = jq ]; then printf '/outside/jq\n'; else builtin command "$@"; fi +} +`}, + } + for _, name := range tools { + cases = append(cases, struct{ name, remove, inject string }{name: "missing " + name, remove: name}) + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cwd := filepath.Join(t.TempDir(), "fixture root with spaces") + if err := os.Mkdir(cwd, 0700); err != nil { + t.Fatal(err) + } + f := newCodexIntentFixture(t) + env := codexHandoffEnv(t, cwd, "http://127.0.0.1:19437") + if tc.remove != "" { + if err := os.Remove(filepath.Join(cwd, "bin", tc.remove)); err != nil { + t.Fatal(err) + } + } + trace := filepath.Join(cwd, "validation-pids") + bin := codexIntentQuote(filepath.ToSlash(filepath.Join(cwd, "bin"))) + if f.cygpath != "" { + bin = "$(" + codexIntentQuote(f.cygpath) + " -u -- " + bin + ")" + } + script := "bin=" + bin + "\nexport PATH=\"$bin\"\n" + tc.inject + + "type() { builtin printf '%s\\n' \"${BASHPID-unavailable}\" >> " + codexIntentQuote(filepath.ToSlash(trace)) + "; builtin type \"$@\"; }\n" + + codexHandoffToolGuard() + "printf 'accepted\\n'\n" + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, f.bash, "--noprofile", "--norc", "-p", "-c", script) + cmd.WaitDelay = time.Second + cmd.Env, cmd.Dir = env, cwd + output, err := cmd.CombinedOutput() + if tc.remove != "" || tc.inject != "" { + if err == nil || cmd.ProcessState.ExitCode() != 92 || len(output) != 0 { + t.Fatalf("guard accepted invalid lookup: err=%v output=%q", err, output) + } + return + } + if err != nil || string(output) != "accepted\n" { + t.Fatalf("owned tools rejected: %v %q", err, output) + } + data, err := os.ReadFile(trace) + if err != nil { + t.Fatal(err) + } + pids := map[string]bool{} + calls := strings.Fields(string(data)) + for _, pid := range calls { + pids[pid] = true + } + if len(calls) != len(tools) { + t.Fatalf("validated %d tools, want %d", len(calls), len(tools)) + } + // Legacy macOS Bash lacks BASHPID. All lookup/rejection behavior + // still runs there; native Windows must expose process identity + // for the MSYS subprocess-budget regression. + if pids["unavailable"] && runtime.GOOS != "windows" { + return + } + if pids["unavailable"] || len(pids) != 1 { + t.Fatalf("validation used %d subprocesses for %d tool checks; want one subprocess and %d checks", len(pids), len(calls), len(tools)) + } + }) + } +} diff --git a/plugin/codex_session_handoff_test.go b/plugin/codex_session_handoff_test.go index 40999038a..d5d0954a8 100644 --- a/plugin/codex_session_handoff_test.go +++ b/plugin/codex_session_handoff_test.go @@ -607,6 +607,22 @@ func codexFixtureProject(t *testing.T, cwd, project string) { } } +func codexHandoffToolGuard() string { + // MSYS pays for each command-substitution fork. Collect all lookups in one + // subprocess while retaining the exact type/path pair for every tool. + return `resolved=$(for name in engram curl cat dirname jq date touch bash; do + type -t "$name" && command -v "$name" || exit 92 +done) || exit 92 +expected= +for name in engram curl cat dirname jq date touch bash; do + expected="${expected}file +$bin/$name +" +done +[ "$resolved" = "${expected%$'\n'}" ] || exit 92 +` +} + func codexHandoffEnv(t *testing.T, cwd, serverURL string, binary ...string) []string { t.Helper() target, err := url.Parse(strings.TrimSpace(serverURL)) @@ -704,9 +720,7 @@ bin=$(convert "$bin") || exit 91 cd "$root" || exit 91 export PATH="$bin" hash -r -for name in engram curl cat dirname jq date touch bash; do - [ "$(type -t "$name")" = file ] && [ "$(command -v "$name")" = "$bin/$name" ] || exit 92 -done +` + codexHandoffToolGuard() + ` if command -v engram.exe >/dev/null || command -v curl.exe >/dev/null; then exit 93 fi