fix(rest): wire BoxOptions.network from CreateBoxDto to lower-layer sandbox#687
Draft
G4614 wants to merge 3 commits into
Draft
fix(rest): wire BoxOptions.network from CreateBoxDto to lower-layer sandbox#687G4614 wants to merge 3 commits into
BoxOptions.network from CreateBoxDto to lower-layer sandbox#687G4614 wants to merge 3 commits into
Conversation
…andbox
`box = rt.create(BoxOptions(network=NetworkSpec(mode='disabled')))` over
REST silently turned into "no network restriction" — the SDK serialised
`network` into the request body, but the NestJS CreateBoxDto didn't
include the field, so class-validator stripped it before the mapper saw
anything. The mapper then carried no `networkBlockAll` /
`networkAllowList` into `CreateSandboxDto`, and the runner inherited
default-allow.
Surface fixed end-to-end:
- apps/api/src/boxlite-rest/dto/create-box.dto.ts
Adds `CreateBoxNetworkDto { mode, allow_net }` mirroring the SDK
shape; `network?: CreateBoxNetworkDto` on the parent DTO.
`@IsIn(['enabled', 'disabled'])` matches the on-the-wire format
the SDK already serializes from NetworkSpec.
- apps/api/src/boxlite-rest/mappers/sandbox-to-box.mapper.ts
`mode === 'disabled'` → `networkBlockAll = true`.
`allow_net=[...]` → `networkAllowList` (CSV).
Absent → no field set, so callers that never opted in see
identical behaviour.
The lower-layer SandboxService.createFromSnapshot already understands
`networkBlockAll` / `networkAllowList` (see sandbox.service.ts:492–497),
and the runner already plumbs them through to libboxlite via
`boxlite.WithNetwork()` (apps/runner/pkg/boxlite/client.go:196). So this
patch is *only* a DTO + mapper fix at the seam — no runner changes.
E2E regression test:
sdks/python/tests/e2e/test_network_allow_net.py
::test_network_mode_disabled_blocks_all
Volume forwarding (the other BoxOptions field that gets silently
dropped) is NOT in this PR — the sandbox-side model uses pre-registered
Volume entities while BoxOptions takes inline (host, guest, ro) bind
mounts, which is an architectural mismatch needing a separate RFC.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…IDR validator Without this filter, BoxOptions.network.allow_net=['example.com'] caused the entire create call to 400 because the sandbox-side validator rejects anything that's not IPv4 CIDR. Restoring pre-fix behaviour for hostname inputs (silent no-op) while keeping CIDR entries working.
The fix in the preceding commit closes the gap this test exercises. Mirrors the layout of `scripts/test/e2e/cases/` on main.
G4614
added a commit
to G4614/boxlite
that referenced
this pull request
Jun 9, 2026
test_exec_user.py → boxlite-ai#686, test_network_allow_net.py → boxlite-ai#687, test_files_io.py → boxlite-ai#688, test_box_metrics.py → boxlite-ai#689, test_snapshot_clone.py → boxlite-ai#694, test_images_pull_list.py → boxlite-ai#696. Remaining 3 cases (test_exec_attach.py, test_volume_readonly.py, test_cli_detach_recovery.py) stay here because they pin REST-path gaps that don't have a matching fix PR in this session — they document the contract for future work to land against.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
box = rt.create(BoxOptions(network=NetworkSpec(mode='disabled')))over REST silently turned into "no network restriction" — the SDK serialisednetworkinto the request body, but the NestJS CreateBoxDto didn't include the field, so class-validator stripped it before the mapper saw anything. Adds the field + mapsmode/allow_netto the lower-layernetworkBlockAll/networkAllowListthat SandboxService and the runner already understand.Includes a fixup (commit 2) that filters non-CIDR entries out of
allow_netbefore passing to the sandbox-side validator, becausevalidateNetworkAllowListrejects hostnames as "Invalid IP address" — without this, callers who pass hostnames (per SDK docs) saw their box create call 400 instead of the previous silent no-op.Test plan — two-sided verified
Pin:
scripts/test/e2e/cases/test_network_allow_net.pyStack: local e2e. Only API restart needed (mapper + DTO change).
test_network_mode_disabled_blocks_all(wget https://example.comwithmode='disabled')network=disabledenforcedtest_allow_net_lets_listed_host_through(wget example.comwithallow_net=['example.com'])test_allow_net_blocks_unlisted_host(wget captive.apple.comwithallow_net=['example.com'])allow_nethas no path through the sandbox modelThe third case is out of scope: BoxOptions accepts hostnames in
allow_net(per SDK docs); the sandbox-sidenetworkAllowListonly takes CIDRs. Wiring hostname → CIDR (DNS resolve, or hostname filter at the runner egress) is an architectural extension, not a DTO fix. Documented as known gap; this PR closes themode='disabled'and CIDR cases.Commit 1: DTO + mapper.
Commit 2 (fixup): filter non-CIDR before forwarding, restores pre-fix behaviour for hostname inputs (no regression).