Add browser proxy to automatically forward browser connections to local#281
Add browser proxy to automatically forward browser connections to local#281KevinFairise2 wants to merge 20 commits into
Conversation
…al laptop
When running `dda env dev start`, a small HTTP daemon is now started on the
host that listens on a deterministic port (derived from the container name).
Inside the container, /usr/local/bin/xdg-open is replaced with a Python script
that forwards any xdg-open call to the host daemon via host.docker.internal.
For OAuth/auth flows that redirect back to localhost:{port}/callback, the daemon
detects the redirect_uri in the URL query parameters and sets up an SSH local
port forward (host 127.0.0.1:{port} → container localhost:{port}) before
opening the browser, so the auth callback reaches the service running inside
the container.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🎯 Code Coverage (details) 🔗 Commit SHA: 66f26fd | Docs | Datadog PR Page | Give us feedback! |
…other tunnel When two containers run OAuth simultaneously with the same callback port, the second SSH tunnel can't bind the port. Previously _wait_for_port_bound would incorrectly return True (EADDRINUSE detected but from the wrong process). Now it also verifies that our ssh process is still alive after detecting the port is taken, returning False if ssh exited (meaning the port belongs to someone else). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
spawn_daemon uses subprocess.Popen which is not mocked in tests. On Windows, the spawned process inherits the test CWD (inside a temp dir), holding a directory handle that blocks pytest cleanup and causes WinError 32 in subsequent tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b4c214c34
ℹ️ 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".
| port = os.environ.get("DDA_BROWSER_PROXY_PORT", "") | ||
| if not port: | ||
| sys.exit(0) |
There was a problem hiding this comment.
Do not rely on Docker env inside SSH sessions
When commands are run through this dev env they go through SSH, and the repo's SSH config only sets TERM (src/dda/env/ssh.py), so DDA_BROWSER_PROXY_PORT from docker run -e is not propagated into those sessions. In that common path xdg-open reaches this branch and exits successfully without contacting the proxy, so browser opens from shells/commands inside the container become silent no-ops; the port needs to be embedded in the mounted script or otherwise made available to SSH sessions.
Useful? React with 👍 / 👎.
| "-L", | ||
| f"127.0.0.1:{callback_port}:localhost:{callback_port}", | ||
| "dd@localhost", |
There was a problem hiding this comment.
Fail the tunnel when local forwarding cannot bind
If the callback port is already in use on the host, OpenSSH does not exit by default (ssh -G localhost reports exitonforwardfailure no), so this ssh -N process can stay alive even though -L 127.0.0.1:<callback_port> was not installed. _wait_for_port_bound then treats the pre-existing listener as success and opens the browser, causing the OAuth callback to hit the wrong local service instead of the container; add ExitOnForwardFailure=yes or otherwise verify that this process actually owns the forward.
Useful? React with 👍 / 👎.
Without ExitOnForwardFailure=yes, ssh -N stays alive even if -L fails to bind, so _wait_for_port_bound (which checks proc.poll() is None after seeing EADDRINUSE) incorrectly treated a pre-existing listener as our own tunnel, opening the browser and sending the OAuth callback to the wrong local service. Fix: use -F /dev/null to suppress user SSH configs (the earlier reason for removing ExitOnForwardFailure was that ~/.ssh/workspaces config adds a failing reverse-forward that kills SSH), then re-add ExitOnForwardFailure=yes so SSH exits immediately on bind failure. Also add _active_tunnels dict + lock so concurrent /open requests for the same callback port share the existing tunnel instead of racing to start a second SSH process. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Docker -e variables are not propagated into SSH sessions (sshd only passes TERM per the SSH config), so DDA_BROWSER_PROXY_PORT was always empty in shells/commands run via `dda env dev shell/run`, causing xdg-open to silently exit without contacting the proxy. Fix: bake the port directly into the generated script at container start time. The script is already per-container, so the port is always correct and available regardless of how the session was opened. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per-container daemons meant a crash affected only one container but
required a per-container restart to recover. With a shared daemon, one
restart fixes all containers simultaneously.
Changes:
- browser_proxy.py: remove _ssh_port global; each request now carries
its container's ssh_port as a query param (?ssh_port=<port>), so the
stateless daemon can set up the right SSH tunnel per request.
_active_tunnels is now keyed by (ssh_port, callback_port) so tunnels
from different containers to the same callback port are tracked
independently.
- linux_container.py: browser_proxy_port derives from the fixed key
"dda-browser-proxy" (same port for all containers). _make_xdg_open_script
now embeds both the shared proxy port and the container's ssh_port.
_start_browser_proxy uses a shared PID file at
config.storage.join("browser-proxy"). _stop_browser_proxy removed —
the daemon is not owned by any single container. _start_browser_proxy
is also called in launch_shell, run_command, and code so a crashed
daemon is automatically recovered on the next user interaction.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The browser proxy daemon is a no-op on Windows, so the container-side plumbing that supports it (--add-host host.docker.internal:host-gateway, -e BROWSER, and the xdg-open volume mount) is also pointless there. Guard all three under the existing sys.platform != "win32" block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ishirui
left a comment
There was a problem hiding this comment.
I think this "browser proxy" pattern will be quite useful to have for all types of dev envs, not just linux containers (thinking about stuff like remote windows VMs)
Right now it feels like the implementation is very coupled to the linux_container type, with very little generic infrastructure on the parent DeveloperEnvironmentInterface class. I think a few of the properties and methods can be declared on that parent and then concretely implemented on linux_container, that way we have an easily-extendable pattern if we want to add browser proxy support to other dev env types.
Would also be nice to add a little bit of doc explaining how this works
| sys.executable, | ||
| "-m", | ||
| "dda.env.dev.browser_proxy", | ||
| str(self.browser_proxy_port), |
There was a problem hiding this comment.
I would be wary of manually calling python modules with sys.executable like this. You never know what PYTHONPATH shenanigans are going on.
Discussed offline: despite this, it's probably the best alternative since we need to spawn an actual daemon process i.e. calling a dda command would not work. We could also ship a binary in say, Go, but that would require setting up tooling for Go in this repo and also shipping the binary alongside the dda wheel.
98a74f0 to
8f7e9ef
Compare
Summary
Automatically forwards browser connections from inside a dev container to the local laptop, including OAuth callback support.
dda env dev startruns, listening on a deterministic port derived from the container name (same scheme as SSH/MCP ports)xdg-openscript into the container at/usr/local/bin/xdg-openand setsBROWSER=xdg-open+DDA_BROWSER_PROXY_PORT, so any tool callingxdg-openor$BROWSERworks automaticallyredirect_uri=http://localhost:{port}/...and sets up an SSH local port forward (127.0.0.1:{port}on host →localhost:{port}in container) before opening the browser, so the auth callback reaches the service running inside the container (e.g.ddtool auth gitlab login)--add-host host.docker.internal:host-gatewaytodocker runfor Linux host compatibility (it is automatic on Docker Desktop for Mac/Windows)spawn_daemon+ PID file pattern as the MCP server manager; daemon is started onstartand killed onstop