diff --git a/.roborev.toml b/.roborev.toml index c66819b95..a33799c12 100644 --- a/.roborev.toml +++ b/.roborev.toml @@ -45,14 +45,17 @@ security findings: ## Agent tool permissions -Review agents run with shell access to inspect diffs and source files. The -allow-all-tools plus deny-list approach used by agents like Copilot is -intentional — it matches the trust level of Codex (--sandbox read-only still -permits arbitrary shell execution) and Gemini (--approval-mode plan). The +Review agents run with shell access to inspect diffs and source files. Codex +runs with --sandbox danger-full-access because the bwrap sandbox (used by both +read-only and workspace-write modes) blocks loopback networking, which prevents +git commands (git diff, git log, git show) from working in CI review jobs. +This is intentional — roborev is a local-only tool that runs in trusted +environments where the code is the operator's own. The allow-all-tools plus +deny-list approach used by agents like Copilot matches this trust level. The deny-list is defense-in-depth, not a security boundary. Users reviewing untrusted code are expected to run roborev in a sandboxed environment -(container, VM). Do not flag agent tool permission patterns as security -vulnerabilities when they follow this model. +(container, VM). Do not flag agent sandbox or tool permission patterns as +security vulnerabilities when they follow this model. ## Config loading and filesystem fallback diff --git a/internal/agent/codex.go b/internal/agent/codex.go index fb16ba4f2..a2bd18d17 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -141,10 +141,12 @@ func (a *CodexAgent) commandArgs(opts codexArgOptions) []string { args = append(args, codexDangerousFlag) } if opts.autoApprove { - // Use read-only sandbox for review mode instead of --full-auto - // (which implies --sandbox workspace-write). Background review - // jobs run in the user's repo and must not take index.lock. - args = append(args, "--sandbox", "read-only") + // Use full-access sandbox for review mode. The read-only + // sandbox blocks loopback networking, which prevents git + // commands from working in CI review jobs. roborev runs in + // trusted environments where the code is the operator's own, + // so sandbox enforcement is unnecessary. + args = append(args, "--sandbox", "danger-full-access") } if !opts.preview { args = append(args, "-C", opts.repoPath) @@ -181,7 +183,7 @@ func codexSupportsDangerousFlag(ctx context.Context, command string) (bool, erro } // codexSupportsNonInteractive checks that codex supports --sandbox, -// needed for non-agentic review mode (--sandbox read-only). +// needed for non-agentic review mode (--sandbox danger-full-access). func codexSupportsNonInteractive(ctx context.Context, command string) (bool, error) { if cached, ok := codexAutoApproveSupport.Load(command); ok { return cached.(bool), nil @@ -210,8 +212,8 @@ func (a *CodexAgent) Review(ctx context.Context, repoPath, commitSHA, prompt str } } - // Non-agentic review mode uses --sandbox read-only for - // non-interactive execution without writing to the working tree. + // Non-agentic review mode uses --sandbox danger-full-access for + // non-interactive execution that can still run git commands. autoApprove := false if !agenticMode { supported, err := codexSupportsNonInteractive(ctx, a.Command) diff --git a/internal/agent/codex_test.go b/internal/agent/codex_test.go index e58bb101f..fa405285c 100644 --- a/internal/agent/codex_test.go +++ b/internal/agent/codex_test.go @@ -36,7 +36,7 @@ func TestCodex_buildArgs(t *testing.T) { name: "NonAgenticAutoApprove", agentic: false, autoApprove: true, - wantFlags: []string{"--sandbox", "read-only", "--json"}, + wantFlags: []string{"--sandbox", "danger-full-access", "--json"}, wantMissingFlags: []string{codexDangerousFlag, codexAutoApproveFlag}, }, { @@ -96,7 +96,7 @@ func TestCodexCommandLineOmitsRuntimeOnlyArgs(t *testing.T) { assert.Contains(t, cmdLine, "exec resume --json") assert.Contains(t, cmdLine, "session-123") - assert.Contains(t, cmdLine, "--sandbox read-only") + assert.Contains(t, cmdLine, "--sandbox danger-full-access") assert.NotContains(t, cmdLine, " -C ") assert.False(t, strings.HasSuffix(cmdLine, " -"), "command line should omit stdin marker: %q", cmdLine) } @@ -118,7 +118,7 @@ func TestCodexReviewUnsafeMissingFlagErrors(t *testing.T) { assert.Contains(t, err.Error(), "does not support") } -func TestCodexReviewUsesReadOnlySandbox(t *testing.T) { +func TestCodexReviewUsesSandboxNone(t *testing.T) { a, mock := setupMockCodex(t, false, MockCLIOpts{ HelpOutput: "usage --sandbox", CaptureArgs: true, @@ -133,10 +133,10 @@ func TestCodexReviewUsesReadOnlySandbox(t *testing.T) { args, err := os.ReadFile(mock.ArgsFile) require.NoError(t, err) argsStr := string(args) - assert.Contains(t, argsStr, "--sandbox read-only", - "expected --sandbox read-only in args, got %s", strings.TrimSpace(argsStr)) + assert.Contains(t, argsStr, "--sandbox danger-full-access", + "expected --sandbox danger-full-access in args, got: %s", strings.TrimSpace(argsStr)) assert.NotContains(t, argsStr, codexAutoApproveFlag, - "expected no %s in review mode, got %s", codexAutoApproveFlag, strings.TrimSpace(argsStr)) + "expected no %s in review mode, got: %s", codexAutoApproveFlag, strings.TrimSpace(argsStr)) } func TestCodexReviewWithSessionResumePassesResumeArgs(t *testing.T) { diff --git a/scripts/changelog.sh b/scripts/changelog.sh index 462a1232f..6829f216e 100755 --- a/scripts/changelog.sh +++ b/scripts/changelog.sh @@ -82,7 +82,7 @@ case "$AGENT" in codex) CODEX_RUST_LOG="${CHANGELOG_CODEX_RUST_LOG:-${RUST_LOG:-error,codex_core::rollout::list=off}}" set +e - RUST_LOG="$CODEX_RUST_LOG" codex exec --json --skip-git-repo-check --sandbox read-only -c reasoning_effort=high -o "$TMPFILE" - >/dev/null < "$PROMPTFILE" 2>"$ERRFILE" + RUST_LOG="$CODEX_RUST_LOG" codex exec --json --skip-git-repo-check --sandbox danger-full-access -c reasoning_effort=high -o "$TMPFILE" - >/dev/null < "$PROMPTFILE" 2>"$ERRFILE" AGENT_EXIT=$? set -e ;;