feat: add --sandbox so a run can reach a database without granting write access - #759
coincobber wants to merge 3 commits into
Conversation
`--write` is the only control over the Codex sandbox: without it a task runs under `sandbox: "read-only"` (codex-companion.mjs), which blocks database and network access, not merely file edits. The rescue skill and agent told the forwarder to drop `--write` when the user "only wants review, diagnosis, or research without edits". Those words describe the deliverable, not the sandbox, so a request phrased as "review this, read-only, make no changes" silently removed the ability to run anything — and the failure surfaces only as an opaque `EPERM` from deep inside a database client, which reads like a broken tool rather than a missing flag. Observed: a performance review asked to measure query cost with a read-only database script. The forwarder dropped `--write` as instructed, every measurement attempt failed with `EPERM`, and the review silently fell back to reasoning from source. Its headline figure was a cap-based maximum that overstated the real cost by 56%. Guidance only; no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The previous commit claimed a run needing to execute something "needs `--write`". Testing showed that is wrong. `--write` selects `workspace-write`, which permits writes inside the workspace but still blocks the network and the database. Verified against the same database script, three ways: no flag (read-only) -> code: 'EPERM' --write (workspace-write) -> code: 'EPERM' danger-full-access -> exit 0, real query output No flag selects `danger-full-access`, so a task that must measure against a live dependency cannot be completed through the forwarder at all. The guidance now says that plainly, so the run reports the limit instead of silently degrading to source-only reasoning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
coincobber
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c06faefe3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Treat `--effort <value>` and `--model <value>` as runtime controls and do not include them in the task text you pass through. | ||
| - Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. | ||
| - Default to a write-capable Codex run by adding `--write`. | ||
| - `--write` selects the SANDBOX, not merely permission to edit files. Without it Codex runs in the `read-only` sandbox and cannot reach the database, the network, or any command that needs them. A read-only investigation that must still RUN something — a database script, a test, a build, a profiler — needs `--write`. |
There was a problem hiding this comment.
For rescue tasks that need a networked database or another remote service, adding --write still leaves them unable to connect. The wrapper only maps the flag to sandbox: "workspace-write" in codex-companion.mjs:491; I checked the installed Codex 0.144.0-alpha.4 app server, whose thread/start response for that mode reports networkAccess: false (and codex --help describes --sandbox only as selecting the policy for model-generated shell commands). Consequently these new instructions both promise access that the flag does not grant and misdiagnose the resulting EPERM, so the motivating measurement workflow will continue to fail unless network access is configured separately.
Useful? React with 👍 / 👎.
| - Treat "read-only", "review", "diagnosis" and "research" in the user's wording as a description of the DELIVERABLE, not of the sandbox. They are not by themselves a reason to omit `--write`. | ||
| - Omit `--write` only when reading source files is genuinely sufficient. When the task must measure or execute anything, pass `--write` and forbid edits in the forwarded prompt text instead, for example: "Make no file changes and no commits." |
There was a problem hiding this comment.
When a user explicitly requests read-only execution, these rules now grant the delegated model workspace write access merely because it needs to run a command, while the prompt-level phrase “Make no file changes” is not an enforcement boundary. I verified with the installed CLI that the read-only sandbox does execute commands successfully (printf and git rev-parse both ran), while only a write attempt was rejected; therefore read-only tests or diagnostics do not inherently require --write. This removes the protection the user explicitly selected and allows an accidental model edit, so the forwarder should retain read-only mode unless a concrete required write is identified and the user agrees to broaden access.
Useful? React with 👍 / 👎.
`--write` was the only control over the sandbox, and it conflated two unrelated permissions: may Codex edit my files, and may Codex reach the network and the database. Neither `read-only` nor `workspace-write` reaches a database, so a review that needed to measure something had no way to run at all. `--sandbox <read-only|workspace-write|danger-full-access>` separates them. `runAppServerTurn` already accepts `options.sandbox` and forwards it to both `startThread` and `resumeThread`, so this only threads the value from the CLI through `buildTaskRequest`. The background worker spreads the stored request, so queued runs carry it without further change. Default behaviour is unchanged: with no `--sandbox`, the mode is still `request.write ? "workspace-write" : "read-only"`. Verified against a read-only database script: no flag -> code: 'EPERM' --write -> code: 'EPERM' --sandbox danger-full-access -> connects, runs, exits 0 --sandbox bogus -> rejected, valid modes named The useful combination this unlocks is `--sandbox danger-full-access` WITHOUT `--write`: Codex can measure against a live dependency while still being unable to edit the repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
coincobber
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dbf96a294
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| model: request.model, | ||
| effort: request.effort, | ||
| sandbox: request.write ? "workspace-write" : "read-only", | ||
| sandbox: request.sandbox ?? (request.write ? "workspace-write" : "read-only"), |
There was a problem hiding this comment.
When a review needs a live dependency, the new rescue rules explicitly invoke --sandbox danger-full-access without --write, but this line makes the sandbox override the write flag entirely. request.write is used only for job metadata/rendering, so nothing prevents that Codex turn from modifying the repository—or other host files—despite the documentation promising that it “still cannot edit”; a prompt such as “Make no file changes” is not an enforcement boundary. The newly added sandbox override is fresh evidence beyond the earlier read-only concern: either preserve an actual no-write boundary for networked reviews or stop presenting this combination as read-only.
Useful? React with 👍 / 👎.
1 participant
Problem
--writeis the only control over the Codex sandbox:That one flag conflates two unrelated permissions: may Codex edit my files, and may Codex reach the network and the database. You cannot ask for one without the other, and the combination most reviews actually want — measure a live dependency, touch no files — cannot be expressed at all.
Verified against a read-only database script:
read-onlycode: 'EPERM'--writeworkspace-writecode: 'EPERM'--sandbox danger-full-access--sandbox bogusSo
--writedoes not help a task that needs to measure, and before this PR nothing selecteddanger-full-access.The guidance made it worse. Both
skills/codex-cli-runtime/SKILL.mdandagents/codex-rescue.mdsaid:"Read-only", "review", "diagnosis" and "research" describe the deliverable, not the sandbox. A request phrased "review this, read-only, make no changes" silently removed the ability to run anything — and the failure surfaces only as an opaque
EPERMfrom inside a database client, which reads like a broken tool rather than a sandbox boundary.Observed
A performance review was asked to measure query cost with a read-only database script. The forwarder dropped
--writeas instructed, every measurement failed withEPERM, and the review fell back to reasoning from source without flagging that its evidence base had collapsed. Its headline figure was a cap-based maximum that overstated real cost by 56% — a number one successful query would have corrected.Change
Code. Adds
--sandbox <read-only|workspace-write|danger-full-access>totask.runAppServerTurnalready acceptsoptions.sandboxand forwards it tostartThreadandresumeThread, so this only threads the value from the CLI throughbuildTaskRequest.normalizeReasoningEffort, including naming the valid modes in the error.--sandbox, the mode is stillrequest.write ? "workspace-write" : "read-only".Docs. Updates the skill and agent so the forwarder can use it:
--writeselects the sandbox, not just edit permission.read-onlynorworkspace-writereaches the network or a database.--sandbox danger-full-accesswhen the task must measure against a live dependency;--writewill not achieve it.--sandbox danger-full-accesswithout--writeis the right shape for a measuring review.--sandboxis a routing flag, stripped from the task text and passed through.🤖 Generated with Claude Code