Split NetworkSpec into outbound/inbound directions - #996
Merged
DorianZheng merged 3 commits intoAug 19, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR restructures network configuration into nested outbound and inbound policies across the core, REST API, C, Node, and Python SDKs. It adds inbound service-access configuration, Node box tunnel APIs, validation coverage, preview-access E2E tests, and a CLI Tunnel command. ChangesNetwork policy and REST integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
July 17, 2026 05:57
8246d7d to
e83feda
Compare
3 tasks
|
Does the current implementation establish a new runner-to-box connection for every HTTP request? Does each request require re-authentication? Is the user base characterized by low concurrency? Is the performance acceptable? |
G4614
changed the title
G4614
marked this pull request as ready for review
July 21, 2026 07:09
📦 BoxLite review — couldn't completepowered by BoxLite |
G4614
enabled auto-merge
July 21, 2026 07:10
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/boxlite/src/net/mod.rs (1)
651-673: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale comments after removing
into_fd(). The test no longer recovers an fd, but lines 651–653 and the trailing line 672 still describe fd recoverability / "SDK fd-bridge relies on it," which no longer holds. Trim these so the test intent stays accurate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/boxlite/src/net/mod.rs` around lines 651 - 673, Remove the stale fd-recovery and SDK fd-bridge claims from the test comments surrounding BoxInternalTunnel::from_local. Keep comments describing the Unix socketpair, bidirectional AsyncRead/AsyncWrite behavior, and peer handling, but delete the trailing comment about recovering an owned OS fd.
🧹 Nitpick comments (6)
src/cli/src/commands/tunnel.rs (1)
22-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffNo test coverage for the new
tunnelcommand.
executehas no accompanying unit test in this batch (unlike the port-validation checks mirrored in the SDKtunnel()bindings, which do have tests). Consider adding at least a test for theport == 0rejection and theBoxEndpoint::UnixSocketerror path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/src/commands/tunnel.rs` around lines 22 - 50, Add unit tests for the tunnel command’s execute flow, covering port == 0 rejection and the BoxEndpoint::UnixSocket error path. Reuse existing CLI test patterns and fixtures/mocks around execute, asserting the validation error and remote REST profile error respectively without changing production behavior.sdks/python/tests/test_tunnel.py (1)
38-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd comprehensive docstrings to public test functions.
As per coding guidelines, comprehensive docstrings must be written for all public functions and classes. While these are test functions, they are public and should include a brief docstring explaining the test's purpose.
sdks/python/tests/test_tunnel.py#L38-L38: Add a docstring totest_endpoint_returns_stable_unix_socket_path.sdks/python/tests/test_tunnel.py#L49-L49: Add a docstring totest_connect_opens_fresh_sockets.sdks/python/tests/test_tunnel.py#L73-L73: Add a docstring totest_tunnel_requires_a_started_box.sdks/python/tests/test_dev_tunnel_e2e.py#L136-L136: Add a docstring totest_local_box_tunnel_endpoint_and_repeated_connects.sdks/python/tests/test_dev_tunnel_e2e.py#L152-L152: Add a docstring totest_dev_cloud_tunnel_endpoint_and_repeated_connects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/tests/test_tunnel.py` at line 38, Add brief purpose-focused docstrings to test_endpoint_returns_stable_unix_socket_path, test_connect_opens_fresh_sockets, and test_tunnel_requires_a_started_box in sdks/python/tests/test_tunnel.py at lines 38-38, 49-49, and 73-73, respectively. Also document test_local_box_tunnel_endpoint_and_repeated_connects and test_dev_cloud_tunnel_endpoint_and_repeated_connects in sdks/python/tests/test_dev_tunnel_e2e.py at lines 136-136 and 152-152, describing the behavior each test verifies.Source: Coding guidelines
sdks/python/boxlite/simplebox.py (1)
216-222: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant attribute check.
Since
self._networkis unconditionally initialized in__init__(line 138), thehasattrcheck here is redundant and can be simplified.♻️ Proposed refactor
`@property` def network(self) -> _SimpleBoxNetwork: """Get the box-scoped network handle.""" - if not hasattr(self, "_network"): - self._network = _SimpleBoxNetwork(self) return self._network🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/boxlite/simplebox.py` around lines 216 - 222, Update the network property to return the already-initialized self._network directly, removing the redundant hasattr check and lazy _SimpleBoxNetwork construction while preserving the existing _SimpleBoxNetwork handle.sdks/python/boxlite/sync_api/_network.py (1)
18-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd type hints for
connectandendpointreturn values.To match the
BoxTunnelinterface insimplebox.py, consider adding explicit return type hints (socket.socketandstr) to these methods.Add the
socketimport at the top of the file:import socket♻️ Proposed refactor
- def connect(self): + def connect(self) -> socket.socket: """Open a blocking socket to the target service.""" tunnel = self._box._sync(self._tunnel.connect()) tunnel.setblocking(True) return tunnel - def endpoint(self): + def endpoint(self) -> str: """Return the cloud URL or local Unix socket path.""" return self._box._sync(self._tunnel.endpoint())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/boxlite/sync_api/_network.py` around lines 18 - 26, Update the synchronous tunnel methods connect and endpoint with explicit return annotations of socket.socket and str, respectively, and import socket in the module so the annotations resolve.apps/api/src/boxlite-rest/boxlite-proxy.controller.ts (1)
195-208: 🚀 Performance & Scalability | 🔵 TrivialRe: reviewer question on per-request runner connections/re-auth.
Confirming from this code: yes — every octet-stream tunnel request flowing through
proxyNetworkTunnel→proxyToRunnerperforms a freshboxService.findOneByIdOrNameDB lookup, a freshrunnerService.findOne, and instantiates a brand-newcreateProxyMiddleware(...)(with its own outgoing connection to the runner) per HTTP request. This mirrors the pre-existing pattern used byproxyExec/proxyFiles/proxyMetrics, so it isn't a regression from this PR, but it does mean concurrency/performance for guest-port tunneling is bounded by this per-request overhead (DB round-trip + new outbound connection) rather than a persistently reused runner connection. Worth keeping in mind if/when the expected request rate per tunnel grows, since (unlike the WS proxy inboxlite-ws-proxy.service.ts, which builds onecreateProxyMiddlewareinstance in the constructor) this path builds a new middleware instance on every call.Also applies to: 242-290
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts` around lines 195 - 208, Review the per-request connection behavior in proxyNetworkTunnel and proxyToRunner, including the related logic around the alternate tunnel path, and avoid creating a fresh createProxyMiddleware instance and runner lookup for every streaming request. Reuse a persistent proxy middleware/runner connection pattern comparable to boxlite-ws-proxy.service.ts while preserving authentication, request routing, and octet-stream validation.apps/runner/pkg/api/controllers/proxy.go (1)
263-264: 🚀 Performance & Scalability | 🔵 TrivialPer-request transport prevents connection reuse.
NewGuestPortTransportbuilds a fresh transport that dials a new guest tunnel per request, anddefer transport.CloseIdleConnections()discards any keep-alive connection once the handler returns, so nothing is reused across requests. This is fine for the expected low-concurrency preview traffic called out in the PR discussion, but if request volume grows, consider caching a transport per(boxId, port)to amortize tunnel setup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/runner/pkg/api/controllers/proxy.go` around lines 263 - 264, Update the proxy handler around NewGuestPortTransport to reuse transports across requests by caching them per (boxId, port) pair instead of creating and closing one for every request. Ensure cached transports remain available for connection reuse and are safely managed when boxes or ports are no longer needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts`:
- Around line 182-212: Update proxyNetworkTunnel to await this.startHint(boxId,
authContext) before either the streaming proxyToRunner path or the
getPortPreviewUrl path, matching the startup handling in proxyExec, proxyFiles,
and proxyMetrics. Ensure the notification occurs after validating the port and
before any guest-facing request can autostart the box.
In `@sdks/python/boxlite/sync_api/_network.py`:
- Around line 35-38: Update SyncBoxTunnel's tunnel method to validate port
before calling _owner._create_tunnel, ensuring it is a valid integer port within
the supported range and raising the established input-validation error for
invalid values. Preserve the existing tunnel creation and SyncBoxTunnel return
flow for valid ports, matching SimpleBox._create_tunnel behavior.
In `@sdks/python/boxlite/sync_api/_simplebox.py`:
- Around line 258-264: Update the docstring of SyncSimpleBox.tunnel to describe
returning a lazy SyncBoxTunnel handle, not immediately opening a socket; mention
that callers must explicitly invoke .connect() to obtain the socket, while
leaving the method behavior unchanged.
In `@src/boxlite/src/litebox/box_impl.rs`:
- Around line 75-90: Update the listener accept loop around
listener.accept().await so transient accept errors are logged and the loop
continues instead of breaking and terminating the accept task. Preserve the
existing client-handling spawn and tunnel error logging, while reserving loop
termination only for explicitly unrecoverable listener states if such handling
already exists.
---
Outside diff comments:
In `@src/boxlite/src/net/mod.rs`:
- Around line 651-673: Remove the stale fd-recovery and SDK fd-bridge claims
from the test comments surrounding BoxInternalTunnel::from_local. Keep comments
describing the Unix socketpair, bidirectional AsyncRead/AsyncWrite behavior, and
peer handling, but delete the trailing comment about recovering an owned OS fd.
---
Nitpick comments:
In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts`:
- Around line 195-208: Review the per-request connection behavior in
proxyNetworkTunnel and proxyToRunner, including the related logic around the
alternate tunnel path, and avoid creating a fresh createProxyMiddleware instance
and runner lookup for every streaming request. Reuse a persistent proxy
middleware/runner connection pattern comparable to boxlite-ws-proxy.service.ts
while preserving authentication, request routing, and octet-stream validation.
In `@apps/runner/pkg/api/controllers/proxy.go`:
- Around line 263-264: Update the proxy handler around NewGuestPortTransport to
reuse transports across requests by caching them per (boxId, port) pair instead
of creating and closing one for every request. Ensure cached transports remain
available for connection reuse and are safely managed when boxes or ports are no
longer needed.
In `@sdks/python/boxlite/simplebox.py`:
- Around line 216-222: Update the network property to return the
already-initialized self._network directly, removing the redundant hasattr check
and lazy _SimpleBoxNetwork construction while preserving the existing
_SimpleBoxNetwork handle.
In `@sdks/python/boxlite/sync_api/_network.py`:
- Around line 18-26: Update the synchronous tunnel methods connect and endpoint
with explicit return annotations of socket.socket and str, respectively, and
import socket in the module so the annotations resolve.
In `@sdks/python/tests/test_tunnel.py`:
- Line 38: Add brief purpose-focused docstrings to
test_endpoint_returns_stable_unix_socket_path, test_connect_opens_fresh_sockets,
and test_tunnel_requires_a_started_box in sdks/python/tests/test_tunnel.py at
lines 38-38, 49-49, and 73-73, respectively. Also document
test_local_box_tunnel_endpoint_and_repeated_connects and
test_dev_cloud_tunnel_endpoint_and_repeated_connects in
sdks/python/tests/test_dev_tunnel_e2e.py at lines 136-136 and 152-152,
describing the behavior each test verifies.
In `@src/cli/src/commands/tunnel.rs`:
- Around line 22-50: Add unit tests for the tunnel command’s execute flow,
covering port == 0 rejection and the BoxEndpoint::UnixSocket error path. Reuse
existing CLI test patterns and fixtures/mocks around execute, asserting the
validation error and remote REST profile error respectively without changing
production behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 56686a1b-9c8c-4da7-9b22-b8e5d3ba672d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (68)
apps/api-client-go/api/openapi.yamlapps/api/src/box/dto/create-box.dto.tsapps/api/src/box/dto/port-preview-url.dto.tsapps/api/src/box/services/box.service.spec.tsapps/api/src/box/services/box.service.tsapps/api/src/boxlite-rest/boxlite-proxy.controller.spec.tsapps/api/src/boxlite-rest/boxlite-proxy.controller.tsapps/api/src/boxlite-rest/boxlite-rest-routing.spec.tsapps/api/src/boxlite-rest/boxlite-ws-proxy.service.tsapps/api/src/main.tsapps/common-go/pkg/proxy/proxy.goapps/common-go/pkg/proxy/proxy_test.goapps/infra/sst.config.tsapps/proxy/pkg/proxy/get_box_target.goapps/proxy/pkg/proxy/get_box_target_test.goapps/runner/pkg/api/controllers/proxy.goapps/runner/pkg/api/controllers/proxy_integration_test.goapps/runner/pkg/api/controllers/proxy_test.goapps/runner/pkg/api/server.goapps/runner/pkg/boxlite/guest_port_tunnel.goopenapi/box.openapi.yamlscripts/build/build-runtime.shscripts/test/e2e/cases/test_node_tunnel.pyscripts/test/e2e/cases/test_sdk_tunnel.pyscripts/test/e2e/sdks/node/e2e_tunnel.tssdks/c/Cargo.tomlsdks/c/include/boxlite.hsdks/c/src/lib.rssdks/c/src/network.rssdks/c/src/tests.rssdks/go/constants.gosdks/go/tunnel.gosdks/node/Cargo.tomlsdks/node/lib/index.tssdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/node/src/box_handle.rssdks/node/src/lib.rssdks/node/src/network.rssdks/node/tests/tunnel.test.tssdks/python/Cargo.tomlsdks/python/boxlite/__init__.pysdks/python/boxlite/simplebox.pysdks/python/boxlite/sync_api/__init__.pysdks/python/boxlite/sync_api/_box.pysdks/python/boxlite/sync_api/_network.pysdks/python/boxlite/sync_api/_simplebox.pysdks/python/pytest.inisdks/python/src/box_handle.rssdks/python/src/lib.rssdks/python/src/network.rssdks/python/tests/test_dev_tunnel_e2e.pysdks/python/tests/test_tunnel.pysrc/boxlite/Cargo.tomlsrc/boxlite/src/litebox/box_impl.rssrc/boxlite/src/litebox/mod.rssrc/boxlite/src/litebox/network.rssrc/boxlite/src/net/gvproxy/services.rssrc/boxlite/src/net/mod.rssrc/boxlite/src/rest/client.rssrc/boxlite/src/rest/litebox.rssrc/boxlite/src/rest/runtime.rssrc/boxlite/src/runtime/backend.rssrc/boxlite/tests/gvproxy_backend.rssrc/cli/src/cli.rssrc/cli/src/commands/mod.rssrc/cli/src/commands/tunnel.rssrc/cli/src/main.rs
💤 Files with no reviewable changes (2)
- src/boxlite/src/net/gvproxy/services.rs
- src/boxlite/tests/gvproxy_backend.rs
G4614
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/api/src/boxlite-rest/boxlite-proxy.controller.ts (1)
186-205:⚠️ Potential issue | 🟠 MajorMissing
startHintautostart notification.
proxyNetworkTunnelis missing thestartHintautostart notification. Every other guest-facing proxy method (proxyExec,proxyFiles,proxyMetrics) callsawait this.startHint(boxId, authContext)before proxying. WithoutstartHint, tunneling into a guest port on a stopped box will trigger an auto-start/re-stop race because the control plane'ssync-stateswill promptly stop the box when it seesdesiredState=STOPPED.🐛 Proposed fix
if (port < 1 || port > 65535) { return res.status(400).json({ error: 'port must be between 1 and 65535' }) } + await this.startHint(boxId, authContext) + const { url: uri } = await this.boxService.getPortPreviewUrl(boxId, authContext.organizationId, port)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts` around lines 186 - 205, Update proxyNetworkTunnel to await this.startHint(boxId, authContext) before requesting the preview URL or issuing the tunnel ticket, matching the existing proxyExec, proxyFiles, and proxyMetrics flows so stopped boxes receive the autostart notification first.
🧹 Nitpick comments (2)
openapi/box.openapi.yaml (1)
1298-1313: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
snake_casefor API response properties.The
BoxServiceEndpointschema introducesconnectUriincamelCase, which is inconsistent with thesnake_caseconvention used across the rest of the API (e.g.,box_id,created_at,disk_size_gb).
openapi/box.openapi.yaml#L1298-L1313: RenameconnectUritoconnect_uri.apps/api/src/boxlite-rest/boxlite-proxy.controller.ts#L186-L205: Returnconnect_uriinstead ofconnectUriin the JSON response payload.apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.ts#L78-L99: Update the test assertion to expectconnect_uri.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/box.openapi.yaml` around lines 1298 - 1313, Rename the BoxServiceEndpoint schema property from connectUri to connect_uri in openapi/box.openapi.yaml (1298-1313), update the boxlite-proxy controller response to return connect_uri in apps/api/src/boxlite-rest/boxlite-proxy.controller.ts (186-205), and change the corresponding assertion in apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.ts (78-99) to expect connect_uri.apps/runner/pkg/api/controllers/proxy_test.go (1)
19-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't verify
proxyBidirectionalStreamwaits for both directions to finish.The test runs
proxyBidirectionalStreamviagoand never synchronizes on its return, so it can't detect the single-<-done-receive bug (seeproxy.golines 159-171): the function could return well beforeguest's pong is actually delivered and the test would still pass. Consider running it synchronously (or with a completion channel) and asserting it returns only after both writes/reads complete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/runner/pkg/api/controllers/proxy_test.go` around lines 19 - 46, The test currently does not synchronize with proxyBidirectionalStream, so it cannot verify completion of both relay directions. Update TestProxyBidirectionalStreamRelaysBothDirections to use a completion channel or equivalent synchronization, wait for proxyBidirectionalStream to return after both ping and pong exchanges, and assert completion occurs only after both writes and reads finish.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/proxy/pkg/proxy/proxy.go`:
- Around line 200-204: Update the http.Server initialization in the proxy setup
to add a suitable ReadTimeout alongside ReadHeaderTimeout, so routed non-CONNECT
requests have an overall read deadline. Keep the existing Addr, Handler, and
header-timeout configuration unchanged.
In `@apps/proxy/pkg/proxy/tunnel.go`:
- Around line 94-98: Update the TLS configuration in the HTTPS branch of the
tunnel connection logic to explicitly set MinVersion to tls.VersionTLS12 (or the
project’s approved TLS 1.3 minimum) when constructing tls.Config. Keep the
existing ServerName behavior and non-HTTPS dialing path unchanged.
- Around line 130-137: Add a CloseWrite method to bufferedConn that detects
whether the embedded net.Conn supports CloseWrite() error and forwards the call,
returning the underlying result; otherwise return an appropriate
unsupported-operation error. This ensures proxyTunnelStreams can detect and
half-close buffered connections.
- Around line 78-101: Update dialRunnerTunnel to enforce an explicit bounded
timeout for the runner-tunnel connection and TLS handshake, rather than relying
solely on the incoming request context. Configure the net.Dialer used by both
plain and TLS dialing with the established tunnel setup timeout, preserving
context cancellation while ensuring unreachable or slow runners cannot block
indefinitely.
In `@apps/runner/pkg/api/controllers/proxy.go`:
- Around line 159-171: Both bidirectional tunnel helpers return after only one
copy direction completes. In apps/runner/pkg/api/controllers/proxy.go lines
159-171, update proxyBidirectionalStream to wait for both done signals before
returning; apply the identical change in apps/proxy/pkg/proxy/tunnel.go lines
139-151 for proxyTunnelStreams. Preserve the existing two-goroutine streaming
behavior and deferred connection cleanup.
---
Duplicate comments:
In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts`:
- Around line 186-205: Update proxyNetworkTunnel to await this.startHint(boxId,
authContext) before requesting the preview URL or issuing the tunnel ticket,
matching the existing proxyExec, proxyFiles, and proxyMetrics flows so stopped
boxes receive the autostart notification first.
---
Nitpick comments:
In `@apps/runner/pkg/api/controllers/proxy_test.go`:
- Around line 19-46: The test currently does not synchronize with
proxyBidirectionalStream, so it cannot verify completion of both relay
directions. Update TestProxyBidirectionalStreamRelaysBothDirections to use a
completion channel or equivalent synchronization, wait for
proxyBidirectionalStream to return after both ping and pong exchanges, and
assert completion occurs only after both writes and reads finish.
In `@openapi/box.openapi.yaml`:
- Around line 1298-1313: Rename the BoxServiceEndpoint schema property from
connectUri to connect_uri in openapi/box.openapi.yaml (1298-1313), update the
boxlite-proxy controller response to return connect_uri in
apps/api/src/boxlite-rest/boxlite-proxy.controller.ts (186-205), and change the
corresponding assertion in
apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.ts (78-99) to expect
connect_uri.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d7ebc8b-d917-4dde-a7e8-9664c51c7fa8
📒 Files selected for processing (25)
apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.tsapps/api/src/boxlite-rest/boxlite-proxy.controller.tsapps/api/src/boxlite-rest/boxlite-rest.module.tsapps/api/src/boxlite-rest/boxlite-tunnel-ticket.service.spec.tsapps/api/src/boxlite-rest/boxlite-tunnel-ticket.service.tsapps/api/src/config/configuration.tsapps/common-go/pkg/cache/redis_cache.goapps/infra/sst.config.tsapps/proxy/pkg/proxy/proxy.goapps/proxy/pkg/proxy/tunnel.goapps/proxy/pkg/proxy/tunnel_test.goapps/runner/pkg/api/controllers/proxy.goapps/runner/pkg/api/controllers/proxy_test.goapps/runner/pkg/api/server.goopenapi/box.openapi.yamlsdks/c/include/boxlite.hsdks/c/src/network.rssdks/go/tunnel.gosdks/node/src/network.rssdks/python/src/network.rssrc/boxlite/Cargo.tomlsrc/boxlite/src/litebox/network.rssrc/boxlite/src/rest/client.rssrc/boxlite/src/rest/litebox.rssrc/cli/src/commands/tunnel.rs
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/runner/pkg/api/server.go
- src/cli/src/commands/tunnel.rs
- src/boxlite/Cargo.toml
- src/boxlite/src/rest/litebox.rs
- sdks/node/src/network.rs
- sdks/c/include/boxlite.h
- sdks/c/src/network.rs
- sdks/go/tunnel.go
- sdks/python/src/network.rs
- src/boxlite/src/litebox/network.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/proxy/pkg/proxy/proxy.go (1)
103-113: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInitialize
tunnelTicketswhen Redis is disabled.The new cache is assigned only in the
config.Redis != nilbranch, whileconnectAwareHandlerroutes CONNECT requests in both modes. In a no-Redis configuration, tunnel handling can therefore dereference a nil cache or reject every ticket.} else { proxy.boxRunnerCache = common_cache.NewMapCache[RunnerInfo](ctx) proxy.runnerCache = common_cache.NewMapCache[RunnerInfo](ctx) proxy.boxPublicCache = common_cache.NewMapCache[bool](ctx) proxy.boxAuthKeyValidCache = common_cache.NewMapCache[bool](ctx) proxy.boxLastActivityUpdateCache = common_cache.NewMapCache[bool](ctx) + proxy.tunnelTickets = common_cache.NewMapCache[TunnelTicket](ctx) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/proxy/pkg/proxy/proxy.go` around lines 103 - 113, Initialize proxy.tunnelTickets in the config.Redis == nil branch using the in-memory cache implementation, alongside the other MapCache fields. Ensure connectAwareHandler can access a non-nil tunnelTickets cache in both Redis-enabled and no-Redis configurations.
♻️ Duplicate comments (2)
apps/proxy/pkg/proxy/proxy.go (1)
200-203: 🩺 Stability & Availability | 🟠 MajorAdd read deadlines to the HTTP server.
This remains unresolved from the previous review: the server has neither
ReadHeaderTimeoutnorReadTimeout. Non-CONNECT requests delegated toroutercan hold connections open with slow request bodies, exhausting proxy resources. Choose values compatible with supported request sizes and CONNECT handshakes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/proxy/pkg/proxy/proxy.go` around lines 200 - 203, Update the http.Server initialization in the proxy handler setup to configure both ReadHeaderTimeout and ReadTimeout with finite values. Choose timeouts that allow supported request sizes and CONNECT handshakes while preventing slow non-CONNECT requests routed through router from holding connections indefinitely.Source: Linters/SAST tools
apps/api/src/boxlite-rest/boxlite-proxy.controller.ts (1)
186-204: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
proxyNetworkTunnelis missing thestartHintautostart notification.Every other guest-facing proxy method (
proxyExec,proxyFiles,proxyMetrics) callsawait this.startHint(boxId, authContext)before proxying, because a proxied call into a stopped box's runtime auto-starts the VM (Box::live_state()) without the control-plane DB knowing — withoutstartHint,sync-stateswill seedesiredState=STOPPEDand re-stop the box shortly after tunnel traffic arrives.proxyNetworkTunnelskips this call, so tunneling into a guest port on a stopped box is likely to trigger the exact auto-start/re-stop race theensureStartedForProxydocstring describes.🐛 Proposed fix
if (port < 1 || port > 65535) { return res.status(400).json({ error: 'port must be between 1 and 65535' }) } + await this.startHint(boxId, authContext) + const { url: uri } = await this.boxService.getPortPreviewUrl(boxId, authContext.organizationId, port)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts` around lines 186 - 204, Update proxyNetworkTunnel to await startHint(boxId, authContext) before requesting the port preview URL or issuing the tunnel ticket, matching the startup notification flow used by proxyExec, proxyFiles, and proxyMetrics.
🧹 Nitpick comments (2)
sdks/python/boxlite/sync_api/_simplebox.py (1)
258-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public networking API contract completely.
The lazy-tunnel wording is now correct, but both public APIs should document their arguments/return values and the
RuntimeErrorraised before startup, as required by the SDK guidelines.📝 Suggested documentation
def tunnel(self, port: int): - """Return a lazy tunnel handle; call ``connect()`` to open its socket.""" + """Return a lazy tunnel handle for a guest port. + + Args: + port: Guest port to expose. + + Returns: + A tunnel handle; call ``connect()`` to open its socket. + + Raises: + RuntimeError: If the box has not been started. + """ `@property` def network(self): - """Get the box-scoped network handle.""" + """Return the box-scoped network handle. + + Raises: + RuntimeError: If the box has not been started. + """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/boxlite/sync_api/_simplebox.py` around lines 258 - 273, Complete the docstrings for the public SyncSimpleBox.tunnel and SyncSimpleBox.network APIs, documenting the tunnel port argument, returned handle/value, and the RuntimeError raised when the box has not started. Preserve the existing lazy tunnel behavior and startup validation.Source: Coding guidelines
src/boxlite/src/rest/client.rs (1)
346-346: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTower
Service::callrequirespoll_ready.Calling
.call()directly on atower::Servicewithout first ensuring.poll_ready()returnsReadyviolates the Tower service contract. Althoughhyper_rustls::HttpsConnectorhappens to always be ready without blocking, it's safer and more idiomatic to usetower::ServiceExt::oneshot.🛠️ Proposed fix
Import
ServiceExtat the use-site and apply.oneshot():- let io = tokio::time::timeout(TUNNEL_SETUP_TIMEOUT, connector.call(uri.clone())) + use tower::ServiceExt; + let io = tokio::time::timeout(TUNNEL_SETUP_TIMEOUT, connector.oneshot(uri.clone()))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/boxlite/src/rest/client.rs` at line 346, Update the tunnel setup flow around the connector service to import and use Tower’s ServiceExt::oneshot instead of calling connector.call directly, ensuring readiness is polled before dispatch while preserving the existing timeout and URI handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/proxy/pkg/proxy/proxy.go`:
- Line 202: Update the CONNECT handling around proxy.handleTunnelConnect and
connectAwareHandler so established hijacked tunnels are registered with
shutdownWg before being served and released when they close, ensuring graceful
shutdown waits for active CONNECT streams. Preserve the existing handler routing
while covering all CONNECT tunnel lifetimes.
---
Outside diff comments:
In `@apps/proxy/pkg/proxy/proxy.go`:
- Around line 103-113: Initialize proxy.tunnelTickets in the config.Redis == nil
branch using the in-memory cache implementation, alongside the other MapCache
fields. Ensure connectAwareHandler can access a non-nil tunnelTickets cache in
both Redis-enabled and no-Redis configurations.
---
Duplicate comments:
In `@apps/api/src/boxlite-rest/boxlite-proxy.controller.ts`:
- Around line 186-204: Update proxyNetworkTunnel to await startHint(boxId,
authContext) before requesting the port preview URL or issuing the tunnel
ticket, matching the startup notification flow used by proxyExec, proxyFiles,
and proxyMetrics.
In `@apps/proxy/pkg/proxy/proxy.go`:
- Around line 200-203: Update the http.Server initialization in the proxy
handler setup to configure both ReadHeaderTimeout and ReadTimeout with finite
values. Choose timeouts that allow supported request sizes and CONNECT
handshakes while preventing slow non-CONNECT requests routed through router from
holding connections indefinitely.
---
Nitpick comments:
In `@sdks/python/boxlite/sync_api/_simplebox.py`:
- Around line 258-273: Complete the docstrings for the public
SyncSimpleBox.tunnel and SyncSimpleBox.network APIs, documenting the tunnel port
argument, returned handle/value, and the RuntimeError raised when the box has
not started. Preserve the existing lazy tunnel behavior and startup validation.
In `@src/boxlite/src/rest/client.rs`:
- Line 346: Update the tunnel setup flow around the connector service to import
and use Tower’s ServiceExt::oneshot instead of calling connector.call directly,
ensuring readiness is polled before dispatch while preserving the existing
timeout and URI handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e5dd83d-bc5d-41b0-a7e8-ece7c7072532
📒 Files selected for processing (14)
apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.tsapps/api/src/boxlite-rest/boxlite-proxy.controller.tsapps/proxy/pkg/proxy/proxy.goapps/proxy/pkg/proxy/tunnel.goapps/proxy/pkg/proxy/tunnel_test.goapps/runner/pkg/api/controllers/proxy.goapps/runner/pkg/api/controllers/proxy_test.goopenapi/box.openapi.yamlsdks/python/boxlite/sync_api/_network.pysdks/python/boxlite/sync_api/_simplebox.pysdks/python/tests/test_tunnel.pysrc/boxlite/Cargo.tomlsrc/boxlite/src/litebox/box_impl.rssrc/boxlite/src/rest/client.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/proxy/pkg/proxy/tunnel_test.go
- apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.ts
- sdks/python/boxlite/sync_api/_network.py
- openapi/box.openapi.yaml
- src/boxlite/src/litebox/box_impl.rs
- apps/proxy/pkg/proxy/tunnel.go
- src/boxlite/Cargo.toml
- apps/runner/pkg/api/controllers/proxy_test.go
- apps/runner/pkg/api/controllers/proxy.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/proxy/pkg/proxy/tunnel.go (1)
117-123: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPath is stripped from
CONNECTrequests, causing tunnel failure.Go's
http.Request.Writemethod specifically handles theCONNECTmethod by usingreq.URL.Hostas the Request-URI and completely ignoring the path. Becausereqis created with"http://"+host+path,req.URL.Hostis populated, andreq.Write(conn)will outputCONNECT {host} HTTP/1.1. The runner will never receive the/v1/boxes/...path and will likely return a 404 Not Found.To preserve the path in the Request-URI while maintaining the
Hostheader, initialize the request with just thepath.🐛 Proposed fix
- path := fmt.Sprintf("/v1/boxes/%s/network/tunnel?port=%d", url.PathEscape(boxID), port) - req, err := http.NewRequestWithContext(ctx, http.MethodConnect, "http://"+host+path, nil) + path := fmt.Sprintf("/v1/boxes/%s/network/tunnel?port=%d", url.PathEscape(boxID), port) + req, err := http.NewRequestWithContext(ctx, http.MethodConnect, path, nil) if err != nil { conn.Close() return nil, err } req.Host = host🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/proxy/pkg/proxy/tunnel.go` around lines 117 - 123, Update the CONNECT request construction near req.Host so http.NewRequestWithContext receives only path rather than "http://"+host+path, then retain host in req.Host. Preserve the existing context, method, error cleanup, and escaped tunnel path so req.Write sends the full path as the Request-URI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/proxy/pkg/proxy/tunnel.go`:
- Around line 28-37: Fix handleTunnelConnect and tunnelTarget so a plain box ID
from the Host header cannot authorize a tunnel: require and validate a securely
signed preview token or restored Proxy-Authorization/ticket credential, reject
decoding or validation failures, and verify the requested port is marked public
before calling getBoxRunnerInfo or dialing the runner.
- Line 63: Update the tunnel setup around proxyTunnelStreams to wrap clientConn
with bufferedConn, matching the existing runner connection handling, and pass
the wrapped client connection into proxyTunnelStreams so bytes already held by
hijacker.Hijack’s buffered.Reader are forwarded.
---
Outside diff comments:
In `@apps/proxy/pkg/proxy/tunnel.go`:
- Around line 117-123: Update the CONNECT request construction near req.Host so
http.NewRequestWithContext receives only path rather than "http://"+host+path,
then retain host in req.Host. Preserve the existing context, method, error
cleanup, and escaped tunnel path so req.Write sends the full path as the
Request-URI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18693219-6bf9-4a71-8f2e-fb66e2643ec1
📒 Files selected for processing (9)
apps/api/src/boxlite-rest/boxlite-proxy.controller.spec.tsapps/api/src/boxlite-rest/boxlite-proxy.controller.tsapps/infra/sst.config.tsapps/proxy/pkg/proxy/proxy.goapps/proxy/pkg/proxy/tunnel.goapps/proxy/pkg/proxy/tunnel_test.goopenapi/box.openapi.yamlsrc/boxlite/src/rest/client.rssrc/boxlite/src/rest/litebox.rs
💤 Files with no reviewable changes (2)
- apps/proxy/pkg/proxy/proxy.go
- apps/infra/sst.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/boxlite/src/rest/litebox.rs
3 tasks
Contributor
Author
auto-merge was automatically disabled
July 21, 2026 22:53
Pull request was closed
2 tasks
G4614
marked this pull request as draft
July 22, 2026 04:13
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
July 22, 2026 08:59
894039b to
8f50bcd
Compare
5 tasks
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
July 23, 2026 05:09
8f50bcd to
75751a9
Compare
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
July 23, 2026 05:16
75751a9 to
b1ba38a
Compare
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
July 23, 2026 05:54
1182eb8 to
ae6a01e
Compare
This was referenced Jul 29, 2026
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
2 times, most recently
from
August 6, 2026 05:11
ff32d14 to
071136f
Compare
DorianZheng
reviewed
Aug 7, 2026
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
August 11, 2026 10:31
24cb7c2 to
415e3ec
Compare
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
August 12, 2026 05:46
dc10e6b to
f8b14f8
Compare
G4614
changed the title
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
4 times, most recently
from
August 13, 2026 07:32
9fe5615 to
e673df9
Compare
DorianZheng
reviewed
Aug 13, 2026
| rootfs: RootfsSpec::default(), | ||
| volumes: Vec::new(), | ||
| network: NetworkSpec::default(), | ||
| network: NetworkPolicy::default(), |
Contributor
Author
There was a problem hiding this comment.
because if we want to accept the old pub enum NetworkSpec: {Enabled{allow_net}, Disabled } for Rust SDK, we can't just change it to pub struct NetworkSpec { outbound, inbound } and let it accept the old non-struct format at the same time
Contributor
Author
There was a problem hiding this comment.
like this?
pub enum NetworkSpec {
Enabled { allow_net: Vec<String> },
Disabled,
Policy { outbound: OutboundNetworkSpec, inbound: InboundNetworkSpec },
}
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
August 13, 2026 08:07
e673df9 to
0c517b1
Compare
DorianZheng
pushed a commit
that referenced
this pull request
Aug 13, 2026
## Summary
`apps/api`'s `CreateBoxDto.network`/`box-to-box.mapper.ts` still
expected the old flat `service_access` field. Update
`InboundNetworkSpecDto` to the same `{mode, allow_net?}` shape as
`OutboundNetworkSpecDto`, matching the Rust core/CLI/SDK shape landing
in #996.
## Before/after
```
CreateBoxDto.network.inbound: { service_access?: 'public'|'private' }
← BUG: doesn't match the outbound shape, no allow_net slot
```
```
CreateBoxDto.network.inbound: { mode: 'enabled'|'disabled', allow_net?: string[] }
```
`box-to-box.mapper.ts` now derives `createDto.public` from
`inbound.mode` instead of `inbound.service_access`.
`openapi/box.openapi.yaml` and the e2e case (`test_box_management.py`)
updated to match.
## Scope note
Not part of the same Cargo workspace as the Rust core/CLI/SDKs (#996) —
no compile-time coupling, so this ships as its own PR. It does depend on
#996 landing first for the REST path to work end-to-end (the running
`boxlite` binary needs to actually send this shape); this PR only
changes what the REST server accepts and maps.
## Test plan
- `npx nx test api -- --testPathPatterns="create-box.dto.spec.ts"` —
29/29 passed
- `npx nx test api -- --testPathPatterns="box-to-box.mapper.spec.ts"` —
5/5 passed
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added separate inbound and outbound network policies for boxes.
* Added configurable outbound allow-lists and inbound access modes.
* Inbound settings now control public preview availability.
* **Bug Fixes**
* Improved network configuration validation and mapping.
* Legacy flat network settings remain supported through normalization.
* Invalid or mixed network formats are rejected.
* Unsupported inbound allow-list values are rejected.
* **Documentation**
* Updated the API specification with the new network policy structure.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
3 times, most recently
from
August 19, 2026 10:21
ee721a2 to
dd33e18
Compare
DorianZheng
previously approved these changes
Aug 19, 2026
NetworkSpec was a single enum modeling guest egress only; whether a
box's exposed services are publicly reachable had no field anywhere.
Reshape it into a struct with two directions:
Before:
BoxOptions.network: NetworkSpec::Enabled{allow_net}|Disabled
<- egress only; no inbound reachability concept
After:
BoxOptions.network: NetworkSpec{
outbound: OutboundNetworkSpec::Enabled{allow_net}|Disabled,
inbound: InboundNetworkSpec::Enabled{allow_net}|Disabled,
}
Inbound: Enabled = publicly reachable (default), Disabled = private.
Its allow_net exists for shape symmetry but is rejected when non-empty
(try_from and sanitize) until a runtime sink enforces it. The legacy
flat wire shape still deserializes (untagged fallback) with a
deprecation warning.
CLI/serve/REST client and the C/Node/Python bindings are adapted to
compile against the new shape without exposing inbound configuration;
those surfaces follow in a separate PR. Go is untouched (C ABI
unchanged).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
NetworkSpec::enabled/disabled read as whole-spec constructors but only set the outbound direction, leaving inbound at its default — invisible at the call site now that inbound exists. Rename to outbound_enabled/outbound_disabled and document what each leaves untouched. Same problem in the messages: errors saying "network.mode" point nested callers at a field that no longer exists. Say network.outbound.mode where the check is outbound-specific, and make NetworkMode::from_str's message direction-neutral — it parses both directions now, so "invalid network.mode" was wrong for inbound input. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`NetworkSpec` modeled guest egress only; whether a box's exposed services
are reachable from outside had no field anywhere.
Add it as a sibling field on `BoxOptions` rather than wrapping the two
directions in a new type. `BoxOptions` is already a flat bag of ~20
independent fields, and the two directions are independent — a box may
refuse egress while its exposed services stay reachable. Keeping them
separate leaves `NetworkSpec` untouched: same name, same variants, same
wire form, same field, so pre-split literals, match arms and
`BoxOptions { network: .. }` all compile unchanged, with no conversion at
the assignment site.
`inbound_network` reuses `NetworkSpec` — both directions have identical
shape. `Enabled` means reachable (the default), `Disabled` means private.
Its `allow_net` must be empty: no layer enforces an inbound allowlist yet,
so a non-empty one is rejected at the wire boundary and again in
`sanitize()` for FFI callers that set the spec directly.
Box configs persisted before this field load with it defaulted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
G4614
force-pushed
the
codex/runner-guest-port-connect
branch
from
August 19, 2026 11:06
dd33e18 to
88679bc
Compare
DorianZheng
approved these changes
Aug 19, 2026
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
3 participants
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.
Summary
NetworkSpecmodeled guest egress only; whether a box's exposed services are reachable from outside had no field anywhere. This PR splits the two directions in the core type and adapts every dependent surface to compile — without exposing inbound configuration yet (that's #1206, stacked on this).Before/after
Rust source compatibility
NetworkSpecis untouched — same name, same variants, same wire form — so pre-split source and already-persisted box configs keep working. Verified bynetwork_spec.rs::pre_split_network_spec_source_shape_still_compiles, which compiles as an external crate against the public API:NetworkSpec::Enabled { allow_net: v }literalmatch spec { NetworkSpec::Enabled { .. } => … }{"Enabled":{...}}/"Disabled"BoxOptions { network: spec, .. }.into()match opts.network { … }opts.network.outboundThe last two are unavoidable: a struct field has exactly one type, and the pre-split enum cannot also be the two-direction container. rustc points at both and suggests the fix.
Stack
NetworkInforead side (stacked on this)Align REST network DTO with inbound/outbound core shape #1199 — apps/api REST DTO alignment— merged (2a567eb5); its API accepts both the flat and nested wire shapes, so Expose inbound network policy across CLI, SDKs, serve, and NetworkInfo #1206's client is unblockedTest plan
boxlitecore 999/999 (incl. the source-compat test),boxlite-cli191/191, C 72/72 (1 ignored), Node 21/21cargo check -p boxlite-python --testsclean (cargo test -p boxlite-pythonfails to link libpython on current main in this environment — pre-existing, verified against a clean origin/main worktree)make fmt:check:rustandmake fmt:check:gocleanDocs
docs/reference/rust/README.mdnow documents theNetworkPolicycontainer, both direction types, theOutboundNetworkPolicyalias, whyinbound.allow_netmust be empty, and theFrom<NetworkSpec>path pre-split callers use.🤖 Generated with Claude Code