diff --git a/lab/internal/harness/harness_cache_test.go b/lab/internal/harness/harness_cache_test.go index 948bafc..801890c 100644 --- a/lab/internal/harness/harness_cache_test.go +++ b/lab/internal/harness/harness_cache_test.go @@ -63,8 +63,8 @@ func TestHarnessCacheReachesCodexCodingInvocation(t *testing.T) { if !reflect.DeepEqual(plainEnv, []string{"GOCACHE=/user/go-build", "GOMODCACHE=/user/mod", "GOTMPDIR=/user/gotmp"}) { t.Fatalf("a coding run without the cache changed the environment: %q", plainEnv) } - if slices.Contains(plainArgv, "--add-dir") { - t.Fatalf("a coding run without the cache got --add-dir: %q", plainArgv) + if slices.Contains(plainArgv, "--add-dir") || slices.ContainsFunc(plainArgv, func(a string) bool { return strings.HasPrefix(a, "shell_environment_policy.") }) { + t.Fatalf("a coding run without the cache got --add-dir or a policy override: %q", plainArgv) } cachedArgv, cachedEnv := runCodexEcho(t, WithHarnessCache(request, root), base) want := []string{"GOCACHE=" + filepath.Join(root, "go-build"), "GOMODCACHE=/user/mod", "GOTMPDIR=" + filepath.Join(root, "gotmp")} @@ -78,9 +78,51 @@ func TestHarnessCacheReachesCodexCodingInvocation(t *testing.T) { if cachedArgv[len(cachedArgv)-1] != "-" || cachedArgv[slices.Index(cachedArgv, "--sandbox")+1] != "workspace-write" { t.Fatalf("sandbox or stdin argument changed: %q", cachedArgv) } - stripped := slices.Delete(slices.Clone(cachedArgv), i, i+2) + overrides := []string{ + "-c", `shell_environment_policy.set.GOCACHE="` + filepath.Join(root, "go-build") + `"`, + "-c", `shell_environment_policy.set.GOTMPDIR="` + filepath.Join(root, "gotmp") + `"`, + } + if i+2+len(overrides) > len(cachedArgv) || !reflect.DeepEqual(cachedArgv[i+2:i+2+len(overrides)], overrides) { + t.Fatalf("argv lacks the shell_environment_policy overrides %q: %q", overrides, cachedArgv) + } + stripped := slices.Delete(slices.Clone(cachedArgv), i, i+2+len(overrides)) if !reflect.DeepEqual(withoutOutputPaths(stripped), withoutOutputPaths(plainArgv)) { - t.Fatalf("argv differs beyond --add-dir:\n%q\n%q", stripped, plainArgv) + t.Fatalf("argv differs beyond --add-dir and the overrides:\n%q\n%q", stripped, plainArgv) + } +} + +func TestHarnessCacheOverridesQuoteTheRoot(t *testing.T) { + for _, c := range []struct{ root, gocache, gotmpdir string }{ + {`/lab/a"b`, `"/lab/a\"b/go-build"`, `"/lab/a\"b/gotmp"`}, + {`/lab/a\b`, `"/lab/a\\b/go-build"`, `"/lab/a\\b/gotmp"`}, + {`/lab/a\"b`, `"/lab/a\\\"b/go-build"`, `"/lab/a\\\"b/gotmp"`}, + } { + call, err := BuildInvocation("codex", WithHarnessCache(map[string]any{"kind": "molecule"}, c.root), "/fixture/result") + if err != nil { + t.Fatalf("%q: %v", c.root, err) + } + for _, want := range []string{"shell_environment_policy.set.GOCACHE=" + c.gocache, "shell_environment_policy.set.GOTMPDIR=" + c.gotmpdir} { + if i := slices.Index(call.Args, want); i < 1 || call.Args[i-1] != "-c" { + t.Fatalf("%q: argv lacks -c %s: %q", c.root, want, call.Args) + } + } + } +} + +func TestHarnessCacheOverridesRefuseANewline(t *testing.T) { + for _, s := range []string{"/lab/a\nb", "/lab/a\rb"} { + if _, err := tomlBasicString(s); err == nil { + t.Fatalf("quoted %q", s) + } + if _, err := codexHarnessCacheOverrides(s); err == nil { + t.Fatalf("built overrides for %q", s) + } + if _, err := BuildInvocation("codex", WithHarnessCache(map[string]any{"kind": "molecule"}, s), "/fixture/result"); err == nil { + t.Fatalf("built an invocation for %q", s) + } + } + if got, err := tomlBasicString(`/lab/plain`); err != nil || got != `"/lab/plain"` { + t.Fatalf("plain path quoted as %q, %v", got, err) } } diff --git a/lab/internal/harness/isolation_test.go b/lab/internal/harness/isolation_test.go index 268b003..a504e84 100644 --- a/lab/internal/harness/isolation_test.go +++ b/lab/internal/harness/isolation_test.go @@ -587,6 +587,12 @@ func TestAdapterIsolationReport(t *testing.T) { t.Fatalf("the claude-code reason omits %q: %s", mention, claude) } } + codex := AdapterIsolation("codex").Reason + for _, mention := range []string{"shell_environment_policy.set.GOCACHE", "GOTMPDIR"} { + if !strings.Contains(codex, mention) { + t.Fatalf("the codex reason omits %q: %s", mention, codex) + } + } native := []string{"/forgecell", "__adapter", "claude-code", "/claude"} if got := BindingIsolation(native); got != AdapterIsolation("claude-code") { t.Fatalf("%+v", got) diff --git a/lab/internal/harness/protocol.go b/lab/internal/harness/protocol.go index 0cf40fb..76e595d 100644 --- a/lab/internal/harness/protocol.go +++ b/lab/internal/harness/protocol.go @@ -8,6 +8,7 @@ import ( "github.com/DanielNetzer/forgecell/lab/internal/formula" "github.com/DanielNetzer/forgecell/lab/internal/process" "github.com/DanielNetzer/forgecell/lab/internal/readiness" + "github.com/DanielNetzer/forgecell/lab/internal/resources" "io" "maps" "math" @@ -35,7 +36,9 @@ const analysisInstruction = "You are assessing ticket readiness during the intak // reviewedAcceptance empty. // Version 8 reads the Lab-owned harness Go cache (harnessGoCache) on a Codex coding // request: --add-dir names it, and GOCACHE and GOTMPDIR point into it. -const AdapterProtocolVersion = "8" +// Version 9 also sets GOCACHE and GOTMPDIR through shell_environment_policy.set +// overrides, which the user's Codex shell_environment_policy cannot filter. +const AdapterProtocolVersion = "9" // harnessCacheRequestKey carries the root of the Lab-owned harness Go cache in a Codex // coding request. Like harnessModel it is private to the Lab and never reaches the @@ -56,6 +59,31 @@ func requestHarnessCache(request map[string]any) (string, error) { return root, nil } +// tomlBasicString quotes s as a TOML basic string, escaping backslash and double quote. +// A newline cannot be written into a single-line override, so it is refused. +func tomlBasicString(s string) (string, error) { + if strings.ContainsAny(s, "\r\n") { + return "", fmt.Errorf("a newline cannot be quoted in a Codex config override") + } + return `"` + strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(s) + `"`, nil +} + +// codexHarnessCacheOverrides returns the -c overrides that set GOCACHE and GOTMPDIR for +// every command Codex runs. shell_environment_policy.set applies after inherit and the +// include and exclude filters, so the user's policy cannot drop them. +func codexHarnessCacheOverrides(root string) ([]string, error) { + cache := resources.HarnessCacheAt(root) + var args []string + for _, v := range []struct{ name, path string }{{"GOCACHE", cache.BuildCache}, {"GOTMPDIR", cache.TmpDir}} { + quoted, err := tomlBasicString(v.path) + if err != nil { + return nil, err + } + args = append(args, "-c", "shell_environment_policy.set."+v.name+"="+quoted) + } + return args, nil +} + // WithHarnessCache returns the request with the harness Go cache root attached, or the // request itself when root is empty. The caller's map is never modified. BuildInvocation // accepts it only on a Codex coding request. @@ -292,7 +320,7 @@ func AdapterIsolation(id string) Isolation { case "claude-code": return Isolation{IsolationPartial, "Coding, formula-improvement and analysis with extensions run with --setting-sources project; analysis and review without extensions run with --safe-mode; MCP servers are limited to the Lab-approved set (--strict-mcp-config). User settings, plugins and hooks are excluded, so user hooks that rewrite tool output no longer change what the harness sees (observed in a live probe of --setting-sources user versus project). Not verified, hence partial: managed (enterprise) policy settings, and whether a user-level CLAUDE.md, user agents or user skills still load under --setting-sources project. Project .claude/settings.json can still load: it is fingerprinted from the base commit, a coding or formula-improvement run is refused when .claude holds settings, hooks or agents, and a coding run that changes .claude fails as a scope violation."} case "codex": - return Isolation{IsolationPartial, "Codex coding and formula-improvement runs still read the user's CODEX_HOME/config.toml (profiles, hooks, MCP servers); they run with approval_policy never and a sandbox, and config.toml is fingerprinted. Analysis and review add --strict-config, --ignore-rules and disable shell, plugins and hooks. A pinned model is not supported."} + return Isolation{IsolationPartial, "Codex coding and formula-improvement runs still read the user's CODEX_HOME/config.toml (profiles, hooks, MCP servers); they run with approval_policy never and a sandbox, and config.toml is fingerprinted. Coding runs with the Lab-owned Go cache override shell_environment_policy.set.GOCACHE and GOTMPDIR, so the user's shell_environment_policy cannot drop them. Analysis and review add --strict-config, --ignore-rules and disable shell, plugins and hooks. A pinned model is not supported."} case "cursor": return Isolation{IsolationUnknown, "Cursor Agent's user configuration (rules, MCP servers, plugins, hooks) is not verified to be excluded: runs only pass --sandbox enabled and --trust (and --mode ask for formula-improvement), and no Cursor configuration is fingerprinted. Analysis and review are unavailable. A pinned model is not supported."} } @@ -443,8 +471,14 @@ func BuildInvocation(id string, request map[string]any, output string) (Invocati } } if cache != "" { - // The workspace-write sandbox may also write exactly the Lab-owned cache. + // The workspace-write sandbox may also write exactly the Lab-owned cache, and + // every command Codex runs is pointed at it. + overrides, err := codexHarnessCacheOverrides(cache) + if err != nil { + return Invocation{}, err + } result.Args = append(result.Args, "--add-dir", cache) + result.Args = append(result.Args, overrides...) } result.Args = append(result.Args, "-") case "claude-code": diff --git a/lab/internal/harness/protocol_contract_test.go b/lab/internal/harness/protocol_contract_test.go index 3db5500..1ca898c 100644 --- a/lab/internal/harness/protocol_contract_test.go +++ b/lab/internal/harness/protocol_contract_test.go @@ -20,6 +20,7 @@ var adapterContractDigests = map[string]string{ "6": "65f4a5f18b8bcdcd538ab3ac5ce44c26de3fd62ebd07c8577f3413f14fc0e2fb", "7": "9a2549ee76ecae7395ab23915224d6294bd4ad8e46ca326f83ea6b0692da0d69", "8": "df91601b9703e2a050087e824b5106cdb54fba19adbd3f4a86c9ea34a365b4bd", + "9": "4d29883a0a8602024f44e7031fd4e72bda1845992dcaa6d2c6177c101548d8de", } func adapterContractDigest(t *testing.T) string {