mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 17:56:14 -10:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Vasanthdev2004
|
9371890bf4 | ||
|
Vasanthdev2004
|
94695aaad7 |
No files matched your search
@@ -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
|
||||
|
||||
@@ -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`.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -160,7 +160,8 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
|
||||
// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
|
||||
// FileTracker re-baseline: recording pre-format content would make the very
|
||||
// next edit look like an external modification and trip the conflict guard.
|
||||
updated = maybeFormatWrittenFile(ctx, absolutePath, updated)
|
||||
formatting := maybeFormatWrittenFile(ctx, absolutePath, updated)
|
||||
updated = formatting.Content
|
||||
// Re-baseline to the content we just wrote so subsequent edits in this session
|
||||
// compare against the current on-disk state, not the pre-edit version.
|
||||
newInfo, _ := os.Stat(absolutePath)
|
||||
@@ -193,6 +194,7 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
|
||||
suffix = "s"
|
||||
}
|
||||
summary := fmt.Sprintf("Successfully edited %s (replaced %d occurrence%s).", relativePath, replacedCount, suffix)
|
||||
summary += formatting.notice(relativePath)
|
||||
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
||||
result := okResult(summary)
|
||||
result.ChangedFiles = []string{relativePath}
|
||||
|
||||
@@ -2,6 +2,7 @@ package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -22,8 +23,59 @@ import (
|
||||
// the conflict guard.
|
||||
|
||||
// formatOnWriteTimeout bounds one formatter run; a wedged formatter must never
|
||||
// hang a tool call. On timeout the unformatted write stands.
|
||||
const formatOnWriteTimeout = 10 * time.Second
|
||||
// hang a tool call. On timeout the unformatted write stands, and the caller
|
||||
// says so: see formatOnWriteResult.
|
||||
//
|
||||
// A var rather than a const so a test can shorten it. Nothing outside a test
|
||||
// assigns to it, and the deadline path was previously unreachable in a test at
|
||||
// any speed, which is part of why it went unnoticed that it reported nothing.
|
||||
var formatOnWriteTimeout = 10 * time.Second
|
||||
|
||||
// formatOnWriteResult is the content after formatting, plus whether the
|
||||
// formatting that was supposed to happen actually did.
|
||||
//
|
||||
// A TIMEOUT IS NOT THE SAME KIND OF MISS AS THE OTHERS. Every other way this
|
||||
// falls back is a standing fact about the environment: the toggle is off, the
|
||||
// extension has no formatter, the binary is not installed. Those are silent on
|
||||
// purpose, because nothing is wrong and saying so on every write would be
|
||||
// noise. A deadline firing is different: formatting was configured, available
|
||||
// and expected, and the file was written unformatted anyway, on a machine that
|
||||
// was merely slow. Left silent, the caller believes it wrote canonical style
|
||||
// and finds out from a CI format check it cannot see, which is the thing this
|
||||
// feature exists to prevent.
|
||||
type formatOnWriteResult struct {
|
||||
Content string
|
||||
// Formatter is the binary that was run, named in the notice so the user can
|
||||
// tell a slow gofmt from a slow prettier.
|
||||
Formatter string
|
||||
TimedOut bool
|
||||
// RestoreFailed means the file on disk is not known to hold Content.
|
||||
//
|
||||
// These formatters edit in place, so one that is killed or fails partway
|
||||
// can leave the target truncated or half-rewritten: what a dead
|
||||
// `prettier --write` leaves behind is not the input and not the output.
|
||||
// Returning the written bytes while disk holds something else would put the
|
||||
// tracker baseline, the diff preview and the file itself into three
|
||||
// different states, so the failure paths write the bytes back. When even
|
||||
// that fails the user has to hear about it: it is their file.
|
||||
RestoreFailed bool
|
||||
}
|
||||
|
||||
// notice is the line appended to the tool summary when formatting was expected
|
||||
// and did not happen, and empty in every other case.
|
||||
func (result formatOnWriteResult) notice(relativePath string) string {
|
||||
if result.RestoreFailed {
|
||||
return "\n\nWARNING: " + relativePath + " may not hold what was written. " +
|
||||
result.Formatter + " was interrupted while rewriting it in place and the " +
|
||||
"content could not be written back. Re-read the file before trusting it."
|
||||
}
|
||||
if !result.TimedOut {
|
||||
return ""
|
||||
}
|
||||
return "\n\nNote: " + relativePath + " was written but not formatted: " +
|
||||
result.Formatter + " did not finish within " + formatOnWriteTimeout.String() +
|
||||
". The file holds exactly what was written, so a project format check may still flag it."
|
||||
}
|
||||
|
||||
// formatterCommands maps a file extension to the formatter argv; the file path
|
||||
// is appended as the final argument. Only in-place, config-respecting,
|
||||
@@ -69,18 +121,20 @@ func formatOnWriteEnabled() bool {
|
||||
// enabled and on PATH) and returns the file's content afterwards. Best-effort
|
||||
// throughout: any failure — no formatter, formatter error, timeout, unreadable
|
||||
// result — returns writtenContent so the caller's state matches the last write
|
||||
// it performed itself.
|
||||
func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenContent string) string {
|
||||
// it performed itself. Only the timeout is reported back, for the reason on
|
||||
// formatOnWriteResult.
|
||||
func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenContent string) formatOnWriteResult {
|
||||
unformatted := formatOnWriteResult{Content: writtenContent}
|
||||
if !formatOnWriteEnabled() {
|
||||
return writtenContent
|
||||
return unformatted
|
||||
}
|
||||
command, ok := formatterCommands[strings.ToLower(filepath.Ext(absolutePath))]
|
||||
if !ok {
|
||||
return writtenContent
|
||||
return unformatted
|
||||
}
|
||||
binaryPath, err := exec.LookPath(command[0])
|
||||
if err != nil {
|
||||
return writtenContent
|
||||
return unformatted
|
||||
}
|
||||
formatCtx, cancel := context.WithTimeout(ctx, formatOnWriteTimeout)
|
||||
defer cancel()
|
||||
@@ -89,11 +143,35 @@ func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenCon
|
||||
formatter.Dir = filepath.Dir(absolutePath)
|
||||
formatter.Stdin = strings.NewReader("")
|
||||
if err := formatter.Run(); err != nil {
|
||||
return writtenContent
|
||||
unformatted.Formatter = command[0]
|
||||
// THE FORMATTER EDITS IN PLACE, SO A FAILED RUN CAN LEAVE THE FILE
|
||||
// NEITHER FORMATTED NOR AS WRITTEN. Killed by the deadline or by the
|
||||
// caller, or exiting partway through its own rewrite, the target can hold
|
||||
// a truncation. Returning the written bytes on top of that would leave the
|
||||
// tracker baseline and the diff preview describing a file that is not on
|
||||
// disk, which is a worse failure than the missing formatting: the next
|
||||
// edit compares against content the file does not have.
|
||||
//
|
||||
// Written back unconditionally on this path rather than only when the
|
||||
// bytes differ. Comparing first means reading the file to find out, and a
|
||||
// read that fails leaves the same ambiguity this exists to remove.
|
||||
if restoreErr := os.WriteFile(absolutePath, []byte(writtenContent), 0o644); restoreErr != nil {
|
||||
unformatted.RestoreFailed = true
|
||||
}
|
||||
// OUR deadline, not the caller's cancellation and not the formatter's own
|
||||
// exit status. A cancelled tool call is already being reported as
|
||||
// cancelled, and a formatter that ran and refused the file usually means
|
||||
// content it could not parse, which the write itself does not promise to
|
||||
// fix. Neither is this notice's business; the restore above is, for all
|
||||
// three.
|
||||
if errors.Is(formatCtx.Err(), context.DeadlineExceeded) && ctx.Err() == nil {
|
||||
unformatted.TimedOut = true
|
||||
}
|
||||
return unformatted
|
||||
}
|
||||
formatted, err := os.ReadFile(absolutePath)
|
||||
if err != nil {
|
||||
return writtenContent
|
||||
return unformatted
|
||||
}
|
||||
return string(formatted)
|
||||
return formatOnWriteResult{Content: string(formatted), Formatter: command[0]}
|
||||
}
|
||||
@@ -97,9 +97,12 @@ func TestFormatOnWriteFormatsAndKeepsTrackerConsistent(t *testing.T) {
|
||||
|
||||
func TestFormatOnWriteSkipsUnknownExtensions(t *testing.T) {
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
content := maybeFormatWrittenFile(context.Background(), filepath.Join(t.TempDir(), "notes.xyz"), "raw text")
|
||||
if content != "raw text" {
|
||||
t.Fatalf("unknown extension must pass through: %q", content)
|
||||
formatting := maybeFormatWrittenFile(context.Background(), filepath.Join(t.TempDir(), "notes.xyz"), "raw text")
|
||||
if formatting.Content != "raw text" {
|
||||
t.Fatalf("unknown extension must pass through: %q", formatting.Content)
|
||||
}
|
||||
if notice := formatting.notice("notes.xyz"); notice != "" {
|
||||
t.Fatalf("an extension with no formatter is not a miss worth reporting, got %q", notice)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -111,8 +114,11 @@ func TestFormatOnWriteFormatterLookupFailure(t *testing.T) {
|
||||
if err := os.WriteFile(targetPath, []byte(uglyContent), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
content := maybeFormatWrittenFile(context.Background(), targetPath, uglyContent)
|
||||
if content != uglyContent {
|
||||
t.Fatalf("missing formatter must return written content, got %q", content)
|
||||
formatting := maybeFormatWrittenFile(context.Background(), targetPath, uglyContent)
|
||||
if formatting.Content != uglyContent {
|
||||
t.Fatalf("missing formatter must return written content, got %q", formatting.Content)
|
||||
}
|
||||
if notice := formatting.notice("a.go"); notice != "" {
|
||||
t.Fatalf("an uninstalled formatter is a standing fact, not a miss worth reporting, got %q", notice)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,280 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// A FORMATTER THAT RUNS OUT OF TIME MUST NOT LOOK LIKE ONE THAT SUCCEEDED.
|
||||
//
|
||||
// Every other way format-on-write falls back is a standing fact about the
|
||||
// environment: the toggle is off, the extension has no formatter, the binary is
|
||||
// not installed. Those are silent because nothing is wrong. A deadline firing is
|
||||
// different: formatting was configured, available and expected, and the file was
|
||||
// written unformatted anyway because the machine was slow. Silent, the caller
|
||||
// believes it wrote canonical style and learns otherwise from a CI format check
|
||||
// it cannot see, which is the failure this feature exists to prevent.
|
||||
//
|
||||
// THE DEADLINE IS SHRUNK RATHER THAN THE FORMATTER SLOWED. A formatter that
|
||||
// really sleeps has to be killed, and on Windows the batch file that hosts the
|
||||
// sleep leaves the sleeping grandchild holding the working directory, so the
|
||||
// test then fails in TempDir cleanup rather than on anything it asserts. An
|
||||
// expired budget against an ordinary fast formatter reaches the same branch by
|
||||
// the same route, deterministically and in microseconds.
|
||||
func TestFormatOnWriteReportsATimeout(t *testing.T) {
|
||||
formatter := installFakeFormatter(t, ".fastfmt", "fastfmt", succeedingFormatterScript())
|
||||
shortenFormatOnWriteTimeout(t, time.Nanosecond)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
|
||||
target := filepath.Join(t.TempDir(), "subject.fastfmt")
|
||||
const written = "written but not formatted\n"
|
||||
if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), target, written)
|
||||
|
||||
if formatting.Content != written {
|
||||
t.Fatalf("content = %q, want the bytes that were written", formatting.Content)
|
||||
}
|
||||
if !formatting.TimedOut {
|
||||
t.Fatal("a formatter cut off by the deadline was reported as an ordinary miss, so the write claims a formatting that never happened")
|
||||
}
|
||||
notice := formatting.notice("subject.fastfmt")
|
||||
for _, want := range []string{"subject.fastfmt", "not formatted", formatter} {
|
||||
if !strings.Contains(notice, want) {
|
||||
t.Errorf("notice %q does not mention %q", notice, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// And the same formatter inside its budget says nothing at all, or the notice
|
||||
// above would be reporting the deadline rather than the miss.
|
||||
func TestFormatOnWriteStaysQuietWhenTheFormatterFinishes(t *testing.T) {
|
||||
installFakeFormatter(t, ".fastfmt", "fastfmt", succeedingFormatterScript())
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
|
||||
target := filepath.Join(t.TempDir(), "subject.fastfmt")
|
||||
const written = "written\n"
|
||||
if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), target, written)
|
||||
if formatting.TimedOut {
|
||||
t.Error("a formatter that finished was reported as timed out")
|
||||
}
|
||||
if notice := formatting.notice("subject.fastfmt"); notice != "" {
|
||||
t.Errorf("a successful format produced a notice: %q", notice)
|
||||
}
|
||||
}
|
||||
|
||||
// The caller cancelling is not this notice's business: that run is already
|
||||
// being reported as cancelled, and saying the formatter was too slow on top of
|
||||
// it would be wrong about why.
|
||||
func TestFormatOnWriteStaysQuietWhenTheCallerCancels(t *testing.T) {
|
||||
installFakeFormatter(t, ".fastfmt", "fastfmt", succeedingFormatterScript())
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
|
||||
target := filepath.Join(t.TempDir(), "subject.fastfmt")
|
||||
const written = "written\n"
|
||||
if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
formatting := maybeFormatWrittenFile(ctx, target, written)
|
||||
|
||||
if formatting.Content != written {
|
||||
t.Fatalf("content = %q, want the bytes that were written", formatting.Content)
|
||||
}
|
||||
if formatting.TimedOut {
|
||||
t.Error("a cancelled run was reported as a formatter timeout")
|
||||
}
|
||||
if notice := formatting.notice("subject.fastfmt"); notice != "" {
|
||||
t.Errorf("a cancelled run produced a notice: %q", notice)
|
||||
}
|
||||
}
|
||||
|
||||
// A formatter that runs and refuses the file is also not a timeout. It usually
|
||||
// means content it could not parse, which the write does not promise to fix,
|
||||
// and reporting it as a slow machine would be wrong about the cause.
|
||||
func TestFormatOnWriteStaysQuietWhenTheFormatterFails(t *testing.T) {
|
||||
installFakeFormatter(t, ".failfmt", "failfmt", failingFormatterScript())
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
|
||||
target := filepath.Join(t.TempDir(), "subject.failfmt")
|
||||
const written = "unparseable\n"
|
||||
if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), target, written)
|
||||
if formatting.TimedOut {
|
||||
t.Error("a formatter that exited non-zero was reported as a timeout")
|
||||
}
|
||||
if notice := formatting.notice("subject.failfmt"); notice != "" {
|
||||
t.Errorf("a failing formatter produced a timeout notice: %q", notice)
|
||||
}
|
||||
}
|
||||
|
||||
// installFakeFormatter puts a formatter on PATH and registers it in the command
|
||||
// table for the test's duration, returning the binary name the notice carries.
|
||||
func installFakeFormatter(t *testing.T, extension, name, script string) string {
|
||||
t.Helper()
|
||||
binaryName := name + formatterScriptExtension()
|
||||
directory := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(directory, binaryName), []byte(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("PATH", directory+string(os.PathListSeparator)+os.Getenv("PATH"))
|
||||
|
||||
previous, existed := formatterCommands[extension]
|
||||
formatterCommands[extension] = []string{binaryName}
|
||||
t.Cleanup(func() {
|
||||
if existed {
|
||||
formatterCommands[extension] = previous
|
||||
return
|
||||
}
|
||||
delete(formatterCommands, extension)
|
||||
})
|
||||
return binaryName
|
||||
}
|
||||
|
||||
func formatterScriptExtension() string {
|
||||
if runtime.GOOS == "windows" {
|
||||
return ".bat"
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func succeedingFormatterScript() string {
|
||||
if runtime.GOOS == "windows" {
|
||||
return "@echo off\r\nexit /b 0\r\n"
|
||||
}
|
||||
return "#!/bin/sh\nexit 0\n"
|
||||
}
|
||||
|
||||
func failingFormatterScript() string {
|
||||
if runtime.GOOS == "windows" {
|
||||
return "@echo off\r\nexit /b 3\r\n"
|
||||
}
|
||||
return "#!/bin/sh\nexit 3\n"
|
||||
}
|
||||
|
||||
// shortenFormatOnWriteTimeout narrows the production deadline for one test.
|
||||
func shortenFormatOnWriteTimeout(t *testing.T, timeout time.Duration) {
|
||||
t.Helper()
|
||||
previous := formatOnWriteTimeout
|
||||
formatOnWriteTimeout = timeout
|
||||
t.Cleanup(func() { formatOnWriteTimeout = previous })
|
||||
}
|
||||
|
||||
// A FAILED FORMATTER MUST NOT LEAVE THE FILE HALF-REWRITTEN.
|
||||
//
|
||||
// These commands edit in place, so one killed by the deadline, killed by the
|
||||
// caller, or exiting partway through its own rewrite can leave the target
|
||||
// truncated: neither the input nor the output. Returning the written bytes on
|
||||
// top of that would leave the tracker baseline and the diff preview describing
|
||||
// a file that is not on disk, and the next edit would compare against content
|
||||
// the file does not have.
|
||||
func TestFormatOnWriteRestoresTheFileWhenTheFormatterFails(t *testing.T) {
|
||||
installFakeFormatter(t, ".clobberfmt", "clobberfmt", clobberingFormatterScript())
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
|
||||
target := filepath.Join(t.TempDir(), "subject.clobberfmt")
|
||||
const written = "the bytes the caller wrote\n"
|
||||
if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
requireFormatterClobbers(t, written)
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), target, written)
|
||||
|
||||
onDisk, err := os.ReadFile(target)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(onDisk) != written {
|
||||
t.Errorf("file on disk = %q, want the bytes that were written back", onDisk)
|
||||
}
|
||||
if formatting.Content != written {
|
||||
t.Errorf("content = %q, want the bytes that were written", formatting.Content)
|
||||
}
|
||||
if formatting.RestoreFailed {
|
||||
t.Error("restoration was reported as failed on a writable file")
|
||||
}
|
||||
}
|
||||
|
||||
// And the same for a run cut off by the deadline, which is the case the notice
|
||||
// already covers: the disclosure and the file have to agree.
|
||||
func TestFormatOnWriteRestoresTheFileOnTimeout(t *testing.T) {
|
||||
installFakeFormatter(t, ".clobberfmt", "clobberfmt", clobberingFormatterScript())
|
||||
shortenFormatOnWriteTimeout(t, time.Nanosecond)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
|
||||
target := filepath.Join(t.TempDir(), "subject.clobberfmt")
|
||||
const written = "the bytes the caller wrote\n"
|
||||
if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
requireFormatterClobbers(t, written)
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), target, written)
|
||||
|
||||
onDisk, err := os.ReadFile(target)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(onDisk) != written {
|
||||
t.Errorf("file on disk = %q, want the bytes that were written back", onDisk)
|
||||
}
|
||||
if !formatting.TimedOut {
|
||||
t.Error("the deadline path stopped being reported once restoration was added")
|
||||
}
|
||||
if notice := formatting.notice("subject.clobberfmt"); !strings.Contains(notice, "not formatted") {
|
||||
t.Errorf("notice = %q, want the timeout note", notice)
|
||||
}
|
||||
}
|
||||
|
||||
// clobberingFormatterScript truncates the file it is handed and then fails, the
|
||||
// way an interrupted in-place formatter leaves a partial rewrite.
|
||||
func clobberingFormatterScript() string {
|
||||
if runtime.GOOS == "windows" {
|
||||
return "@echo off\r\necho CLOBBERED> %1\r\nexit /b 3\r\n"
|
||||
}
|
||||
return "#!/bin/sh\necho CLOBBERED > \"$1\"\nexit 3\n"
|
||||
}
|
||||
|
||||
// requireFormatterClobbers proves the fixture really does damage the file it is
|
||||
// handed, on a throwaway copy.
|
||||
//
|
||||
// It cannot be checked on the real target: restoration is the behaviour under
|
||||
// test, so when it works the evidence is gone, and asserting on the target
|
||||
// afterwards would either pass vacuously or report the opposite of what it saw.
|
||||
func requireFormatterClobbers(t *testing.T, written string) {
|
||||
t.Helper()
|
||||
probe := filepath.Join(t.TempDir(), "probe.clobberfmt")
|
||||
if err := os.WriteFile(probe, []byte(written), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
binary, err := exec.LookPath("clobberfmt" + formatterScriptExtension())
|
||||
if err != nil {
|
||||
t.Fatalf("SETUP INVALID: the fake formatter is not on PATH: %v", err)
|
||||
}
|
||||
_ = exec.Command(binary, probe).Run()
|
||||
after, err := os.ReadFile(probe)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(string(after), "CLOBBERED") {
|
||||
t.Fatalf("SETUP INVALID: the fake formatter left %q, so it does not damage the file and restoration is not under test", after)
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -114,7 +114,8 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
|
||||
// FileTracker baseline: recording pre-format content would make the very
|
||||
// next edit look like an external modification and trip the conflict guard.
|
||||
content = maybeFormatWrittenFile(ctx, absolutePath, content)
|
||||
formatting := maybeFormatWrittenFile(ctx, absolutePath, content)
|
||||
content = formatting.Content
|
||||
// Baseline the freshly written content so a later edit/overwrite in this
|
||||
// session compares against what is now on disk.
|
||||
newInfo, _ := os.Stat(absolutePath)
|
||||
@@ -137,6 +138,7 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
lines++
|
||||
}
|
||||
summary := fmt.Sprintf("%s %s (%d lines).", verb, relativePath, lines)
|
||||
summary += formatting.notice(relativePath)
|
||||
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
||||
result := okResult(summary)
|
||||
result.ChangedFiles = []string{relativePath}
|
||||
|
||||
Reference in new issue
Block a user