Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions .roborev.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 9 additions & 7 deletions internal/agent/codex.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions internal/agent/codex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
{
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion scripts/changelog.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
;;
Expand Down
Loading