Skip to content

Use sandboxed runtime defaults with on-request approvals#34

Open
lelonco wants to merge 2 commits intofriuns2:mainfrom
lelonco:feature/default-sandbox-on-request
Open

Use sandboxed runtime defaults with on-request approvals#34
lelonco wants to merge 2 commits intofriuns2:mainfrom
lelonco:feature/default-sandbox-on-request

Conversation

@lelonco
Copy link
Copy Markdown

@lelonco lelonco commented Apr 5, 2026

  • switch default app-server runtime from danger-full-access / never to workspace-write / on-request
  • centralize runtime arg construction in a shared server config module
  • add CLI overrides for sandbox mode and approval policy
  • update docs and manual test coverage

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Switch to sandboxed defaults with on-request approvals

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Switch default app-server runtime from danger-full-access/never to workspace-write/on-request
• Centralize runtime configuration in new shared appServerRuntimeConfig module
• Add CLI flags for sandbox mode and approval policy overrides
• Update documentation and test coverage for new defaults
Diagram
flowchart LR
  CLI["CLI Options<br/>--sandbox-mode<br/>--approval-policy"]
  ENV["Environment Variables<br/>CODEXUI_SANDBOX_MODE<br/>CODEXUI_APPROVAL_POLICY"]
  CONFIG["appServerRuntimeConfig<br/>resolveAppServerRuntimeConfig"]
  DEFAULTS["Default Values<br/>workspace-write<br/>on-request"]
  ARGS["App Server Args<br/>buildAppServerArgs"]
  ROUTES["accountRoutes &<br/>codexAppServerBridge"]
  
  CLI -->|parsed & validated| CONFIG
  ENV -->|read & normalized| CONFIG
  DEFAULTS -->|fallback| CONFIG
  CONFIG -->|builds| ARGS
  ARGS -->|used by| ROUTES
Loading

Grey Divider

File Changes

1. src/server/appServerRuntimeConfig.ts ✨ Enhancement +73/-0

New centralized runtime configuration module

• New module centralizing runtime configuration management
• Defines sandbox modes (read-only, workspace-write, danger-full-access) and approval policies
 (untrusted, on-failure, on-request, never)
• Implements environment variable reading with fallback to new defaults (workspace-write,
 on-request)
• Exports functions for parsing CLI arguments and building app-server command arguments

src/server/appServerRuntimeConfig.ts


2. src/cli/index.ts ✨ Enhancement +34/-0

Add CLI options for runtime configuration

• Import new runtime configuration functions from appServerRuntimeConfig module
• Add --sandbox-mode and --approval-policy CLI options with descriptions
• Parse and validate CLI arguments for sandbox mode and approval policy
• Set environment variables CODEXUI_SANDBOX_MODE and CODEXUI_APPROVAL_POLICY when provided
• Display resolved sandbox mode and approval policy in startup logs

src/cli/index.ts


3. src/server/accountRoutes.ts ✨ Enhancement +2/-7

Use centralized runtime configuration

• Replace hardcoded APP_SERVER_ARGS with call to buildAppServerArgs() function
• Removes inline configuration of danger-full-access and never defaults
• Now uses centralized runtime configuration with new defaults

src/server/accountRoutes.ts


View more (3)
4. src/server/codexAppServerBridge.ts ✨ Enhancement +2/-7

Use centralized runtime configuration

• Import buildAppServerArgs from appServerRuntimeConfig module
• Replace hardcoded appServerArgs property with call to buildAppServerArgs()
• Removes inline configuration of danger-full-access and never defaults

src/server/codexAppServerBridge.ts


5. docs/index.html 📝 Documentation +2/-2

Update documentation for safer defaults

• Update feature description from "Full Auto-Approval" to "Sandboxed By Default"
• Change description to highlight workspace-write sandbox mode and on-request approvals
• Emphasize safer defaults instead of unrestricted access

docs/index.html


6. tests.md 🧪 Tests +23/-0

Add manual test for runtime defaults

• Add comprehensive manual test case for default runtime configuration
• Test verifies workspace-write sandbox mode and on-request approval policy defaults
• Test confirms CLI flags override defaults when provided
• Includes prerequisites, steps, expected results, and cleanup instructions

tests.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 5, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Action required

1. Account routes ignore CLI overrides🐞 Bug ≡ Correctness
Description
src/server/accountRoutes.ts computes APP_SERVER_ARGS at module load time, before startServer()
sets CODEXUI_SANDBOX_MODE/CODEXUI_APPROVAL_POLICY from CLI flags. As a result,
--sandbox-mode/--approval-policy do not affect the codex app-server spawned inside
withTemporaryCodexAppServer(), causing inconsistent runtime behavior.
Code

src/server/accountRoutes.ts[81]

+const APP_SERVER_ARGS = buildAppServerArgs()
Evidence
Because ESM imports are evaluated before startServer() runs, accountRoutes.ts is imported (and
its module-scope APP_SERVER_ARGS computed) before the CLI has a chance to set
process.env.CODEXUI_SANDBOX_MODE / process.env.CODEXUI_APPROVAL_POLICY. The temporary app-server
spawned later uses that precomputed args array, so CLI overrides won’t take effect for account
flows.

src/server/accountRoutes.ts[7-83]
src/server/accountRoutes.ts[389-401]
src/cli/index.ts[1-27]
src/cli/index.ts[466-477]
src/server/httpServer.ts[6-9]
src/server/codexAppServerBridge.ts[13-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/server/accountRoutes.ts` computes `APP_SERVER_ARGS` at module scope (`const APP_SERVER_ARGS = buildAppServerArgs()`), which runs during module import. CLI overrides are applied later in `startServer()` by setting env vars, so the temporary `codex app-server` launched by `withTemporaryCodexAppServer()` will keep using the earlier, pre-override args.
## Issue Context
- `startServer()` sets `process.env.CODEXUI_SANDBOX_MODE` / `process.env.CODEXUI_APPROVAL_POLICY` after module imports are already evaluated.
- `accountRoutes.ts` is imported via `httpServer.ts -> codexAppServerBridge.ts -> accountRoutes.ts`.
## Fix Focus Areas
- src/server/accountRoutes.ts[81-82]
- src/server/accountRoutes.ts[389-401]
## Suggested change
Remove the module-level `APP_SERVER_ARGS` constant and instead call `buildAppServerArgs()` at the point of spawning (inside `withTemporaryCodexAppServer`), so the args reflect the current env (including CLI-provided overrides).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant