Repository navigation
feat(sandbox): allow Docker sandboxes to disable networking - #4452
Conversation
seratch
left a comment
There was a problem hiding this comment.
Thanks for the contribution. The need described in #4451 is demonstrated, and the narrow Docker-specific network_mode="none" design is sound. Before merging, please make three focused changes:
- Append
network_modeafter the existing keyword-onlytypeparameter to preserve public constructor ordering. - Update the current unreleased RunState 1.16 summary to include the persisted Docker network-isolation state.
- Add a deserialization test for an invalid persisted value such as
bridge, asserting rejection before any Docker provider access.
These changes keep the released API ordering and durable-state contract explicit while proving the none-only security boundary. Once they are addressed and CI is green, the PR should be ready for another review.
f73b3cc to
2298047
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f73b3cc5a9
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd19ee0b95
ℹ️ 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".
seratch
left a comment
There was a problem hiding this comment.
Thanks for the updates. The current head resolves the earlier constructor-ordering, RunState schema-summary, and persisted-value validation requests.
Can you continue working until the PR review says 👍 ?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 812f23bf90
ℹ️ 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".
Summary
This pull request adds an opt-in way to create Docker sandboxes with networking completely disabled.
DockerSandboxClientOptionsnow accepts:The new option is deliberately narrow:
When omitted, Docker container creation remains unchanged and no
network_modeargument is passed.When set to
"none", the value is forwarded todocker_client.containers.create(), causing Docker to create the sandbox without networking.The setting is also persisted in
DockerSandboxSessionStateand reapplied whenDockerSandboxClient.resume()needs to recreate a missing container. This prevents a sandbox originally created without networking from silently regaining Docker's default network configuration after recreation.Older serialized session states remain compatible because the new field defaults to
None.The public option intentionally does not expose arbitrary Docker networking modes such as
host,bridge, orcontainer:<id>.Test plan
Added regression coverage for:
network_mode="none""none"through the publicDockerSandboxClient.create()pathnetwork_mode="none"to Docker container creationDockerSandboxClientOptionsserialization and parsingDockerSandboxSessionStateserializationnetwork_modenetwork_mode="none"whenresume()recreates a missing containerFocused sandbox tests passed:
Full repository verification was run with:
All verification steps passed, including:
Issue number
Closes #4451
Checks
.agents/skills/code-change-verification/scripts/run.sh/reviewbefore submitting this PR (not applicable; this change was not prepared with Codex)