test(tools): restore the drain guard and gate the TTY loop on the exit too

Two follow-ups from review.

The anti-redelivery poll my previous commit message described was not in the
tree. It was written, it passed, and then a `git checkout -- .` in the
falsification run that followed reverted it, because it was the one edit made
after the checkpoint commit. The message was amended afterwards and recorded work
that no longer existed. jatmn caught the mismatch. Restored here, and the claim
now matches the code: one more read after the served line, asserting it is not
repeated, which is the only guard for drain semantics on Windows since the
manager-level test that counts deliveries is skipped there.

The TTY loop stopped on the echo alone while asserting the exit as well. collect
can return the drained echo on a yield timer before markDone has run, so a poll
can carry got:hello with no exit_code meta yet and the assertion below would then
be about an exit nobody observed. It now waits for both. The echo term stays in
the condition deliberately: gating on exit_code alone would stop polling before a
slow shell start delivers the echo, which is the race this change exists to
remove.

README.md

@@ -273,19 +273,6 @@ zero sandbox grants list
 Zero includes local file/search/edit/shell tools, `web_fetch` for public URLs,
 and MCP support for additional tools.
 
-`web_fetch` refuses loopback, private and other special-use addresses: it checks
-the URL before asking permission, resolves the host, and dials the address it
-validated so a name cannot resolve to something else in between.
-
-If `HTTP_PROXY`/`HTTPS_PROXY` is set, `web_fetch` and the provider connectivity
-probe use it, and that last step changes: the proxy is dialed and the target
-hostname is sent to it, so the proxy decides which address the request actually
-reaches. The URL is still checked and resolved locally first, but a proxy that
-answers differently can reach a private service. A forward proxy already sees
-and can rewrite every request through it, so this is the trust you accept by
-configuring one. Leave the variables unset for the checks to be enforced end to
-end.
-
 For local dev servers, use shell commands such as `curl` through `exec_command`
 so the normal sandbox and permission policy applies. Long-running commands stay
 attached to a background terminal session and can be listed or stopped from the

docs/HOW_ZERO_WORKS.md

@@ -627,12 +627,7 @@ Core tool groups include:
   skills, `ask_user`, permission requests.
 - **Write tools**: `write_file`, `edit_file`, `apply_patch`, `update_plan`.
 - **Shell tools**: `exec_command`, `write_stdin`, legacy `bash`.
-- **Network tools**: `web_fetch` and optional web search backend. `web_fetch`
-  validates the URL before the permission prompt, refuses loopback/private/
-  special-use addresses, and dials the address it resolved. With
-  `HTTP_PROXY`/`HTTPS_PROXY` set, the proxy is dialed instead and resolves the
-  target itself, so destination enforcement ends at the proxy; see the note in
-  the README.
+- **Network tools**: `web_fetch` and optional web search backend.
 - **Local-control tools**: config-gated wrappers for browser, terminal, desktop,
   and artifact helpers backed by packages such as `internal/localcontrol` and
   `internal/browser`.

internal/providerhealth/providerhealth.go

@@ -19,7 +19,6 @@ import (
 	"github.com/Gitlawb/zero/internal/providercatalog"
 	"github.com/Gitlawb/zero/internal/providers"
 	"github.com/Gitlawb/zero/internal/providers/providerio"
-	"github.com/Gitlawb/zero/internal/proxydial"
 	"github.com/Gitlawb/zero/internal/redaction"
 )
 
@@ -519,15 +518,9 @@ func newConnectivityClient(timeout time.Duration, resolver Resolver, sensitiveHe
 		transport = &http.Transport{}
 	}
 	transport.DialContext = safeDialContext(resolver, allowLoopbackOrPrivate)
-	// WRAPPED, SO THE DIALER LEARNS THE ROUTE THIS TRANSPORT CHOSE. The wrapper
-	// records the proxy selected for each request on that request, and the
-	// dialer reads it back on the dial that request causes; installing the bare
-	// transport instead would leave a proxied dial looking like a loopback
-	// target and fail the probe (#569). The cloned default transport already
-	// reads HTTPS_PROXY and friends; nothing new is enabled.
 	return &http.Client{
 		Timeout:   timeout,
-		Transport: proxydial.Wrap(transport),
+		Transport: transport,
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			if len(via) >= maxConnectivityRedirects {
 				return fmt.Errorf("provider connectivity exceeded %d redirects", maxConnectivityRedirects)
@@ -590,17 +583,6 @@ func sensitiveAuthHeaderNames(profile config.ProviderProfile, kind config.Provid
 func safeDialContext(resolver Resolver, allowLoopbackOrPrivate bool) func(context.Context, string, string) (net.Conn, error) {
 	dialer := &net.Dialer{Timeout: 10 * time.Second, KeepAlive: 30 * time.Second}
 	return func(ctx context.Context, network, address string) (net.Conn, error) {
-		// THE PROXY IS NOT THE TARGET. With HTTPS_PROXY set to a local forward
-		// proxy, the transport dials the proxy first and tunnels the request
-		// through it, and this dialer used to refuse that dial as a loopback
-		// address: "proxyconnect tcp: ... loopback hosts are blocked" for a
-		// request whose real target had already been validated (#569). The
-		// target validation is unchanged; only the dial the transport opens to
-		// the proxy it picked for this very request is let past the guard, and a
-		// direct dial is checked as it always was.
-		if proxydial.IsProxyDial(ctx) {
-			return dialer.DialContext(ctx, network, address)
-		}
 		host, port, err := net.SplitHostPort(address)
 		if err != nil {
 			return nil, err

internal/providerhealth/proxy_dial_test.go

@@ -1,218 +0,0 @@
-package providerhealth
-
-import (
-	"context"
-	"errors"
-	"net"
-	"net/http"
-	"net/netip"
-	"net/url"
-	"strconv"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/Gitlawb/zero/internal/proxydial"
-)
-
-// connectivityClientWithProxy builds the real connectivity client and swaps in
-// a proxy function, which is how the transport learns its route. The
-// environment is left alone deliberately: http.ProxyFromEnvironment reads it
-// once per process, so an environment-driven test would depend on test order.
-func connectivityClientWithProxy(t *testing.T, resolver Resolver, proxyFor func(*http.Request) (*url.URL, error)) *http.Client {
-	t.Helper()
-	client := newConnectivityClient(3*time.Second, resolver, nil, false)
-	wrapped, ok := client.Transport.(*proxydial.Transport)
-	if !ok {
-		t.Fatalf("transport = %T, want the route-recording wrapper; without it no dial can be recognised as the proxy dial", client.Transport)
-	}
-	wrapped.Proxy = proxyFor
-	return client
-}
-
-// recordingProxy is a listener standing in for a local forward proxy. It
-// records the first line it receives and hangs up.
-func recordingProxy(t *testing.T) (*url.URL, <-chan string) {
-	t.Helper()
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatal(err)
-	}
-	t.Cleanup(func() { _ = listener.Close() })
-	arrived := make(chan string, 1)
-	go func() {
-		conn, err := listener.Accept()
-		if err != nil {
-			return
-		}
-		defer conn.Close()
-		buffer := make([]byte, 256)
-		n, _ := conn.Read(buffer)
-		arrived <- string(buffer[:n])
-	}()
-	proxyURL, err := url.Parse("http://" + listener.Addr().String())
-	if err != nil {
-		t.Fatal(err)
-	}
-	return proxyURL, arrived
-}
-
-// THE PROXY IS NOT THE TARGET, AND THE GUARD HAS TO KNOW THE DIFFERENCE.
-//
-// With HTTPS_PROXY set to a local forward proxy, the transport dials the proxy
-// first and tunnels the request through it. This dialer refused that dial as a
-// loopback address, so `zero doctor --connectivity` failed with "proxyconnect
-// tcp: ... loopback hosts are blocked" for a target that had already been
-// validated and was public (#569). Driven through the real client, so the route
-// is the transport's own decision.
-func TestConnectivityClientTunnelsThroughTheConfiguredProxy(t *testing.T) {
-	proxyURL, arrived := recordingProxy(t)
-	client := connectivityClientWithProxy(t,
-		staticResolver{err: errors.New("resolver must not be called for a proxied request")},
-		func(*http.Request) (*url.URL, error) { return proxyURL, nil })
-
-	request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://api.example.com/v1/models", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, err = client.Do(request)
-	select {
-	case first := <-arrived:
-		if line := strings.SplitN(first, "\r\n", 2)[0]; line != "CONNECT api.example.com:443 HTTP/1.1" {
-			t.Fatalf("proxy received %q, want a CONNECT for the validated target", line)
-		}
-	case <-time.After(5 * time.Second):
-		t.Fatalf("the proxy never received a connection; request error: %v", err)
-	}
-}
-
-// AND A DIRECT REQUEST STAYS GUARDED, EVEN WHEN ITS ORIGIN IS THE PROXY'S OWN
-// ADDRESS.
-//
-// This is the NO_PROXY shape: the proxy function sends one host direct. An
-// exemption derived from asking that function about a request of our own
-// invention would answer "proxied" for the excluded host too, and this dial,
-// whose address equals the configured proxy endpoint, would skip the resolver
-// and reach whatever the name resolves to at connect time.
-func TestDirectRequestToTheProxyEndpointIsStillGuarded(t *testing.T) {
-	proxyURL, err := url.Parse("http://rebind.example:80")
-	if err != nil {
-		t.Fatal(err)
-	}
-	lookups := 0
-	resolver := countingResolver{
-		onLookup: func() { lookups++ },
-		addr:     netip.MustParseAddr("10.0.0.5"),
-	}
-	client := connectivityClientWithProxy(t, resolver, func(request *http.Request) (*url.URL, error) {
-		if request.URL.Hostname() == "rebind.example" {
-			return nil, nil
-		}
-		return proxyURL, nil
-	})
-
-	request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://rebind.example/data", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, err = client.Do(request)
-
-	if lookups == 0 {
-		t.Fatal("the direct dial skipped the safety resolver because its address matched the configured proxy endpoint")
-	}
-	var safety endpointSafetyError
-	if !errors.As(err, &safety) {
-		t.Fatalf("err = %v, want the private answer to be refused", err)
-	}
-}
-
-// A proxy configured with a Unicode hostname is dialed by its IDNA-encoded
-// name, and it is still the proxy: the request must reach it rather than be
-// refused as an ordinary private address.
-func TestUnicodeProxyHostnameIsStillTheProxy(t *testing.T) {
-	dialed := ""
-	client := connectivityClientWithProxy(t,
-		staticResolver{err: errors.New("resolver must not be called for a proxied request")},
-		func(*http.Request) (*url.URL, error) { return url.Parse("http://büro.example:3067") })
-	wrapped := client.Transport.(*proxydial.Transport)
-	guarded := wrapped.DialContext
-	wrapped.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) {
-		dialed = address
-		return guarded(ctx, network, address)
-	}
-
-	request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://api.example.com/v1/models", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, err = client.Do(request)
-
-	if dialed != "xn--bro-hoa.example:3067" {
-		t.Fatalf("SETUP INVALID: dialed %q, want the IDNA-encoded proxy endpoint", dialed)
-	}
-	var safety endpointSafetyError
-	if errors.As(err, &safety) {
-		t.Fatalf("the configured proxy was refused by the guard: %v", err)
-	}
-}
-
-// Without a proxy the guard is exactly what it was: loopback is refused.
-func TestSafeDialContextStillRefusesLoopbackWithoutAProxy(t *testing.T) {
-	dial := safeDialContext(staticResolver{err: errors.New("resolver must not be called")}, false)
-	conn, err := dial(context.Background(), "tcp", "127.0.0.1:3067")
-	if conn != nil {
-		_ = conn.Close()
-		t.Fatal("loopback was dialed with no proxy configured")
-	}
-	var safety endpointSafetyError
-	if !errors.As(err, &safety) {
-		t.Fatalf("err = %v, want endpointSafetyError", err)
-	}
-}
-
-// A sibling loopback port is not the proxy, and the guard is unchanged for it.
-func TestSafeDialContextRefusesALoopbackPortThatIsNotTheProxy(t *testing.T) {
-	proxyURL, arrived := recordingProxy(t)
-	client := connectivityClientWithProxy(t,
-		staticResolver{err: errors.New("resolver must not be called")},
-		func(request *http.Request) (*url.URL, error) {
-			// Only the https probe is proxied; the http one below is direct and
-			// happens to point at the neighbouring port.
-			if request.URL.Scheme == "https" {
-				return proxyURL, nil
-			}
-			return nil, nil
-		})
-
-	_, port, _ := net.SplitHostPort(proxyURL.Host)
-	number, _ := strconv.Atoi(port)
-	sibling := "http://" + net.JoinHostPort("127.0.0.1", strconv.Itoa(number+1)) + "/"
-	request, err := http.NewRequestWithContext(context.Background(), http.MethodGet, sibling, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, err = client.Do(request)
-	var safety endpointSafetyError
-	if !errors.As(err, &safety) {
-		t.Fatalf("err = %v, want a loopback port beside the proxy to stay blocked", err)
-	}
-	select {
-	case first := <-arrived:
-		t.Fatalf("the proxy received a connection it should not have: %q", first)
-	default:
-	}
-}
-
-// countingResolver answers with one address and reports each lookup, so a test
-// can tell a skipped guard from a guard that ran and allowed the address.
-type countingResolver struct {
-	onLookup func()
-	addr     netip.Addr
-}
-
-func (resolver countingResolver) LookupNetIP(context.Context, string, string) ([]netip.Addr, error) {
-	if resolver.onLookup != nil {
-		resolver.onLookup()
-	}
-	return []netip.Addr{resolver.addr}, nil
-}

internal/proxydial/proxydial.go

@@ -1,93 +0,0 @@
-// Package proxydial answers one question for an SSRF-guarded dialer: is this
-// dial the connection dial of a request the transport routed through a forward
-// proxy?
-//
-// A guarded transport validates the TARGET of a request before dialing, and its
-// dialer refuses loopback, private and link-local addresses so a redirect or a
-// DNS rebind cannot walk the request onto the local network. When a forward
-// proxy is configured, the transport dials the PROXY first and tunnels the
-// request through it, and a local proxy is by far the common shape:
-// HTTPS_PROXY=http://127.0.0.1:3067. The dialer then sees 127.0.0.1, refuses it,
-// and the user gets "loopback hosts are blocked" for a request whose real target
-// was validated and public (#569).
-//
-// The proxy address is not the request target, and the guard was never about it.
-// It is configuration the user set on purpose, and the transport is going to dial
-// it for every request whether the guard likes it or not. So that dial is let
-// through, and only that dial.
-//
-// THE EXEMPTION FOLLOWS THE TRANSPORT'S OWN ROUTE DECISION, NOT A GUESS AT IT.
-// Asking the Proxy function a question of one's own invention answers for a
-// request the transport never made: with NO_PROXY excluding a host, the real
-// request to that host goes direct while a synthetic probe still comes back
-// proxied, and a direct dial whose address happens to equal the proxy endpoint
-// would then skip the guard entirely. Host-dependent proxy callbacks make the
-// mismatch general, and no amount of care over the probe's spelling closes it,
-// because the two calls are about different requests.
-//
-// So the decision is recorded where it is made, on the request, and read back on
-// the dial that request causes. Transport.RoundTrip asks the wrapped transport's
-// Proxy function about the actual request, stores the answer in that request's
-// context, and hands it on; the transport derives the dial context from the
-// request context, so the dialer reads back the route this very connection is
-// being opened for. Per-request, so concurrent requests cannot overwrite each
-// other, and unforgeable from outside the package: the context key is private.
-package proxydial
-
-import (
-	"context"
-	"net/http"
-	"net/url"
-)
-
-// selectedProxyKey addresses the route decision inside a request context. It is
-// unexported so nothing outside this package can plant one.
-type selectedProxyKey struct{}
-
-// Transport records the proxy its wrapped transport selects for each request in
-// that request's context, so a guarded DialContext installed on the same
-// transport can tell the dial to a proxy from a direct dial.
-//
-// The embedded transport stays reachable: callers configure Proxy, DialContext
-// and the rest on it exactly as before, and tests can read them back.
-type Transport struct {
-	*http.Transport
-}
-
-// Wrap returns a Transport around transport. The result is the http.RoundTripper
-// to install on the client; installing the bare transport instead leaves every
-// dial looking direct, which is safe (the guard applies) but reopens #569.
-func Wrap(transport *http.Transport) *Transport {
-	return &Transport{Transport: transport}
-}
-
-// RoundTrip resolves the route for this request and passes the decision down to
-// the dialer through the request context.
-func (transport *Transport) RoundTrip(request *http.Request) (*http.Response, error) {
-	if transport.Transport.Proxy != nil {
-		// A Proxy error is left for the transport to report: it fails the
-		// request before dialing, so there is no dial to classify. The function
-		// is called once here and once by the transport, which is what a
-		// host-dependent callback already expects from redirects and retries.
-		if proxyURL, err := transport.Transport.Proxy(request); err == nil && proxyURL != nil {
-			request = request.WithContext(context.WithValue(request.Context(), selectedProxyKey{}, proxyURL))
-		}
-	}
-	return transport.Transport.RoundTrip(request)
-}
-
-// IsProxyDial reports whether ctx belongs to a request this package's Transport
-// routed through a proxy, which makes the dial about to happen the dial to that
-// proxy.
-//
-// THE ADDRESS IS NOT COMPARED, AND DOES NOT NEED TO BE. A transport opens
-// exactly one connection dial per connection, and when a proxy is selected that
-// dial goes to the proxy; the target is reached through it, never dialed. So the
-// route decision on the context already identifies the dial, and matching the
-// address against the proxy URL on top of it could only ever disagree with the
-// transport, which is what a punycoded Unicode proxy hostname does: Go dials
-// xn--bro-hoa.example while the URL still spells buero as the user wrote it.
-func IsProxyDial(ctx context.Context) bool {
-	proxyURL, _ := ctx.Value(selectedProxyKey{}).(*url.URL)
-	return proxyURL != nil
-}

internal/proxydial/proxydial_test.go

@@ -1,224 +0,0 @@
-package proxydial
-
-import (
-	"context"
-	"errors"
-	"net"
-	"net/http"
-	"net/url"
-	"sync"
-	"testing"
-)
-
-// errDialed ends the request at the dial, after the route decision has been
-// observed. Nothing here needs a live server.
-var errDialed = errors.New("dial recorded")
-
-type dialRecord struct {
-	address string
-	proxied bool
-}
-
-// driveRequest sends one request through a wrapped transport whose dialer
-// records what it was asked for and whether the context says this dial belongs
-// to a proxied request.
-func driveRequest(t *testing.T, target string, proxyFor func(*http.Request) (*url.URL, error)) dialRecord {
-	t.Helper()
-	var record dialRecord
-	transport := &http.Transport{Proxy: proxyFor}
-	transport.DialContext = func(ctx context.Context, _, address string) (net.Conn, error) {
-		record = dialRecord{address: address, proxied: IsProxyDial(ctx)}
-		return nil, errDialed
-	}
-	client := &http.Client{Transport: Wrap(transport)}
-	request, err := http.NewRequest(http.MethodGet, target, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if _, err := client.Do(request); !errors.Is(err, errDialed) {
-		t.Fatalf("%s: request ended before the dial: %v", target, err)
-	}
-	if record.address == "" {
-		t.Fatalf("%s: the dialer was never called", target)
-	}
-	return record
-}
-
-func fixedProxy(raw string) func(*http.Request) (*url.URL, error) {
-	parsed, err := url.Parse(raw)
-	if err != nil {
-		panic(err)
-	}
-	return func(*http.Request) (*url.URL, error) { return parsed, nil }
-}
-
-// THE DIAL TO A CONFIGURED PROXY IS THE ONE THAT IS EXEMPT. A proxied request
-// dials the proxy, never the target, so the route on the context is what
-// identifies the dial.
-func TestProxiedRequestDialsTheProxyAndSaysSo(t *testing.T) {
-	record := driveRequest(t, "https://api.example.com/v1/models", fixedProxy("http://127.0.0.1:3067"))
-	if !record.proxied {
-		t.Error("a request routed through a proxy did not reach the dialer as proxied")
-	}
-	if record.address != "127.0.0.1:3067" {
-		t.Errorf("dial address = %q, want the proxy endpoint", record.address)
-	}
-}
-
-// AND A DIRECT REQUEST IS NOT, EVEN WHEN ITS ORIGIN IS THE PROXY'S OWN ADDRESS.
-//
-// This is the NO_PROXY shape: the proxy function excludes one host, so a request
-// to that host goes direct. Asking that same function about a request of our own
-// invention would answer "proxied" (the invented host is not excluded) and the
-// direct dial, whose address equals the proxy endpoint, would skip the guard
-// while resolving to whatever the name resolves to at connect time.
-func TestDirectRequestToTheProxyEndpointIsNotAProxyDial(t *testing.T) {
-	proxyURL, err := url.Parse("http://rebind.example:80")
-	if err != nil {
-		t.Fatal(err)
-	}
-	noProxyForRebind := func(request *http.Request) (*url.URL, error) {
-		if request.URL.Hostname() == "rebind.example" {
-			return nil, nil
-		}
-		return proxyURL, nil
-	}
-
-	record := driveRequest(t, "http://rebind.example/data", noProxyForRebind)
-	if record.address != "rebind.example:80" {
-		t.Fatalf("SETUP INVALID: dial address = %q, want the direct origin, which is also the proxy endpoint", record.address)
-	}
-	if record.proxied {
-		t.Error("a direct dial was reported as the proxy dial because its address matched the configured proxy")
-	}
-
-	// The same function still routes another host through that proxy, so the
-	// exclusion above is a route difference and not a disabled proxy.
-	if record := driveRequest(t, "http://elsewhere.example/data", noProxyForRebind); !record.proxied {
-		t.Error("the configured proxy was not used for a host outside the exclusion")
-	}
-}
-
-// The inverse mismatch: a callback that routes by host must not have its answer
-// taken from some other host. A proxy selected only for the real target is
-// honoured.
-func TestHostDependentProxySelectionIsHonoured(t *testing.T) {
-	proxyURL, err := url.Parse("http://127.0.0.1:3067")
-	if err != nil {
-		t.Fatal(err)
-	}
-	onlyForOneHost := func(request *http.Request) (*url.URL, error) {
-		if request.URL.Hostname() == "api.example.com" {
-			return proxyURL, nil
-		}
-		return nil, nil
-	}
-	if record := driveRequest(t, "https://api.example.com/v1", onlyForOneHost); !record.proxied || record.address != "127.0.0.1:3067" {
-		t.Errorf("selected proxy not reached: %+v", record)
-	}
-	if record := driveRequest(t, "https://other.example/v1", onlyForOneHost); record.proxied {
-		t.Error("an unproxied host was reported as proxied")
-	}
-}
-
-// Scheme separation: an https-only proxy leaves an http request direct, and the
-// decision recorded is the one made for that request's own scheme.
-func TestSchemeSeparation(t *testing.T) {
-	proxyURL, err := url.Parse("http://127.0.0.1:3067")
-	if err != nil {
-		t.Fatal(err)
-	}
-	httpsOnly := func(request *http.Request) (*url.URL, error) {
-		if request.URL.Scheme == "https" {
-			return proxyURL, nil
-		}
-		return nil, nil
-	}
-	if record := driveRequest(t, "https://api.example.com/v1", httpsOnly); !record.proxied {
-		t.Error("the https proxy was not applied to an https request")
-	}
-	if record := driveRequest(t, "http://api.example.com/v1", httpsOnly); record.proxied {
-		t.Error("an http request was treated as proxied by an https-only proxy")
-	}
-}
-
-// A UNICODE PROXY HOSTNAME IS STILL THE PROXY. Go dials proxies by their
-// IDNA-encoded name, so a matcher comparing the URL's own spelling would call
-// this dial direct and refuse it as an ordinary private address. Nothing is
-// compared here, so both spellings of one host are one host.
-func TestUnicodeProxyHostnameIsRecognised(t *testing.T) {
-	unicodeRecord := driveRequest(t, "https://api.example.com/v1", fixedProxy("http://büro.example:3067"))
-	if !unicodeRecord.proxied {
-		t.Error("a proxy configured with a Unicode hostname was not recognised as the proxy")
-	}
-	asciiRecord := driveRequest(t, "https://api.example.com/v1", fixedProxy("http://xn--bro-hoa.example:3067"))
-	if !asciiRecord.proxied {
-		t.Error("a proxy configured with the ASCII spelling was not recognised as the proxy")
-	}
-	// The two spellings name one endpoint, and it is the ASCII one that is
-	// dialed. This is the mismatch a string comparison would have had to close.
-	if unicodeRecord.address != asciiRecord.address {
-		t.Errorf("dial addresses differ across spellings of one host: %q vs %q", unicodeRecord.address, asciiRecord.address)
-	}
-}
-
-// A transport with no proxy leaves every dial direct.
-func TestNoProxyMeansNoExemption(t *testing.T) {
-	if record := driveRequest(t, "http://127.0.0.1:3067/", nil); record.proxied {
-		t.Error("a dial was reported as proxied by a transport with no Proxy function")
-	}
-	if IsProxyDial(context.Background()) {
-		t.Error("a context that never went through the wrapper reported a proxy dial")
-	}
-}
-
-// THE DECISION IS PER REQUEST. A shared record of the last selected proxy would
-// let a proxied request hand its exemption to a direct one running beside it.
-func TestConcurrentRequestsKeepTheirOwnRoute(t *testing.T) {
-	proxyURL, err := url.Parse("http://127.0.0.1:3067")
-	if err != nil {
-		t.Fatal(err)
-	}
-	byHost := func(request *http.Request) (*url.URL, error) {
-		if request.URL.Hostname() == "proxied.example" {
-			return proxyURL, nil
-		}
-		return nil, nil
-	}
-
-	var mutex sync.Mutex
-	seen := map[string]bool{}
-	transport := &http.Transport{Proxy: byHost}
-	transport.DialContext = func(ctx context.Context, _, address string) (net.Conn, error) {
-		mutex.Lock()
-		seen[address] = IsProxyDial(ctx)
-		mutex.Unlock()
-		return nil, errDialed
-	}
-	client := &http.Client{Transport: Wrap(transport)}
-
-	var group sync.WaitGroup
-	for range 40 {
-		for _, target := range []string{"http://proxied.example/", "http://direct.example/"} {
-			group.Add(1)
-			go func() {
-				defer group.Done()
-				request, _ := http.NewRequest(http.MethodGet, target, nil)
-				_, _ = client.Do(request)
-			}()
-		}
-	}
-	group.Wait()
-
-	mutex.Lock()
-	defer mutex.Unlock()
-	if !seen["127.0.0.1:3067"] {
-		t.Error("the proxy dial was not recorded as proxied under concurrency")
-	}
-	if seen["direct.example:80"] {
-		t.Error("a direct dial picked up another request's proxy exemption")
-	}
-	if len(seen) != 2 {
-		t.Fatalf("SETUP INVALID: dialed %v, want exactly the proxy and the direct origin", seen)
-	}
-}

internal/sandbox/profile.go

@@ -96,28 +96,15 @@ var protectedMetadataNames = []string{".git", ".zero", ".agents"}
 // gitMetadataWriteCarveouts below.
 var sandboxFullyProtectedMetadataNames = []string{".zero", ".agents"}
 
-// gitMetadataWriteCarveouts returns the .git paths that stay write-denied under
-// the OS-level sandbox even though the rest of .git is writable to git
-// subprocesses. Worktrees and submodules store .git as a regular pointer file,
-// so they protect that file itself: constructing .git/hooks below it makes
-// bwrap fail before the command starts. All other entry and error states retain
-// the legacy child paths. The pointer is deliberately not parsed here;
-// repository-controlled metadata must not redirect sandbox rules elsewhere.
+// gitMetadataWriteCarveouts returns the .git subpaths that stay write-denied
+// under the OS-level sandbox even though the rest of .git is writable to git
+// subprocesses. Nonexistent paths are harmless no-ops in every backend's
+// enforcement (seatbelt regex, bwrap ro-bind, Windows ACL deny entry).
 func gitMetadataWriteCarveouts(root string) []string {
-	return gitMetadataWriteCarveoutsWithLstat(root, os.Lstat)
-}
-
-func gitMetadataWriteCarveoutsWithLstat(root string, lstat func(string) (os.FileInfo, error)) []string {
-	gitPath := filepath.Join(root, ".git")
-	children := []string{
-		filepath.Join(gitPath, "hooks"),
-		filepath.Join(gitPath, "config"),
+	return []string{
+		filepath.Join(root, ".git", "hooks"),
+		filepath.Join(root, ".git", "config"),
 	}
-	info, err := lstat(gitPath)
-	if err != nil || !info.Mode().IsRegular() {
-		return children
-	}
-	return []string{gitPath}
 }
 
 func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile {

internal/sandbox/profile_worktree_test.go

@@ -1,115 +0,0 @@
-package sandbox
-
-import (
-	"errors"
-	"os"
-	"path/filepath"
-	"testing"
-)
-
-func mustEqualCarveouts(t *testing.T, got, want []string) {
-	t.Helper()
-	if len(got) != len(want) {
-		t.Fatalf("carveouts = %v, want %v", got, want)
-	}
-	for i := range want {
-		if got[i] != want[i] {
-			t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i])
-		}
-	}
-}
-
-// Worktree/submodule checkouts store .git as a pointer file. Protect the file
-// itself and never create a bwrap mount target beneath it.
-func TestGitMetadataWriteCarveoutsProtectsWorktreePointer(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	if err := os.WriteFile(gitPath, []byte("gitdir: ../outside\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	got := gitMetadataWriteCarveouts(root)
-	mustEqualCarveouts(t, got, []string{gitPath})
-	for _, carveout := range got {
-		if carveout == filepath.Join(gitPath, "hooks") || carveout == filepath.Join(gitPath, "config") {
-			t.Fatalf("carveout points beneath .git file: %q", carveout)
-		}
-	}
-}
-
-func TestLinuxBwrapPlanDoesNotMountBelowWorktreePointer(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	if err := os.WriteFile(gitPath, []byte("gitdir: ../outside\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	profile := PermissionProfile{FileSystem: FileSystemPolicy{
-		Kind: FileSystemRestricted,
-		WriteRoots: []WritableRoot{{
-			Root:             root,
-			ReadOnlySubpaths: gitMetadataWriteCarveouts(root),
-		}},
-	}}
-	args := linuxBwrapFilesystemArgs(profile)
-	assertArgsContainSequence(t, args, "--ro-bind", gitPath, gitPath)
-	for _, child := range []string{"hooks", "config"} {
-		bogus := filepath.Join(gitPath, child)
-		if argsContainSequence(args, "--tmpfs", bogus) || argsContainSequence(args, "--ro-bind", bogus, bogus) {
-			t.Fatalf("bwrap args contain mount below .git file %q: %v", bogus, args)
-		}
-	}
-}
-
-func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) {
-	root := t.TempDir()
-	want := []string{
-		filepath.Join(root, ".git", "hooks"),
-		filepath.Join(root, ".git", "config"),
-	}
-	mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want)
-
-	if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want)
-}
-
-func TestGitMetadataWriteCarveoutsPreservesSymlinkBehavior(t *testing.T) {
-	for _, targetKind := range []string{"directory", "file", "dangling"} {
-		t.Run(targetKind, func(t *testing.T) {
-			root := t.TempDir()
-			target := filepath.Join(root, "real.git")
-			switch targetKind {
-			case "directory":
-				if err := os.MkdirAll(target, 0o755); err != nil {
-					t.Fatal(err)
-				}
-			case "file":
-				if err := os.WriteFile(target, []byte("gitdir: ../outside\n"), 0o644); err != nil {
-					t.Fatal(err)
-				}
-			}
-			if err := os.Symlink(target, filepath.Join(root, ".git")); err != nil {
-				t.Skipf("symlink unavailable: %v", err)
-			}
-			want := []string{
-				filepath.Join(root, ".git", "hooks"),
-				filepath.Join(root, ".git", "config"),
-			}
-			mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want)
-		})
-	}
-}
-
-func TestGitMetadataWriteCarveoutsPreservesInspectionErrorBehavior(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	want := []string{
-		filepath.Join(gitPath, "hooks"),
-		filepath.Join(gitPath, "config"),
-	}
-	got := gitMetadataWriteCarveoutsWithLstat(root, func(string) (os.FileInfo, error) {
-		return nil, errors.New("inspection failed")
-	})
-	mustEqualCarveouts(t, got, want)
-}

internal/sandbox/profile_worktree_unix_test.go

@@ -1,21 +0,0 @@
-//go:build unix
-
-package sandbox
-
-import (
-	"path/filepath"
-	"syscall"
-	"testing"
-)
-
-func TestGitMetadataWriteCarveoutsPreservesSpecialGitEntryBehavior(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	if err := syscall.Mkfifo(gitPath, 0o644); err != nil {
-		t.Skip(err)
-	}
-	mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), []string{
-		filepath.Join(gitPath, "hooks"),
-		filepath.Join(gitPath, "config"),
-	})
-}

internal/tools/bash_tool_test.go

@@ -70,6 +70,9 @@ func runBashToolHelper(command string) {
 		fmt.Println("listening", listener.Addr().String())
 		server := &http.Server{
 			Handler: http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+				// Printed per request, so a test can prove the session delivers output
+				// mid-flight: this line cannot exist before the first read has happened.
+				fmt.Println("served", request.URL.Path)
 				_, _ = response.Write([]byte("zero-server-ok"))
 			}),
 		}
@@ -522,7 +525,14 @@ func TestBashToolTimeoutKillsBackgroundChildren(t *testing.T) {
 	if result.Status != StatusError {
 		t.Fatalf("expected timeout error status, got %s: %q", result.Status, result.Output)
 	}
-	if elapsed > time.Second {
+	// The regression this guards is Run blocking until the background child's own
+	// sleep ends, so the bound only has to sit under that. One second sat almost
+	// exactly on it: the call legitimately costs a sandbox plan, a fork and exec of
+	// a shell plus a subshell, the 300ms timeout itself and the post-kill drain,
+	// and spawn alone was measured here at 720ms to 1.44s with every core busy. A
+	// merely slow runner therefore failed a correct system. Three seconds is still
+	// far below the child's lifetime, so nothing is given up.
+	if elapsed > 3*time.Second {
 		t.Fatalf("Run blocked %s past the 300ms timeout; background child held the pipes", elapsed)
 	}
 

internal/tools/exec_command_test.go

@@ -3,6 +3,7 @@ package tools
 import (
 	"context"
 	"io"
+	"net"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -373,10 +374,6 @@ func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
 	if err != nil {
 		t.Fatalf("foreground server should return session_id, meta=%#v output=%q", start.Meta, start.Output)
 	}
-	addr := parseListeningAddress(start.Output)
-	if addr == "" {
-		t.Fatalf("server output did not include listening address: %q", start.Output)
-	}
 	t.Cleanup(func() {
 		writeTool.Run(context.Background(), map[string]any{
 			"session_id": sessionID,
@@ -384,7 +381,12 @@ func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
 		})
 	})
 
-	response, err := http.Get("http://" + addr)
+	addr := waitForListeningAddress(t, writeTool, sessionID, start.Output)
+
+	// Bounded, so a server that accepts a connection and never answers fails here
+	// with its address instead of hanging until the package timeout kills the run.
+	client := &http.Client{Timeout: 30 * time.Second}
+	response, err := client.Get("http://" + addr)
 	if err != nil {
 		t.Fatalf("foreground exec server was not reachable at %s: %v; output=%q", addr, err, start.Output)
 	}
@@ -396,6 +398,153 @@ func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
 	if string(bytes) != "zero-server-ok" {
 		t.Fatalf("server response = %q", string(bytes))
 	}
+
+	// AND THE SESSION DELIVERS OUTPUT MID-FLIGHT, PINNED DETERMINISTICALLY.
+	//
+	// The helper prints this line only when it serves a request, so it cannot have
+	// been in the first read: the request had not happened yet. It can only arrive
+	// through a poll of a process that is still running, which is the delivery the
+	// address parse used to cover by accident and only on a machine slow enough to
+	// miss the first read. A regression where a live session buffers everything
+	// until it exits leaves the rest of this test green and fails here.
+	deadline := time.Now().Add(60 * time.Second)
+	var served strings.Builder
+	for !strings.Contains(served.String(), "served") && time.Now().Before(deadline) {
+		poll := writeTool.Run(context.Background(), map[string]any{
+			"session_id":    sessionID,
+			"chars":         "",
+			"yield_time_ms": 250,
+		})
+		served.WriteString(poll.Output)
+		if poll.Status != StatusOK {
+			t.Fatalf("the exec session was gone before it reported serving the request:\n%s", served.String())
+		}
+	}
+	if !strings.Contains(served.String(), "served") {
+		t.Fatalf("a still-running session never delivered the line the server printed while serving:\n%s", served.String())
+	}
+
+	// AND A READ DRAINS WHAT IT RETURNS.
+	//
+	// One more poll, because "the line showed up" is satisfied just as well by
+	// output that is re-delivered forever. drain copies and then nils the buffer,
+	// so this read carries nothing; if it stopped clearing, every session in
+	// production would repeat its whole output on every poll while every other
+	// assertion here stayed green. The manager-level test that asserts the same
+	// count is skipped on Windows, so this is the only place guarding it there.
+	again := writeTool.Run(context.Background(), map[string]any{
+		"session_id":    sessionID,
+		"chars":         "",
+		"yield_time_ms": 250,
+	})
+	if strings.Contains(again.Output, "served") {
+		t.Errorf("a second read returned output the first had already delivered, so reads are no longer draining:\n%s", again.Output)
+	}
+}
+
+// waitForListeningAddress polls the exec session until the helper says where it
+// is listening.
+//
+// ONE YIELD IS AN ALLOWANCE, NOT A GUARANTEE, AND THIS TEST TREATED IT AS ONE.
+//
+// The helper is this test binary re-executed. The first read used to be its only
+// chance: 500ms to be spawned through the process manager, reach net.Listen and
+// have its line drained back. A Windows CI runner under load does not get there
+// in time, so the read returned "Command is still running." with no address and
+// the test failed as though the server were broken. It failed that way on
+// unrelated contributor PRs, which is worse than a slow test: it reports somebody
+// else's change as the fault.
+//
+// Polling is what the banner in that very output tells the caller to do, so it is
+// what this does, up to a bound far above any plausible start.
+//
+// AND IT STILL FAILS FAST WHEN THE FAILURE IS REAL. A session that has gone away
+// answers with StatusError, and a process that exited before listening answers
+// with an exit_code line. Either is a genuine defect, so neither is worth waiting
+// out: they end the loop immediately with what the session actually said.
+//
+// Each read DRAINS: managedProcess.collect returns only the bytes produced since
+// the previous call, so these chunks are incremental and none of them is the whole
+// output. Each is parsed on its own, which is sound because the helper writes its
+// address with a single Println and a write that small does not tear. The whole
+// transcript is kept anyway, so a failure shows every chunk rather than the last.
+func waitForListeningAddress(t *testing.T, writeTool Tool, sessionID int, first string) string {
+	t.Helper()
+	if addr := validListeningAddress(first); addr != "" {
+		return addr
+	}
+	// AND WHEN IT DOES NOT, THIS TEST STOPS COVERING THE FIRST READ.
+	//
+	// That parse used to be the only assertion in the repo that a still-running
+	// session's first read carries the child's stdout at all: every other test
+	// either uses a tiny yield against a helper that prints later, or uses a large
+	// one against a helper that exits, which is the collect-on-exit branch. The
+	// manager-level equivalent is skipped on Windows because it shells out to
+	// /bin/sh. The coverage cannot be pinned deterministically without
+	// reintroducing the race, since collect() blocks the whole window either way,
+	// so it is given up deliberately here and paid back after the request below,
+	// where a line that cannot exist before the first read proves the poll path.
+	transcript := []string{first}
+	// Sixty seconds, not thirty. Starting this helper was measured here at 0.74s
+	// to 4.79s with every core saturated, on a box faster than a CI runner, and
+	// windows-latest additionally scans a freshly linked test binary on its first
+	// execution. A deadline of the same order as the latency it tolerates is the
+	// mistake that produced the 500ms budget; the point is a bound, and every
+	// genuine failure below leaves before reaching it anyway.
+	deadline := time.Now().Add(60 * time.Second)
+	for time.Now().Before(deadline) {
+		poll := writeTool.Run(context.Background(), map[string]any{
+			"session_id":    sessionID,
+			"chars":         "",
+			"yield_time_ms": 250,
+		})
+		transcript = append(transcript, poll.Output)
+		if addr := validListeningAddress(poll.Output); addr != "" {
+			return addr
+		}
+		// FAST-FAIL, BECAUSE THE LOOP HAS NO PACING OF ITS OWN. All the spacing
+		// comes from collect() waiting out its yield inside a LIVE session; every
+		// error return happens in microseconds. Continue removes the process the
+		// moment it observes the exit, so without these two the loop free-runs on
+		// a dead id: measured at 3028 polls in 5 seconds, which on a 4-vCPU runner
+		// would peg a core for the whole deadline and hand t.Fatalf hundreds of
+		// kilobytes of the same sentence. The exit result is also delivered
+		// exactly once, so folding it into the blob and continuing loses the one
+		// poll that says what went wrong.
+		if poll.Status != StatusOK {
+			t.Fatalf("the exec session was gone before the server reported an address:\n%s", strings.Join(transcript, "\n--- poll ---\n"))
+		}
+		if strings.Contains(poll.Output, "exit_code:") {
+			t.Fatalf("the server process exited before it reported an address:\n%s", strings.Join(transcript, "\n--- poll ---\n"))
+		}
+	}
+	t.Fatalf("the server never reported a listening address within the deadline:\n%s", strings.Join(transcript, "\n--- poll ---\n"))
+	return ""
+}
+
+// validListeningAddress is parseListeningAddress plus the check that what it
+// found is actually an address.
+//
+// A read DRAINS, and the formatter wraps each drain in its own body, so a line
+// split across two reads arrives as two fragments with banner text wedged between
+// them and no concatenation can rejoin it. The formatter also terminates the
+// partial chunk, so the fragment looks like a whole line and the bare parse
+// returns a truncated address: the test would then fail on an unreachable port
+// and point the next reader at the network instead of at the parse. Rejecting
+// anything that is not host:port keeps polling instead.
+func validListeningAddress(output string) string {
+	addr := parseListeningAddress(output)
+	if addr == "" {
+		return ""
+	}
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil || host == "" {
+		return ""
+	}
+	if number, err := strconv.Atoi(port); err != nil || number <= 0 {
+		return ""
+	}
+	return addr
 }
 
 func parseListeningAddress(output string) string {
@@ -427,7 +576,12 @@ func TestExecCommandReapsFinishedUnpolledSession(t *testing.T) {
 		t.Fatalf("session_id is not numeric: %v", err)
 	}
 
-	deadline := time.Now().Add(2 * time.Second)
+	// Ten seconds, not two. Inside this the helper must cold-start, run its 250ms
+	// sleep, exit and pass the reap, and a cold start alone was measured at up to
+	// 1.44s under load: two seconds was a budget with almost nothing left in it. The
+	// loop returns as soon as the session is gone, so the bound only costs anything
+	// when the reap genuinely never happens.
+	deadline := time.Now().Add(10 * time.Second)
 	for {
 		if _, ok := manager.Snapshot(sessionID); !ok {
 			return
@@ -739,6 +893,13 @@ func TestExecCommandTTYSessionAcceptsInputOnLinux(t *testing.T) {
 		t.Fatalf("session_id is not numeric: %v", err)
 	}
 
+	// SAME SHAPE AS THE SERVER TEST ABOVE, one window narrower.
+	//
+	// The shell is started with a 10ms yield, so it has almost certainly not
+	// reached `read` yet, and the echo then had exactly one fixed 1000ms window to
+	// travel back through the PTY. Writing early is fine, the terminal buffers it,
+	// but a loaded runner can spend that whole second still getting the shell up.
+	// So the echo is polled for rather than demanded within one window.
 	result := writeTool.Run(context.Background(), map[string]any{
 		"session_id":    sessionID,
 		"chars":         "hello\n",
@@ -747,11 +908,39 @@ func TestExecCommandTTYSessionAcceptsInputOnLinux(t *testing.T) {
 	if result.Status != StatusOK {
 		t.Fatalf("write_stdin status = %s: %s", result.Status, result.Output)
 	}
-	if !strings.Contains(result.Output, "got:hello") {
-		t.Fatalf("expected PTY input output, got %q", result.Output)
+	var transcript strings.Builder
+	transcript.WriteString(result.Output)
+	exitCode := result.Meta["exit_code"]
+	// BOTH THE ECHO AND THE EXIT, because the test asserts both.
+	//
+	// collect can return the drained echo on a yield timer before markDone has
+	// run, so a poll can carry got:hello with no exit_code meta yet. Stopping on
+	// the echo alone would then assert an exit that had not been observed. The
+	// condition deliberately keeps the echo term as well: gating on exit_code
+	// alone would stop polling before a slow shell start delivers the echo, which
+	// is the race this whole change exists to remove.
+	deadline := time.Now().Add(30 * time.Second)
+	for (!strings.Contains(transcript.String(), "got:hello") || exitCode == "") && time.Now().Before(deadline) {
+		poll := writeTool.Run(context.Background(), map[string]any{
+			"session_id":    sessionID,
+			"chars":         "",
+			"yield_time_ms": 250,
+		})
+		transcript.WriteString(poll.Output)
+		if code := poll.Meta["exit_code"]; code != "" {
+			exitCode = code
+		}
+		if poll.Status != StatusOK {
+			// The session is gone. Either the echo already arrived, which the loop
+			// condition will see, or it never will and the assertions below say so.
+			break
+		}
 	}
-	if result.Meta["exit_code"] != "0" {
-		t.Fatalf("expected exited session, got meta=%#v output=%q", result.Meta, result.Output)
+	if !strings.Contains(transcript.String(), "got:hello") {
+		t.Fatalf("expected PTY input output, got %q", transcript.String())
+	}
+	if exitCode != "0" {
+		t.Fatalf("expected exited session, got exit_code=%q output=%q", exitCode, transcript.String())
 	}
 }
 

internal/tools/web_fetch.go

@@ -13,7 +13,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Gitlawb/zero/internal/proxydial"
 	"github.com/Gitlawb/zero/internal/redaction"
 	zeroSandbox "github.com/Gitlawb/zero/internal/sandbox"
 )
@@ -309,48 +308,19 @@ func webFetchSafeTransport(roundTripper http.RoundTripper, resolver webFetchReso
 		transport = http.DefaultTransport.(*http.Transport).Clone()
 	case *http.Transport:
 		transport = typed.Clone()
-	case *proxydial.Transport:
-		// Already guarded: rebuild from the inner transport so a second pass
-		// installs one dialer rather than nesting wrappers.
-		transport = typed.Transport.Clone()
 	default:
 		return roundTripper
 	}
 
 	dialer := &net.Dialer{Timeout: webFetchTimeout, KeepAlive: 30 * time.Second}
-	// THE PROXY IS HONOURED, AND THAT IS A DELIBERATE REVERSAL. The original
-	// guarded fetch set Proxy to nil as part of tightening the SSRF guardrails,
-	// and the reasoning was sound as far as it went: a proxied request is dialed
-	// to the proxy, so the dial-time address pin below never sees the target and
-	// the DNS-rebind protection it provides does not apply. What that traded away
-	// was every fetch on a machine that reaches the network only through a
-	// configured proxy, which fails outright rather than degrades (#569).
-	//
-	// The target is still validated before the request is made, lexically and
-	// through the resolver, with the same block list the dialer applies. What a
-	// forward proxy removes is only the re-check at dial time, and that is
-	// inherent to using any forward proxy; a user who sets HTTPS_PROXY has chosen
-	// it.
-	//
-	// The returned round tripper is wrapped so the dialer learns which route
-	// the transport chose for each request: the one dial it lets past the guard
-	// is the one this transport opens to the proxy it picked for that request.
-	transport.Proxy = http.ProxyFromEnvironment
+	transport.Proxy = nil
 	transport.DialContext = webFetchSafeDialContext(resolver, dialer)
 	transport.DialTLSContext = nil
-	return proxydial.Wrap(transport)
+	return transport
 }
 
 func webFetchSafeDialContext(resolver webFetchResolver, dialer webFetchDialer) func(context.Context, string, string) (net.Conn, error) {
 	return func(ctx context.Context, network string, address string) (net.Conn, error) {
-		// The proxy is not the target. See webFetchSafeTransport for why the
-		// proxy is honoured at all; here, the dial the transport opens to the
-		// proxy it selected for this request skips the pin, and nothing else
-		// does. A direct dial is pinned as it always was, including one whose
-		// address happens to equal a configured proxy endpoint.
-		if proxydial.IsProxyDial(ctx) {
-			return dialer.DialContext(ctx, network, address)
-		}
 		pinnedAddress, err := webFetchSafeDialAddress(ctx, resolver, address)
 		if err != nil {
 			return nil, err

internal/tools/web_fetch_proxy_test.go

@@ -1,163 +0,0 @@
-package tools
-
-import (
-	"context"
-	"errors"
-	"net"
-	"net/http"
-	"net/netip"
-	"net/url"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/Gitlawb/zero/internal/proxydial"
-)
-
-// webFetchTransportWithProxy builds the guarded web_fetch round tripper and
-// swaps in a proxy function, which is how the transport learns its route. The
-// environment is left alone deliberately: http.ProxyFromEnvironment reads it
-// once per process, so an environment-driven test would depend on test order.
-func webFetchTransportWithProxy(t *testing.T, resolver webFetchResolver, proxyFor func(*http.Request) (*url.URL, error)) *proxydial.Transport {
-	t.Helper()
-	roundTripper := webFetchSafeTransport(nil, resolver)
-	wrapped, ok := roundTripper.(*proxydial.Transport)
-	if !ok {
-		t.Fatalf("transport = %T, want the route-recording wrapper; without it no dial can be recognised as the proxy dial", roundTripper)
-	}
-	wrapped.Proxy = proxyFor
-	return wrapped
-}
-
-// THE PROXY IS NOT THE TARGET. With HTTPS_PROXY pointing at a local forward
-// proxy, the transport dials the proxy and tunnels the request through it. The
-// dialer used to refuse that dial as a loopback address, so every fetch on a
-// machine behind such a proxy failed with "loopback hosts are blocked" for a
-// public target that had already been validated (#569).
-func TestWebFetchTunnelsThroughTheConfiguredProxy(t *testing.T) {
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer listener.Close()
-	arrived := make(chan string, 1)
-	go func() {
-		conn, err := listener.Accept()
-		if err != nil {
-			return
-		}
-		defer conn.Close()
-		buffer := make([]byte, 256)
-		n, _ := conn.Read(buffer)
-		arrived <- string(buffer[:n])
-	}()
-	proxyURL, err := url.Parse("http://" + listener.Addr().String())
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// A proxied fetch never resolves the target on this side; the CONNECT
-	// carries the hostname and the proxy resolves it.
-	transport := webFetchTransportWithProxy(t,
-		webFetchResolverFunc(func(context.Context, string, string) ([]netip.Addr, error) {
-			return nil, errors.New("resolver must not be called for a proxied fetch")
-		}),
-		func(*http.Request) (*url.URL, error) { return proxyURL, nil })
-	client := &http.Client{Transport: transport, Timeout: 3 * time.Second}
-
-	_, err = client.Get("https://public.example/resource")
-	select {
-	case first := <-arrived:
-		if line := strings.SplitN(first, "\r\n", 2)[0]; line != "CONNECT public.example:443 HTTP/1.1" {
-			t.Fatalf("proxy received %q, want a CONNECT for the target", line)
-		}
-	case <-time.After(5 * time.Second):
-		t.Fatalf("the proxy never received a connection; request error: %v", err)
-	}
-}
-
-// AND A DIRECT FETCH STAYS PINNED, EVEN WHEN ITS ORIGIN IS THE PROXY'S OWN
-// ADDRESS.
-//
-// This is the NO_PROXY shape: the proxy function sends one host direct. An
-// exemption derived from asking that function about a request of our own
-// invention would answer "proxied" for the excluded host too, and this dial
-// would skip the pin and reach whatever the name resolves to at connect time.
-func TestWebFetchDirectRequestToTheProxyEndpointIsStillPinned(t *testing.T) {
-	proxyURL, err := url.Parse("http://rebind.example:80")
-	if err != nil {
-		t.Fatal(err)
-	}
-	lookups := 0
-	transport := webFetchTransportWithProxy(t,
-		webFetchResolverFunc(func(_ context.Context, _ string, host string) ([]netip.Addr, error) {
-			lookups++
-			return []netip.Addr{netip.MustParseAddr("127.0.0.1")}, nil
-		}),
-		func(request *http.Request) (*url.URL, error) {
-			if request.URL.Hostname() == "rebind.example" {
-				return nil, nil
-			}
-			return proxyURL, nil
-		})
-	client := &http.Client{Transport: transport, Timeout: 3 * time.Second}
-
-	_, err = client.Get("http://rebind.example/data")
-
-	if lookups == 0 {
-		t.Fatal("the direct dial skipped the address pin because its address matched the configured proxy endpoint")
-	}
-	if err == nil || !strings.Contains(err.Error(), "loopback hosts are blocked") {
-		t.Fatalf("err = %v, want the private answer to be refused", err)
-	}
-}
-
-// A proxy configured with a Unicode hostname is dialed by its IDNA-encoded
-// name, and it is still the proxy.
-func TestWebFetchUnicodeProxyHostnameIsStillTheProxy(t *testing.T) {
-	dialed := ""
-	transport := webFetchTransportWithProxy(t,
-		webFetchResolverFunc(func(context.Context, string, string) ([]netip.Addr, error) {
-			return nil, errors.New("resolver must not be called for a proxied fetch")
-		}),
-		func(*http.Request) (*url.URL, error) { return url.Parse("http://büro.example:3067") })
-	guarded := transport.DialContext
-	transport.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) {
-		dialed = address
-		return guarded(ctx, network, address)
-	}
-	client := &http.Client{Transport: transport, Timeout: 3 * time.Second}
-
-	_, err := client.Get("https://public.example/resource")
-
-	if dialed != "xn--bro-hoa.example:3067" {
-		t.Fatalf("SETUP INVALID: dialed %q, want the IDNA-encoded proxy endpoint", dialed)
-	}
-	if err != nil && strings.Contains(err.Error(), "hosts are blocked") {
-		t.Fatalf("the configured proxy was refused by the guard: %v", err)
-	}
-}
-
-// Without a proxy the guard is exactly what it was: loopback is refused and
-// the dialer never runs.
-func TestWebFetchSafeDialStillRefusesLoopbackWithoutAProxy(t *testing.T) {
-	dialCalled := false
-	dial := webFetchSafeDialContext(
-		webFetchResolverFunc(func(_ context.Context, network string, host string) ([]netip.Addr, error) {
-			t.Fatalf("resolver consulted for a loopback literal: network=%q host=%q", network, host)
-			return nil, nil
-		}),
-		webFetchDialFunc(func(context.Context, string, string) (net.Conn, error) {
-			dialCalled = true
-			return nil, errors.New("dial should not run")
-		}),
-	)
-
-	_, err := dial(context.Background(), "tcp", "127.0.0.1:3067")
-	if err == nil || !strings.Contains(err.Error(), "loopback hosts are blocked") {
-		t.Fatalf("expected loopback rejection, got %v", err)
-	}
-	if dialCalled {
-		t.Fatal("loopback was dialed with no proxy configured")
-	}
-}

internal/tools/web_fetch_test.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/Gitlawb/zero/internal/proxydial"
 	zeroSandbox "github.com/Gitlawb/zero/internal/sandbox"
 )
 
@@ -326,26 +325,15 @@ func TestWebFetchToolConfiguresDialTimeSafetyForDefaultTransport(t *testing.T) {
 	}
 
 	client := tool.clientForRun()
-	// The round tripper is the route-recording wrapper, not the bare transport:
-	// it is what tells the dialer which dial is the one to the proxy the
-	// transport picked for a given request. The guarded transport it wraps
-	// stays reachable, and is what the rest of this test inspects.
-	transport, ok := client.Transport.(*proxydial.Transport)
+	transport, ok := client.Transport.(*http.Transport)
 	if !ok {
-		t.Fatalf("client transport = %T, want *proxydial.Transport", client.Transport)
+		t.Fatalf("client transport = %T, want *http.Transport", client.Transport)
 	}
 	if transport.DialContext == nil {
 		t.Fatal("expected web_fetch transport to install a safe DialContext")
 	}
-	// REVERSED ON PURPOSE. The guarded fetch used to pin Proxy to nil as part of
-	// the SSRF hardening, and this assertion held it there. #569 is what that
-	// cost: on a machine that reaches the network only through a configured
-	// forward proxy, every fetch failed outright. The proxy is honoured now, and
-	// the wrapper hands the dialer the route chosen for each request so the one
-	// dial it lets past the guard is the one to that proxy. The target is still
-	// validated before the request, which the tests below pin.
-	if transport.Proxy == nil {
-		t.Fatal("expected web_fetch transport to honour the environment proxy; a machine behind a forward proxy cannot fetch at all without it (#569)")
+	if transport.Proxy != nil {
+		t.Fatal("expected web_fetch transport to disable proxy resolution")
 	}
 }