[adil-a]

Summary

M29 of the Harbor task-format stack: the remaining sandbox capabilities. Docker gets interactive PTY sessions on a par with OpenSandbox, the Harbor verify phase gets its network policy on OpenSandbox, and a static policy test pins the rule that task code runs only in the sandbox and the sandbox is never exposed outside the harness session. The in-sandbox tool bridge with grants for [[environment.mcp_servers]] is dropped from this row: host-loop harnesses keep the 422 for tasks with MCP servers.

What changes

Docker PTY (SupportsSandboxPty). nemo_gym/sandbox/providers/docker/pty.py runs one docker exec -i -t client per session under a host pseudo-terminal, so the CLI allocates a terminal in the container and forwards bytes both ways; the session reads and writes the host pty's master. rows/cols size the terminal before the client starts (the CLI applies the size at exec start), resize() sets the host size and sends the client SIGWINCH; cwd/env/user map to -w/--env/--user; pty=False runs the client on pipes with stdout and stderr split. The program starts through a one-line launcher that records its pid in /tmp/.nemo-gym-pty-<id>.pid inside the container (in /dev/shm/.nemo-gym-pty-<id>.pid when /tmp refuses the write, as under create.read_only: true without a tmpfs on /tmp; a session that can record it nowhere, say --ipc=none as well, is not started: the launcher says so on stderr and exits 1, so the session ends at once with the message rather than running unsignallable), which send_signal() (kill through a second exec, as root) and close() use. The read of the record waits up to two seconds for the launcher to write it, because create_pty() returns as soon as the client is spawned and a send_signal()/close() issued right away reaches the container through a second docker exec that races the session's own. The read is bounded against the record's owner too: a record that is not a regular file (a FIFO, or a link to a device, put at the path by the session's user) is refused at once rather than opened, the file is opened read-write on a pinned descriptor and tested again there (an O_RDWR open of a FIFO does not wait for a writer, and the test on /proc/self/fd/3 catches whatever was swapped in after the path was tested), at most 32 bytes are read, and the exec that reads it runs under PID_READ_TIMEOUT_S (30 s, or the shorter exec.default_timeout_s) instead of the 180 s default, so the session's user can make its record useless but cannot stall send_signal() or close() on it. The file is the session user's, who can rewrite it or replace it by a link to a file it does not own, so neither its content nor its owner is trusted: it is read once, as the session's own user (/proc/<pid>/environ is readable by the process's own uid but not by a root without CAP_SYS_PTRACE, which Docker's default capability set does not grant, so a root-side read fails for every non-root session; the kill and the cleanup run as root and no added capability is needed), and followed only to a plain pid above 1 whose process carries the session's marker in its environment (NEMO_GYM_PTY_SESSION=<id>, set by the exec's own --env), whose parent is outside the container (PPid 0, the runtime's execs) and, in PTY mode, which leads its session; a rewritten, hard-linked or renamed file can never aim the root-side kill at a process the session did not start. In PTY mode send_signal() addresses the terminal's foreground process group, read from /proc/<pid>/stat, which is what a key press reaches: the program of a command session, or the job an interactive shell is running (a shell with job control keeps each job in a group of its own, so a signal to the shell's group would never reach it); on pipes the process's own group and then the process. A docker client that goes away does not end the process it started, so closing a live session hangs up its process group, gives a shell up to a second (bounded with an integer-only sleep too) to relay the hangup to its jobs, then SIGKILLs every process left in the recorded process's session, and removes the pid file. Every in-container line is spelled for busybox sh as well as dash and bash (kill -INT -<pgid>), since sh is busybox in Alpine-based images. Sessions live in the Gym process (no attach() by id; pty.exec(detach=True) runs attached) and are released when the container is closed and at provider aclose(). sandbox.pty.create/exec and the interactive sandbox shell (python -m nemo_gym.sandbox.shell) now work on Docker exactly as the facade expects, for root and non-root sessions alike (a session program that changes its user or execs a setuid program loses its signalling; documented). A harness that drives a terminal session, a Terminus-2-style PTY harness say, can build on them; no harness in the repository consumes sandbox.pty yet (the shipped Terminus-2 sandboxed agent drives tmux over exec), so the PTY is exercised by the facade, the shell and the tests, not by a rollout.

OpenSandbox network policy at verify. Verified live with the OpenSandbox 0.1.15 SDK against an OpenSandbox deployment (transcript in the report; the probes did not read the control plane's own version): a sandbox created without a network_policy has no egress sidecar and cannot be restricted afterwards (the egress endpoints answer 502); a sandbox created with a policy keeps its defaultAction for life, the SDK's patch_egress_rules/delete_egress_rules change rules within seconds but nothing changes the default action, and * is not a catch-all target. So Harbor's _phase_network_policy switch has no equivalent on the agent's sandbox, and the restricted policy is applied where it can be: the separate verifier's own sandbox is created under it. Concretely:

• nemo_gym.sandbox.agent_tools.phase_network_policy(provider_name, network_mode, allowed_hosts) turns a Harbor phase's no-network/allowlist into the OpenSandbox egress policy (deny by default, the hosts as ordered allow rules: host names, *.domain wildcards and bare IP literals as Harbor spells them; CIDR ranges refused, since the OpenSandbox egress sidecar documents FQDN and wildcard targets only and they were not verified live, a parity gap with Harbor, which admits them; and the spellings Harbor's normalize_allowed_hosts refuses, bracketed or zoned IPv6 literals and wildcards over an IP literal, refused as Harbor does), failing closed for any other provider as restricted_network_policy does.
• The loader admits [verifier].network_mode/allowed_hosts for separate-mode tasks and refuses them on shared-mode tasks with a message that names the fix (a shared-mode task with artifacts that the server would verify separately under separate_verifier_for_artifacts is refused too, since the loader cannot see that setting: declare environment_mode = "separate"); [environment], [agent], [verifier.environment] and step restrictions stay refused.
• The environment server builds the policy at seed (a provider other than OpenSandbox, or an entry the policy cannot express, is a terminal 422 before the rollout) and creates the separate verifier's sandbox with it under provider_options.network_policy, replacing the deployment's sandbox_provider_options.network_policy for that sandbox as Harbor's explicit phase policy replaces the baseline. The window is wider than Harbor's, which wraps verifier.verify() only: here the verifier sandbox lives its whole life under the policy, so the verifier image's ENTRYPOINT/HEALTHCHECK and the artifact staging run restricted too (documented, with the advice to allowlist hosts a verifier image needs at start). verifier_network in the result reports the mode. No new config keys or CLI flags.

Static sandbox policy test (tests/unit_tests/test_sandbox_policy.py), rules derived from the code and stated in its docstring: no host process spawning under environment_servers/harbor under any import alias (calls are resolved through the module's imports, from imports of spawn functions and bare import subprocess|multiprocessing|pty are refused outside the spawning modules unless type-only; the check is self-tested on snippets it must flag) and only git/the configured build tool/docker in the three dataset-tooling modules under nemo_gym/tasks/harbor (never shell=True); no dynamic code loading; verifier_command(...) is only ever handed to a sandbox .exec; the server registers no route of its own (/run and /aggregate_metrics only, no command-shaped request field), sandbox_access= is passed exactly once in the whole server package, inside the POST /v1/agent_sessions to the configured agent server, serialize() is called only by _sandbox_access, and no response model carries the descriptor; no shipped Harbor config selects the host-execution local provider, resolved the way the server does (the top-level block any sandbox_provider/gpu_sandbox_provider value names, under whatever name, plus any sandbox*-named block; self-tested on synthetic configs).

Docs: the Docker and OpenSandbox provider pages, the sandbox index (interactive terminals), the Docker provider README and the Harbor environment server README.

Test plan

• tests/unit_tests/test_docker_pty.py (54): the real terminal plumbing against a local sh under a host pty (sizing, resize, the SIGWINCH resize() sends to the client, CRLF, exit, EOF, pipe-mode stderr split, close-before-exit, run_detached, the facade's pty.exec end to end through DockerProvider), the docker exec command line, signal and cleanup execs against a recorded fake CLI, and real signals: send_signal("SIGINT") on an interactive bash and dash session (under the pty as controlling tty, as runc gives an exec) ends the foreground sleep with status 130 and leaves the shell usable, with the in-container lines run by this host's sh and by busybox sh; the pipe-mode fallback ends a command session with SIGTERM; close() of a live bash session ends its background job (hangup relayed, then the session sweep); a pid file rewritten to 1, to another user's pid or to the session's own child, or replaced by a hard link or a symlink to another file, is refused before any kill, by send_signal() and by close(); close()'s hangup grace stays about a second under a sleep that rejects fractions; check_recorded_pid on 15 crafted records; a failed kill surfaces as Could not signal unless the process exits within the second; provider.close() abandons a live session with no exec into the container; session release on container close and aclose(); close() and send_signal() issued right after create_pty() with the launcher held back half a second still reach the process (the pid read waits up to two seconds for the record, bounded under an integer-only sleep too); a pid directory that refuses the write (a read-only /tmp) sends the record to the /dev/shm fallback and signals and cleanup work from it, and a session that can record its pid nowhere ends at once with the launcher's message; and a non-root session in a stand-in container (a user+pid+mount namespace: session as uid 1000, root-side execs as uid 0 without CAP_SYS_PTRACE, the session's process with a real PPid 0 and the pid file at its real path in a private /tmp), where the root-side read of the old code is refused by the kernel and the read as the session's user, send_signal on the foreground job, close() of the shell and its background job and the pipe-mode SIGTERM all work, and the same session under a read-only tmpfs mounted over /tmp records its pid under /dev/shm and is signalled and closed from there (skipped where unshare --map-auto/nsenter/setpriv/setsid are unavailable); a pid record replaced by a FIFO, by a symlink to one at the fallback path or by a link to /dev/zero is refused at once by send_signal() and by close() (within the two-second wait, nothing killed, the process left to the container, the record removed) under sh and busybox, a 1 MB record is read 32 bytes at most, the pinned record open comes back at once for a FIFO with or without a writer attached, a symlink to one, a device and a directory, and the pid read's exec runs under PID_READ_TIMEOUT_S (30 s, or the shorter exec.default_timeout_s) while the kill and the cleanup keep the default
• tests/unit_tests/test_sandbox_agent_tools.py::TestPhaseNetworkPolicy (15), loader tests in test_harbor_tasks.py (18 cases), environment_servers/harbor/tests/test_app.py::TestVerifierNetwork (8)
• tests/unit_tests/test_sandbox_policy.py (30: the five rules on the tree, 16 self-tests of the spawn check on snippets that must be flagged or accepted, 7 self-tests of the R5 provider-block resolution)
• Environment server suite: 417 passed, 2 deselected (409 on the base); oracle suite: 22 passed; sandbox and Harbor unit files together: 1114 passed, 5 skipped; ruff check/ruff format --check clean; pre-commit on the changed files passes
• Live on OpenSandbox: the public sandbox cannot be switched (curl 200 before and after the SDK's patch attempt), a sandbox created under phase_network_policy("opensandbox", "no-network") refuses https://example.com (exit 6) and a raw IP (exit 28), under the allowlist of example.com reaches it and refuses www.nvidia.com
• Real rollout: gym dataset validate on OpenSandbox with a no-network and an allowlist separate-verifier task whose test.sh curls from the verifier and reads the agent's own curl result (see the report), and a shared-mode task with the same restriction skipped at prepare with the loader's reason
• A live Docker PTY check needs a Docker host (this node has no daemon and podman is broken); the plumbing is exercised with a real local process under a real pty, and the docker exec side with fakes

🤖 Generated with Claude Code

[adil-a]

/ok to test 6ac6cfa

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-4862-c3f95b11fae8.docs.buildwithfern.com/nemo/gym

[adil-a]

/ok to test b69effd

[adil-a]

/ok to test b21a912

[adil-a]

/ok to test 7110d16

[ffrujeri]

The PTY lifecycle fixes look solid, and each has a regression test that uses the real terminal plumbing. I ran the Docker PTY path against a live daemon to cover the unchecked item in the test plan (python:3.12-slim and alpine:3.20, as root and as nobody): terminal sizing, SIGINT on a foreground job (exit 130), resize, close() ending a background job and removing the PID files, command and pipe-mode exits, the paused-container SandboxPtyError, and session release on container close all behaved as described.

The one thing I'd like settled before merge is the verifier-versus-deployment network policy thread; the fail-closed intersection you proposed sounds right to me. The only CI failure is the non-blocking kilocode conformance job.

[adil-a]

/ok to test 22fe4e2

[adil-a]

/ok to test 97ebc23

[adil-a]

/ok to test f921994

[adil-a]

/ok to test 4a6f40f

[adil-a]

/ok to test 890edb2

[adil-a]

/ok to test b118c1e

[ffrujeri]

One remaining case: network_policy: {} is deny-all in the 0.1.15 SDK, but bool(deployment) treats it as absent, so a task allowlist can still widen it. Could this check deployment is not None instead?

[adil-a]

/ok to test 50d38ef

[adil-a]

Fixed in 50d38ef (folded into the M29 commit): _verifier_network_policy now treats a present network_policy as the deployment's policy whatever its contents, so network_policy: {} (deny-all in the 0.1.15 SDK) refuses a task allowlist the same way an explicit deny-by-default policy does, and only a missing key is allow-all; a present non-mapping value is reported as a 422 seed refusal instead of crashing. Tests: test_an_empty_network_policy_is_deny_all_and_refuses_an_allowlist_but_not_no_network (allowlist refused with the same message and no sandbox created; no-network still passes and narrows) and test_a_network_policy_that_is_not_a_mapping_is_refused_at_seed, both failing before the change.

[adil-a]

/ok to test ace1a65

[yuhezhang-ai]

The blocking findings from my review are addressed on ace1a658 against stack parent 5529c2f3, including the terminal lifecycle/input issues and the empty deny-all network-policy bypass. All 40 focused network-policy and terminal regression tests pass, and the original empty-policy reproduction now refuses the widening task allowlist before creating a sandbox.

The remaining documentation wording comment is nonblocking. The main unit-test job is still running, so merge should wait for required checks.

[adil-a]

/review role=automation

[ffrujeri]

Thanks for the fix, LGTM.

[svcnvidia-nemo-ci]

HOLD — one BLOCKER async write hang, two RISK gaps in the static policy checks, and one NOTE about network-probe evidence.

Reviewed all 18 changed files and directly affected contracts. The earlier terminal lifecycle/input fixes and the empty deny-all deployment-policy refusal are present in the current source and regression assertions. The delegated network report was unavailable, so that scope was inspected directly.

This was a source-only review: no tests or live Docker/OpenSandbox checks were executed. The suggested corrections below require author verification.

Note: formal approval is not available for this request, so this review is posted as a comment only.

Review generated by codex. This was a full review. Request a full review with /review reset=true.

[adil-a]

/ok to test 8342487

[adil-a]

/ok to test 6f35ff3

[adil-a]

/ok to test ae053cb

[adil-a]

/ok to test c3f95b1

[ffrujeri]

LGTM.

[adil-a]

/review role=automation

[svcnvidia-nemo-ci]

SHIP — no unresolved prior findings or qualifying new regressions found.

Verified all four findings from review 5660201709 against the current source:

• PTY writes: write() now serializes the entire operation and checks closure after acquiring the lock. Regression assertions cover complete A+B delivery, abandonment of both writers, and descriptor reuse without leaked input.
• R2 dynamic loading: The production check and self-tests resolve import aliases, reject the reported loaders, and preserve regex compilation and ordinary sandbox/model methods.
• R5 local provider: The check and regression cases now reject null-valued sandbox provider blocks while retaining the intended reference/name boundaries.
• Network-probe wording: The documentation separates the observed raw-IP timeout from its unestablished enforcement layer and retains the deployment-setting caveat.

The earlier empty deny-all deployment-policy refusal also remains present, with assertions covering allowlist rejection before provisioning and successful no-network narrowing.

This completed follow-up covered the reported fixes, preservation cases, and directly affected contracts—not a fresh whole-PR review. The previously reviewed tree was unavailable, limiting exact inter-review comparison. Assessment was source-only; no tests or live Docker/OpenSandbox checks were executed.

Note: formal approval is not available for this request, so this review is posted as a comment only.

Review generated by codex. This was a findings-only review. Request a full review with /review reset=true.

[github-actions]

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40533122604

[github-actions]

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40540769119