Repository navigation
feat(codex): restrict the egress of agent-created sandboxes - #5556
Conversation
Benchmarks whose Resources server supplies no sandbox, such as HLE, have pi_agent create one per session. pi_sandboxed_agent ran those sandboxes with model-only egress; pi_agent could not, so a reasoning benchmark moved to agent sessions would gain open internet. - network_access (inherit, model_only, model_and_tools) sets an OpenSandbox egress policy on a sandbox the agent creates. A borrowed task sandbox keeps its owner's network, so a session that borrows one rejects any other value. The policy helper, restrict_sandbox_network, lives in nemo_gym.agents.utils.sandbox_network and is exported from nemo_gym.agents so other session agents can share it. - sandbox_runtime_dir points setup at a runtime baked into the image and checks its version, since a model-only sandbox cannot download Node or Pi. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
As for pi_agent: network_access sets the OpenSandbox egress policy on a sandbox Codex creates, and a session that borrows a task sandbox rejects any other value than inherit. The policy comes from the shared restrict_sandbox_network helper, imported from nemo_gym.agents, so it cannot be combined with sandbox_config.provider_options.network_policy. A model-only sandbox cannot download Node or Codex, so sandbox_runtime_dir points setup at a runtime baked into the image and checks that it reports codex-cli <codex_version> instead of installing. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test e3336c7 |
svcnvidia-nemo-ci
left a comment
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP WITH CARE
RISK — Add representative Pi smoke-rollout evidence
The supplied real rollout results all use Codex, but this changeset also changes Pi's network policy and preinstalled-runtime setup. No representative Pi rollout is recorded. The repository quality bar requires model-backed smoke evidence for behavior-changing agent work. This is a validation gap affecting the new Pi session settings, not a demonstrated runtime failure.
Correction: Record a representative Pi rollout using an agent-owned OpenSandbox sandbox, a preinstalled runtime, and restricted egress. Include the Pi version, image, runtime path, model, configuration/commands, and observed agent and verifier results. No source patch is indicated by this gap.
Acceptance: A proposed model_and_tools smoke run should complete a real model request, successfully call a granted MCP tool, produce a verifier result, and destroy the owned sandbox on close. Record any failure and resolve it before claiming validation. I couldn't run this check here.
Preservation: Retain the existing session-test cases: inherit must continue borrowing without stopping the Resources-owned sandbox; restricted settings must reject borrowed access before sandbox execution; model_only must allow only the model host, while model_and_tools adds the grant host; and a matching preinstalled version must skip installation while a mismatch fails setup.
I reviewed all nine supplied changed files, including the stacked shared helper and Pi changes, and traced policy forwarding, runtime launch, and ownership-aware cleanup. I found no additional qualifying source-level defects. This was a static review; I didn't execute tests or rollouts.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
restrict_sandbox_network raised ValueError when a restricted network_access could not be enforced: an explicit provider network policy as well, a provider without network policies, or an endpoint the sandbox cannot reach. The seed route answered it with a 500, which the Environment Server retries, though no attempt can fix the configuration. The helper now raises AgentSeedRejectedError, still a ValueError, so every adapter that calls it refuses the seed once with HTTP 422, before any sandbox is created. Pi also built its tool egress from every MCP grant, including an optional one it leaves out at activation, so such a grant at a loopback URL failed the seed. Egress now covers only the grants Pi keeps. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
…egress change Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
…th a 422 With NVIDIA-NeMo#5554's restrict_sandbox_network change, a restricted network_access the provider cannot enforce (here an explicit network_policy as well) refuses the seed with HTTP 422 before any sandbox is created, instead of a retried 500. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
On the Pi smoke-evidence finding: the Pi changes in this diff are #5554's, which this PR is stacked on, and their rollout evidence is in #5554's description (the Rollouts section): #5554. It covers internal HLE runs of Pi on an agent-owned OpenSandbox with the preinstalled runtime ( This PR now also has #5554's review fix merged in (8ba3e02): |
|
/ok to test 01c758a |
Both sides added public exports to nemo_gym.agents; both are kept. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
…x egress change Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test 61b28c9 |
Addresses Yu Yao's review of the Pi egress change: - restrict_sandbox_network takes the required tool grants and refuses them under model_only with AgentSeedRejectedError. The policy blocks their hosts, so a session would otherwise fail only after the sandbox and the runtime exist, as a retried 5xx. Pi passes its required MCP grants. - A host written with a trailing dot no longer yields a rule the egress sidecar never matches, and provider_options: null is restricted like an empty mapping instead of raising a 500. - Pi's tests cover both restricted values for a borrowed sandbox, a model URL that fails to resolve (the provider is closed), and a preinstalled runtime that exits nonzero or reports a version prefix. - The profile comment says that with sandbox_runtime_dir: null the image must hold the runtime at the default path with its ready marker. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Under model_and_tools, a grant at an unreachable address refuses the seed even when optional, and model_only refuses a required grant. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Addresses Yu Yao's review: - The preinstalled-runtime check runs with CODEX_HOME under the session directory, so it writes nothing into the task's home, and checks for Python 3.8+ first, as the installer does. A runtime that is missing, runs another version (exact match) or finds no usable Python refuses the seed with AgentSeedRejectedError; a transport error or timeout stays retryable. - sandbox_transport.py postpones annotation evaluation, so it imports on the Python 3.8 and 3.9 the README and installer allow (checked under both). - Codex passes its required MCP grants to restrict_sandbox_network, so model_only refuses them at seed instead of failing after setup. - The README says how to build sandbox_runtime_dir, that the image needs bash and python3, that the setting also applies to a borrowed sandbox (checked, never stopped), which grants a restricted seed refuses, and that full isolation needs OpenSandbox's dns+nft egress mode. - Tests: several grants (required, optional, direct_http) in the egress test, both restricted values for the borrowed refusal, a route test per refusal cause with the provider closed and no session record left, and a runtime-check table (prefix, failed run, timeout, runtime used). Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
github-actions
Bot
removed
the
sla:review-overdue
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com> NVIDIA-NeMo#5554 was squash-merged, so its files conflicted with this branch's copy of them; main's versions are kept. This PR's own changes are unchanged.
|
/ok to test 0bfd75b |
Brings in main's latest four commits; this PR's own changes are unchanged. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test 4b94f94 |
yaoyu-33
left a comment
yaoyu-33
left a comment
There was a problem hiding this comment.
Re-reviewed at 4b94f94 (the change is git diff main...head now that #5554 has merged). Everything from my earlier review is addressed, and CI is green on this head.
- The preinstalled-runtime check now separates the two cases:
error_typestays a retryableRuntimeError; a non-zero exit or any other version (exact match) is anAgentSeedRejectedError, so a 422. - The check runs with
CODEX_HOMEunder the session directory, so nothing lands in the task's home, and it checks Python 3.8+ first as the installer does;sandbox_transport.pynow postpones annotation evaluation so the README's 3.8+ holds. - Required MCP grants are refused under
model_onlyat seed; the README says how to buildsandbox_runtime_dir, that it also applies to a borrowed sandbox, and thedns+nftcaveat. - The tests are a table (pinned, other version,
0.144.40prefix, failed run with the right banner, timeout) and the refusal test is parametrized like Pi's.
Not run here: I did not re-run the suites locally; I read the diff and relied on the green CI at this head.
yaoyu-33
added
the
ready-to-merge
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/42137145215 |
What does this PR do?
codex_agentgainsnetwork_access(defaultinherit) for the sandboxes it creates. The value comes from feat(pi): restrict the egress of agent-created sandboxes #5554'srestrict_sandbox_network, with the Model Server's host and, formodel_and_tools, each MCP grant's host. A session that borrows a task sandbox rejects any value other thaninherit.codex_agentgainssandbox_runtime_dir: an absolute sandbox path that holds a preinstalled runtime in the installer'snode/andcodex/layout. Setup checks that it reportscodex-cli <codex_version>instead of installing, and a mismatch fails setup and stops the sandbox.Why. The same reason as #5554, for Codex. Without this, a sandbox Codex creates has the provider's default network, and a restricted sandbox cannot download Codex. Stacked on #5554, whose branch this PR targets. Part of #5552.
Validation
gym env test +entrypoint=responses_api_agents/codex_agent +should_validate_data=true: 481 passed, 7 skipped. The skips already existed: they needpython3.8or aCODEX_TEST_BIN.With the behavior disabled, the new tests fail: model-only and model-and-tools egress, the borrowed-sandbox rejection, both preinstalled-runtime cases and the absolute-path check. The
inheritcontrol passes.Review follow-up: feat(pi): restrict the egress of agent-created sandboxes #5554's review fix merged in (8ba3e02), so this adapter's
restrict_sandbox_networkcall refuses an unenforceablenetwork_accesswith a terminal 422.test_unenforceable_network_access_is_a_terminal_seed_refusalchecks the 422 and that no sandbox is started; it fails with feat(pi): restrict the egress of agent-created sandboxes #5554's helper change reverted. Suite in the adapter's own venv: 482 passed, 7 skipped.pre-commit run --all-files: clean.Follow-up to the feat(pi): restrict the egress of agent-created sandboxes #5554 review: merged its latest head; the helper call passes the provider; a borrowed sandbox with a restricted value is refused with
AgentSeedRejectedError; the README says a borrowed sandbox relies on its owner's policy. Its benchmark profile setsnetwork_access: model_onlyand requiressandbox_runtime_dir. Tool egress now leaves out the MCP grants Codex skips, with a regression test. The adapter's owntest_benchmark_profile.pyexpectsmodel_only(removing it fails the test). Suite: Codex: 521 passed, 7 skipped; profile and helper tests pass.Review follow-up (Yu Yao) at 623cf9a: the preinstalled-runtime check runs with
CODEX_HOMEunder the session directory and checks Python first; a missing runtime, another version (exact) or no usable Python is a 422, a timeout stays retryable;sandbox_transport.pyimports on Python 3.8 and 3.9 (checked under both); required MCP grants are refused undermodel_only; README on buildingsandbox_runtime_dir. Tests: a runtime-check table, route refusal tests per cause, several grants in the egress test. Codex suite with the profile and helper tests: 560 passed, 7 skipped.Rollouts
On the internal OpenSandbox deployment used for #5554's egress checks, before this PR's review changes, an image with Codex 0.144.4 preinstalled under
/opt/codex-runtimereportedcodex-cli 0.144.4with egress limited to the model and tool hosts, whilenodejs.org, the npm registry and GitHub's DNS name were blocked.Internal runs on the
georgea/nemo-gymfork at ee833ed, which carries this change before its review updates (Nemotron 3.5 Super on vLLM, 5 text tasks each, that image):model_only: 5/5 rollouts completed, graded and healthy.model_and_toolsand Gym's Tavily server: 5/5 healthy, with the search tools called in 3 of 5.model_only: 5/5 healthy.Compatibility and benchmark impact
Defaults unchanged:
network_access: inheritand nosandbox_runtime_dir.The adapter's own default stays
inherit. Its benchmark profile (from #5550) now setsnetwork_access: model_onlyand requiressandbox_runtime_dir, so a run that uses the profile gets the restricted sandbox; open-network scores are not comparable.Checklist
pre-commit run --all-files).git commit -s).