diff --git a/docs/codex-sandbox-troubleshooting.md b/docs/codex-sandbox-troubleshooting.md new file mode 100644 index 0000000000..cd6ebd02b4 --- /dev/null +++ b/docs/codex-sandbox-troubleshooting.md @@ -0,0 +1,107 @@ +# Codex sandbox troubleshooting (macOS `no such host`) + +This doc explains the failure mode that caused [MUL-963][mul-963] and the +matrix the daemon now follows when writing Codex's per-task `config.toml`. + +[mul-963]: https://multica-api.copilothub.ai/issues/28c34ad2-102a-4f46-91ac-336ed78c5859 + +## Symptom fingerprint + +| Error text | Likely cause | +| ------------------------------------------------------------- | ------------------------------------------------------------------------------- | +| `dial tcp: lookup HOST: no such host` | **Codex Seatbelt sandbox blocking DNS** (macOS, `workspace-write` mode). | +| `dial tcp IP:PORT: connect: connection refused` | Server/daemon not running on that port (app-level, not sandbox). | +| `dial tcp IP:PORT: i/o timeout` | Container-level network policy or firewall (not Codex sandbox). | +| `x509: certificate signed by unknown authority` | TLS/CA issue, unrelated. | + +If you see `no such host` *inside a Codex session on macOS* but `curl https://multica-api.copilothub.ai` from a plain shell on the same machine works, you are hitting the Seatbelt bug below. + +## Root cause + +Upstream issue: [openai/codex#10390][codex-10390]. On macOS, Codex's Seatbelt +profile for `sandbox_mode = "workspace-write"` silently ignores the +`[sandbox_workspace_write] network_access = true` setting. The seatbelt +policy hard-codes `CODEX_SANDBOX_NETWORK_DISABLED=1`, which blocks DNS/UDP +syscalls. Go's `net.LookupHost` surfaces that as `no such host`. + +Linux (Landlock) is **not** affected — only macOS Seatbelt. + +[codex-10390]: https://github.com/openai/codex/issues/10390 + +## What the daemon does now + +The daemon writes a *multica-managed* block into each task's +`$CODEX_HOME/config.toml`, delimited by `# BEGIN multica-managed` / +`# END multica-managed` markers. Anything outside the markers is left +untouched so users can still tune Codex behavior. + +Decision matrix (see [`server/internal/daemon/execenv/codex_sandbox.go`](../server/internal/daemon/execenv/codex_sandbox.go)): + +| Host OS | Codex version | Managed block emits | +| --------- | ------------------------------------------------ | ------------------------------------------------------------------------- | +| non-darwin | any | `sandbox_mode = "workspace-write"` + `sandbox_workspace_write.network_access = true` (dotted-key form) | +| darwin | ≥ `CodexDarwinNetworkAccessFixedVersion` | same as above (upstream fix in effect) | +| darwin | older / unknown (current default) | `sandbox_mode = "danger-full-access"` + warn-level log | + +The managed block is always hoisted to the top of `config.toml` and uses +TOML dotted-key syntax rather than a `[sandbox_workspace_write]` section +header. Both are load-bearing: if the block sat after a user table like +`[permissions.multica]`, a bare `sandbox_mode = "..."` line would be parsed +as `permissions.multica.sandbox_mode` and Codex would silently ignore it. + +`CodexDarwinNetworkAccessFixedVersion` is an empty string today, meaning *no +known fixed release yet*. Bump it once a tagged Codex release includes the +upstream fix. + +When the daemon falls back to `danger-full-access`, it logs at `WARN`: + +``` +codex sandbox: falling back to danger-full-access on macOS + reason=codex on macOS: seatbelt ignores sandbox_workspace_write.network_access (openai/codex#10390) ... + codex_version=0.121.0 + hint=upgrade Codex CLI (e.g. `brew upgrade codex` or `npm i -g @openai/codex`) ... + config_path=/.../codex-home/config.toml +``` + +## Quick self-check commands + +From the host shell (outside the sandbox): + +```bash +# Is the Multica API reachable at all? +curl -sSf https://multica-api.copilothub.ai/healthz +``` + +From inside a Codex session (after the daemon writes its config): + +```bash +multica issue list --limit 1 --output json >/dev/null && echo OK +``` + +If the host curl works but the Codex-session call fails with `no such host`, +the sandbox is the culprit; confirm the daemon picked the right policy by +looking at the managed block in `$CODEX_HOME/config.toml`. + +## Options and trade-offs + +- **A. Domain-scoped `permissions` profile** (tight): when the upstream + `network_access` fix is available, prefer writing a `permissions.multica` + profile that allows only `multica-api.copilothub.ai` and + `multica-static.copilothub.ai`. Keeps filesystem sandbox intact. +- **B. `danger-full-access`** (current macOS fallback): drops the whole + Seatbelt profile. Simplest reliable workaround until the upstream fix is + released. +- **C. Upgrade Codex CLI**: `brew upgrade codex` or `npm i -g @openai/codex`. + Once a release containing [openai/codex#10390][codex-10390] is installed, + bump `CodexDarwinNetworkAccessFixedVersion` in `codex_sandbox.go` and + option A/the workspace-write path takes over automatically. + +## If you need to hand-verify + +```bash +# Inspect the managed block the daemon wrote for a given task. +sed -n '/# BEGIN multica-managed/,/# END multica-managed/p' \ + ~/multica_workspaces/$WORKSPACE_ID/$TASK_SHORT/codex-home/config.toml +``` + +The block is idempotent — re-running a task rewrites it in place. diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 32a4fbe66b..1f799d8489 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -45,6 +45,9 @@ type Daemon struct { runtimeIndex map[string]Runtime // runtimeID -> Runtime for provider lookups reloading sync.Mutex // prevents concurrent workspace syncs + versionsMu sync.RWMutex // guards agentVersions + agentVersions map[string]string // provider -> detected CLI version (set during registration) + cancelFunc context.CancelFunc // set by Run(); called by triggerRestart restartBinary string // non-empty after a successful update; path to the new binary updating atomic.Bool // prevents concurrent update attempts @@ -55,15 +58,32 @@ type Daemon struct { func New(cfg Config, logger *slog.Logger) *Daemon { cacheRoot := filepath.Join(cfg.WorkspacesRoot, ".repos") return &Daemon{ - cfg: cfg, - client: NewClient(cfg.ServerBaseURL), - repoCache: repocache.New(cacheRoot, logger), - logger: logger, - workspaces: make(map[string]*workspaceState), - runtimeIndex: make(map[string]Runtime), + cfg: cfg, + client: NewClient(cfg.ServerBaseURL), + repoCache: repocache.New(cacheRoot, logger), + logger: logger, + workspaces: make(map[string]*workspaceState), + runtimeIndex: make(map[string]Runtime), + agentVersions: make(map[string]string), } } +// setAgentVersion records the detected CLI version for an agent provider so +// later task-dispatch code (e.g. Codex sandbox policy) can read it. +func (d *Daemon) setAgentVersion(provider, version string) { + d.versionsMu.Lock() + defer d.versionsMu.Unlock() + d.agentVersions[provider] = version +} + +// agentVersion returns the last-detected CLI version for an agent provider, +// or an empty string if unknown. +func (d *Daemon) agentVersion(provider string) string { + d.versionsMu.RLock() + defer d.versionsMu.RUnlock() + return d.agentVersions[provider] +} + // Run starts the daemon: resolves auth, registers runtimes, then polls for tasks. func (d *Daemon) Run(ctx context.Context) error { // Wrap context so handleUpdate can cancel the daemon for restart. @@ -188,6 +208,7 @@ func (d *Daemon) registerRuntimesForWorkspace(ctx context.Context, workspaceID s d.logger.Warn("skip registering runtime: version too old", "name", name, "version", version, "error", err) continue } + d.setAgentVersion(name, version) displayName := strings.ToUpper(name[:1]) + name[1:] if d.cfg.DeviceName != "" { displayName = fmt.Sprintf("%s (%s)", displayName, d.cfg.DeviceName) @@ -893,8 +914,9 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, taskLo // Try to reuse the workdir from a previous task on the same (agent, issue) pair. var env *execenv.Environment + codexVersion := d.agentVersion("codex") if task.PriorWorkDir != "" { - env = execenv.Reuse(task.PriorWorkDir, provider, taskCtx, d.logger) + env = execenv.Reuse(task.PriorWorkDir, provider, codexVersion, taskCtx, d.logger) } if env == nil { var err error @@ -904,6 +926,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, taskLo TaskID: task.ID, AgentName: agentName, Provider: provider, + CodexVersion: codexVersion, Task: taskCtx, }, d.logger) if err != nil { diff --git a/server/internal/daemon/execenv/codex_home.go b/server/internal/daemon/execenv/codex_home.go index 181b2e95b7..c81fbff735 100644 --- a/server/internal/daemon/execenv/codex_home.go +++ b/server/internal/daemon/execenv/codex_home.go @@ -6,7 +6,6 @@ import ( "log/slog" "os" "path/filepath" - "strings" ) // Directories to symlink from the shared ~/.codex/ into the per-task CODEX_HOME. @@ -30,10 +29,33 @@ var codexCopiedFiles = []string{ "instructions.md", } -// prepareCodexHome creates a per-task CODEX_HOME directory and seeds it with -// config from the shared ~/.codex/ home. Auth is symlinked (shared), config -// files are copied (isolated). +// CodexHomeOptions carries optional inputs for prepareCodexHomeWithOpts that +// affect the generated per-task config.toml. +type CodexHomeOptions struct { + // CodexVersion is the detected Codex CLI version (e.g. "0.121.0"). Empty + // means unknown; on macOS, unknown is treated as "probably broken" so the + // daemon falls back to danger-full-access for network access. See + // codex_sandbox.go for details. + CodexVersion string + // GOOS overrides the target platform when deciding the sandbox policy. + // Empty means use runtime.GOOS. Primarily exists so tests can exercise + // both macOS and Linux paths deterministically. + GOOS string +} + +// prepareCodexHome is a thin wrapper around prepareCodexHomeWithOpts kept for +// tests that don't care about platform-aware sandbox configuration. It +// assumes a Linux-like environment where workspace-write + network_access +// works correctly. func prepareCodexHome(codexHome string, logger *slog.Logger) error { + return prepareCodexHomeWithOpts(codexHome, CodexHomeOptions{GOOS: "linux"}, logger) +} + +// prepareCodexHomeWithOpts creates a per-task CODEX_HOME directory and seeds +// it with config from the shared ~/.codex/ home. Auth is symlinked (shared), +// config files are copied (isolated). The per-task config.toml gets a +// daemon-managed sandbox block picked by codexSandboxPolicyFor. +func prepareCodexHomeWithOpts(codexHome string, opts CodexHomeOptions, logger *slog.Logger) error { sharedHome := resolveSharedCodexHome() if err := os.MkdirAll(codexHome, 0o755); err != nil { @@ -67,10 +89,12 @@ func prepareCodexHome(codexHome string, logger *slog.Logger) error { } } - // Ensure config.toml has workspace-write sandbox with network access enabled. - // Codex needs network access to reach the Multica API (api.multica.ai). - if err := ensureCodexNetworkAccess(filepath.Join(codexHome, "config.toml")); err != nil { - logger.Warn("execenv: codex-home ensure network access failed", "error", err) + // Write a daemon-managed sandbox block into config.toml. On macOS we may + // need to fall back to danger-full-access because of openai/codex#10390; + // see codex_sandbox.go for the full rationale. + policy := codexSandboxPolicyFor(opts.GOOS, opts.CodexVersion) + if err := ensureCodexSandboxConfig(filepath.Join(codexHome, "config.toml"), policy, opts.CodexVersion, logger); err != nil { + logger.Warn("execenv: codex-home ensure sandbox config failed", "error", err) } return nil @@ -144,53 +168,11 @@ func ensureSymlink(src, dst string) error { return createFileLink(src, dst) } -// defaultCodexConfig is the minimal config.toml for Codex tasks. -// It sets workspace-write sandbox mode with network access enabled so the -// Multica CLI can reach api.multica.ai. -const defaultCodexConfig = `sandbox_mode = "workspace-write" +// (The daemon used to write a minimal inline config here; the authoritative +// sandbox/network directives now live in a managed block rendered by +// codex_sandbox.go's ensureCodexSandboxConfig so they can be updated +// idempotently without touching user-managed keys.) -[sandbox_workspace_write] -network_access = true -` - -// ensureCodexNetworkAccess ensures that config.toml exists and contains the -// sandbox_workspace_write section with network_access = true. If the file -// doesn't exist, it creates one with defaults. If it exists but lacks the -// network_access setting, the section is appended. -func ensureCodexNetworkAccess(configPath string) error { - data, err := os.ReadFile(configPath) - if os.IsNotExist(err) { - // No config.toml — create with defaults. - return os.WriteFile(configPath, []byte(defaultCodexConfig), 0o644) - } - if err != nil { - return fmt.Errorf("read config.toml: %w", err) - } - - content := string(data) - - // If the file already has network_access configured under sandbox_workspace_write, leave it alone. - if strings.Contains(content, "[sandbox_workspace_write]") && strings.Contains(content, "network_access") { - return nil - } - - // Append the section. If sandbox_mode is already set, only append the section block. - var appendStr string - if strings.Contains(content, "[sandbox_workspace_write]") { - // Section exists but missing network_access — append the key under it. - content = strings.Replace(content, "[sandbox_workspace_write]", "[sandbox_workspace_write]\nnetwork_access = true", 1) - return os.WriteFile(configPath, []byte(content), 0o644) - } - - // Section doesn't exist — append both sandbox_mode (if missing) and the section. - appendStr = "\n" - if !strings.Contains(content, "sandbox_mode") { - appendStr += "sandbox_mode = \"workspace-write\"\n" - } - appendStr += "\n[sandbox_workspace_write]\nnetwork_access = true\n" - - return os.WriteFile(configPath, append(data, []byte(appendStr)...), 0o644) -} // copyFileIfExists copies src to dst. If src doesn't exist, it's a no-op. // If dst already exists, it's not overwritten. diff --git a/server/internal/daemon/execenv/codex_sandbox.go b/server/internal/daemon/execenv/codex_sandbox.go new file mode 100644 index 0000000000..b9403d28ee --- /dev/null +++ b/server/internal/daemon/execenv/codex_sandbox.go @@ -0,0 +1,280 @@ +package execenv + +import ( + "fmt" + "log/slog" + "os" + "regexp" + "runtime" + "strconv" + "strings" +) + +// Background +// +// On macOS, Codex's Seatbelt sandbox in the `workspace-write` mode silently +// ignores `[sandbox_workspace_write] network_access = true`. DNS resolution is +// blocked at the syscall layer, so processes inside the sandbox see +// `no such host` errors when calling out (for example, `multica issue get` +// hitting the Multica API). See upstream issue openai/codex#10390. +// +// Until a fixed Codex release ships, the per-task Codex config on macOS needs +// to fall back to `sandbox_mode = "danger-full-access"` so the agent can +// actually reach the Multica API. On Linux (and on macOS once the upstream +// fix is released), the normal `workspace-write` + `network_access = true` +// combo is preferred because it keeps the filesystem sandbox intact. +// +// CodexDarwinNetworkAccessFixedVersion is the earliest Codex CLI version in +// which `network_access = true` is honored under Seatbelt on macOS. Bump this +// constant when the upstream fix ships. Empty string means "no known fixed +// release yet — always treat macOS Codex as broken for network access". +const CodexDarwinNetworkAccessFixedVersion = "" + +// codexSandboxPolicy describes how the per-task Codex config.toml should +// configure the sandbox. +type codexSandboxPolicy struct { + // Mode is the value written as `sandbox_mode = "..."`. + Mode string + // NetworkAccess controls `[sandbox_workspace_write] network_access`. + // Only meaningful when Mode is "workspace-write". + NetworkAccess bool + // Reason is a short human-readable label used in warn-level logs. + Reason string +} + +// codexSandboxPolicyFor picks the right policy for the given platform and +// detected Codex CLI version. +// +// - Non-darwin: always workspace-write with network access (Landlock is not +// affected by the macOS Seatbelt bug). +// - darwin with a version at or above CodexDarwinNetworkAccessFixedVersion: +// workspace-write with network access (upstream bug fixed). +// - darwin otherwise (including when the version is unknown): fall back to +// danger-full-access so the Multica CLI can reach the API. +func codexSandboxPolicyFor(goos, detectedVersion string) codexSandboxPolicy { + if goos == "" { + goos = runtime.GOOS + } + if goos != "darwin" { + return codexSandboxPolicy{ + Mode: "workspace-write", + NetworkAccess: true, + Reason: "non-darwin platform — seatbelt bug does not apply", + } + } + if codexDarwinNetworkAccessFixed(detectedVersion) { + return codexSandboxPolicy{ + Mode: "workspace-write", + NetworkAccess: true, + Reason: "codex version includes macOS network_access fix", + } + } + reason := "codex on macOS: seatbelt ignores sandbox_workspace_write.network_access (openai/codex#10390)" + if detectedVersion == "" { + reason += " — version unknown, assuming broken" + } + return codexSandboxPolicy{ + Mode: "danger-full-access", + NetworkAccess: false, + Reason: reason, + } +} + +// codexDarwinNetworkAccessFixed returns true if the given detected version is +// known to honor `network_access = true` under Seatbelt on macOS. +func codexDarwinNetworkAccessFixed(detectedVersion string) bool { + if CodexDarwinNetworkAccessFixedVersion == "" || detectedVersion == "" { + return false + } + fixed, err := parseCodexSemver(CodexDarwinNetworkAccessFixedVersion) + if err != nil { + return false + } + got, err := parseCodexSemver(detectedVersion) + if err != nil { + return false + } + return !got.lessThan(fixed) +} + +// codexUpgradeHint returns a short, actionable hint for users running a Codex +// version that suffers from the macOS network_access bug. +func codexUpgradeHint() string { + return "upgrade Codex CLI (e.g. `brew upgrade codex` or `npm i -g @openai/codex`) once a release including openai/codex#10390 is available to restore workspace-write + network_access" +} + +// multicaManagedBeginMarker / multicaManagedEndMarker delimit the block the +// daemon writes into the per-task config.toml. Everything between the markers +// is owned by the daemon and will be rewritten idempotently; anything outside +// the markers is preserved as-is. +const ( + multicaManagedBeginMarker = "# BEGIN multica-managed (do not edit; regenerated by daemon)" + multicaManagedEndMarker = "# END multica-managed" +) + +// renderMulticaManagedBlock produces the managed block for the given policy. +// +// The block contains only top-level key=value assignments — no `[table]` +// headers — and uses TOML dotted-key syntax for nested values. This is +// important because the block is inserted into a user-owned config.toml: +// +// - If the block opened a `[sandbox_workspace_write]` header, any user +// content that happened to sit below it would be silently reparented into +// that table. +// - If the block were appended after a file that already ends inside some +// other table (e.g. `[permissions.multica]`), a bare `sandbox_mode = ...` +// key would be parsed as a child of that preceding table. +// +// Keeping the block as pure top-level dotted-key assignments, and placing it +// at the top of the file (see upsertMulticaManagedBlock), avoids both traps. +func renderMulticaManagedBlock(policy codexSandboxPolicy) string { + var b strings.Builder + b.WriteString(multicaManagedBeginMarker) + b.WriteString("\n") + b.WriteString(fmt.Sprintf("sandbox_mode = %q\n", policy.Mode)) + if policy.Mode == "workspace-write" { + b.WriteString(fmt.Sprintf("sandbox_workspace_write.network_access = %t\n", policy.NetworkAccess)) + } + b.WriteString(multicaManagedEndMarker) + b.WriteString("\n") + return b.String() +} + +// managedBlockRe captures the daemon-owned block (including the surrounding +// markers) so it can be replaced idempotently. +var managedBlockRe = regexp.MustCompile( + `(?ms)^` + regexp.QuoteMeta(multicaManagedBeginMarker) + + `.*?^` + regexp.QuoteMeta(multicaManagedEndMarker) + `\n?`) + +// upsertMulticaManagedBlock returns the config content with the multica-managed +// block placed at the very top of the file. Any previously written managed +// block is removed in place; user content outside the markers is preserved. +// +// The block is always hoisted to the top (rather than replaced in place or +// appended to EOF) so that its top-level keys are parsed at the TOML root, +// regardless of whether the user's config ends inside a table like +// `[permissions.multica]` or `[profiles.foo]`. Combined with the dotted-key +// form used by renderMulticaManagedBlock, this means the managed block neither +// leaks into nor inherits from any surrounding table scope. +func upsertMulticaManagedBlock(content string, policy codexSandboxPolicy) string { + // Drop any previously written managed block (wherever it sits). + content = managedBlockRe.ReplaceAllString(content, "") + block := renderMulticaManagedBlock(policy) + // Trim leading blank lines left behind by the removal so we don't grow + // the file on every idempotent rewrite. + content = strings.TrimLeft(content, "\n") + if content == "" { + return block + } + return block + "\n" + content +} + +// stripLegacySandboxDirectives removes top-level `sandbox_mode = ...` lines +// and any `[sandbox_workspace_write]` section that would otherwise conflict +// with the managed block. This lets the daemon migrate tasks whose config.toml +// was produced by an older daemon that wrote those values inline. +// +// Only top-level entries are stripped; anything under an unrelated section +// header (like `[permissions.foo]`) is preserved untouched. +func stripLegacySandboxDirectives(content string) string { + lines := strings.Split(content, "\n") + out := make([]string, 0, len(lines)) + inLegacyWorkspaceWrite := false + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "[") { + // Entering a new section. Exit legacy-tracking if we were in one. + inLegacyWorkspaceWrite = trimmed == "[sandbox_workspace_write]" + if inLegacyWorkspaceWrite { + continue + } + out = append(out, line) + continue + } + if inLegacyWorkspaceWrite { + // Drop the legacy section body until the next section. + continue + } + if strings.HasPrefix(trimmed, "sandbox_mode") { + // Drop legacy top-level sandbox_mode declarations. + continue + } + out = append(out, line) + } + return strings.Join(out, "\n") +} + +// ensureCodexSandboxConfig writes the multica-managed sandbox block into the +// given config.toml according to the policy. It is idempotent: running it +// twice produces the same file contents. The file is created if it doesn't +// exist. +// +// The function logs (at warn level) when it falls back to danger-full-access +// on macOS so the incident is visible in daemon logs. +func ensureCodexSandboxConfig(configPath string, policy codexSandboxPolicy, detectedVersion string, logger *slog.Logger) error { + data, err := os.ReadFile(configPath) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("read config.toml: %w", err) + } + existing := string(data) + + // Drop inline sandbox_mode / [sandbox_workspace_write] from older daemon + // versions so they don't collide with the managed block. + if existing != "" && !managedBlockRe.MatchString(existing) { + existing = stripLegacySandboxDirectives(existing) + } + + updated := upsertMulticaManagedBlock(existing, policy) + if updated == string(data) { + return nil + } + + if policy.Mode == "danger-full-access" && logger != nil { + version := detectedVersion + if version == "" { + version = "unknown" + } + logger.Warn("codex sandbox: falling back to danger-full-access on macOS", + "reason", policy.Reason, + "codex_version", version, + "hint", codexUpgradeHint(), + "config_path", configPath, + ) + } + + if err := os.WriteFile(configPath, []byte(updated), 0o644); err != nil { + return fmt.Errorf("write config.toml: %w", err) + } + return nil +} + +// --- small semver helper, scoped to this package to avoid an import cycle +// with server/pkg/agent. The agent package already has a similar parser; we +// duplicate the minimal bits here because execenv cannot depend on agent. + +type codexSemver struct { + Major, Minor, Patch int +} + +var codexSemverRe = regexp.MustCompile(`v?(\d+)\.(\d+)\.(\d+)`) + +func parseCodexSemver(raw string) (codexSemver, error) { + m := codexSemverRe.FindStringSubmatch(raw) + if m == nil { + return codexSemver{}, fmt.Errorf("cannot parse version %q", raw) + } + maj, _ := strconv.Atoi(m[1]) + min, _ := strconv.Atoi(m[2]) + pat, _ := strconv.Atoi(m[3]) + return codexSemver{Major: maj, Minor: min, Patch: pat}, nil +} + +func (v codexSemver) lessThan(o codexSemver) bool { + if v.Major != o.Major { + return v.Major < o.Major + } + if v.Minor != o.Minor { + return v.Minor < o.Minor + } + return v.Patch < o.Patch +} diff --git a/server/internal/daemon/execenv/execenv.go b/server/internal/daemon/execenv/execenv.go index a40623517f..5d0f037659 100644 --- a/server/internal/daemon/execenv/execenv.go +++ b/server/internal/daemon/execenv/execenv.go @@ -20,11 +20,12 @@ type RepoContextForEnv struct { // PrepareParams holds all inputs needed to set up an execution environment. type PrepareParams struct { - WorkspacesRoot string // base path for all envs (e.g., ~/multica_workspaces) - WorkspaceID string // workspace UUID — tasks are grouped under this - TaskID string // task UUID — used for directory name - AgentName string // for git branch naming only - Provider string // agent provider ("claude", "codex") — determines skill injection paths + WorkspacesRoot string // base path for all envs (e.g., ~/multica_workspaces) + WorkspaceID string // workspace UUID — tasks are grouped under this + TaskID string // task UUID — used for directory name + AgentName string // for git branch naming only + Provider string // agent provider ("claude", "codex") — determines skill injection paths + CodexVersion string // detected Codex CLI version (only used when Provider == "codex") Task TaskContextForEnv // context data for writing files } @@ -110,7 +111,7 @@ func Prepare(params PrepareParams, logger *slog.Logger) (*Environment, error) { // For Codex, set up a per-task CODEX_HOME seeded from ~/.codex/ with skills. if params.Provider == "codex" { codexHome := filepath.Join(envRoot, "codex-home") - if err := prepareCodexHome(codexHome, logger); err != nil { + if err := prepareCodexHomeWithOpts(codexHome, CodexHomeOptions{CodexVersion: params.CodexVersion}, logger); err != nil { return nil, fmt.Errorf("execenv: prepare codex-home: %w", err) } if len(params.Task.AgentSkills) > 0 { @@ -127,7 +128,11 @@ func Prepare(params PrepareParams, logger *slog.Logger) (*Environment, error) { // Reuse wraps an existing workdir into an Environment and refreshes context files. // Returns nil if the workdir does not exist (caller should fall back to Prepare). -func Reuse(workDir, provider string, task TaskContextForEnv, logger *slog.Logger) *Environment { +// +// codexVersion is the detected Codex CLI version, used (only when provider is +// "codex") to pick the right sandbox policy for the per-task config.toml. +// Pass an empty string when the version is unknown. +func Reuse(workDir, provider, codexVersion string, task TaskContextForEnv, logger *slog.Logger) *Environment { if _, err := os.Stat(workDir); err != nil { return nil } @@ -144,11 +149,11 @@ func Reuse(workDir, provider string, task TaskContextForEnv, logger *slog.Logger } // Restore CodexHome for Codex provider — the per-task codex-home directory - // lives alongside the workdir. Re-run prepareCodexHome to ensure config - // (especially network access) is up to date. + // lives alongside the workdir. Re-run prepareCodexHomeWithOpts to ensure + // config (especially sandbox/network access) is up to date. if provider == "codex" { codexHome := filepath.Join(env.RootDir, "codex-home") - if err := prepareCodexHome(codexHome, logger); err != nil { + if err := prepareCodexHomeWithOpts(codexHome, CodexHomeOptions{CodexVersion: codexVersion}, logger); err != nil { logger.Warn("execenv: refresh codex-home failed", "error", err) } else { env.CodexHome = codexHome diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index 095d29c83e..e7c4f1b080 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -762,13 +762,14 @@ func TestPrepareCodexHomeSkipsMissingFiles(t *testing.T) { } } -func TestEnsureCodexNetworkAccessCreatesDefault(t *testing.T) { +func TestEnsureCodexSandboxConfigCreatesDefaultLinux(t *testing.T) { t.Parallel() dir := t.TempDir() configPath := filepath.Join(dir, "config.toml") - if err := ensureCodexNetworkAccess(configPath); err != nil { - t.Fatalf("ensureCodexNetworkAccess failed: %v", err) + policy := codexSandboxPolicyFor("linux", "0.121.0") + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("ensureCodexSandboxConfig failed: %v", err) } data, err := os.ReadFile(configPath) @@ -776,51 +777,74 @@ func TestEnsureCodexNetworkAccessCreatesDefault(t *testing.T) { t.Fatalf("failed to read config.toml: %v", err) } s := string(data) + if !strings.Contains(s, multicaManagedBeginMarker) || !strings.Contains(s, multicaManagedEndMarker) { + t.Errorf("missing managed block markers, got:\n%s", s) + } if !strings.Contains(s, `sandbox_mode = "workspace-write"`) { t.Error("missing sandbox_mode") } - if !strings.Contains(s, "[sandbox_workspace_write]") { - t.Error("missing [sandbox_workspace_write] section") + // The managed block uses TOML dotted-key form rather than a + // `[sandbox_workspace_write]` section header so it cannot leak into or + // inherit from any surrounding table scope. See upsertMulticaManagedBlock + // for why. + if strings.Contains(s, "[sandbox_workspace_write]") { + t.Errorf("managed block must not open a [sandbox_workspace_write] table header, got:\n%s", s) } - if !strings.Contains(s, "network_access = true") { - t.Error("missing network_access = true") + if !strings.Contains(s, "sandbox_workspace_write.network_access = true") { + t.Errorf("missing dotted-key network_access = true, got:\n%s", s) } } -func TestEnsureCodexNetworkAccessPreservesExisting(t *testing.T) { +func TestEnsureCodexSandboxConfigDarwinFallsBack(t *testing.T) { t.Parallel() dir := t.TempDir() configPath := filepath.Join(dir, "config.toml") - existing := `model = "o3" - -[sandbox_workspace_write] -network_access = true -` - os.WriteFile(configPath, []byte(existing), 0o644) - - if err := ensureCodexNetworkAccess(configPath); err != nil { - t.Fatalf("ensureCodexNetworkAccess failed: %v", err) + policy := codexSandboxPolicyFor("darwin", "0.121.0") + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("ensureCodexSandboxConfig failed: %v", err) } + s, _ := os.ReadFile(configPath) + if !strings.Contains(string(s), `sandbox_mode = "danger-full-access"`) { + t.Errorf("expected danger-full-access fallback on macOS, got:\n%s", s) + } + if strings.Contains(string(s), "[sandbox_workspace_write]") { + t.Errorf("should not emit workspace-write section on macOS fallback, got:\n%s", s) + } +} + +func TestEnsureCodexSandboxConfigIsIdempotent(t *testing.T) { + t.Parallel() + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + + policy := codexSandboxPolicyFor("linux", "0.121.0") + for i := 0; i < 3; i++ { + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("pass %d: %v", i, err) + } + } data, _ := os.ReadFile(configPath) - if string(data) != existing { - t.Errorf("config should be unchanged, got:\n%s", data) + // The managed block should appear exactly once. + if n := strings.Count(string(data), multicaManagedBeginMarker); n != 1 { + t.Errorf("expected exactly 1 managed block, got %d in:\n%s", n, data) } } -func TestEnsureCodexNetworkAccessAppendsToExisting(t *testing.T) { +func TestEnsureCodexSandboxConfigPreservesUserContent(t *testing.T) { t.Parallel() dir := t.TempDir() configPath := filepath.Join(dir, "config.toml") existing := `model = "o3" -sandbox_mode = "workspace-write" +approval_policy = "on-failure" ` os.WriteFile(configPath, []byte(existing), 0o644) - if err := ensureCodexNetworkAccess(configPath); err != nil { - t.Fatalf("ensureCodexNetworkAccess failed: %v", err) + policy := codexSandboxPolicyFor("linux", "0.121.0") + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("ensureCodexSandboxConfig failed: %v", err) } data, _ := os.ReadFile(configPath) @@ -828,36 +852,184 @@ sandbox_mode = "workspace-write" if !strings.Contains(s, `model = "o3"`) { t.Error("lost existing model setting") } - if !strings.Contains(s, "[sandbox_workspace_write]") { - t.Error("missing [sandbox_workspace_write] section") + if !strings.Contains(s, "approval_policy") { + t.Error("lost existing approval_policy") } if !strings.Contains(s, "network_access = true") { t.Error("missing network_access = true") } } -func TestEnsureCodexNetworkAccessAddsMissingKey(t *testing.T) { +func TestEnsureCodexSandboxConfigStripsLegacyInlineDirectives(t *testing.T) { t.Parallel() dir := t.TempDir() configPath := filepath.Join(dir, "config.toml") - // Section exists but without network_access. - existing := `[sandbox_workspace_write] -allow_commands = ["git"] + // Simulate a config.toml produced by an older daemon version that wrote + // sandbox directives inline (no managed block markers). After migration, + // the inline directives should be gone and only the managed block should + // carry them. + existing := `model = "o3" +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +network_access = true ` os.WriteFile(configPath, []byte(existing), 0o644) - if err := ensureCodexNetworkAccess(configPath); err != nil { - t.Fatalf("ensureCodexNetworkAccess failed: %v", err) + policy := codexSandboxPolicyFor("darwin", "0.121.0") + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("ensureCodexSandboxConfig failed: %v", err) } data, _ := os.ReadFile(configPath) s := string(data) - if !strings.Contains(s, "network_access = true") { - t.Error("missing network_access = true") + if !strings.Contains(s, `model = "o3"`) { + t.Error("should have preserved unrelated user config") } - if !strings.Contains(s, `allow_commands = ["git"]`) { - t.Error("lost existing allow_commands") + // Inline sandbox_mode and [sandbox_workspace_write] should be stripped. + if strings.Count(s, "sandbox_mode") != 1 { + t.Errorf("expected exactly one sandbox_mode line (inside managed block), got:\n%s", s) + } + if strings.Contains(s, "[sandbox_workspace_write]") { + t.Errorf("darwin fallback should not retain workspace-write section:\n%s", s) + } + if !strings.Contains(s, `sandbox_mode = "danger-full-access"`) { + t.Errorf("expected danger-full-access on macOS, got:\n%s", s) + } +} + +func TestEnsureCodexSandboxConfigHoistsAboveUserTables(t *testing.T) { + t.Parallel() + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + + // User config that ends inside a table. If the managed block were + // appended at EOF, `sandbox_mode = "..."` would be parsed as + // permissions.multica.sandbox_mode and Codex would never see it — see + // review of MUL-963 PR #1246. The block must be hoisted above any + // user-defined table headers so it lives at the TOML root. + existing := `model = "o3" + +[permissions.multica] +trust = "always" +` + os.WriteFile(configPath, []byte(existing), 0o644) + + policy := codexSandboxPolicyFor("linux", "0.121.0") + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("ensureCodexSandboxConfig failed: %v", err) + } + + data, _ := os.ReadFile(configPath) + s := string(data) + + beginIdx := strings.Index(s, multicaManagedBeginMarker) + endIdx := strings.Index(s, multicaManagedEndMarker) + tableIdx := strings.Index(s, "[permissions.multica]") + if beginIdx < 0 || endIdx < 0 || tableIdx < 0 { + t.Fatalf("expected managed block and user table to both be present, got:\n%s", s) + } + // The entire managed block must sit before the user's table header so + // that sandbox_mode and sandbox_workspace_write.network_access are + // parsed at the TOML root. + if !(beginIdx < endIdx && endIdx < tableIdx) { + t.Errorf("managed block must be hoisted above [permissions.multica]; got begin=%d end=%d table=%d:\n%s", beginIdx, endIdx, tableIdx, s) + } + // User content must be preserved verbatim. + if !strings.Contains(s, `model = "o3"`) { + t.Error("lost user top-level key") + } + if !strings.Contains(s, `trust = "always"`) { + t.Error("lost user permissions.multica content") + } + + // Running again must be idempotent even when the preceding content ends + // inside a table. + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("second pass: %v", err) + } + data2, _ := os.ReadFile(configPath) + if string(data2) != s { + t.Errorf("second pass should be idempotent:\n--- first ---\n%s\n--- second ---\n%s", s, data2) + } + if n := strings.Count(string(data2), multicaManagedBeginMarker); n != 1 { + t.Errorf("expected exactly one managed block after idempotent rewrite, got %d", n) + } +} + +func TestEnsureCodexSandboxConfigMovesLegacyTrailingBlockToTop(t *testing.T) { + t.Parallel() + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + + // Simulate a config.toml produced by the pre-fix PR #1246 logic, which + // appended the managed block to EOF — so the block sits below a user + // table. On the next daemon run, the block must be hoisted back to the + // top; otherwise sandbox_mode remains trapped inside the preceding table. + legacy := `model = "o3" + +[permissions.multica] +trust = "always" + +` + multicaManagedBeginMarker + ` +sandbox_mode = "workspace-write" + +[sandbox_workspace_write] +network_access = true +` + multicaManagedEndMarker + ` +` + os.WriteFile(configPath, []byte(legacy), 0o644) + + policy := codexSandboxPolicyFor("linux", "0.121.0") + if err := ensureCodexSandboxConfig(configPath, policy, "0.121.0", testLogger()); err != nil { + t.Fatalf("ensureCodexSandboxConfig failed: %v", err) + } + data, _ := os.ReadFile(configPath) + s := string(data) + + beginIdx := strings.Index(s, multicaManagedBeginMarker) + tableIdx := strings.Index(s, "[permissions.multica]") + if beginIdx < 0 || tableIdx < 0 || beginIdx > tableIdx { + t.Errorf("expected managed block to be hoisted above [permissions.multica], got:\n%s", s) + } + if strings.Count(s, multicaManagedBeginMarker) != 1 { + t.Errorf("expected exactly one managed block, got:\n%s", s) + } + // The old inline `[sandbox_workspace_write]` header must be gone — the + // new block uses dotted-key form only. + if strings.Contains(s, "[sandbox_workspace_write]") { + t.Errorf("managed block must not emit [sandbox_workspace_write] table header, got:\n%s", s) + } +} + +func TestCodexSandboxPolicyFor(t *testing.T) { + t.Parallel() + cases := []struct { + name string + goos string + version string + wantMode string + wantNet bool + }{ + {"linux any version", "linux", "0.100.0", "workspace-write", true}, + {"linux unknown version", "linux", "", "workspace-write", true}, + {"darwin old version", "darwin", "0.121.0", "danger-full-access", false}, + {"darwin unknown version", "darwin", "", "danger-full-access", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := codexSandboxPolicyFor(tc.goos, tc.version) + if p.Mode != tc.wantMode { + t.Errorf("mode = %q, want %q", p.Mode, tc.wantMode) + } + if p.NetworkAccess != tc.wantNet { + t.Errorf("network_access = %v, want %v", p.NetworkAccess, tc.wantNet) + } + if p.Reason == "" { + t.Error("expected non-empty Reason") + } + }) } } @@ -869,6 +1041,7 @@ func TestPrepareCodexHomeEnsuresNetworkAccess(t *testing.T) { t.Setenv("CODEX_HOME", sharedHome) codexHome := filepath.Join(t.TempDir(), "codex-home") + // Default prepareCodexHome assumes linux-like behavior. if err := prepareCodexHome(codexHome, testLogger()); err != nil { t.Fatalf("prepareCodexHome failed: %v", err) } @@ -914,7 +1087,7 @@ func TestReuseRestoresCodexHome(t *testing.T) { } // Reuse should restore CodexHome. - reused := Reuse(env.WorkDir, "codex", TaskContextForEnv{IssueID: "reuse-test"}, testLogger()) + reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{IssueID: "reuse-test"}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -922,13 +1095,14 @@ func TestReuseRestoresCodexHome(t *testing.T) { t.Fatal("expected CodexHome to be restored after Reuse") } - // Verify config.toml has network access. + // Verify config.toml has a managed block (exact mode depends on host + // platform; either workspace-write or danger-full-access is valid). data, err := os.ReadFile(filepath.Join(reused.CodexHome, "config.toml")) if err != nil { t.Fatalf("config.toml not found in reused CodexHome: %v", err) } - if !strings.Contains(string(data), "network_access = true") { - t.Error("reused config.toml missing network_access = true") + if !strings.Contains(string(data), multicaManagedBeginMarker) { + t.Error("reused config.toml missing multica-managed block") } }