Repository navigation
Conversation
|
Commit: Regression test reportSelected 4 changed tests (limit=20, 0 not selected) 4 tests: regression=4 unreleased=0 coverage=0 failing=0
TestAccept/cmd/sandbox/forward/forward: branch ✅ | main (a572447) ❌ | latest (v1.26.0) ❌main (a572447): latest (v1.26.0): same output as main (a572447) TestAccept/cmd/sandbox/forward/invalid-forward: branch ✅ | main (a572447) ❌ | latest (v1.26.0) ❌main (a572447): latest (v1.26.0): same output as main (a572447) TestAccept/cmd/sandbox/forward/no-default: branch ✅ | main (a572447) ❌ | latest (v1.26.0) ❌main (a572447): latest (v1.26.0): same output as main (a572447) TestAccept/cmd/sandbox/forward/not-found: branch ✅ | main (a572447) ❌ | latest (v1.26.0) ❌main (a572447): latest (v1.26.0): same output as main (a572447) |
✅ Integration test reportCommit: 4ee9807
Top 3 slowest tests (at least 2 minutes):
|
pietern
left a comment
There was a problem hiding this comment.
(from my agent) There is no acceptance test for an actual forward: the Forwarding http://127.0.0.1:N -> id:10080 / Press Ctrl-C to stop. output and a request going through the new /sandbox-proxy/sandboxes/<id>/ports/<port>/ route are only covered by unit tests against the handler. Could one acceptance test run the command in the background with -L 0:10080, curl the printed URL against a testserver handler for that route, and print the request it received? Or is that not worth it with the current harness?
95ca5f1 to
3532674
Compare
databricks sandbox port-forwarddatabricks sandbox forward
Forward local ports to apps in a sandbox: each -L LOCAL:REMOTE serves http://127.0.0.1:LOCAL and relays HTTP and WebSocket traffic through the workspace's /sandbox-proxy/sandboxes/<id>/ports/<port>/ route with the caller's credentials. Forwards accept only localhost-addressed requests and refuse dot-segment paths, so neither a DNS-rebinding page nor a crafted path can use those credentials outside the port route. Co-authored-by: Isaac <no-reply@databricks.com>
…oxes Refuse browser requests from other sites (Origin, Sec-Fetch-Site) so a web page can't use a forward, including over WebSockets. Reject remote ports outside 10000-65535 at parse time, start a stopped sandbox before forwarding like `sandbox ssh`, also serve the IPv6 loopback, and keep net/http's error reports in the debug log. Co-authored-by: Isaac <no-reply@databricks.com>
The sandbox command group is now visible as a Beta feature, so the new subcommand belongs in the release notes. Co-authored-by: Isaac <no-reply@databricks.com>
- Share sandbox resolution with `sandbox ssh` (resolveSandbox), and reject IDs that are not a single path segment. A 400 from the lookup now fails instead of warning. - Let a loopback Origin take precedence over Sec-Fetch-Site, so a frontend on http://localhost:N can call the printed http://127.0.0.1:N URL. - Send the package's X-Databricks-Org-Id routing header, as the sandbox API does. - Add an acceptance test that forwards a request end to end. Co-authored-by: Isaac <no-reply@databricks.com>
Match the Python SDK's port forward: forward a request only if each path segment decodes once to plain text other than "." or "..". Segments with ";", a backslash, a control character, a malformed escape, or an encoded "/", "\", "%", "?", or "#" get a local 400. Also drop caller-supplied S2S-* headers alongside X-Databricks-*. Co-authored-by: Isaac <no-reply@databricks.com>
The command relays HTTP and WebSocket traffic, so "port-forward" suggests generic TCP forwarding like `ssh -L` that it doesn't provide. A protocol-neutral name lets TCP support land later without a new command. This matches the Python SDK, where `SandboxExt.port_forward` becomes `SandboxExt.forward`. Co-authored-by: Isaac <no-reply@databricks.com>
…box hint - Forward top-level GET and HEAD navigations from other sites, as the SDK does, so links and sign-in redirects to the app work. Iframes, fetches and cross-site form posts are still refused. - Share the hint that ends the missing- and stale-default errors, and point users at setting an existing sandbox as the default. - Say "Could not resolve sandbox" when the lookup only warns. Co-authored-by: Isaac <no-reply@databricks.com>
9a6a405 to
ffb0d8c
Compare
Co-authored-by: Isaac <no-reply@databricks.com>
Changes
Add
databricks sandbox forward [sandbox-id] -L LOCAL:REMOTE.... Each-Lserveshttp://127.0.0.1:LOCALand relays HTTP and WebSocket traffic to the app on sandbox portREMOTEthrough the workspace's/sandbox-proxy/sandboxes/<id>/ports/<port>/route, authenticated as the caller. Without an ID it uses the default sandbox.Every forwarded request carries the caller's credentials, so a forward:
localhost;OriginandSec-Fetch-Site), so neither a DNS-rebinding page nor a page on another site can use it, including over WebSockets;Authorization,X-Databricks-*, andS2S-*headers with the CLI's;.or.., so every request stays on the port route. Segments with;, a backslash, a control character, a malformed escape, or an encoded/,\,%,?, or#get a local 400, the same rule as the Python SDK'sforward.Remote ports outside 10000-65535 are rejected up front, and a stopped sandbox is started before forwarding, like
sandbox ssh.Replaces #7367, which was opened from a fork.
Why
Users can open apps running in their sandbox without setting up SSH. This is the CLI counterpart of the Python SDK's
SandboxExt.forward(renamed fromport_forward). The command is namedforward, notport-forward, because it carries HTTP and WebSocket only for now. That leaves room for TCP support later without a new command.Tests
-Lparsing, the request rewrite and its guards, WebSocket relay, auth failures, and sandbox resolution. Acceptance tests for argument and lookup errors.This pull request and its description were written by Isaac.