Skip to content

fix(acp): let full access edit the workspace - #418

Open
pjdoland wants to merge 3 commits into
plmbr:mainfrom
pjdoland:fix/acp-full-access-sandbox
Open

pjdoland wants to merge 3 commits into
plmbr:mainfrom
pjdoland:fix/acp-full-access-sandbox

Conversation

@pjdoland

Copy link
Copy Markdown
Collaborator

Summary

With ACP full access on and an API key configured, Codex could not edit any file. NBI passed only -c approval_policy="never". NBI's isolated CODEX_HOME has no config, so Codex used its default read-only sandbox, and under never nothing can be approved past it. Every edit failed with patch rejected: writing is blocked by read-only sandbox; rejected by user approval settings, while the ACP tab says full access "lets it run tools (edits, shell) without asking." In practice, turning on full access made the agent less capable than the default mode.

Solution

The pin. When full access is on, codex_approval_args now also passes -c sandbox_mode="workspace-write". That sandbox allows writes to the workspace (the Jupyter server's root directory) and temp directories, and keeps outbound network access off unless Codex's own config enables it. It matches what the setting promises. danger-full-access would also have removed the network and filesystem boundary that no administrator opted into by enabling full access. The default posture (approval_policy="untrusted", no sandbox pin) is unchanged. The pin is applied in the same place as the approval policy, so it follows the existing policy clamp and the NBI_ACP_AGENT_COMMAND path.

A shadowing fix made necessary by the pin. Codex starts NBI's MCP server outside its sandbox, with the workspace as its working directory, and NBI launched it with python -m notebook_intelligence.acp_mcp_server. -m puts the working directory first on sys.path, so now that full access can write the workspace without asking, a notebook_intelligence package planted there would run in place of the server. I reproduced that from a directory containing a planted package. The server is now launched by absolute file path. It imports only the standard library, so this works on every supported Python, including 3.10, where -P is unavailable.

Docs. The admin guide, the README policy row, the ACP tab copy, and the acp_full_access_policy traitlet help now describe the sandbox precisely:

  • What full access cannot do: network access and writes outside the workspace fail without prompting.
  • How the pins rank against Codex's user, project, and managed configuration, including the platform differences.
  • That command allow rules run outside the sandbox.
  • That files written in the workspace can later run outside the sandbox, so the workspace root matters.
  • That a replacement NBI_ACP_AGENT_COMMAND must pass NBI's -c options through.

Testing

Automated

  • TestApprovalArgs: the full-access pins, exact list.
  • TestFullAccessLaunch, through the real launch path:
    • Full access launches with both pins.
    • A missing full_access setting launches with the default posture.
    • A force-off policy keeps a stored full_access: true off. This goes through the real NBIConfig.
    • force-on turns full access on.
    • An NBI_ACP_AGENT_COMMAND override keeps the pins.
  • TestNbiMcpServerLaunch: the server launches by absolute path, and a spawn test runs the real launch command from a workspace containing a planted notebook_intelligence package and checks that the genuine server answers initialize.
  • Mutation-checked:
    • Reverting the pin, returning danger-full-access, or dropping the pin on the override path each fails the relevant tests.
    • Removing apply_acp_policies from the settings read path now fails the clamp tests. Previously the whole suite still passed.
    • Restoring the -m launch fails both server-launch tests.
  • pytest tests/ --ignore=tests/test_claude_client.py: 1760 passed. jlpm tsc --noEmit, stylelint, prettier, and eslint are clean. jlpm jest: 423 passed.

Live, against a JupyterLab running this branch (codex-acp 0.16.0, Codex 0.137.0, API-key auth, NBI_ACP_FULL_ACCESS_POLICY=force-on):

  • Before: the launch used -c approval_policy="never"; the transcript showed a read-only sandbox; the edit was rejected and the file was unchanged.
  • After:
    • The launch uses -c approval_policy="never" -c sandbox_mode="workspace-write", and the transcript shows workspace-write.
    • Codex fixed a bug in a workspace script and verified the result with no approval prompt and zero sandbox rejections, and the file changed on disk.
    • The MCP server process ran as python .../notebook_intelligence/acp_mcp_server.py.
    • The ACP tab showed the new copy, and the browser console had zero errors.

Behavior change

  • Deployments where full access is on (user-choice with the toggle on, or force-on) will now see the agent write to the workspace without asking. That is what the setting has always described, but it did not happen with API-key auth before.
  • With ChatGPT auth, a broader sandbox_mode in the user's ~/.codex (for example danger-full-access) is narrowed to workspace-write while full access is on, so commands that need the network fail without prompting.

No CHANGELOG entry: main has no 5.4.1 section yet, and I did not want to decide that header in this PR. Happy to add one wherever you prefer.

Risks / follow-ups

  • Workspace root. Files written in the workspace, such as a project .venv, IPython startup scripts, or Claude project hooks (and NBI's own config when the root is a home directory), can run later outside the sandbox. This is documented here. A guard for roots that contain ~/.jupyter is a product decision I left open.
  • Default path. It does not pin a sandbox or approvals_reviewer. With ChatGPT auth, the user's Codex config decides both in the default mode. That is unchanged by this PR but worth a look.
  • Platform sandbox. Full access depends on Codex enforcing its platform sandbox. Where it cannot, commands fail: containers without unprivileged user namespaces, and Windows without Codex's Windows sandbox, where Codex reads workspace-write as read-only. The Windows behavior and the managed_config.toml precedence come from reading the Codex source, not from a test run.
  • Network access. There is no NBI setting for network access under full access.
  • Naming. codex_approval_args now returns the sandbox pin too, so a rename may be worth it.
  • Client file handlers. _NbiAcpClient.read_text_file and write_text_file have no workspace containment. codex-acp 0.16.0 does not call them, so this is separate and pre-existing, and I plan to raise it on its own.
  • Blog. The 5.4 blog post in docs(site): add 5.4.0 release post and an ACP agent mode deep dive #417 describes the current full access behavior. That paragraph should be updated once this merges.

No issue was filed for this. The reproduction is in the Summary and Testing sections.

Full access passed only approval_policy="never" to codex-acp. With an API
key, NBI's isolated CODEX_HOME has no config, so Codex used its default
read-only sandbox, and under "never" nothing can be approved past it. Every
edit was refused ("patch rejected: writing is blocked by read-only
sandbox") while the ACP tab promised edits and shell commands without
asking.

Pin sandbox_mode="workspace-write" alongside the approval policy when full
access is on. That sandbox allows writes to the workspace and temp
directories and keeps network access off by default, which matches what the
setting promises without widening to danger-full-access. The default
posture is unchanged.

Update the admin guide, the README policy row, and the ACP tab copy to
describe the new behavior.
The first pass described the sandbox too absolutely and left the policy path
to the launch command untested.

- State Codex's trust rule correctly, and scope the pins' precedence to user
  and project config, since managed config outranks them.
- Document what full access cannot do (network access and writes outside
  the workspace fail without prompting), how the pin narrows a broader
  sandbox_mode under ChatGPT auth, command allow rules that bypass the
  sandbox, the risk of a workspace root that contains ~/.jupyter/nbi, and
  platforms where the sandbox cannot be enforced.
- Add launch tests that a force-off policy keeps a stored full_access off,
  that force-on turns it on, and that an NBI_ACP_AGENT_COMMAND override
  keeps the pins. Removing apply_acp_policies from the settings read path
  previously left the whole suite passing.
Codex starts NBI's MCP server outside its sandbox with the workspace as its
working directory, and NBI launched it with `python -m`, which puts that
directory first on sys.path. Now that full access can write the workspace
without asking, a notebook_intelligence package planted there would run in
place of the server, with network access. Launch the server by absolute
file path instead. It imports only the standard library, so this works on
every supported Python.

Tighten the docs as well: files written in the workspace can later run
outside the sandbox, so full access amounts to letting the agent run code as
the Jupyter server's account; managed Codex config differs by platform, and
on Windows the managed_config.toml read from CODEX_HOME is writable by that
account; and a replacement NBI_ACP_AGENT_COMMAND must pass NBI's -c options
through.

Adds a test that spawns the server from a workspace containing a planted
package, and a test that a missing full_access setting launches with the
default posture.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant