fix(cdp): remote attach takes its proxy from the caller at session open

docs/adr/0016-attach-proxy-comes-from-the-caller.md

@@ -0,0 +1,66 @@
+# A remote `--attach` takes its proxy from the caller's environment, once, at session open
+
+Status: accepted and implemented (2026-10-08) by #136. Supersedes the
+"never proxy the CDP control channel" half of #20.
+
+## Context
+
+Reaching a remote CDP browser (`--attach=<url>`) takes three connections:
+`/json/version` discovery over HTTP, the session's own CDP websocket, and one
+websocket per Playwright client through the facade bridge. Before #136 they
+disagreed about proxies, and all of them asked the wrong process:
+
+- discovery used httpx with `trust_env=True` for any non-loopback endpoint;
+- both websockets were hard-coded `proxy=None` (#20);
+- every decision read the **daemon's** environment.
+
+The daemon is long-lived and is started either by a LaunchAgent (no proxy
+vars) or from a shell (whatever that shell exported). So one
+`session new --attach=…` command worked against one daemon and failed against
+another. Under a SOCKS `all_proxy` without `socksio`, httpx raised while
+*constructing* the client, before it looked at `NO_PROXY` at all.
+
+Sometimes the proxy is needed: a cloud browser behind a corporate egress, for
+example. "Never proxy", which is what #20 chose, is too strong.
+
+## Decision
+
+1. **The CLI that opens the session decides.** `session new --attach=<url>` and
+   the throwaway sessions behind `markdown` / `search --attach` resolve the
+   proxy for the attach URL from their **own** environment:
+   `http_proxy` / `https_proxy` / `all_proxy` / `no_proxy`, in either case. They
+   read only those variables, never the OS proxy settings that
+   `urllib.request.getproxies()` falls back to on macOS. `NO_PROXY` accepts
+   `*`, domains (which match subdomains), IPs, and CIDR ranges. Loopback
+   targets, including `*.localhost` and `0.0.0.0`, are always direct.
+2. **The result is pinned on the session.** It is stored as
+   `workspace["proxy"]` in the ledger and is fixed for the session's lifetime.
+   Later calls (`-e`, tasks) do not carry or change it. Every reconnect the
+   daemon performs on its own (ADR-0013) reuses it. To change the proxy, end
+   the session and open a new one.
+3. **The daemon never reads proxy env vars of its own.** Discovery uses httpx
+   with `trust_env=False` and the pinned proxy. Both websockets get the pinned
+   proxy explicitly, or `None`. Proxy variables in the daemon process have no
+   effect on any of these connections.
+4. **The opt-out is `NO_PROXY` at the call site.** For example,
+   `NO_PROXY=100.64.0.0/10 browserwright session new --attach=…`. There is no
+   `--proxy` flag.
+5. **SOCKS works out of the box.** `httpx[socks]` and `python-socks` are
+   default dependencies, because macOS proxy apps export
+   `all_proxy=socks5://…` by default.
+6. **A dead proxy is reported as a dead proxy.** A connect failure through a
+   proxy names the proxy (redacted) and the `NO_PROXY=<host>` bypass, so it
+   cannot be mistaken for a dead remote browser.
+
+## Consequences
+
+- #20's scenario (LAN, Tailscale, CloakBrowser) still works when the user's
+  proxy app routes those addresses DIRECT, which Surge and Clash do in rule
+  mode. Otherwise the user lists them in `NO_PROXY`; the daemon no longer
+  guesses.
+- Behaviour no longer depends on which daemon is running.
+- A proxy URL can carry `user:pass@`. It is redacted wherever a record is
+  printed (`session list --json`, daemon logs, errors).
+- Verified end to end by `tests/daemon/e2e/test_issue136_attach_proxy.py`,
+  which uses recording HTTP and SOCKS5 proxies and a daemon whose own env
+  points at a dead proxy.

pyproject.toml

@@ -22,7 +22,13 @@ license = "AGPL-3.0-only"
 dependencies = [
     "websockets==15.0.1",
     "pillow==12.2.0",
-    "httpx>=0.27",
+    # SOCKS support is default, not an extra (#136): an `--attach` session's
+    # proxy comes from the caller's environment, and macOS proxy apps (Surge,
+    # Clash) routinely export `all_proxy=socks5://...`. `socks` gives httpx
+    # socksio for CDP discovery; python-socks is what websockets 15 needs for
+    # the CDP websocket itself.
+    "httpx[socks]>=0.27",
+    "python-socks>=2.7",
     # Phase C: the skill heredoc injects a real Playwright `page`/`context`
     # bound to the daemon facade, so playwright is a runtime dependency now (it
     # was previously dev-only). `playwright install chromium` is still needed

src/browserwright/daemon/_net.py

@@ -11,6 +11,9 @@
   question decides both whether the user's `ALL_PROXY` should apply and
   whether Chrome's `DevToolsActivePort` file is worth reading. Neither is
   correct for a remote endpoint.
+- **`proxy_for`** — "which proxy reaches this attach target?" Answered once,
+  by the CLI from *its* environment when a session is opened, and stored on
+  the session (#136). The daemon never reads proxy env vars of its own.
 - **`redact_url`** — a CDP endpoint routinely carries a reusable token, in the
   userinfo or the query string. Anything that prints an endpoint (daemon logs,
   `daemon ps --json`, `session list --json`, error messages) must go through
@@ -19,6 +22,7 @@
 from __future__ import annotations
 
 import ipaddress
+from collections.abc import Mapping
 from urllib.parse import urlsplit, urlunsplit
 
 #: Hostnames that mean "this machine" without being parseable as an address.
@@ -53,12 +57,132 @@ def is_loopback_host(host_or_url: str) -> bool:
     host = host.strip().strip("[]").lower()
     if not host:
         return False
-    if host in _LOOPBACK_NAMES:
+    # RFC 6761: every `*.localhost` name resolves to loopback.
+    if host in _LOOPBACK_NAMES or host.rstrip(".").endswith(".localhost"):
         return True
     try:
-        return ipaddress.ip_address(host).is_loopback
+        addr = ipaddress.ip_address(host)
     except ValueError:
         return False
+    # `0.0.0.0` / `::` as a *destination* is this machine on every OS we run on.
+    return addr.is_loopback or addr.is_unspecified
+
+
+#: Which env var serves which target scheme. `ws` rides the same proxy as
+#: `http` and `wss` the same as `https`, matching curl and `websockets`.
+_PROXY_VAR_FOR_SCHEME = {"http": "http", "ws": "http",
+                         "https": "https", "wss": "https"}
+
+
+def _env_value(env: Mapping[str, str], name: str) -> str:
+    """`name_proxy` from `env`, lowercase winning over uppercase as in curl."""
+    for key in (name.lower(), name.upper()):
+        value = env.get(key)
+        if value and value.strip():
+            return value.strip()
+    return ""
+
+
+def _no_proxy_matches(host: str, port: int | None, no_proxy: str) -> bool:
+    """Does a `NO_PROXY` list exempt `host`?
+
+    Entries are comma- or space-separated: `*` (everything), an IP or CIDR
+    (`100.64.0.0/10`), or a domain, which also covers its subdomains whether
+    written `example.com` or `.example.com`. An entry may carry a `:port`,
+    which then has to match too. `urllib`'s own bypass check does not
+    understand CIDR, which is the form a tailnet or LAN is usually written in.
+    """
+    try:
+        addr = ipaddress.ip_address(host)
+    except ValueError:
+        addr = None
+    for raw in no_proxy.replace(" ", ",").split(","):
+        entry = raw.strip().lower()
+        if not entry:
+            continue
+        if entry == "*":
+            return True
+        entry_port: int | None = None
+        if entry.startswith("[") and "]" in entry:
+            body, _, rest = entry[1:].partition("]")
+            if rest.startswith(":") and rest[1:].isdigit():
+                entry_port = int(rest[1:])
+            entry = body
+        elif entry.count(":") == 1:
+            body, _, maybe_port = entry.partition(":")
+            if maybe_port.isdigit():
+                entry, entry_port = body, int(maybe_port)
+        if entry_port is not None and entry_port != port:
+            continue
+        if addr is not None:
+            try:
+                if addr in ipaddress.ip_network(entry, strict=False):
+                    return True
+            except ValueError:
+                pass
+            continue
+        domain = entry.lstrip(".").rstrip(".")
+        if domain and (host == domain or host.endswith("." + domain)):
+            return True
+    return False
+
+
+def proxy_for(url: str, env: Mapping[str, str]) -> str | None:
+    """The proxy `env` says reaches `url`, or None for a direct connection.
+
+    Reads exactly the `http_proxy` / `https_proxy` / `all_proxy` / `no_proxy`
+    variables (either case) in `env` — never the OS proxy settings, which
+    `urllib.request.getproxies()` falls back to on macOS. Loopback targets are
+    always direct: proxying to your own machine is never what anyone wants.
+
+    The caller passes `env` explicitly because *whose* environment it is is
+    the whole decision (#136): the CLI that opens a session passes its own,
+    and the daemon never calls this with its own.
+    """
+    if is_loopback_host(url):
+        return None
+    try:
+        parts = urlsplit(url)
+        host = (parts.hostname or "").lower()
+        port = parts.port
+    except ValueError:
+        return None
+    var = _PROXY_VAR_FOR_SCHEME.get(parts.scheme.lower())
+    if not host or var is None:
+        return None
+    if _no_proxy_matches(host, port, _env_value(env, "no_proxy")):
+        return None
+    proxy = _env_value(env, f"{var}_proxy") or _env_value(env, "all_proxy")
+    if not proxy:
+        return None
+    # `host:port` without a scheme is a common way to write an HTTP proxy.
+    return proxy if "://" in proxy else f"http://{proxy}"
+
+
+def proxy_toward(url: str, proxy: str | None) -> str | None:
+    """`proxy`, unless `url` is on this machine.
+
+    A session's proxy was resolved for its attach URL, but discovery can hand
+    back a websocket URL on a different host. A loopback one is still direct.
+    """
+    return None if proxy is None or is_loopback_host(url) else proxy
+
+
+def proxy_hint(proxy: str | None, url: str) -> str:
+    """The suffix that turns a connect failure through `proxy` into an
+    actionable one, or "" when the connection was direct.
+
+    Without it a dead proxy reads as a dead remote browser.
+    """
+    if proxy is None:
+        return ""
+    try:
+        host = urlsplit(url).hostname or url
+    except ValueError:
+        host = url
+    return (f" (via proxy {redact_url(proxy)}, taken from the environment "
+            f"that opened this session; if that proxy is not reachable, end "
+            f"the session and open it again with NO_PROXY={host} set)")
 
 
 def redact_url(url: object) -> object:

src/browserwright/daemon/backends/cdp.py

@@ -36,7 +36,7 @@
 
 import httpx
 
-from .._net import is_loopback_host, redact_url
+from .._net import is_loopback_host, proxy_hint, redact_url
 from ..config import Config
 from ..errors import Unavailable
 from ..platforms import profile_paths
@@ -62,12 +62,10 @@ class RealCdpBackend:
     kind = "UPSTREAM_WS"
     recommended_mode: str = "A"
     ux_cost = "none"
-    # cdp overrides to False: the user's HTTP(S)_PROXY / ALL_PROXY must not be
-    # applied to localhost probes — proxying to your own loopback is never what
-    # anyone wants and triggers httpx[socks] import errors when
-    # ALL_PROXY=socks5://... env keeps the default: its target may be a remote
-    # host where the proxy is intentional.
-    _trust_env: bool = True
+    # The proxy discovery goes through, or None for direct. Never read from
+    # the daemon's environment (#136): `CdpBackend` takes it from the
+    # session's Config, where the CLI that opened the session pinned it.
+    _proxy: str | None = None
 
     def __init__(self, cfg: Config):
         self._cfg = cfg
@@ -97,7 +95,12 @@ async def _resolve_source(self, timeout: float) -> tuple[str, dict]:
     # ---- shared plumbing ----------------------------------------------------
 
     async def _get(self, url: str, timeout: float) -> httpx.Response:
-        async with httpx.AsyncClient(timeout=timeout, trust_env=self._trust_env) as client:
+        # `trust_env=False`: httpx would otherwise build a transport for every
+        # proxy env var at construction — before `NO_PROXY` is consulted — so
+        # a stray daemon-side `all_proxy` could crash a request it never
+        # applied to (#136).
+        async with httpx.AsyncClient(timeout=timeout, trust_env=False,
+                                     proxy=self._proxy) as client:
             return await client.get(url)
 
     async def _discover_ws_url(self, base_url: str, timeout: float) -> str:
@@ -118,7 +121,8 @@ async def _discover_ws_url(self, base_url: str, timeout: float) -> str:
             resp = await self._get(url, timeout)
         except (httpx.HTTPError, OSError) as e:
             raise Unavailable(
-                f"{self.name}: cannot reach {shown}: {e}",
+                f"{self.name}: cannot reach {shown}: {e}"
+                f"{proxy_hint(self._proxy, base_url)}",
                 attempts={self.name: f"GET {shown} -> {type(e).__name__}: {e}"},
             ) from e
         if resp.status_code == 404:
@@ -166,7 +170,7 @@ class CdpBackend(RealCdpBackend):
 
     | | old | new |
     |---|---|---|
-    | apply the user's `ALL_PROXY` | `cdp` no, `env` yes | `not _loopback` |
+    | apply a proxy | `cdp` no, `env` yes | `not _loopback` |
     | DevToolsActivePort 404 fallback | `cdp` yes, `env` never | `_loopback` |
 
     That is strictly more correct than the names were: `--attach=http://127.0.0.1:9222`
@@ -179,6 +183,7 @@ def __init__(self, cfg: Config):
         super().__init__(cfg)
         self.port = cfg.backends.cdp.port
         self.endpoint = cfg.backends.cdp.endpoint
+        self.proxy = cfg.backends.cdp.proxy
 
     # ---- the one predicate the two old backends disagreed about -------------
 
@@ -190,15 +195,9 @@ def _loopback(self) -> bool:
         return is_loopback_host(self.endpoint)
 
     @property
-    def _trust_env(self) -> bool:  # overrides RealCdpBackend's class attribute
-        """Apply the user's HTTP(S)_PROXY / ALL_PROXY?
-
-        Never for a local browser: proxying to your own loopback is never what
-        anyone wants, and it trips httpx[socks] import errors under
-        `ALL_PROXY=socks5://...`. Always for a remote endpoint, where reaching
-        it through the proxy is usually the whole point.
-        """
-        return not self._loopback
+    def _proxy(self) -> str | None:  # overrides RealCdpBackend's class attribute
+        """The proxy the session pinned (#136); never one for a local browser."""
+        return None if self._loopback else self.proxy
 
     # ---- URL sources --------------------------------------------------------
 

src/browserwright/daemon/config.py

@@ -197,6 +197,10 @@ class CdpConfig:
     """
     port: int = 9222
     endpoint: str | None = None
+    #: The proxy that reaches `endpoint`, or None for direct. Per-session only,
+    #: like `endpoint`: resolved by the CLI from the environment of whoever
+    #: opened the session (#136). The daemon never reads its own proxy env.
+    proxy: str | None = None
 
 
 @dataclass