Compare commits

..
Author SHA1 Message Date
Vasanthdev2004 221fb93041 fix(proxydial): bind the proxy exemption to the route the transport chose
Asking the Proxy function a question of our own invention answered 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, so a direct dial whose address happened to equal the configured
proxy endpoint skipped the guard and reached whatever the name resolved to
at connect time. A probe on the previous head dials rebind.example:80 with
zero safety lookups; the guard now runs and refuses its private answer.
Host-dependent proxy callbacks make the mismatch general, and no spelling
of the probe host closes it, because the two calls are about different
requests.

The decision is now recorded where it is made. proxydial.Transport wraps
the transport, asks its Proxy function about the actual request, and puts
the answer in that request's context; 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 the context key is unexported so nothing
outside the package can plant one.

Address matching is gone with it. A transport opens one connection dial,
and when a proxy is selected that dial goes to the proxy, so the recorded
route already identifies the dial. Comparing the address on top could only
disagree with the transport, which is what a Unicode proxy hostname did:
Go dials xn--bro-hoa.example while the URL still spells it as configured,
and the proxy was refused as an ordinary private address. Both spellings
are now one endpoint with no comparison to get wrong.

Documents where destination enforcement ends: with a proxy configured, the
target hostname is sent to the proxy and the proxy resolves it, so the
local pre-request checks no longer bind the address that is reached. A
forward proxy already sees and can rewrite every request through it, so
that trust comes with configuring one; the alternative is refusing to use
proxies at all, which is the outage in #569.
2026-09-09 11:03:14 +05:30
Vasanthdev2004 ac3958f860 fix(proxydial): have the dialer ask its transport for the proxy at dial time
Copying transport.Proxy into the dialer at construction left the wiring
between the two unpinned: a dialer handed nil would pass every unit test
while a proxied request still died as loopback. The dialer now consults
the transport it is installed on, and each package drives a real request
through a loopback listener acting as the proxy and checks that the
CONNECT arrives. The proxy is substituted on the transport rather than
through HTTPS_PROXY, since http.ProxyFromEnvironment reads the
environment once per process and an env-based test would depend on
test order.
2026-09-08 10:04:41 +05:30
Vasanthdev2004andbaoyu0 63767f3e42 fix(providerhealth,tools): let the configured forward proxy through the loopback guard
With HTTPS_PROXY pointing at a local forward proxy, the transport dials the
proxy first and tunnels the request through it. The SSRF-guarded dialer in
providerhealth refused that dial as a loopback address, so
`zero doctor --connectivity` failed with "proxyconnect tcp: provider
connectivity URL is unsafe: loopback hosts are blocked" for a public
provider that had already been validated, and web_fetch had no proxy
support at all (#569).

Each dialer is now handed its transport's own Proxy function and lets a dial
through unpinned only when the address is exactly the proxy endpoint that
function names for the request scheme: same host, same port, with a missing
proxy port taken as the scheme default. A sibling loopback port is still
refused. The shared check lives in internal/proxydial since providerhealth
and tools otherwise share nothing.

web_fetch honours http.ProxyFromEnvironment again. The guarded fetch had set
Proxy to nil as part of its SSRF hardening; the target is still validated
before the request, and what a forward proxy removes is only the dial-time
rebind pin, which is inherent to using any forward proxy. The pre-permission
URL validation stays in place.

Started from #594 by baoyu0. This version keeps the pre-permission check,
matches the proxy port exactly instead of any port when the proxy URL has
none, and injects the transport's Proxy function instead of reading the
environment a second time.

Closes #569

Co-authored-by: baoyu0 <[email protected]>
2026-09-08 09:59:50 +05:30
f30f550e60 fix(sandbox): protect worktree git pointers (#805)
* fix(sandbox): resolve git metadata carveouts for worktrees

.git is a file (gitdir: ...) in worktrees/submodules, not a dir. The
hooks/config write-deny carveout assumed a directory and pointed at
<root>/.git/hooks, which has no mkdir-able parent under the .git file.
bwrap then failed 'Can't mkdir parents ... Not a directory', blocking
every sandboxed tool in worktree checkouts.

Resolve the real (common) git dir via the gitdir pointer + commondir so
worktrees deny the shared hooks/config like a plain clone. Plain
checkouts (.git dir) and .git-absent roots keep the literal paths.

* fix(sandbox): bound worktree git metadata carveouts to the write root

Refuse gitdir/commondir pointers that escape the workspace instead of
carving outside confinement. Invalid, stale, or non-directory pointers
return no carveouts so bwrap does not mkdir under a .git file.

* fix(sandbox): evaluate gitdir/commondir through symlinks

Containment must use the real path so an in-root symlink cannot punch
hooks/config carveouts outside the write root.

* fix(sandbox): fail closed on gitdir eval and protect the pointer file

Refuse carveouts when EvalSymlinks cannot produce a real in-root path,
compare evaluated roots and targets, and write-deny the .git pointer so
it cannot redirect hooks/config between commands.

* fix(sandbox): deny worktree-local gitdir metadata and parse first gitdir line

Keep hooks/config write-denied on both the common dir and the in-root
worktree gitdir, emit those paths under the lexical write root, and
accept a gitdir: pointer that has trailing comment lines.

* fix(sandbox): skip special git metadata files and re-check carveouts at apply

Read .git and commondir only when they are regular files so a FIFO cannot
block profile construction. At sandbox apply, drop ReadOnlySubpaths that
no longer sit inside the write root after a symlink swap.

* fix(sandbox): follow directory .git symlinks and unix-tag FIFO tests

A .git symlink to a directory is a normal checkout, so keep lexical
hooks/config carveouts. FIFO coverage stays on unix so Windows still
compiles.

* fix(sandbox): protect worktree git pointer directly

* test(sandbox): skip unavailable symlink fixture

* fix(sandbox): preserve git symlink semantics

* fix(sandbox): limit git pointer carveout to regular files

---------

Co-authored-by: jatmn <[email protected]>
Co-authored-by: jatmn <[email protected]>
2026-09-08 07:54:22 +08:00
Vasanth T aadb4a27e9 fix(specialist): keep the pinned model when a specialist is resumed (#1009)
* fix(specialist): keep the pinned model when a specialist is resumed

Metadata.Model exists so a bounded, delegated task can run on a cheaper model
than its parent, and BuildArgs appends it to the child argv through
appendModelArgs. BuildResumeArgs never did.

So a specialist pinned to a cheap model ran on that model exactly once. The
moment the orchestrator resumed it, the child fell back to whatever the
parent's configured model resolved to. Nothing surfaced it: the resumed child
starts normally and does the work, so the only symptom is the bill.
Cost-motivated delegation quietly stopped saving anything.

Resuming does not restore the recorded model on its own. sessions.PrepareExec
records the model a run used but never feeds it back into provider
construction, so the flag has to be passed again rather than relied upon.

BuildResumeArgsInput now carries ParentModel and ParentReasoningEffort, the
same fallbacks the fresh path takes, and runResume passes what TaskRunOptions
already held. The reasoning-effort rule travels with the model unchanged: the
parent's effort is inherited only when the manifest pins no model of its own,
because a manifest that chose a different model has not agreed to the parent's
effort for it.

Regressions drive both builders and compare them, so the two paths cannot
drift again: a pinned model survives resume, an unpinned one still inherits the
parent's, both halves of the effort rule hold, and the flag keeps its position
relative to --auto in both.

Refs #554

* test(specialist): cover the resume call site, not just its builder

The builder tests all call BuildResumeArgs directly, so dropping the
ParentModel field from the runResume call site still compiled and still passed
every one of them. The defect this fixes lived at the call site, so it needs a
test that goes through Run.

Driven through the real dispatch with the RunChild seam capturing argv.

* test(specialist): guard the fresh call site as well

runFresh and runResume each construct their builder input by hand and carry a
byte-identical ParentModel line. Deleting either compiles and, until now,
deleting the fresh one was silent.

The resume half is what this branch repairs. This covers the other half so the
pair cannot drift again in the direction nobody was looking.
2026-09-07 16:02:39 +05:30
KRATOS a6ec0435e3 feat(acp): surface safe browser tool metadata (#1000)
* feat(acp): surface safe browser tool metadata

* fix(acp): align browser permission titles

* fix(acp): namespace browser metadata

* fix(acp): reject unsafe browser title text
2026-09-07 15:50:49 +05:30
14 changed files with 1274 additions and 27 deletions

No files matched your search

+13
View File
@@ -273,6 +273,19 @@ 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
+6 -1
View File
@@ -627,7 +627,12 @@ 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.
- **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.
- **Local-control tools**: config-gated wrappers for browser, terminal, desktop,
and artifact helpers backed by packages such as `internal/localcontrol` and
`internal/browser`.
+19 -1
View File
@@ -19,6 +19,7 @@ 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"
)
@@ -518,9 +519,15 @@ 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: transport,
Transport: proxydial.Wrap(transport),
CheckRedirect: func(req *http.Request, via []*http.Request) error {
if len(via) >= maxConnectivityRedirects {
return fmt.Errorf("provider connectivity exceeded %d redirects", maxConnectivityRedirects)
@@ -583,6 +590,17 @@ 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
+218
View File
@@ -0,0 +1,218 @@
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
}
+93
View File
@@ -0,0 +1,93 @@
// 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
}
+224
View File
@@ -0,0 +1,224 @@
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)
}
}
+20 -7
View File
@@ -96,15 +96,28 @@ var protectedMetadataNames = []string{".git", ".zero", ".agents"}
// gitMetadataWriteCarveouts below.
var sandboxFullyProtectedMetadataNames = []string{".zero", ".agents"}
// 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).
// 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.
func gitMetadataWriteCarveouts(root string) []string {
return []string{
filepath.Join(root, ".git", "hooks"),
filepath.Join(root, ".git", "config"),
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"),
}
info, err := lstat(gitPath)
if err != nil || !info.Mode().IsRegular() {
return children
}
return []string{gitPath}
}
func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile {
+115
View File
@@ -0,0 +1,115 @@
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)
}
@@ -0,0 +1,21 @@
//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"),
})
}
+31 -12
View File
@@ -76,12 +76,17 @@ type BuildArgsInput struct {
}
type BuildResumeArgsInput struct {
SessionID string
Prompt string
CurrentDepth int
Manifest Manifest
Cwd string
PermissionMode string
SessionID string
Prompt string
CurrentDepth int
Manifest Manifest
Cwd string
// ParentModel and ParentReasoningEffort are the same fallbacks the fresh
// path takes, and they are here for the same reason: appendModelArgs uses
// them only when the manifest pins nothing of its own.
ParentModel string
ParentReasoningEffort string
PermissionMode string
}
type BuildArgsResult struct {
@@ -344,6 +349,18 @@ func (executor Executor) BuildResumeArgs(input BuildResumeArgsInput) (BuildArgsR
}
args := []string{"exec", "--resume", sessionID}
args = append(args, promptArgs...)
// A RESUMED SPECIALIST KEEPS THE MODEL ITS MANIFEST PINNED.
//
// The fresh path appends this and the resume path did not, so a specialist
// pinned to a cheap model ran on that model once and then silently reverted
// to the parent's configured model on every resume. Nothing reports it: the
// child starts normally and the only symptom is the bill.
//
// Resuming does not restore the recorded model on its own. sessions.PrepareExec
// records the model it ran under but never feeds it back into provider
// construction, so without this the child takes whatever the config default
// resolves to now.
args = appendModelArgs(args, input.Manifest, input.ParentModel, input.ParentReasoningEffort)
args = append(args, "--auto", specialistAutonomy(input.PermissionMode), "--output-format", "stream-json")
// See BuildArgs: only plan/spec-draft propagate --permission-mode so --auto
// remains the authority for member/auto/ask/unsafe child resolution.
@@ -424,12 +441,14 @@ func (executor Executor) runResume(ctx context.Context, params TaskParameters, o
return ExecResult{}, err
}
built, err := executor.BuildResumeArgs(BuildResumeArgsInput{
SessionID: params.Resume,
Prompt: params.Prompt,
CurrentDepth: options.CurrentDepth,
Manifest: manifest,
Cwd: options.Cwd,
PermissionMode: options.PermissionMode,
SessionID: params.Resume,
Prompt: params.Prompt,
CurrentDepth: options.CurrentDepth,
Manifest: manifest,
Cwd: options.Cwd,
ParentModel: options.ParentModel,
ParentReasoningEffort: options.ParentReasoningEffort,
PermissionMode: options.PermissionMode,
})
if err != nil {
return ExecResult{}, err
+303
View File
@@ -0,0 +1,303 @@
package specialist
import (
"context"
"strings"
"testing"
"github.com/Gitlawb/zero/internal/sessions"
"github.com/Gitlawb/zero/internal/streamjson"
)
// argValue returns the value following flag in argv, and whether it was present.
func argValue(args []string, flag string) (string, bool) {
for index, arg := range args {
if arg == flag && index+1 < len(args) {
return args[index+1], true
}
}
return "", false
}
func pinnedManifest(model string, effort string) Manifest {
return Manifest{
Metadata: Metadata{
Name: "skim",
Description: "Bounded read-only lookups.",
Model: model,
ReasoningEffort: effort,
Tools: []string{"read_file"},
},
SystemPrompt: "Find the thing and stop.",
ResolvedTools: []string{"read_file"},
}
}
// A RESUMED SPECIALIST KEEPS THE MODEL ITS MANIFEST PINNED.
//
// Metadata.Model exists so a bounded, delegated task can run on a cheaper model
// than its parent. BuildArgs appended it; BuildResumeArgs did not. So the
// specialist ran on the cheap model once, and the moment the orchestrator
// resumed it the child fell back to whatever the parent's configured model
// resolved to. Nothing surfaced it: the resumed child starts normally, does the
// work, and the only symptom is the bill.
//
// Resuming does not restore the recorded model on its own, which is why the flag
// has to be passed again rather than relied upon.
func TestAResumedSpecialistKeepsItsPinnedModel(t *testing.T) {
manifest := pinnedManifest("claude-haiku-4.5", "")
fresh, err := (Executor{}).BuildArgs(BuildArgsInput{
Manifest: manifest,
Prompt: "find the thing",
CurrentDepth: 0,
ParentModel: "claude-opus-4.1",
})
if err != nil {
t.Fatalf("BuildArgs: %v", err)
}
// SETUP: the fresh path really does pin it, or the comparison below is
// asserting against a path that never worked either.
freshModel, ok := argValue(fresh.Args, "--model")
if !ok || freshModel != "claude-haiku-4.5" {
t.Fatalf("SETUP INVALID: the fresh path passed --model %q (present=%t), want the manifest's model", freshModel, ok)
}
resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
SessionID: "01HZZZZZZZZZZZZZZZZZZZZZZZ",
Prompt: "keep going",
CurrentDepth: 0,
Manifest: manifest,
ParentModel: "claude-opus-4.1",
})
if err != nil {
t.Fatalf("BuildResumeArgs: %v", err)
}
resumedModel, ok := argValue(resumed.Args, "--model")
if !ok {
t.Fatalf("a resumed specialist carries no --model at all, so it reverts to the parent's configured model: %v", resumed.Args)
}
if resumedModel != "claude-haiku-4.5" {
t.Fatalf("a resumed specialist runs on %q, want the manifest's pinned %q", resumedModel, "claude-haiku-4.5")
}
}
// And a manifest that pins nothing still inherits the parent's model, which is
// what makes the fallback in appendModelArgs meaningful rather than the pinned
// case being special-cased.
func TestAResumedSpecialistWithNoPinInheritsTheParentModel(t *testing.T) {
resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
SessionID: "01HZZZZZZZZZZZZZZZZZZZZZZZ",
Prompt: "keep going",
CurrentDepth: 0,
Manifest: pinnedManifest("", ""),
ParentModel: "claude-opus-4.1",
})
if err != nil {
t.Fatalf("BuildResumeArgs: %v", err)
}
model, ok := argValue(resumed.Args, "--model")
if !ok || model != "claude-opus-4.1" {
t.Fatalf("a resumed specialist with no pinned model passed --model %q (present=%t), want the parent's", model, ok)
}
}
// The reasoning-effort rule travels with the model, or the two paths disagree
// about what a pinned model implies. appendModelArgs inherits the parent's
// effort ONLY when the manifest pins no model of its own: a manifest that chose
// a different model has not agreed to the parent's effort for it.
func TestAResumedSpecialistFollowsTheSameReasoningEffortRule(t *testing.T) {
t.Run("pinned model does not inherit parent effort", func(t *testing.T) {
resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
SessionID: "01HZZZZZZZZZZZZZZZZZZZZZZZ",
Prompt: "keep going",
CurrentDepth: 0,
Manifest: pinnedManifest("claude-haiku-4.5", ""),
ParentModel: "claude-opus-4.1",
ParentReasoningEffort: "high",
})
if err != nil {
t.Fatalf("BuildResumeArgs: %v", err)
}
if effort, ok := argValue(resumed.Args, "--reasoning-effort"); ok {
t.Fatalf("a manifest that pinned its own model inherited the parent's effort %q", effort)
}
})
t.Run("no pinned model inherits parent effort", func(t *testing.T) {
resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
SessionID: "01HZZZZZZZZZZZZZZZZZZZZZZZ",
Prompt: "keep going",
CurrentDepth: 0,
Manifest: pinnedManifest("", ""),
ParentModel: "claude-opus-4.1",
ParentReasoningEffort: "high",
})
if err != nil {
t.Fatalf("BuildResumeArgs: %v", err)
}
effort, ok := argValue(resumed.Args, "--reasoning-effort")
if !ok || effort != "high" {
t.Fatalf("a manifest pinning nothing passed --reasoning-effort %q (present=%t), want the parent's", effort, ok)
}
})
}
// The two builders agree on where the flag sits relative to the rest of the
// argv, so a future change to one does not silently reorder only the other.
func TestBothBuildersPlaceTheModelBeforeTheAutonomyFlag(t *testing.T) {
manifest := pinnedManifest("claude-haiku-4.5", "")
fresh, err := (Executor{}).BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "go", CurrentDepth: 0})
if err != nil {
t.Fatalf("BuildArgs: %v", err)
}
resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
SessionID: "01HZZZZZZZZZZZZZZZZZZZZZZZ", Prompt: "go", CurrentDepth: 0, Manifest: manifest,
})
if err != nil {
t.Fatalf("BuildResumeArgs: %v", err)
}
for _, argv := range [][]string{fresh.Args, resumed.Args} {
model := indexOf(argv, "--model")
auto := indexOf(argv, "--auto")
if model < 0 || auto < 0 || model > auto {
t.Fatalf("--model at %d, --auto at %d in %s", model, auto, strings.Join(argv, " "))
}
}
}
func indexOf(args []string, want string) int {
for index, arg := range args {
if arg == want {
return index
}
}
return -1
}
// AND THE CALL SITE PASSES IT, WHICH THE BUILDER TESTS ABOVE CANNOT SEE.
//
// Every test above calls BuildResumeArgs directly. Dropping the ParentModel
// field from the runResume call site still compiles and still passes all of
// them, because the builder is doing its job with whatever it is handed. The
// defect this fix repairs lived at the call site, not in the builder, so it
// needs a test that goes through Run.
//
// Driven through the real Run dispatch with the RunChild seam capturing argv.
func TestRunResumePassesTheParentModelThroughToTheChild(t *testing.T) {
store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
parent, err := store.Create(sessions.CreateInput{SessionID: "parent_session"})
if err != nil {
t.Fatalf("create parent: %v", err)
}
if _, err := store.Create(sessions.CreateInput{
SessionID: "child_task",
SessionKind: sessions.SessionKindChild,
Tag: SessionTagSpecialist,
Depth: 1,
ParentSessionID: parent.SessionID,
AgentName: "skim",
TaskID: "child_task",
}); err != nil {
t.Fatalf("create child: %v", err)
}
zero := 0
var captured []string
executor := Executor{
BinaryPath: "/usr/local/bin/zero",
SessionStore: store,
NewSessionID: func() (string, error) { return "child_task", nil },
Load: func(LoadOptions) (LoadResult, error) {
// No pinned model, so the parent's is the only thing that can supply one.
return LoadResult{Specialists: []Manifest{pinnedManifest("", "")}}, nil
},
RunChild: func(_ context.Context, _ string, args []string, _ func(streamjson.Event)) (ChildRunResult, error) {
captured = append([]string(nil), args...)
return ChildRunResult{
Events: []streamjson.Event{
{Type: streamjson.EventRunStart, RunID: "run_1", SessionID: "child_task"},
{Type: streamjson.EventFinal, RunID: "run_1", Text: "done"},
{Type: streamjson.EventRunEnd, RunID: "run_1", Status: "success", ExitCode: &zero},
},
}, nil
},
}
if _, err := executor.Run(context.Background(), TaskParameters{
Name: "skim",
Prompt: "keep going",
Resume: "child_task",
}, TaskRunOptions{
ParentSessionID: parent.SessionID,
ParentModel: "claude-opus-4.1",
}); err != nil {
t.Fatalf("Run(resume): %v", err)
}
// SETUP: this really was the resume path, not a fresh spawn.
if index := indexOf(captured, "--resume"); index < 0 {
t.Fatalf("SETUP INVALID: the child was not resumed: %v", captured)
}
model, ok := argValue(captured, "--model")
if !ok {
t.Fatalf("the resumed child was launched with no --model, so it runs on whatever the config default resolves to: %v", captured)
}
if model != "claude-opus-4.1" {
t.Fatalf("the resumed child was launched with --model %q, want the parent's %q", model, "claude-opus-4.1")
}
}
// THE FRESH CALL SITE NEEDS THE SAME GUARD, FOR THE SAME REASON.
//
// runFresh and runResume both construct their builder input by hand, and both
// have a byte-identical ParentModel line. Deleting either one compiles. The
// resume half is what this change repairs; this covers the other half so the
// pair cannot drift again in the direction nobody was looking.
func TestRunFreshPassesTheParentModelThroughToTheChild(t *testing.T) {
store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
parent, err := store.Create(sessions.CreateInput{SessionID: "parent_session"})
if err != nil {
t.Fatalf("create parent: %v", err)
}
zero := 0
var captured []string
executor := Executor{
BinaryPath: "/usr/local/bin/zero",
SessionStore: store,
NewSessionID: func() (string, error) { return "child_task", nil },
Load: func(LoadOptions) (LoadResult, error) {
return LoadResult{Specialists: []Manifest{pinnedManifest("", "")}}, nil
},
RunChild: func(_ context.Context, _ string, args []string, _ func(streamjson.Event)) (ChildRunResult, error) {
captured = append([]string(nil), args...)
return ChildRunResult{
Events: []streamjson.Event{
{Type: streamjson.EventRunStart, RunID: "run_1", SessionID: "child_task"},
{Type: streamjson.EventFinal, RunID: "run_1", Text: "done"},
{Type: streamjson.EventRunEnd, RunID: "run_1", Status: "success", ExitCode: &zero},
},
}, nil
},
}
if _, err := executor.Run(context.Background(), TaskParameters{
Name: "skim",
Prompt: "find the thing",
}, TaskRunOptions{
ParentSessionID: parent.SessionID,
ParentModel: "claude-opus-4.1",
}); err != nil {
t.Fatalf("Run(fresh): %v", err)
}
// SETUP: a fresh spawn, not a resume, or this covers the wrong site.
if indexOf(captured, "--resume") >= 0 {
t.Fatalf("SETUP INVALID: the child was resumed rather than freshly spawned: %v", captured)
}
model, ok := argValue(captured, "--model")
if !ok || model != "claude-opus-4.1" {
t.Fatalf("the fresh child was launched with --model %q (present=%t), want the parent's", model, ok)
}
}
+32 -2
View File
@@ -13,6 +13,7 @@ import (
"strings"
"time"
"github.com/Gitlawb/zero/internal/proxydial"
"github.com/Gitlawb/zero/internal/redaction"
zeroSandbox "github.com/Gitlawb/zero/internal/sandbox"
)
@@ -308,19 +309,48 @@ 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}
transport.Proxy = nil
// 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.DialContext = webFetchSafeDialContext(resolver, dialer)
transport.DialTLSContext = nil
return transport
return proxydial.Wrap(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
+163
View File
@@ -0,0 +1,163 @@
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")
}
}
+16 -4
View File
@@ -11,6 +11,7 @@ import (
"strings"
"testing"
"github.com/Gitlawb/zero/internal/proxydial"
zeroSandbox "github.com/Gitlawb/zero/internal/sandbox"
)
@@ -325,15 +326,26 @@ func TestWebFetchToolConfiguresDialTimeSafetyForDefaultTransport(t *testing.T) {
}
client := tool.clientForRun()
transport, ok := client.Transport.(*http.Transport)
// 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)
if !ok {
t.Fatalf("client transport = %T, want *http.Transport", client.Transport)
t.Fatalf("client transport = %T, want *proxydial.Transport", client.Transport)
}
if transport.DialContext == nil {
t.Fatal("expected web_fetch transport to install a safe DialContext")
}
if transport.Proxy != nil {
t.Fatal("expected web_fetch transport to disable proxy resolution")
// 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)")
}
}