feat(api): proxy box network tunnels#998
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR adds box network tunneling across the API, proxy, runner, Rust runtime, CLI, and C, Go, Node, and Python SDKs. It includes ticket-authenticated WebSocket transport, guest-port proxying, reusable local and remote tunnels, preview URL encoding updates, and integration tests validating HTTP relay through the tunnels. ChangesBox network tunnel
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant BoxliteNetworkController
participant BoxliteTunnelTicketService as TicketService
participant BoxliteWsProxyService as WsProxy
participant RunnerProxyController as RunnerProxy
participant GuestTCP
Client->>BoxliteNetworkController: POST /network/tunnel
BoxliteNetworkController->>BoxliteTunnelTicketService: issue(boxId, port)
TicketService-->>Client: { url, ticket }
Client->>BoxliteWsProxyService: WS upgrade /boxes/.../tunnel?ticket=...
BoxliteWsProxyService->>BoxliteTunnelTicketService: consume(ticket)
TicketService-->>BoxliteWsProxyService: { boxId, port }
BoxliteWsProxyService->>RunnerProxyController: forward authenticated tunnel
RunnerProxyController->>GuestTCP: dial 192.168.127.2:port
RunnerProxyController-->>Client: bridge WS binary frames ↔ TCP bytes
Possibly related PRs:
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
31bf38c to
07d77f4
Compare
5244570 to
e60b2bd
Compare
📦 BoxLite review — couldn't completepowered by BoxLite |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 11
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 referencing the removed
into_fdfd-recovery path.With
into_fddeleted, the doc comment claiming "the concrete local stream stays recoverable (the SDK fd-bridge relies on it)" (Line 653) and the dangling trailing comment "the owned OS fd is recoverable with no unsafe downcast (SDK handoff)" (Line 672) no longer describe any tested behavior. Drop/adjust them 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, Update the test comments around BoxInternalTunnel::from_local to remove references to the deleted into_fd path, SDK fd recovery, and owned OS fd handoff. Keep comments focused on the behavior actually tested: bidirectional byte transfer through AsyncRead/AsyncWrite and preservation of the peer address.
🧹 Nitpick comments (6)
sdks/node/lib/native-contracts.ts (2)
257-265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd TSDoc comments to the exported interfaces and methods.
As per coding guidelines, please write comprehensive JSDoc/TSDoc comments for functions, classes, and exported modules, including these new interfaces and their methods.
📝 Proposed documentation
+/** + * Handle for network operations on a box. + */ export interface JsNetworkHandle { + /** + * Prepares a reusable tunnel to a specified service port inside the box. + */ tunnel(port: number): Promise<JsBoxTunnel>; } +/** + * A reusable tunnel to a service port inside a box. + */ export interface JsBoxTunnel { + /** + * Returns the stable URL or Unix socket address prepared for the tunnel. + */ endpoint(): Promise<string>; + /** + * Opens a fresh stream and returns its owned Unix file descriptor. + */ _connectFd(): Promise<number>; }🤖 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/node/lib/native-contracts.ts` around lines 257 - 265, Add comprehensive TSDoc comments to the exported JsNetworkHandle and JsBoxTunnel interfaces and each method (tunnel, endpoint, and _connectFd), documenting their purpose, parameters, return values, and connection behavior where applicable.Source: Coding guidelines
257-265: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd TSDoc comments to the exported interfaces and methods.
As per coding guidelines, please write comprehensive JSDoc/TSDoc comments for functions, classes, and exported modules, including these new interfaces and their methods.
📝 Proposed documentation
+/** + * Handle for network operations on a box. + */ export interface JsNetworkHandle { + /** + * Prepares a reusable tunnel to a specified service port inside the box. + */ tunnel(port: number): Promise<JsBoxTunnel>; } +/** + * A reusable tunnel to a service port inside a box. + */ export interface JsBoxTunnel { + /** + * Returns the stable URL or Unix socket address prepared for the tunnel. + */ endpoint(): Promise<string>; + /** + * Opens a fresh stream and returns its owned Unix file descriptor. + */ _connectFd(): Promise<number>; }🤖 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/node/lib/native-contracts.ts` around lines 257 - 265, Add comprehensive TSDoc comments to the exported JsNetworkHandle and JsBoxTunnel interfaces and each of their methods, documenting their purpose, parameters, return values, and relevant behavior while preserving the existing API signatures.Source: Coding guidelines
sdks/python/boxlite/sync_api/_network.py (1)
18-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return type annotations.
The
connectandendpointmethods are missing return type annotations. (Note: you will also need toimport socketat the top of the file if you prefer not to use string annotations).♻️ Proposed type hints
- 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, Add return type annotations to the connect and endpoint methods in the relevant sync network class: annotate connect as returning socket.socket and endpoint with its appropriate endpoint value type, importing socket if needed. Preserve the existing blocking connection and endpoint delegation behavior.sdks/node/lib/simplebox.ts (1)
274-279: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidate the
portargument.It is recommended to validate that the
portis a valid integer between 1 and 65535 before attempting to open the tunnel.🛡️ Proposed validation
/** Return a lazy tunnel handle for a port inside this box. */ async tunnel(port: number): Promise<BoxTunnel> { + if (!Number.isInteger(port) || port < 1 || port > 65535) { + throw new Error(`Invalid port: ${port}. Port must be an integer between 1 and 65535.`); + } const box = await this.ensureBox(); const tunnel = await box.network.tunnel(port); return tunnel instanceof BoxTunnel ? tunnel : new BoxTunnel(tunnel); }🤖 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/node/lib/simplebox.ts` around lines 274 - 279, Update SimpleBox.tunnel to validate port before calling ensureBox or opening the network tunnel: require an integer in the inclusive range 1–65535, and reject invalid values using the existing argument-validation convention. Preserve the current BoxTunnel wrapping behavior for valid ports.sdks/python/src/network.rs (2)
38-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd docstrings to public Python methods.
As per coding guidelines, comprehensive docstrings should be written for all public functions. The
tunnelmethod exposed to Python lacks a docstring explaining its behavior, arguments, and return type.📝 Proposed fix
#[pymethods] impl PyNetworkHandle { + /// Create a lazy tunnel handle for a guest port. + /// + /// Args: + /// port (int): The port number to tunnel to. + /// + /// Returns: + /// BoxTunnel: A handle to the created tunnel. fn tunnel<'py>(&self, py: Python<'py>, port: u16) -> PyResult<Bound<'py, PyAny>> { if port == 0 {🤖 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/src/network.rs` around lines 38 - 57, Add a comprehensive Python-facing docstring to the public PyNetworkHandle::tunnel method describing its tunneling behavior, port argument, non-zero requirement, and returned PyBoxTunnel value (including its asynchronous nature).Source: Coding guidelines
59-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd docstrings to public Python methods.
As per coding guidelines, comprehensive docstrings should be written for all public functions. The
endpointandconnectmethods exposed to Python lack docstrings.📝 Proposed fix
#[pymethods] impl PyBoxTunnel { + /// Return the cloud URL or local Unix socket path for this tunnel. fn endpoint<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> { let handle = Arc::clone(&self.handle); pyo3_async_runtimes::tokio::future_into_py(py, async move { match handle.endpoint().await.map_err(map_err)? { BoxEndpoint::Url(url) => Ok(url), BoxEndpoint::UnixSocket(path) => Ok(path.to_string_lossy().into_owned()), } }) } + /// Open a connection to the target service, returning a non-blocking socket. fn connect<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> { let handle = Arc::clone(&self.handle);🤖 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/src/network.rs` around lines 59 - 89, Add Python docstrings to the public PyBoxTunnel methods endpoint and connect in the #[pymethods] implementation, documenting each method’s purpose, asynchronous result, and relevant endpoint or socket behavior. Keep the existing implementations unchanged.Source: Coding guidelines
🤖 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/runner/pkg/api/controllers/proxy_integration_test.go`:
- Around line 80-100: Synchronize concurrent stderr access in the test by
replacing the plain strings.Builder with a synchronized buffer type, adding the
required sync import. Update StartExecution to receive &stderrBuf and change the
waitForRealVMTunnelResponse closure to read stderrBuf.String(), preserving the
existing polling and cleanup behavior.
In `@sdks/go/tunnel.go`:
- Around line 131-137: Change the cFD declaration in the tunnel connection flow
to use C.int32_t, matching the pointer type expected by
C.boxlite_box_tunnel_connect. Keep the existing error handling and
negative-descriptor check unchanged.
- Around line 131-137: Change the cFD declaration used by
boxlite_box_tunnel_connect in the tunnel connection flow from C.int to C.int32_t
so its pointer type matches the C function signature. Preserve the existing
error handling and negative-descriptor check.
- Around line 101-118: Change the endpointType declaration in the tunnel
endpoint flow to use the generated cgo enum type expected by
C.boxlite_box_tunnel_endpoint instead of uint32. Preserve the existing switch
mapping and unknown-type error behavior.
- Around line 101-118: Change endpointType in the tunnel endpoint flow to use
the generated cgo enum type expected by C.boxlite_box_tunnel_endpoint instead of
uint32. Keep the existing switch mapping to EndpointTypeURL and
EndpointTypeUnixSocket unchanged, including unknown-type handling.
In `@sdks/node/lib/simplebox.ts`:
- Around line 282-294: Add TSDoc comments to the exported BoxTunnel class and
its public endpoint and connect methods, describing the tunnel wrapper, the
endpoint value returned, and the socket established by connect. Keep the
implementation unchanged and follow the repository’s existing documentation
style.
In `@sdks/python/boxlite/simplebox.py`:
- Around line 140-144: Update _create_tunnel to validate that port is within the
inclusive range 1–65535 before calling self._box.network.tunnel(port), rejecting
invalid user input with the established argument-validation error behavior while
preserving the existing box-existence check.
In `@sdks/python/pytest.ini`:
- Line 13: Update the pytest.skip call in the _required_env helper within
test_dev_tunnel_e2e.py to pass allow_module_level=True, preserving the existing
message and module-level skip behavior during collection.
In `@sdks/python/tests/test_dev_tunnel_e2e.py`:
- Line 10: Add ApiKeyCredential to the public exports in the boxlite package’s
__init__.py so the import in test_dev_tunnel_e2e.py resolves during test
collection, preserving the existing exports.
- Around line 31-33: Run the repository’s Python formatter on
test_dev_tunnel_e2e.py and apply its formatting to the request assignment,
removing the redundant parenthesized string expression as needed. Do not change
the request content or test behavior.
In `@src/boxlite/src/litebox/box_impl.rs`:
- Around line 62-69: Replace the temporary-directory socket setup around the
tunnel listener with the existing box-scoped socket path flow used for short
Unix socket paths. Update the code creating the `service.sock` path and binding
the `tokio::net::UnixListener` to reuse the runtime’s box socket layout, while
preserving the existing Network error mapping.
---
Outside diff comments:
In `@src/boxlite/src/net/mod.rs`:
- Around line 651-673: Update the test comments around
BoxInternalTunnel::from_local to remove references to the deleted into_fd path,
SDK fd recovery, and owned OS fd handoff. Keep comments focused on the behavior
actually tested: bidirectional byte transfer through AsyncRead/AsyncWrite and
preservation of the peer address.
---
Nitpick comments:
In `@sdks/node/lib/native-contracts.ts`:
- Around line 257-265: Add comprehensive TSDoc comments to the exported
JsNetworkHandle and JsBoxTunnel interfaces and each method (tunnel, endpoint,
and _connectFd), documenting their purpose, parameters, return values, and
connection behavior where applicable.
- Around line 257-265: Add comprehensive TSDoc comments to the exported
JsNetworkHandle and JsBoxTunnel interfaces and each of their methods,
documenting their purpose, parameters, return values, and relevant behavior
while preserving the existing API signatures.
In `@sdks/node/lib/simplebox.ts`:
- Around line 274-279: Update SimpleBox.tunnel to validate port before calling
ensureBox or opening the network tunnel: require an integer in the inclusive
range 1–65535, and reject invalid values using the existing argument-validation
convention. Preserve the current BoxTunnel wrapping behavior for valid ports.
In `@sdks/python/boxlite/sync_api/_network.py`:
- Around line 18-26: Add return type annotations to the connect and endpoint
methods in the relevant sync network class: annotate connect as returning
socket.socket and endpoint with its appropriate endpoint value type, importing
socket if needed. Preserve the existing blocking connection and endpoint
delegation behavior.
In `@sdks/python/src/network.rs`:
- Around line 38-57: Add a comprehensive Python-facing docstring to the public
PyNetworkHandle::tunnel method describing its tunneling behavior, port argument,
non-zero requirement, and returned PyBoxTunnel value (including its asynchronous
nature).
- Around line 59-89: Add Python docstrings to the public PyBoxTunnel methods
endpoint and connect in the #[pymethods] implementation, documenting each
method’s purpose, asynchronous result, and relevant endpoint or socket behavior.
Keep the existing implementations unchanged.
🪄 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: 90d9867b-30a2-4759-9695-e5033d96179d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (70)
apps/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-network.controller.spec.tsapps/api/src/boxlite-rest/boxlite-network.controller.tsapps/api/src/boxlite-rest/boxlite-rest-routing.spec.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/boxlite-rest/boxlite-ws-proxy.service.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/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/tests/gvproxy_backend.rs
- src/boxlite/src/net/gvproxy/services.rs
79b6ccd to
fcc0a6a
Compare
fcc0a6a to
b998e38
Compare
Expose box service -- sdks
Test plan:
git diff --checknode_modulesSummary by CodeRabbit
publicnow correctly defaults totruewhen omitted (including warm-pool assignments).