From 02c12c06591e9ee4804a9b3c011b714842f387ac Mon Sep 17 00:00:00 2001 From: Daniel Netzer Date: Sun, 8 Nov 2026 04:09:34 +0200 Subject: [PATCH] Probe the Codex sandbox before coding and declare environment skips (#403) --- lab/internal/harness/execute.go | 25 ++ lab/internal/harness/protocol.go | 61 +++- .../harness/protocol_contract_test.go | 2 + lab/internal/harness/sandbox_probe.go | 217 ++++++++++++++ lab/internal/harness/sandbox_probe_test.go | 282 ++++++++++++++++++ lab/internal/intake/queue_test.go | 3 + lab/internal/molecule/readiness_run.go | 19 +- lab/internal/molecule/run.go | 4 + lab/internal/molecule/sandbox_probe_test.go | 171 +++++++++++ lab/internal/testenv/testenv.go | 32 ++ lab/internal/testenv/testenv_test.go | 62 ++++ 11 files changed, 873 insertions(+), 5 deletions(-) create mode 100644 lab/internal/harness/sandbox_probe.go create mode 100644 lab/internal/harness/sandbox_probe_test.go create mode 100644 lab/internal/molecule/sandbox_probe_test.go create mode 100644 lab/internal/testenv/testenv.go create mode 100644 lab/internal/testenv/testenv_test.go diff --git a/lab/internal/harness/execute.go b/lab/internal/harness/execute.go index 8cc929d..a59fb93 100644 --- a/lab/internal/harness/execute.go +++ b/lab/internal/harness/execute.go @@ -46,6 +46,19 @@ func HarnessCacheEnvironment(env []string, c resources.HarnessCache) []string { return append(out, "GOCACHE="+c.BuildCache, "GOTMPDIR="+c.TmpDir) } +// SandboxDeniesEnvironment returns env with FORGECELL_SANDBOX_DENIES replaced by denies. +// Every other entry is kept. +func SandboxDeniesEnvironment(env []string, denies string) []string { + out := make([]string, 0, len(env)+1) + for _, kv := range env { + if name, _, _ := strings.Cut(kv, "="); name == SandboxDeniesVariable { + continue + } + out = append(out, kv) + } + return append(out, SandboxDeniesVariable+"="+denies) +} + func Execute(ctx context.Context, o ExecuteOptions) (result Execution) { providerStarted := false fail := func(message string) Execution { @@ -104,6 +117,18 @@ func Execute(ctx context.Context, o ExecuteOptions) (result Execution) { } o.Env = HarnessCacheEnvironment(base, resources.HarnessCacheAt(root)) } + // BuildInvocation accepted a denies list only on a Codex coding request. + denies, err := requestSandboxDenies(o.Request) + if err != nil { + return fail(err.Error()) + } + if denies != "" { + base := o.Env + if base == nil { + base = os.Environ() + } + o.Env = SandboxDeniesEnvironment(base, denies) + } } if err = os.WriteFile(output+".schema.json", []byte(schema), 0600); err != nil { return fail(err.Error()) diff --git a/lab/internal/harness/protocol.go b/lab/internal/harness/protocol.go index 30857bf..cc8717b 100644 --- a/lab/internal/harness/protocol.go +++ b/lab/internal/harness/protocol.go @@ -40,7 +40,48 @@ const analysisInstruction = "You are assessing ticket readiness during the intak // overrides, which the user's Codex shell_environment_policy cannot filter. // Version 10 adds the environment-blocked coding outcome to the outcome schema // and the coding instruction. -const AdapterProtocolVersion = "10" +// Version 11 reads the sandbox capabilities a Codex coding sandbox denies +// (harnessSandboxDenies) and sets FORGECELL_SANDBOX_DENIES through a +// shell_environment_policy.set override. +const AdapterProtocolVersion = "11" + +// sandboxDeniesRequestKey carries the comma-separated capabilities the sandbox probe +// found denied in a Codex coding request (#403). Like harnessGoCache it is private to the +// Lab and never reaches the model-visible prompt. +const sandboxDeniesRequestKey = "harnessSandboxDenies" + +// requestSandboxDenies reads the denied capabilities from a request. An absent key means +// none; a present key must list known capabilities, comma-separated, each once. +func requestSandboxDenies(request map[string]any) (string, error) { + value, present := request[sandboxDeniesRequestKey] + if !present { + return "", nil + } + list, ok := value.(string) + if !ok || list == "" { + return "", fmt.Errorf("invalid sandbox denies list") + } + seen := map[string]bool{} + for _, name := range strings.Split(list, ",") { + if !slices.Contains(SandboxCapabilities, name) || seen[name] { + return "", fmt.Errorf("invalid sandbox denies list") + } + seen[name] = true + } + return list, nil +} + +// WithSandboxDenies returns the request with the denied capabilities attached, or the +// request itself when denies is empty. The caller's map is never modified. +// BuildInvocation accepts it only on a Codex coding request. +func WithSandboxDenies(request map[string]any, denies string) map[string]any { + if denies == "" { + return request + } + attached := maps.Clone(request) + attached[sandboxDeniesRequestKey] = denies + return attached +} // 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 @@ -446,13 +487,21 @@ func BuildInvocation(id string, request map[string]any, output string) (Invocati if cache != "" && (id != "codex" || kind != "molecule") { return Invocation{}, fmt.Errorf("a harness Go cache is supported only for Codex coding requests") } - // Server commands, paths, the pinned model and the harness cache are private to the - // Lab, so they never enter the model-visible request. Absent keys leave the prompt - // unchanged. + denies, err := requestSandboxDenies(request) + if err != nil { + return Invocation{}, err + } + if denies != "" && (id != "codex" || kind != "molecule") { + return Invocation{}, fmt.Errorf("sandbox denies are supported only for Codex coding requests") + } + // Server commands, paths, the pinned model, the harness cache and the sandbox denies + // are private to the Lab, so they never enter the model-visible request. Absent keys + // leave the prompt unchanged. visible := maps.Clone(request) delete(visible, "mcpServers") delete(visible, modelRequestKey) delete(visible, harnessCacheRequestKey) + delete(visible, sandboxDeniesRequestKey) payload, err := json.Marshal(visible) if err != nil { return Invocation{}, err @@ -482,6 +531,10 @@ func BuildInvocation(id string, request map[string]any, output string) (Invocati result.Args = append(result.Args, "--add-dir", cache) result.Args = append(result.Args, overrides...) } + if denies != "" { + // The denies list holds only known capability names, so it needs no escaping. + result.Args = append(result.Args, "-c", "shell_environment_policy.set."+SandboxDeniesVariable+`="`+denies+`"`) + } result.Args = append(result.Args, "-") case "claude-code": if meta && len(servers) > 0 { diff --git a/lab/internal/harness/protocol_contract_test.go b/lab/internal/harness/protocol_contract_test.go index 78fe71e..0825317 100644 --- a/lab/internal/harness/protocol_contract_test.go +++ b/lab/internal/harness/protocol_contract_test.go @@ -22,6 +22,7 @@ var adapterContractDigests = map[string]string{ "8": "df91601b9703e2a050087e824b5106cdb54fba19adbd3f4a86c9ea34a365b4bd", "9": "4d29883a0a8602024f44e7031fd4e72bda1845992dcaa6d2c6177c101548d8de", "10": "74c1394d24976dd43685cc84d82e4831c1881d0f276ecbe31de57ec999cfffe4", + "11": "1c889a6a2199dbeaafd9383575e9c934fccaa8cfdb6e662aaa286c8731ff676e", } func adapterContractDigest(t *testing.T) string { @@ -45,6 +46,7 @@ func adapterContractDigest(t *testing.T) string { {"review", map[string]any{"kind": ReviewKind}}, {"review+extensions", map[string]any{"kind": ReviewKind, "mcpServers": servers}}, {"review+model", map[string]any{"kind": ReviewKind, "harnessModel": "model-fixture"}}, {"molecule+harnessCache", map[string]any{"kind": "molecule", "codingAllowlist": allowlist, "harnessGoCache": "/fixture/lab/caches/harness"}}, + {"molecule+sandboxDenies", map[string]any{"kind": "molecule", "codingAllowlist": allowlist, "harnessGoCache": "/fixture/lab/caches/harness", "harnessSandboxDenies": "ps,localhost-bind"}}, } { call, err := BuildInvocation(id, c.request, "/fixture/result") if err != nil { diff --git a/lab/internal/harness/sandbox_probe.go b/lab/internal/harness/sandbox_probe.go new file mode 100644 index 0000000..a027902 --- /dev/null +++ b/lab/internal/harness/sandbox_probe.go @@ -0,0 +1,217 @@ +package harness + +import ( + "context" + "fmt" + "os" + "slices" + "strings" + "time" + + "github.com/DanielNetzer/forgecell/lab/internal/process" + "github.com/DanielNetzer/forgecell/lab/internal/resources" +) + +// Sandbox capabilities the probe checks, in the order it reports them (#403). +const ( + SandboxWorkspaceWrite = "workspace-write" + SandboxHarnessCacheWrite = "harness-cache-write" + SandboxSpawn = "spawn" + SandboxPS = "ps" + SandboxLocalhostBind = "localhost-bind" +) + +// SandboxCapabilities lists every capability the probe reports, in its order. +var SandboxCapabilities = []string{SandboxWorkspaceWrite, SandboxHarnessCacheWrite, SandboxSpawn, SandboxPS, SandboxLocalhostBind} + +// States of one probe check, and of the probe as a whole. +const ( + SandboxOK = "ok" + SandboxDenied = "denied" + SandboxUnknown = "unknown" + SandboxProbeRan = "ran" + SandboxProbeUnavailable = "unavailable" +) + +// SandboxDeniesVariable names the denied capabilities, comma-separated, in a Codex +// coding run's environment. It is unset when nothing is denied. +const SandboxDeniesVariable = "FORGECELL_SANDBOX_DENIES" + +// DefaultSandboxProbeTimeout bounds the probe and, separately, the version read. +const DefaultSandboxProbeTimeout = 20 * time.Second + +// sandboxProbeScript is the fixed POSIX probe. It is never repository code: $1 is the +// workspace and $2 the harness GOCACHE, and every check prints one name=state line. +const sandboxProbeScript = `ws=$1 +cache=$2 +writable() { + f="$1/.forgecell-sandbox-probe-$$" + if : > "$f" 2>/dev/null && rm -f "$f" 2>/dev/null; then echo ok; else rm -f "$f" 2>/dev/null; echo denied; fi +} +echo "workspace-write=$(writable "$ws")" +echo "harness-cache-write=$(writable "$cache")" +if sh -c 'exit 0' >/dev/null 2>&1; then echo spawn=ok; else echo spawn=denied; fi +if ps -p $$ >/dev/null 2>&1; then echo ps=ok; else echo ps=denied; fi +if command -v python3 >/dev/null 2>&1; then + if python3 -c 'import socket; s=socket.socket(); s.bind(("127.0.0.1", 0)); s.listen(1); s.close()' >/dev/null 2>&1; then echo localhost-bind=ok; else echo localhost-bind=denied; fi +elif command -v nc >/dev/null 2>&1; then + nc -l 127.0.0.1 47391 /dev/null 2>&1 & + pid=$! + sleep 1 + if kill -0 "$pid" 2>/dev/null; then kill "$pid" 2>/dev/null; echo localhost-bind=ok; else echo localhost-bind=denied; fi +else + echo localhost-bind=unknown +fi +` + +// SandboxCheck is one probed capability and its state (ok, denied or unknown). +type SandboxCheck struct { + Name string `json:"name"` + State string `json:"state"` +} + +// SandboxProbe is the recorded result of probing a Codex coding sandbox before the +// coding run starts (#403). State is "ran" with every check, or "unavailable" with the +// reason when the probe could not run or its output could not be read. +type SandboxProbe struct { + CodexVersion string `json:"codexVersion,omitempty"` + State string `json:"state"` + Reason string `json:"reason,omitempty"` + Checks []SandboxCheck `json:"checks,omitempty"` +} + +// Denied returns the denied capabilities in probe order; none for an unavailable probe. +func (p SandboxProbe) Denied() []string { + var denied []string + for _, c := range p.Checks { + if c.State == SandboxDenied { + denied = append(denied, c.Name) + } + } + return denied +} + +// Denies reports whether the probe found capability denied. +func (p SandboxProbe) Denies(capability string) bool { + return slices.Contains(p.Denied(), capability) +} + +// DeniesValue is the comma-separated FORGECELL_SANDBOX_DENIES value, empty when nothing +// is denied. +func (p SandboxProbe) DeniesValue() string { + return strings.Join(p.Denied(), ",") +} + +// Instruction is the sentence appended to the coding instruction, empty when nothing is +// denied. +func (p SandboxProbe) Instruction() string { + denied := p.Denied() + if len(denied) == 0 { + return "" + } + return " The sandbox denies: " + strings.Join(denied, ", ") + ". Don't run checks that need them; the Lab runs the approved checks outside the sandbox." +} + +// SandboxProbeArgs is the argv after the provider executable: the same workspace-write +// sandbox and shell_environment_policy.set overrides as the coding run, and the fixed +// probe script run with the workspace and the harness GOCACHE as arguments. `codex +// sandbox` has no --add-dir and accepts -C only with --permission-profile, so the cache +// root is made writable with the writable_roots override and ProbeSandbox runs the +// command with the workspace as its working directory. +func SandboxProbeArgs(workspace, cacheRoot string) ([]string, error) { + if cacheRoot == "" || workspace == "" { + return nil, fmt.Errorf("the sandbox probe needs a workspace and a harness Go cache") + } + root, err := tomlBasicString(cacheRoot) + if err != nil { + return nil, err + } + overrides, err := codexHarnessCacheOverrides(cacheRoot) + if err != nil { + return nil, err + } + args := []string{"sandbox", "-c", `sandbox_mode="workspace-write"`, "-c", "sandbox_workspace_write.writable_roots=[" + root + "]"} + args = append(args, overrides...) + return append(args, "--", "sh", "-c", sandboxProbeScript, "forgecell-sandbox-probe", workspace, resources.HarnessCacheAt(cacheRoot).BuildCache), nil +} + +// ProbeSandbox runs the fixed probe through the Codex provider executable, with no model +// call. Any failure to run it (an older Codex without `sandbox`, a timeout, unreadable +// output) is returned as unavailable with the reason, never as an error, so coding still +// starts. A timeout of zero uses DefaultSandboxProbeTimeout. +func ProbeSandbox(ctx context.Context, provider, workspace, cacheRoot string, timeout time.Duration) SandboxProbe { + if timeout <= 0 { + timeout = DefaultSandboxProbeTimeout + } + probe := SandboxProbe{CodexVersion: codexVersion(ctx, provider, workspace, timeout)} + args, err := SandboxProbeArgs(workspace, cacheRoot) + if err != nil { + return unavailable(probe, err.Error()) + } + env := HarnessCacheEnvironment(os.Environ(), resources.HarnessCacheAt(cacheRoot)) + result := process.Run(ctx, process.Options{Argv: append([]string{provider}, args...), Dir: workspace, Env: env, Stdin: []byte{}, Timeout: timeout, MaxOutputBytes: 64_000}) + switch { + case result.TimedOut || result.ParentDeadline: + return unavailable(probe, fmt.Sprintf("codex sandbox timed out after %s", timeout)) + case !result.OK: + reason := "codex sandbox failed" + if result.Error != "" { + reason += ": " + boundedText(result.Error, 200) + } + return unavailable(probe, reason) + } + checks, err := parseSandboxProbe(result.Stdout) + if err != nil { + return unavailable(probe, err.Error()) + } + probe.State, probe.Checks = SandboxProbeRan, checks + return probe +} + +func unavailable(p SandboxProbe, reason string) SandboxProbe { + p.State, p.Reason, p.Checks = SandboxProbeUnavailable, reason, nil + return p +} + +// codexVersion reads ` --version`, bounded to one printable line; it is empty +// when unreadable. +func codexVersion(ctx context.Context, provider, dir string, timeout time.Duration) string { + r := process.Run(ctx, process.Options{Argv: []string{provider, "--version"}, Dir: dir, Stdin: []byte{}, Timeout: timeout, MaxOutputBytes: 4096}) + if !r.OK { + return "" + } + line, _, _ := strings.Cut(strings.TrimSpace(r.Stdout), "\n") + return ReviewDisplayText(strings.TrimSpace(line), modelFieldRunes) +} + +// parseSandboxProbe reads the name=state lines. Blank lines are skipped; any other line, +// an unknown name or state, a duplicate or a missing check makes the output unreadable. +func parseSandboxProbe(stdout string) ([]SandboxCheck, error) { + states := map[string]string{} + for _, line := range strings.Split(stdout, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + name, state, ok := strings.Cut(line, "=") + if !ok || !slices.Contains(SandboxCapabilities, name) { + return nil, fmt.Errorf("unreadable sandbox probe output: unexpected line %q", boundedText(line, 80)) + } + if state != SandboxOK && state != SandboxDenied && state != SandboxUnknown { + return nil, fmt.Errorf("unreadable sandbox probe output: invalid state for %s", name) + } + if _, seen := states[name]; seen { + return nil, fmt.Errorf("unreadable sandbox probe output: duplicate %s", name) + } + states[name] = state + } + checks := make([]SandboxCheck, 0, len(SandboxCapabilities)) + for _, name := range SandboxCapabilities { + state, ok := states[name] + if !ok { + return nil, fmt.Errorf("unreadable sandbox probe output: missing %s", name) + } + checks = append(checks, SandboxCheck{Name: name, State: state}) + } + return checks, nil +} diff --git a/lab/internal/harness/sandbox_probe_test.go b/lab/internal/harness/sandbox_probe_test.go new file mode 100644 index 0000000..7da77de --- /dev/null +++ b/lab/internal/harness/sandbox_probe_test.go @@ -0,0 +1,282 @@ +package harness + +import ( + "context" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "syscall" + "testing" + "time" + + "github.com/DanielNetzer/forgecell/lab/internal/resources" +) + +// fakeSandboxCodex writes a fake codex that answers --version, records the argv of its +// `sandbox` call and then runs sandbox, the shell body for that subcommand. Every other +// subcommand fails as an unknown one. No real Codex is involved. +func fakeSandboxCodex(t *testing.T, sandbox string) (provider, argv string) { + t.Helper() + dir := t.TempDir() + provider = filepath.Join(dir, "codex") + argv = filepath.Join(dir, "argv.txt") + script := fmt.Sprintf("#!/bin/sh\nif [ \"$1\" = --version ]; then echo 'codex-cli 0.167.0'; exit 0; fi\nif [ \"$1\" = sandbox ]; then\nprintf '%%s\\n' \"$@\" > %q\n%s\nexit 0\nfi\necho \"error: unrecognized subcommand '$1'\" >&2\nexit 2\n", argv, sandbox) + // Holding ForkLock keeps the write handle out of any child forked meanwhile, so the + // exec never meets a busy file. + syscall.ForkLock.RLock() + defer syscall.ForkLock.RUnlock() + if err := os.WriteFile(provider, []byte(script), 0700); err != nil { + t.Fatal(err) + } + return provider, argv +} + +func probeLines(lines ...string) string { + return "printf '%s\\n' " + strings.Join(lines, " ") +} + +func TestSandboxProbeParsesAndRecordsDenials(t *testing.T) { + provider, argv := fakeSandboxCodex(t, probeLines("workspace-write=ok", "harness-cache-write=ok", "spawn=ok", "ps=denied", "localhost-bind=denied")) + workspace, cache := t.TempDir(), t.TempDir() + p := ProbeSandbox(context.Background(), provider, workspace, cache, 0) + if p.State != SandboxProbeRan || p.CodexVersion != "codex-cli 0.167.0" || p.Reason != "" { + t.Fatalf("probe %+v", p) + } + want := []SandboxCheck{{SandboxWorkspaceWrite, SandboxOK}, {SandboxHarnessCacheWrite, SandboxOK}, {SandboxSpawn, SandboxOK}, {SandboxPS, SandboxDenied}, {SandboxLocalhostBind, SandboxDenied}} + if !slices.Equal(p.Checks, want) { + t.Fatalf("checks %+v", p.Checks) + } + if got := p.DeniesValue(); got != "ps,localhost-bind" { + t.Fatalf("denies value %q", got) + } + if got := p.Instruction(); got != " The sandbox denies: ps, localhost-bind. Don't run checks that need them; the Lab runs the approved checks outside the sandbox." { + t.Fatalf("instruction %q", got) + } + if p.Denies(SandboxHarnessCacheWrite) || !p.Denies(SandboxPS) { + t.Fatalf("Denies disagrees with the checks: %+v", p) + } + raw, err := os.ReadFile(argv) + if err != nil { + t.Fatal(err) + } + args := strings.Split(strings.TrimSuffix(string(raw), "\n"), "\n") + overrides, err := codexHarnessCacheOverrides(cache) + if err != nil { + t.Fatal(err) + } + root, err := tomlBasicString(cache) + if err != nil { + t.Fatal(err) + } + head := slices.Concat([]string{"sandbox", "-c", `sandbox_mode="workspace-write"`, "-c", "sandbox_workspace_write.writable_roots=[" + root + "]"}, overrides, []string{"--", "sh", "-c"}) + if len(args) < len(head) || !slices.Equal(args[:len(head)], head) { + t.Fatalf("probe argv %q, want prefix %q", args, head) + } + if tail := args[len(args)-2:]; !slices.Equal(tail, []string{workspace, resources.HarnessCacheAt(cache).BuildCache}) { + t.Fatalf("probe arguments %q", tail) + } +} + +// `codex sandbox` rejects --add-dir and accepts -C only with --permission-profile, so +// the probe makes the cache root writable through writable_roots and runs in the +// workspace instead. +func TestSandboxProbeArgsUseWritableRootsNotAddDirOrC(t *testing.T) { + cache := `/tmp/odd "cache" \root` + args, err := SandboxProbeArgs("/work/space", cache) + if err != nil { + t.Fatal(err) + } + probeOptions := args[:slices.Index(args, "--")] + for _, flag := range []string{"--add-dir", "-C", "--cd"} { + if slices.Contains(probeOptions, flag) { + t.Fatalf("probe argv passes %s: %q", flag, args) + } + } + want := `sandbox_workspace_write.writable_roots=["/tmp/odd \"cache\" \\root"]` + if i := slices.Index(probeOptions, want); i < 1 || probeOptions[i-1] != "-c" { + t.Fatalf("probe argv %q lacks -c %s", args, want) + } + if _, err := SandboxProbeArgs("/work/space", "/tmp/new\nline"); err == nil { + t.Fatal("a cache root with a newline was quoted") + } + + dir := t.TempDir() + pwdFile := filepath.Join(dir, "pwd.txt") + provider, _ := fakeSandboxCodex(t, fmt.Sprintf("pwd -P > %q\n%s", pwdFile, probeLines("workspace-write=ok", "harness-cache-write=ok", "spawn=ok", "ps=ok", "localhost-bind=ok"))) + workspace := t.TempDir() + if p := ProbeSandbox(context.Background(), provider, workspace, t.TempDir(), 0); p.State != SandboxProbeRan { + t.Fatalf("probe %+v", p) + } + raw, err := os.ReadFile(pwdFile) + if err != nil { + t.Fatal(err) + } + real, err := filepath.EvalSymlinks(workspace) + if err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(string(raw)); got != real { + t.Fatalf("probe ran in %q, want the workspace %q", got, real) + } +} + +func TestSandboxProbeNothingDeniedLeavesInstructionUnchanged(t *testing.T) { + provider, _ := fakeSandboxCodex(t, probeLines("workspace-write=ok", "harness-cache-write=ok", "spawn=ok", "ps=ok", "localhost-bind=unknown")) + p := ProbeSandbox(context.Background(), provider, t.TempDir(), t.TempDir(), 0) + if p.State != SandboxProbeRan || len(p.Denied()) != 0 || p.DeniesValue() != "" || p.Instruction() != "" { + t.Fatalf("probe %+v", p) + } +} + +// The real fixed script runs here under plain sh, outside any sandbox: every check +// must report a known state, and the writable directories must be left clean. +func TestSandboxProbeScriptReportsEveryCheck(t *testing.T) { + provider, _ := fakeSandboxCodex(t, `while [ "$#" -gt 0 ] && [ "$1" != -- ]; do shift; done; shift; exec "$@"`) + workspace, cache := t.TempDir(), t.TempDir() + if err := os.Mkdir(resources.HarnessCacheAt(cache).BuildCache, 0700); err != nil { + t.Fatal(err) + } + p := ProbeSandbox(context.Background(), provider, workspace, cache, 0) + if p.State != SandboxProbeRan || len(p.Checks) != len(SandboxCapabilities) { + t.Fatalf("probe %+v", p) + } + for _, c := range p.Checks[:2] { + if c.State != SandboxOK { + t.Fatalf("%s is %s outside a sandbox", c.Name, c.State) + } + } + for _, dir := range []string{workspace, resources.HarnessCacheAt(cache).BuildCache} { + if entries, err := os.ReadDir(dir); err != nil || len(entries) != 0 { + t.Fatalf("probe left files in %s: %v %v", dir, entries, err) + } + } +} + +func TestSandboxProbeUnavailable(t *testing.T) { + for _, c := range []struct { + name, sandbox, reason string + }{ + {"unknown subcommand", "", ""}, + {"unknown name", probeLines("workspace-write=ok", "harness-cache-write=ok", "spawn=ok", "ps=ok", "localhost-bind=ok", "network=ok"), "unexpected line"}, + {"duplicate", probeLines("workspace-write=ok", "workspace-write=ok", "harness-cache-write=ok", "spawn=ok", "ps=ok", "localhost-bind=ok"), "duplicate workspace-write"}, + {"missing", probeLines("workspace-write=ok", "harness-cache-write=ok", "spawn=ok", "ps=ok"), "missing localhost-bind"}, + {"invalid state", probeLines("workspace-write=ok", "harness-cache-write=maybe", "spawn=ok", "ps=ok", "localhost-bind=ok"), "invalid state for harness-cache-write"}, + {"noise", "echo hello", "unexpected line"}, + } { + t.Run(c.name, func(t *testing.T) { + provider, _ := fakeSandboxCodex(t, c.sandbox) + if c.sandbox == "" { + // A Codex without the subcommand: rewrite the fake so `sandbox` is unknown. + syscall.ForkLock.RLock() + err := os.WriteFile(provider, []byte("#!/bin/sh\nif [ \"$1\" = --version ]; then echo 'codex-cli 0.100.0'; exit 0; fi\necho \"error: unrecognized subcommand '$1'\" >&2\nexit 2\n"), 0700) + syscall.ForkLock.RUnlock() + if err != nil { + t.Fatal(err) + } + } + p := ProbeSandbox(context.Background(), provider, t.TempDir(), t.TempDir(), 0) + if p.State != SandboxProbeUnavailable || p.Reason == "" || len(p.Checks) != 0 || p.DeniesValue() != "" || p.Instruction() != "" { + t.Fatalf("probe %+v", p) + } + if c.sandbox == "" && (!strings.HasPrefix(p.Reason, "codex sandbox failed") || p.CodexVersion != "codex-cli 0.100.0") { + t.Fatalf("missing subcommand recorded as %+v", p) + } + if !strings.Contains(p.Reason, c.reason) { + t.Fatalf("reason %q lacks %q", p.Reason, c.reason) + } + }) + } +} + +func TestSandboxProbeTimeoutIsUnavailable(t *testing.T) { + marker := filepath.Join(t.TempDir(), "started") + provider, _ := fakeSandboxCodex(t, fmt.Sprintf("touch %q\nsleep 60", marker)) + // The probe's own timeout is reported the same way as a context deadline; the + // context expires once the fake has provably started, so no wall-clock budget is + // raced. + ctx, cancel := context.WithCancelCause(context.Background()) + defer cancel(context.Canceled) + go func() { + for ctx.Err() == nil { + if _, err := os.Stat(marker); err == nil { + cancel(context.DeadlineExceeded) + return + } + time.Sleep(10 * time.Millisecond) + } + }() + p := ProbeSandbox(ctx, provider, t.TempDir(), t.TempDir(), time.Minute) + if p.State != SandboxProbeUnavailable || !strings.Contains(p.Reason, "timed out") || len(p.Checks) != 0 { + t.Fatalf("probe %+v", p) + } +} + +func TestSandboxDeniesOnlyOnCodexCodingRequests(t *testing.T) { + coding := map[string]any{"kind": "molecule", "codingAllowlist": []string{"Read"}} + call, err := BuildInvocation("codex", WithSandboxDenies(coding, "ps,localhost-bind"), "/out") + if err != nil { + t.Fatal(err) + } + if i := slices.Index(call.Args, `shell_environment_policy.set.FORGECELL_SANDBOX_DENIES="ps,localhost-bind"`); i < 1 || call.Args[i-1] != "-c" || call.Args[len(call.Args)-1] != "-" { + t.Fatalf("Codex argv %q", call.Args) + } + if strings.Contains(call.Input, "harnessSandboxDenies") || strings.Contains(call.Input, "localhost-bind") { + t.Fatal("the denies list reached the model-visible request") + } + if _, present := coding[sandboxDeniesRequestKey]; present { + t.Fatal("WithSandboxDenies modified the caller's map") + } + plain, err := BuildInvocation("codex", WithSandboxDenies(coding, ""), "/out") + if err != nil || slices.ContainsFunc(plain.Args, func(a string) bool { return strings.Contains(a, "FORGECELL_SANDBOX_DENIES") }) { + t.Fatalf("an empty denies list changed the argv: %q %v", plain.Args, err) + } + for _, c := range []struct { + id string + kind string + }{{"claude-code", "molecule"}, {"cursor", "molecule"}, {"codex", "ticket-analysis"}, {"codex", ReviewKind}, {"codex", "formula-improvement"}} { + request := WithSandboxDenies(map[string]any{"kind": c.kind, "codingAllowlist": []string{"Read"}}, "ps") + if _, err := BuildInvocation(c.id, request, "/out"); err == nil || !strings.Contains(err.Error(), "only for Codex coding requests") { + t.Fatalf("%s/%s accepted sandbox denies: %v", c.id, c.kind, err) + } + } + for _, bad := range []any{"", "ps,ps", "ps,network", "ps,", 3} { + request := map[string]any{"kind": "molecule", sandboxDeniesRequestKey: bad} + if _, err := BuildInvocation("codex", request, "/out"); err == nil { + t.Fatalf("invalid denies list %v accepted", bad) + } + } +} + +func TestExecuteSetsSandboxDeniesEnvironment(t *testing.T) { + dir := t.TempDir() + provider := filepath.Join(dir, "codex") + envFile := filepath.Join(dir, "env.txt") + script := fmt.Sprintf("#!/bin/sh\nenv | grep '^FORGECELL_SANDBOX_DENIES=' > %q\nout=\"\"\nwhile [ \"$#\" -gt 0 ]; do\n if [ \"$1\" = \"--output-last-message\" ]; then shift; out=\"$1\"; fi\n shift\ndone\ncat >/dev/null\nprintf '%%s' '{\"schemaVersion\":\"v1\",\"outcome\":\"completed\",\"reason\":\"done\",\"paths\":[]}' > \"$out\"\n", envFile) + syscall.ForkLock.RLock() + err := os.WriteFile(provider, []byte(script), 0700) + syscall.ForkLock.RUnlock() + if err != nil { + t.Fatal(err) + } + coding := map[string]any{"kind": "molecule", "codingAllowlist": []string{"Read"}} + for _, c := range []struct { + denies string + want string + }{{"ps,localhost-bind", "FORGECELL_SANDBOX_DENIES=ps,localhost-bind"}, {"", ""}} { + _ = os.Remove(envFile) + env := []string{"PATH=" + os.Getenv("PATH"), "FORGECELL_SANDBOX_DENIES=stale"} + if c.denies == "" { + env = env[:1] + } + result := Execute(context.Background(), ExecuteOptions{ID: "codex", Executable: provider, Request: WithSandboxDenies(coding, c.denies), Dir: dir, Env: env, Timeout: time.Minute}) + if result.Coding == nil || result.Coding.Outcome != "completed" { + t.Fatalf("%q: execution %+v", c.denies, result) + } + raw, _ := os.ReadFile(envFile) + if got := strings.TrimSpace(string(raw)); got != c.want { + t.Fatalf("%q: provider environment %q, want %q", c.denies, got, c.want) + } + } +} diff --git a/lab/internal/intake/queue_test.go b/lab/internal/intake/queue_test.go index 9e12b9c..223818f 100644 --- a/lab/internal/intake/queue_test.go +++ b/lab/internal/intake/queue_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" "time" + + "github.com/DanielNetzer/forgecell/lab/internal/testenv" ) func queueGH(t *testing.T, body string) { @@ -76,6 +78,7 @@ func TestIssueQueueSelectionAndFailures(t *testing.T) { } func TestIssueQueueCancellationStopsPipeHoldingDescendant(t *testing.T) { + testenv.SkipIfSandboxDenies(t, "ps") if runtime.GOOS != "linux" && runtime.GOOS != "darwin" { t.Skip("Unix process groups") } diff --git a/lab/internal/molecule/readiness_run.go b/lab/internal/molecule/readiness_run.go index 3ee7111..0df675a 100644 --- a/lab/internal/molecule/readiness_run.go +++ b/lab/internal/molecule/readiness_run.go @@ -1343,6 +1343,17 @@ func resumeApprovedIssue(ctx context.Context, o Options, expected formula.Loaded } harnessCache = &c } + // The same Codex sandbox is then probed with no model call (#403). A probe that + // cannot run is recorded as unavailable and coding starts as before; a sandbox + // that denies the Lab-owned cache would fail the run anyway, so it starts none. + var sandboxProbe *harness.SandboxProbe + if harnessCache != nil { + probe := harness.ProbeSandbox(ctx, codingBinding.Command[3], r.Workspace.Path, harnessCache.Root, harness.DefaultSandboxProbeTimeout) + if probe.Denies(harness.SandboxHarnessCacheWrite) { + return finishFailed(r, "Harness Go cache is not writable inside the Codex sandbox; no coding process started.") + } + sandboxProbe = &probe + } // The approval may come long after intake, so the extensions are verified again // here. A required one that is no longer usable starts no coding harness. codingServers, cleanupExtensions, extErr := attachExtensions(ctx, &r, f, formula.CodingHarness) @@ -1383,11 +1394,17 @@ func resumeApprovedIssue(ctx context.Context, o Options, expected formula.Loaded if resumed != "" { codingRequest["instruction"] = codingRequest["instruction"].(string) + resumeInstruction } + if sandboxProbe != nil { + codingRequest["instruction"] = codingRequest["instruction"].(string) + sandboxProbe.Instruction() + } codingRequest = harness.WithModel(codingRequest, codingBinding.Model) if harnessCache != nil { codingRequest = harness.WithHarnessCache(codingRequest, harnessCache.Root) } - attempt := HarnessAttempt{ID: attemptID, PlanDigest: o.Approve, CodingAllowlist: allowlist, Forgecell: &forgecell, Resumed: resumed, HarnessCache: harnessCache} + if sandboxProbe != nil { + codingRequest = harness.WithSandboxDenies(codingRequest, sandboxProbe.DeniesValue()) + } + attempt := HarnessAttempt{ID: attemptID, PlanDigest: o.Approve, CodingAllowlist: allowlist, Forgecell: &forgecell, Resumed: resumed, HarnessCache: harnessCache, SandboxProbe: sandboxProbe} attempt.RecordInvocation(codingBinding.Command, codingRequest) r.HarnessAttempts = append(r.HarnessAttempts, attempt) stage = append(stage, attemptID) diff --git a/lab/internal/molecule/run.go b/lab/internal/molecule/run.go index 0d9a72a..cb791e6 100644 --- a/lab/internal/molecule/run.go +++ b/lab/internal/molecule/run.go @@ -138,6 +138,10 @@ type HarnessAttempt struct { // HarnessCache records the paths of the Lab-owned Go cache a Codex coding attempt // could write (#396). It is absent for every other attempt and in earlier ledgers. HarnessCache *resources.HarnessCache `json:"harnessCache,omitempty"` + // SandboxProbe records the Codex version and the capabilities the probe found in a + // Codex coding attempt's sandbox before it started (#403). It is absent for every + // other attempt and in earlier ledgers. + SandboxProbe *harness.SandboxProbe `json:"sandboxProbe,omitempty"` } // maxTransientResumes bounds the automatic resumes of one coding stage after diff --git a/lab/internal/molecule/sandbox_probe_test.go b/lab/internal/molecule/sandbox_probe_test.go new file mode 100644 index 0000000..17eb273 --- /dev/null +++ b/lab/internal/molecule/sandbox_probe_test.go @@ -0,0 +1,171 @@ +package molecule + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/DanielNetzer/forgecell/lab/internal/harness" +) + +// codexSandboxProbeFixture is codexCacheFixture with a fake codex that also answers +// --version and runs sandbox, the shell body of its `sandbox` subcommand. Its exec call +// records argv, stdin (the prompt) and FORGECELL_SANDBOX_DENIES; its sandbox call leaves +// a marker. No real Codex, model or network is involved. +type sandboxProbeFiles struct{ argv, stdin, env, probed string } + +func codexSandboxProbeFixture(t *testing.T, sandbox string) (Options, sandboxProbeFiles) { + t.Helper() + o, argv, _ := codexCacheFixture(t) + root := filepath.Dir(argv) + files := sandboxProbeFiles{argv: argv, stdin: filepath.Join(root, "stdin.txt"), env: filepath.Join(root, "env.txt"), probed: filepath.Join(root, "probed")} + script := fmt.Sprintf("#!/bin/sh\nif [ \"$1\" = --version ]; then echo 'codex-cli 0.167.0'; exit 0; fi\nif [ \"$1\" = sandbox ]; then\ntouch %q\n%s\nexit 0\nfi\nprintf '%%s\\n' \"$@\" > %q\nenv | grep '^FORGECELL_SANDBOX_DENIES=' > %q\nout=\"\"\nwhile [ \"$#\" -gt 0 ]; do\n if [ \"$1\" = \"--output-last-message\" ]; then shift; out=\"$1\"; fi\n shift\ndone\ncat > %q\nprintf changed > code.txt\nprintf '%%s' '{\"schemaVersion\":\"v1\",\"outcome\":\"completed\",\"reason\":\"implemented\",\"paths\":[]}' > \"$out\"\n", files.probed, sandbox, argv, files.env, files.stdin) + if err := writeStub(filepath.Join(root, "codex"), []byte(script), 0700); err != nil { + t.Fatal(err) + } + return o, files +} + +func probeOutput(states ...string) string { + lines := make([]string, len(harness.SandboxCapabilities)) + for i, name := range harness.SandboxCapabilities { + lines[i] = name + "=" + states[i] + } + return "printf '%s\\n' " + strings.Join(lines, " ") +} + +func TestSandboxProbeRecordedAndPassedToCodexCoding(t *testing.T) { + t.Parallel() + o, files := codexSandboxProbeFixture(t, probeOutput("ok", "ok", "ok", "denied", "denied")) + r, err := runApprovedFixture(t, o) + if err != nil { + t.Fatal(err) + } + if len(r.HarnessAttempts) != 1 || r.HarnessAttempts[0].SandboxProbe == nil { + t.Fatalf("attempt did not record the sandbox probe: %+v", r.HarnessAttempts) + } + probe := *r.HarnessAttempts[0].SandboxProbe + if probe.State != harness.SandboxProbeRan || probe.CodexVersion != "codex-cli 0.167.0" || !slices.Equal(probe.Denied(), []string{"ps", "localhost-bind"}) { + t.Fatalf("probe %+v", probe) + } + data, err := os.ReadFile(filepath.Join(o.LabDir, "ledgers", r.ID+".json")) + if err != nil { + t.Fatal(err) + } + var saved Record + if err = json.Unmarshal(data, &saved); err != nil { + t.Fatal(err) + } + if got := saved.HarnessAttempts[0].SandboxProbe; got == nil || !slices.Equal(got.Checks, probe.Checks) || got.CodexVersion != probe.CodexVersion { + t.Fatalf("persisted ledger lost the sandbox probe: %+v", got) + } + if !strings.Contains(string(data), `"sandboxProbe"`) { + t.Fatal("ledger has no sandboxProbe field") + } + stdin, err := os.ReadFile(files.stdin) + if err != nil { + t.Fatal(err) + } + sentence := "The sandbox denies: ps, localhost-bind. Don't run checks that need them; the Lab runs the approved checks outside the sandbox." + if strings.Count(string(stdin), sentence) != 1 { + t.Fatalf("coding instruction lacks the denied-capabilities sentence: %s", stdin) + } + rawEnv, _ := os.ReadFile(files.env) + if got := strings.TrimSpace(string(rawEnv)); got != "FORGECELL_SANDBOX_DENIES=ps,localhost-bind" { + t.Fatalf("provider environment %q", got) + } + rawArgv, _ := os.ReadFile(files.argv) + if !slices.Contains(strings.Split(string(rawArgv), "\n"), `shell_environment_policy.set.FORGECELL_SANDBOX_DENIES="ps,localhost-bind"`) { + t.Fatalf("provider argv lacks the denies override: %s", rawArgv) + } +} + +func TestSandboxProbeNothingDeniedChangesNothing(t *testing.T) { + t.Parallel() + o, files := codexSandboxProbeFixture(t, probeOutput("ok", "ok", "ok", "ok", "unknown")) + r, err := runApprovedFixture(t, o) + if err != nil { + t.Fatal(err) + } + if len(r.HarnessAttempts) != 1 || r.HarnessAttempts[0].SandboxProbe == nil || r.HarnessAttempts[0].SandboxProbe.State != harness.SandboxProbeRan { + t.Fatalf("attempts %+v", r.HarnessAttempts) + } + stdin, _ := os.ReadFile(files.stdin) + if strings.Contains(string(stdin), "The sandbox denies") { + t.Fatal("an undenied sandbox changed the coding instruction") + } + rawEnv, _ := os.ReadFile(files.env) + rawArgv, _ := os.ReadFile(files.argv) + if strings.TrimSpace(string(rawEnv)) != "" || strings.Contains(string(rawArgv), "FORGECELL_SANDBOX_DENIES") { + t.Fatalf("FORGECELL_SANDBOX_DENIES set without a denial: %q %q", rawEnv, rawArgv) + } +} + +func TestSandboxProbeDeniedHarnessCacheStartsNoCoding(t *testing.T) { + t.Parallel() + o, files := codexSandboxProbeFixture(t, probeOutput("ok", "denied", "ok", "ok", "ok")) + r, err := runApprovedFixture(t, o) + if err != nil { + t.Fatal(err) + } + if r.Status != "failed" || len(r.HarnessAttempts) != 0 { + t.Fatalf("a denied harness cache reached coding: %s %+v", r.Status, r.HarnessAttempts) + } + if detail := r.Readiness.Events[len(r.Readiness.Events)-1].Detail; !strings.Contains(detail, "Harness Go cache is not writable inside the Codex sandbox") { + t.Fatalf("refusal detail %q", detail) + } + if _, err = os.Stat(files.probed); err != nil { + t.Fatalf("the probe did not run: %v", err) + } + if _, err = os.Stat(files.argv); !os.IsNotExist(err) { + t.Fatal("the coding process started despite a denied harness cache") + } +} + +func TestSandboxProbeUnavailableStillCodes(t *testing.T) { + t.Parallel() + o, files := codexSandboxProbeFixture(t, "echo \"error: unrecognized subcommand 'sandbox'\" >&2\nexit 2") + r, err := runApprovedFixture(t, o) + if err != nil { + t.Fatal(err) + } + if len(r.HarnessAttempts) != 1 || r.HarnessAttempts[0].Coding == nil || r.HarnessAttempts[0].Coding.Outcome != "completed" { + t.Fatalf("coding did not proceed: %+v", r.HarnessAttempts) + } + probe := r.HarnessAttempts[0].SandboxProbe + if probe == nil || probe.State != harness.SandboxProbeUnavailable || probe.Reason == "" || len(probe.Checks) != 0 { + t.Fatalf("probe %+v", probe) + } + stdin, _ := os.ReadFile(files.stdin) + rawEnv, _ := os.ReadFile(files.env) + if strings.Contains(string(stdin), "The sandbox denies") || strings.TrimSpace(string(rawEnv)) != "" { + t.Fatalf("an unavailable probe changed the coding run: %q", rawEnv) + } +} + +func TestSandboxProbeAbsentForOtherBindingsAndEarlierLedgers(t *testing.T) { + t.Parallel() + o, argv := claudeAllowlistFixture(t) + r, err := runApprovedFixture(t, o) + if err != nil { + t.Fatal(err) + } + if len(r.HarnessAttempts) != 1 || r.HarnessAttempts[0].SandboxProbe != nil { + t.Fatalf("a Claude Code attempt recorded a sandbox probe: %+v", r.HarnessAttempts) + } + rawArgv, err := os.ReadFile(argv) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(rawArgv), "FORGECELL_SANDBOX_DENIES") || strings.HasPrefix(string(rawArgv), "sandbox\n") { + t.Fatalf("a Claude Code run got the probe or its variable: %s", rawArgv) + } + var legacy HarnessAttempt + if err = json.Unmarshal([]byte(`{"id":"legacy","planDigest":"d","result":{}}`), &legacy); err != nil || legacy.SandboxProbe != nil { + t.Fatalf("an earlier attempt gained a sandbox probe: %+v %v", legacy, err) + } +} diff --git a/lab/internal/testenv/testenv.go b/lab/internal/testenv/testenv.go new file mode 100644 index 0000000..cd9197d --- /dev/null +++ b/lab/internal/testenv/testenv.go @@ -0,0 +1,32 @@ +// Package testenv lets a test declare the environment it needs, so a test run inside a +// sandbox that denies it skips with a reason instead of failing. +package testenv + +import ( + "os" + "slices" + "strings" + "testing" +) + +// SandboxDeniesVariable lists, comma-separated, the capabilities the Codex coding sandbox +// denies (#403). The Lab sets it only inside a Codex coding run; CI and the Lab's checks +// never set it, so the tests that need those capabilities still run there. +const SandboxDeniesVariable = "FORGECELL_SANDBOX_DENIES" + +// SandboxDenies reports whether FORGECELL_SANDBOX_DENIES lists capability, with the skip +// reason that names it. +func SandboxDenies(capability string) (reason string, denied bool) { + if !slices.Contains(strings.Split(os.Getenv(SandboxDeniesVariable), ","), capability) || capability == "" { + return "", false + } + return "the sandbox denies " + capability + " (" + SandboxDeniesVariable + " lists it); the Lab runs this test outside the sandbox", true +} + +// SkipIfSandboxDenies skips t when FORGECELL_SANDBOX_DENIES lists capability. +func SkipIfSandboxDenies(t testing.TB, capability string) { + t.Helper() + if reason, denied := SandboxDenies(capability); denied { + t.Skip(reason) + } +} diff --git a/lab/internal/testenv/testenv_test.go b/lab/internal/testenv/testenv_test.go new file mode 100644 index 0000000..c06b2d1 --- /dev/null +++ b/lab/internal/testenv/testenv_test.go @@ -0,0 +1,62 @@ +package testenv + +import ( + "os" + "strings" + "testing" +) + +func TestSandboxDeniesReadsTheVariable(t *testing.T) { + for _, c := range []struct { + value string + set bool + denied bool + }{ + {"", false, false}, + {"", true, false}, + {"ps", true, true}, + {"localhost-bind,ps", true, true}, + {"localhost-bind,spawn", true, false}, + {"psx", true, false}, + } { + // t.Setenv restores the original value when the test ends, also after Unsetenv. + t.Setenv(SandboxDeniesVariable, c.value) + if !c.set { + os.Unsetenv(SandboxDeniesVariable) + } + reason, denied := SandboxDenies("ps") + if denied != c.denied { + t.Fatalf("%q: denied = %v, want %v", c.value, denied, c.denied) + } + if denied && (!strings.Contains(reason, "ps") || !strings.Contains(reason, SandboxDeniesVariable)) { + t.Fatalf("%q: reason does not name ps and the variable: %q", c.value, reason) + } + if !denied && reason != "" { + t.Fatalf("%q: reason without a denial: %q", c.value, reason) + } + } + t.Setenv(SandboxDeniesVariable, "ps") + if _, denied := SandboxDenies(""); denied { + t.Fatal("an empty capability was denied") + } +} + +func TestSkipIfSandboxDeniesSkipsOnlyListedCapability(t *testing.T) { + for _, c := range []struct { + value string + skipped bool + }{{"", false}, {"localhost-bind", false}, {"ps", true}, {"spawn,ps", true}} { + t.Setenv(SandboxDeniesVariable, c.value) + var ran bool + t.Run("ps", func(t *testing.T) { + SkipIfSandboxDenies(t, "ps") + ran = true + if c.skipped { + t.Error("test ran although ps is denied") + } + }) + if ran == c.skipped { + t.Fatalf("%q: ran = %v, want skipped = %v", c.value, ran, c.skipped) + } + } +}