feat: add Codex permission env fallback#802
Conversation
📝 WalkthroughWalkthroughThis PR adds server-side environment-driven fallback for Codex permission modes. A new module validates and resolves permission mode by choosing between explicit request values and environment-configured defaults ( ChangesCodex Permission Mode Environment Configuration
Sequence DiagramsequenceDiagram
participant Request as Client Request
participant QueryCodex as queryCodex
participant Resolver as resolveCodexPermissionMode
participant Env as Environment Config
Request->>QueryCodex: options with optional permissionMode
QueryCodex->>QueryCodex: detect if permissionMode explicit
QueryCodex->>Resolver: call resolver(mode, hasExplicit, env)
alt Has Explicit Permission Mode
Resolver->>Resolver: validate request mode
Resolver-->>QueryCodex: return validated mode
else Omitted Permission Mode
Resolver->>Env: read CLOUDCLI_CODEX_PERMISSION_MODE
Resolver->>Resolver: validate env value
Resolver-->>QueryCodex: return env default or 'default'
end
QueryCodex->>QueryCodex: derive sandboxMode and approvalPolicy
QueryCodex->>QueryCodex: start/resume Codex thread
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/openai-codex.js (1)
203-203: 💤 Low valueInclude the invalid value in warning messages for easier debugging.
When operators misconfigure the environment variable or clients send invalid modes, the current warnings don't reveal what value was received. Including the offending value helps diagnose issues faster.
🔧 Proposed improvement
- console.warn(`[Codex] Invalid ${CODEX_PERMISSION_MODE_ENV}; falling back to default`); + console.warn(`[Codex] Invalid ${CODEX_PERMISSION_MODE_ENV}="${normalizedPermissionMode}"; falling back to default`);And for the request validation:
- console.warn('[Codex] Invalid request permission mode; falling back to default'); + console.warn(`[Codex] Invalid request permission mode="${normalizedPermissionMode}"; falling back to default`);Also applies to: 217-217
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/openai-codex.js` at line 203, Update the warning log calls so they include the actual invalid value received: where console.warn(`[Codex] Invalid ${CODEX_PERMISSION_MODE_ENV}; falling back to default`) is used, append the runtime value (e.g. the env value or the variable being validated) into the message so it reads something like "Invalid X: <value>; falling back to default". Do the same for the other warning at the request validation site (the other console.warn around line 217) so both console.warn calls include the offending value for easier debugging; locate the console.warn calls and the VARIABLE/identifier used to validate (CODEX_PERMISSION_MODE_ENV or the runtime variable holding the mode) and interpolate it into the message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 155-162: Expand the "Codex Permission Defaults" section to
document the behavior of each mode (default, acceptEdits, bypassPermissions) so
operators can choose appropriately: describe what "default" enforces (strict
permission checks requiring explicit approval or admin-controlled decisions),
what "acceptEdits" permits (automatic acceptance of non-sensitive
client-suggested edits or a reduced-approval workflow), and clarify
"bypassPermissions" (full access with no approval, already mentioned); reference
the server-side env var CLOUDCLI_CODEX_PERMISSION_MODE and show an example usage
of setting it to each value so readers can test configurations.
---
Nitpick comments:
In `@server/openai-codex.js`:
- Line 203: Update the warning log calls so they include the actual invalid
value received: where console.warn(`[Codex] Invalid
${CODEX_PERMISSION_MODE_ENV}; falling back to default`) is used, append the
runtime value (e.g. the env value or the variable being validated) into the
message so it reads something like "Invalid X: <value>; falling back to
default". Do the same for the other warning at the request validation site (the
other console.warn around line 217) so both console.warn calls include the
offending value for easier debugging; locate the console.warn calls and the
VARIABLE/identifier used to validate (CODEX_PERMISSION_MODE_ENV or the runtime
variable holding the mode) and interpolate it into the message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: de4756ba-7f68-4d67-9e0c-d6a7adf00c7a
📒 Files selected for processing (2)
README.mdserver/openai-codex.js
|
Hey @CoderLuii, thanks for the PR. Can u resolve the merge conflict and I'll review it. |
|
Hey @CoderLuii, on the pr description it says "Self-hosted and containerized CloudCLI deployments sometimes need a process-level Codex permission default for clients that do not send permissionMode". When would you need this? When is the 'sometimes'? |
CoderLuii
force-pushed
the
codex-permission-env
branch
from
fbd54d6 to
c7106d0
Compare
CoderLuii
force-pushed
the
codex-permission-env
branch
from
c7106d0 to
3151245
Compare
|
@blackmammoth thanks for calling that out. You were right that sometimes was too vague. The concrete use case came from HolyClaude: CoderLuii/HolyClaude#18. HolyClaude ships CloudCLI inside a trusted self-hosted Docker image where users run multiple coding agents against the same mounted workspace. The current CloudCLI browser chat submit path sends Concrete examples:
That is why this PR keeps explicit request values first, and only uses I also cleaned the generated noise out of the PR description. Sorry about that; it made the PR harder to review than it needed to be. I refreshed the branch against current |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/codex-permission-mode.test.js (1)
80-91: ⚡ Quick winAdd coverage for whitespace-only env values.
Please add a test where
CLOUDCLI_CODEX_PERMISSION_MODEis set to whitespace (e.g.,' ') and assert it resolves to'default'with no warning. This exercises a distinct trim-to-empty branch not currently covered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/codex-permission-mode.test.js` around lines 80 - 91, Add a new test case after the existing test for invalid env values to cover whitespace-only environment values. Create a test that calls getConfiguredCodexPermissionMode with CODEX_PERMISSION_MODE_ENV set to a whitespace-only string (such as ' '), and assert that it resolves to 'default' and that logger.warnings is an empty array, ensuring this trim-to-empty code path is properly covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/codex-permission-mode.test.js`:
- Around line 80-91: Add a new test case after the existing test for invalid env
values to cover whitespace-only environment values. Create a test that calls
getConfiguredCodexPermissionMode with CODEX_PERMISSION_MODE_ENV set to a
whitespace-only string (such as ' '), and assert that it resolves to 'default'
and that logger.warnings is an empty array, ensuring this trim-to-empty code
path is properly covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a0c2590-f063-479a-8a3a-6be4826b26c4
📒 Files selected for processing (4)
README.mdserver/codex-permission-mode.jsserver/codex-permission-mode.test.jsserver/openai-codex.js
✅ Files skipped from review due to trivial changes (2)
- README.md
- server/codex-permission-mode.js
🚧 Files skipped from review as they are similar to previous changes (1)
- server/openai-codex.js
2 participants
Summary
CLOUDCLI_CODEX_PERMISSION_MODEas a server-side fallback for Codex chat requests that omitpermissionMode.default,acceptEdits, andbypassPermissions.default.bypassPermissionssafety tradeoff.Why
This came from the HolyClaude use case in CoderLuii/HolyClaude#18 and the v1.2.6 downstream patch:
The current CloudCLI browser chat submit path sends
permissionMode, so normal UI requests keep their selected mode. The fallback is for direct WebSocket clients or custom frontends that omitpermissionMode, and for container operators who want a deployment-level Codex default.Examples:
default: cautious or shared deployments where omitted-mode requests should stay on the standard sandbox and approval path.acceptEdits: trusted self-hosted workspaces where Codex should apply file edits without repeated prompts while staying inworkspace-write.bypassPermissions: single-user trusted local containers where the operator intentionally wants full access inside the container and mounted workspace.Explicit request values always win over the env var.
Testing
npm cicompleted; npm reported 39 advisories in the dependency tree.node --check server/openai-codex.jsnode --test server/codex-permission-mode.test.jsgit diff --checknpx eslint server/openai-codex.js server/codex-permission-mode.js server/codex-permission-mode.test.jsnpm run lintcompleted with 291 warnings and 0 errors.npm run typechecknpm run buildcompleted with Browserslist, CSS minify, and chunk-size warnings.Not live-tested: an actual Codex chat runtime session.