From a5bbf4903377320651cd3e1eb2c5c4c07e20711f Mon Sep 17 00:00:00 2001 From: samgozman Date: Sat, 24 Oct 2026 08:41:26 +0400 Subject: [PATCH 1/5] feat: run in Linux containers As root on Linux, the headless Chrome step now runs Chrome as the nobody user with its sandbox on, instead of skipping. Chrome discovery also finds Playwright's Chromium, which many Linux images ship instead of Chrome. use_proxy (REFETCH_USE_PROXY) sends connections through HTTPS_PROXY as CONNECT tunnels inside the public-only dialer, so TLS still runs end to end. A name the proxy resolves is checked when it resolves locally. doctor reports the proxy on a new network line, and no longer suggests /reload-plugins for a development checkout run through REFETCH_BIN. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 + README.md | 10 ++- SECURITY.md | 12 ++- cmd/refetch/doctor.go | 14 +++- cmd/refetch/hook.go | 1 + cmd/refetch/main.go | 10 +++ cmd/refetch/proxy_test.go | 35 +++++++++ config/config.go | 3 +- config/config_test.go | 12 +++ fetch/clients.go | 9 ++- fetch/fetch.go | 18 +++-- fetch/proxy.go | 156 ++++++++++++++++++++++++++++++++++++++ fetch/proxy_test.go | 148 ++++++++++++++++++++++++++++++++++++ render/chrome.go | 29 ++++++- render/chrome_test.go | 17 +++++ render/render.go | 15 +++- render/sysproc_linux.go | 40 +++++++++- render/sysproc_other.go | 9 ++- 18 files changed, 515 insertions(+), 25 deletions(-) create mode 100644 cmd/refetch/proxy_test.go create mode 100644 fetch/proxy.go create mode 100644 fetch/proxy_test.go diff --git a/AGENTS.md b/AGENTS.md index 1c054c5..d91dc3e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,8 @@ fetcher and the stats report. The repo root is also the Claude Code plugin. - Every connection goes through the public-only dialer (`fetch.DialContext`); never add a client that bypasses it. Headless Chrome reaches the network only through `render`'s proxy on that dialer. + An upstream proxy (`use_proxy`, `fetch.UseProxy`) is a CONNECT tunnel + inside that dialer; never set `Proxy` on a transport. - Hooks and the MCP server must never break a session: report failures as tool errors, or exit 0 with empty output. Hooks finish in well under 50 ms. diff --git a/README.md b/README.md index 2dc63ce..9b3220a 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,7 @@ another file. Everything is on by default. | `stats` | `REFETCH_STATS` | `true` | `false` records nothing | | `mode` | `REFETCH_MODE` | `"nudge"` | `"inline"`: the hook fetches the page itself and puts its first 8,000 characters next to the failed result | | `user_agent` | `REFETCH_USER_AGENT` | `refetch/` | The plain client's User-Agent | +| `use_proxy` | `REFETCH_USE_PROXY` | `false` | `true` sends connections through `HTTPS_PROXY`, except to hosts `NO_PROXY` lists. Off by default because a proxy that decrypts TLS replaces the Chrome-like handshake with its own | ```toml # ~/.config/refetch/config.toml @@ -198,9 +199,12 @@ stats = false ``` Headless Chrome uses `REFETCH_CHROME` if set, then the standard macOS app -paths, then `google-chrome` or `chromium` on PATH. refetch never downloads a -browser. As root (in a container, for example), the step is skipped, because -Chrome's sandbox does not run as root. +paths, then `google-chrome` or `chromium` on PATH, then Playwright's Chromium +(in `PLAYWRIGHT_BROWSERS_PATH`, `/ms-playwright`, `/opt/pw-browsers` or +`~/.cache/ms-playwright`). refetch never downloads a browser. Chrome's +sandbox does not run as root, so when refetch runs as root (in a container, +for example), it runs Chrome as the `nobody` user on Linux and skips the +step elsewhere. To turn refetch off entirely, disable the plugin in `/plugin`. diff --git a/SECURITY.md b/SECURITY.md index 25a5c95..ec36258 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -9,7 +9,11 @@ injection can choose it. These guarantees are security boundaries: own interface addresses. The check runs on the resolved address of every connection and redirect, so DNS rebinding cannot get around it. Headless Chrome reaches the network only through a local proxy that applies the - same check. + same check. Behind an upstream proxy (see below), the check runs before + each connection instead. +- **Chrome never runs as root.** When refetch runs as root on Linux, Chrome + runs as the `nobody` user, with its sandbox on; elsewhere the step does + not run as root at all. - **Only pinned binaries run.** The plugin shim downloads the binary for its version from this repo's GitHub release and checks the archive and the binary against the sha256 values pinned in the plugin. It checks the @@ -43,6 +47,12 @@ injection can choose it. These guarantees are security boundaries: 127.0.0.1. Another process on your machine could use it to reach public addresses for those few seconds; it never reaches anything the address check refuses. +- **An upstream proxy, when you turn it on.** With `use_proxy`, + connections go through `HTTPS_PROXY`, and the proxy resolves host names + itself. refetch refuses non-public IP addresses and names that resolve on + this machine to non-public addresses, but a name that only the proxy can + resolve is the proxy's to check. Hosts `NO_PROXY` lists are dialled + directly, with the full check. - **What the address check cannot see.** An internal network on public IP space, or a service that trusts your IP address, looks public. diff --git a/cmd/refetch/doctor.go b/cmd/refetch/doctor.go index 4edc7ba..23e012f 100644 --- a/cmd/refetch/doctor.go +++ b/cmd/refetch/doctor.go @@ -42,7 +42,7 @@ func runDoctor(ctx context.Context, stdout io.Writer) error { } else { checks = append(checks, configCheck()) } - checks = append(checks, dataCheck(ctx, cfg), chromeCheck(ctx, cfg), env.configDirCheck(), env.permissionCheck(), env.hooksCheck()) + checks = append(checks, networkCheck(cfg, useProxy(cfg)), dataCheck(ctx, cfg), chromeCheck(ctx, cfg), env.configDirCheck(), env.permissionCheck(), env.hooksCheck()) for _, c := range checks { mark := "✓" if !c.ok { @@ -55,12 +55,24 @@ func runDoctor(ctx context.Context, stdout io.Writer) error { return nil } +func networkCheck(cfg config.Config, proxy string) check { + switch { + case !cfg.UseProxy: + return check{true, "network", "direct connections"} + case proxy == "": + return check{false, "network", "use_proxy is on, but no HTTPS_PROXY is set, so connections go direct"} + } + return check{true, "network", "through the proxy at " + proxy + " (use_proxy)"} +} + func versionCheck() check { bin := buildVersion() plugin := os.Getenv("REFETCH_PLUGIN_VERSION") switch { case plugin == "": return check{true, "version", "refetch " + bin + " (not run through the plugin)"} + case plugin == "0.0.0": + return check{true, "version", "refetch " + bin + ", with the plugin files of a development checkout"} case strings.TrimPrefix(bin, "v") != plugin: return check{false, "version", fmt.Sprintf("binary %s, plugin %s: run /reload-plugins, or check REFETCH_BIN", bin, plugin)} } diff --git a/cmd/refetch/hook.go b/cmd/refetch/hook.go index 6f9c5f3..e177b20 100644 --- a/cmd/refetch/hook.go +++ b/cmd/refetch/hook.go @@ -68,6 +68,7 @@ func runHook(ctx context.Context, args []string, stdin io.Reader, stdout, stderr debug(err) return } + useProxy(cfg) var in hookInput if err := json.UnmarshalRead(stdin, &in); err != nil { debug(err) diff --git a/cmd/refetch/main.go b/cmd/refetch/main.go index f18e7ab..7f8f9d5 100644 --- a/cmd/refetch/main.go +++ b/cmd/refetch/main.go @@ -72,6 +72,7 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { _, _ = fmt.Fprintln(stderr, "refetch:", err) return 1 } + useProxy(cfg) switch cmd { case "mcp": err = runMCP(ctx, cfg, stderr) @@ -91,6 +92,15 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { return 0 } +// useProxy routes connections through HTTPS_PROXY when use_proxy is on, +// and returns the proxy's address ("" for none). +func useProxy(cfg config.Config) string { + if !cfg.UseProxy { + return "" + } + return fetch.UseProxy() +} + // buildVersion is the release version, or the module version for go // install builds. func buildVersion() string { diff --git a/cmd/refetch/proxy_test.go b/cmd/refetch/proxy_test.go new file mode 100644 index 0000000..f7114a4 --- /dev/null +++ b/cmd/refetch/proxy_test.go @@ -0,0 +1,35 @@ +package main + +import ( + "testing" + + "github.com/samgozman/refetch/config" + "github.com/samgozman/refetch/fetch" +) + +func TestNetworkCheck(t *testing.T) { + t.Cleanup(func() { fetch.UseProxy() }) // after the variables below are restored + for _, k := range []string{"https_proxy", "HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy"} { + t.Setenv(k, "") + } + t.Setenv("HTTPS_PROXY", "http://user:pw@127.0.0.1:3128") + cfg := config.Default() + if c := networkCheck(cfg, useProxy(cfg)); !c.ok || c.detail != "direct connections" { + t.Errorf("HTTPS_PROXY is ignored without use_proxy: %+v", c) + } + cfg.UseProxy = true + if c := networkCheck(cfg, useProxy(cfg)); !c.ok || c.detail != "through the proxy at http://127.0.0.1:3128 (use_proxy)" { + t.Errorf("use_proxy: %+v", c) + } + t.Setenv("HTTPS_PROXY", "") + if c := networkCheck(cfg, useProxy(cfg)); c.ok { + t.Errorf("use_proxy without HTTPS_PROXY: %+v", c) + } +} + +func TestVersionOfADevelopmentCheckout(t *testing.T) { + t.Setenv("REFETCH_PLUGIN_VERSION", "0.0.0") + if c := versionCheck(); !c.ok { + t.Errorf("REFETCH_BIN with a development checkout: %+v", c) + } +} diff --git a/config/config.go b/config/config.go index d6a2825..7701bc2 100644 --- a/config/config.go +++ b/config/config.go @@ -27,6 +27,7 @@ type Config struct { Stats bool `toml:"stats"` // record events.db rows Mode string `toml:"mode"` // nudge or inline (hook response) UserAgent string `toml:"user_agent"` // the plain client's User-Agent; empty = refetch/ + UseProxy bool `toml:"use_proxy"` // connect through HTTPS_PROXY; a TLS-decrypting one replaces the Chrome handshake } // Duration is a time.Duration written as "10s" in TOML and env vars. @@ -123,7 +124,7 @@ func (c *Config) applyEnv(getenv func(string) string) error { } } } - bools := map[string]*bool{"REFETCH_RESPECT_ROBOTS": &c.RespectRobots, "REFETCH_STATS": &c.Stats} + bools := map[string]*bool{"REFETCH_RESPECT_ROBOTS": &c.RespectRobots, "REFETCH_STATS": &c.Stats, "REFETCH_USE_PROXY": &c.UseProxy} for k, p := range bools { if v := getenv(k); v != "" { b, err := strconv.ParseBool(v) diff --git a/config/config_test.go b/config/config_test.go index 247c477..87eff4b 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -88,3 +88,15 @@ func TestDurationText(t *testing.T) { t.Error("UnmarshalText accepted \"soon\"") } } + +func TestUseProxyFromEnv(t *testing.T) { + t.Setenv("REFETCH_CONFIG", "/dev/null") + t.Setenv("REFETCH_USE_PROXY", "") + if c, err := Load(); err != nil || c.UseProxy { + t.Errorf("use_proxy on by default: %v", err) + } + t.Setenv("REFETCH_USE_PROXY", "true") + if c, err := Load(); err != nil || !c.UseProxy { + t.Errorf("REFETCH_USE_PROXY=true: %v", err) + } +} diff --git a/fetch/clients.go b/fetch/clients.go index 591bfa5..b961a5d 100644 --- a/fetch/clients.go +++ b/fetch/clients.go @@ -29,15 +29,16 @@ var chromeHeaders = fhttp.Header{ } // chromeGet fetches with a Chrome 150 TLS and HTTP/2 fingerprint. It shares -// the public-only dialer and redirect rules, never uses a proxy or HTTP/3 -// (QUIC would bypass the dialer), and keeps no cookies between fetches. +// the public-only dialer (and through it any proxy UseProxy set) and the +// redirect rules, never uses HTTP/3 (QUIC would bypass the dialer), and keeps +// no cookies between fetches. func chromeGet(ctx context.Context, rawURL string, timeout time.Duration) (int, []byte, string, error) { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() opts := []tlsclient.HttpClientOption{ tlsclient.WithClientProfile(profiles.Chrome_150), tlsclient.WithTimeoutSeconds(int(timeout.Seconds())), - tlsclient.WithDialer(dialer), + tlsclient.WithDialContext(DialContext), tlsclient.WithDisableHttp3(), tlsclient.WithCookieJar(tlsclient.NewCookieJar()), tlsclient.WithCustomRedirectFunc(func(req *fhttp.Request, via []*fhttp.Request) error { @@ -67,7 +68,7 @@ func chromeGet(ctx context.Context, rawURL string, timeout time.Duration) (int, } var plainClient = &http.Client{ - Transport: &http.Transport{Proxy: nil, DialContext: dialer.DialContext, TLSClientConfig: &tls.Config{InsecureSkipVerify: e2eLocal}}, // InsecureSkipVerify is true only in the e2e build + Transport: &http.Transport{Proxy: nil, DialContext: DialContext, TLSClientConfig: &tls.Config{InsecureSkipVerify: e2eLocal}}, // InsecureSkipVerify is true only in the e2e build CheckRedirect: func(req *http.Request, via []*http.Request) error { return checkRedirect(req.URL, len(via)) }, diff --git a/fetch/fetch.go b/fetch/fetch.go index 3084abc..c5942c1 100644 --- a/fetch/fetch.go +++ b/fetch/fetch.go @@ -350,8 +350,16 @@ func (f *Fetcher) allowed(ctx context.Context, rawURL string) bool { var dialer = net.Dialer{Timeout: 10 * time.Second, Control: publicOnly} // DialContext dials the way both clients do: public addresses only, checked -// after DNS resolution. The headless Chrome step's proxy dials with it. +// after DNS resolution, or through the proxy UseProxy set. The headless +// Chrome step's proxy dials with it. func DialContext(ctx context.Context, network, address string) (net.Conn, error) { + proxy, ok, err := proxyFor(address) + if err != nil { + return nil, err + } + if ok { + return dialProxied(ctx, proxy, address) + } return dialer.DialContext(ctx, network, address) } @@ -387,13 +395,7 @@ func publicOnly(_, address string, _ syscall.RawConn) error { if err != nil { return err } - if !isPublic(ip) { - return fmt.Errorf("%s is not a public address", ip) - } - if (!e2eLocal || !ip.IsLoopback()) && isLocal(ip) { - return fmt.Errorf("%s is not a public address: it is this machine's own", ip) - } - return nil + return checkAddr(ip) } // local caches this machine's interface addresses. A public address on an diff --git a/fetch/proxy.go b/fetch/proxy.go new file mode 100644 index 0000000..d503ba9 --- /dev/null +++ b/fetch/proxy.go @@ -0,0 +1,156 @@ +package fetch + +import ( + "bufio" + "cmp" + "context" + "encoding/base64" + "errors" + "fmt" + "net" + "net/http" + "net/netip" + "net/url" + "strings" + "sync/atomic" + "time" + + "golang.org/x/net/http/httpproxy" +) + +// upstream picks the HTTP proxy for a target, or nil to dial directly. It +// is unset unless UseProxy is called. +var upstream atomic.Pointer[func(*url.URL) (*url.URL, error)] + +// proxyDialer reaches the proxy itself. The proxy is the user's own setting +// (often a corporate proxy, or one on 127.0.0.1), so it skips the +// public-only check that every target still gets. +var proxyDialer = net.Dialer{Timeout: 10 * time.Second} + +// UseProxy sends every connection through the proxy in HTTPS_PROXY (or +// HTTP_PROXY) as a CONNECT tunnel, except to hosts NO_PROXY lists. TLS runs +// inside the tunnel, so the site still sees refetch's own handshake. It +// returns the proxy's address, or "" (and dials directly) when none is set. +func UseProxy() string { + env := httpproxy.FromEnvironment() + proxy := cmp.Or(env.HTTPSProxy, env.HTTPProxy) + if proxy == "" { + upstream.Store(nil) + return "" + } + cfg := httpproxy.Config{HTTPSProxy: proxy, NoProxy: env.NoProxy} + pick := cfg.ProxyFunc() + upstream.Store(&pick) + u, err := url.Parse(proxy) + if err != nil || u.Host == "" { + return proxy + } + return u.Scheme + "://" + u.Host +} + +// proxyFor is the proxy for address (host:port); ok is false to dial +// directly. +func proxyFor(address string) (proxy *url.URL, ok bool, err error) { + pick := upstream.Load() + if pick == nil { + return nil, false, nil + } + proxy, err = (*pick)(&url.URL{Scheme: "https", Host: address}) + return proxy, proxy != nil, err +} + +func dialProxied(ctx context.Context, proxy *url.URL, address string) (net.Conn, error) { + if proxy.Scheme != "http" { + return nil, fmt.Errorf("proxy %s: only http:// proxies are supported", proxy.Host) + } + if err := checkTarget(ctx, address); err != nil { + return nil, err + } + proxyAddr := proxy.Host + if proxy.Port() == "" { + proxyAddr = net.JoinHostPort(proxy.Hostname(), "80") + } + conn, err := proxyDialer.DialContext(ctx, "tcp", proxyAddr) + if err != nil { + return nil, fmt.Errorf("proxy %s: %w", proxy.Host, err) + } + if deadline, ok := ctx.Deadline(); ok { + _ = conn.SetDeadline(deadline) + } + req := &http.Request{Method: http.MethodConnect, URL: &url.URL{Opaque: address}, Host: address, Header: http.Header{}} + if u := proxy.User; u != nil { + pass, _ := u.Password() + req.Header.Set("Proxy-Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(u.Username()+":"+pass))) + } + br := bufio.NewReader(conn) + resp, err := connect(conn, br, req) + if err != nil { + _ = conn.Close() + return nil, fmt.Errorf("proxy %s: %w", proxy.Host, err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + _ = conn.Close() + return nil, fmt.Errorf("the proxy refused %s (%s)", address, resp.Status) + } + _ = conn.SetDeadline(time.Time{}) + if br.Buffered() > 0 { + return &bufferedConn{Conn: conn, r: br}, nil + } + return conn, nil +} + +func connect(conn net.Conn, br *bufio.Reader, req *http.Request) (*http.Response, error) { + if err := req.Write(conn); err != nil { + return nil, err + } + return http.ReadResponse(br, req) +} + +// checkTarget is the public-only check for a connection the proxy makes: +// the proxy resolves the name, so only what can be checked here is. +// Literal addresses must be public, and a name that resolves locally must +// resolve only to public addresses. A name that does not resolve locally +// is left to the proxy. +func checkTarget(ctx context.Context, address string) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return err + } + if ip, err := netip.ParseAddr(host); err == nil { + return checkAddr(ip) + } + name := strings.TrimSuffix(strings.ToLower(host), ".") + if name == "localhost" || strings.HasSuffix(name, ".localhost") { + return fmt.Errorf("%s is not a public address", host) + } + ctx, cancel := context.WithTimeout(ctx, 2*time.Second) + defer cancel() + ips, err := net.DefaultResolver.LookupNetIP(ctx, "ip", host) + if err != nil { + return nil + } + var errs []error + for _, ip := range ips { + errs = append(errs, checkAddr(ip)) + } + return errors.Join(errs...) +} + +func checkAddr(ip netip.Addr) error { + if !isPublic(ip) { + return fmt.Errorf("%s is not a public address", ip) + } + if (!e2eLocal || !ip.IsLoopback()) && isLocal(ip) { + return fmt.Errorf("%s is not a public address: it is this machine's own", ip) + } + return nil +} + +// bufferedConn returns bytes the proxy sent right after its reply first. +type bufferedConn struct { + net.Conn + r *bufio.Reader +} + +func (c *bufferedConn) Read(p []byte) (int, error) { return c.r.Read(p) } diff --git a/fetch/proxy_test.go b/fetch/proxy_test.go new file mode 100644 index 0000000..8e5222d --- /dev/null +++ b/fetch/proxy_test.go @@ -0,0 +1,148 @@ +package fetch + +import ( + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" +) + +// connectProxy is a loopback CONNECT proxy that tunnels every target to +// backend, so tests reach a local server under a name that does not +// resolve, the way a proxy that resolves names itself is reached. +type connectProxy struct { + mu sync.Mutex + targets []string + auth []string + status int + backend string +} + +func (p *connectProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { + p.mu.Lock() + p.targets = append(p.targets, r.Host) + p.auth = append(p.auth, r.Header.Get("Proxy-Authorization")) + status := p.status + p.mu.Unlock() + if status != 0 { + w.WriteHeader(status) + return + } + dst, err := net.Dial("tcp", p.backend) + if err != nil { + w.WriteHeader(http.StatusBadGateway) + return + } + src, _, err := w.(http.Hijacker).Hijack() + if err != nil { + return + } + _, _ = src.Write([]byte("HTTP/1.1 200 Connection established\r\n\r\n")) + go func() { _, _ = io.Copy(dst, src); _ = dst.Close() }() + _, _ = io.Copy(src, dst) + _ = src.Close() +} + +// withProxy starts a backend and a proxy in front of it and routes this +// test's connections through the proxy. +func withProxy(t *testing.T, user *url.Userinfo) *connectProxy { + t.Helper() + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "through the proxy") + })) + t.Cleanup(backend.Close) + cp := &connectProxy{backend: backend.Listener.Addr().String()} + srv := httptest.NewServer(cp) + t.Cleanup(srv.Close) + proxy := &url.URL{Scheme: "http", Host: srv.Listener.Addr().String(), User: user} + pick := func(*url.URL) (*url.URL, error) { return proxy, nil } //nolint:unparam // the shape httpproxy's ProxyFunc has + upstream.Store(&pick) + t.Cleanup(func() { upstream.Store(nil) }) + return cp +} + +func TestProxyTunnelsBothClients(t *testing.T) { + cp := withProxy(t, url.UserPassword("agent", "s3cret")) + for name, get := range map[string]func() (int, []byte, error){ + "plain": func() (int, []byte, error) { + s, b, _, err := plainGet(t.Context(), "http://refetch-test.invalid/page", "refetch-test", 5*time.Second) + return s, b, err + }, + "chrome_like": func() (int, []byte, error) { + s, b, _, err := chromeGet(t.Context(), "http://refetch-test.invalid/page", 5*time.Second) + return s, b, err + }, + } { + status, body, err := get() + if err != nil || status != http.StatusOK || !strings.Contains(string(body), "through the proxy") { + t.Errorf("%s through the proxy: %d %q %v", name, status, body, err) + } + } + cp.mu.Lock() + defer cp.mu.Unlock() + if len(cp.targets) == 0 || cp.targets[0] != "refetch-test.invalid:80" { + t.Errorf("CONNECT targets %v", cp.targets) + } + if cp.auth[0] != "Basic YWdlbnQ6czNjcmV0" { + t.Errorf("Proxy-Authorization %q", cp.auth[0]) + } +} + +func TestProxyStillRefusesNonPublicTargets(t *testing.T) { + cp := withProxy(t, nil) + for _, addr := range []string{"127.0.0.1:80", "10.1.2.3:443", "[::1]:443", "localhost:80", "app.localhost:443"} { + if _, err := DialContext(t.Context(), "tcp", addr); err == nil || !strings.Contains(err.Error(), "not a public address") { + t.Errorf("dial %s through the proxy: %v", addr, err) + } + } + cp.mu.Lock() + defer cp.mu.Unlock() + if len(cp.targets) != 0 { + t.Errorf("the proxy was asked for %v", cp.targets) + } +} + +func TestProxyRefusalIsReported(t *testing.T) { + cp := withProxy(t, nil) + cp.status = http.StatusForbidden + _, err := DialContext(t.Context(), "tcp", "refetch-test.invalid:443") + if err == nil || !strings.Contains(err.Error(), "the proxy refused refetch-test.invalid:443 (403 Forbidden)") { + t.Errorf("refused CONNECT: %v", err) + } +} + +func TestUseProxy(t *testing.T) { + t.Cleanup(func() { upstream.Store(nil) }) + for _, k := range []string{"HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy"} { + t.Setenv(k, "") + } + if got := UseProxy(); got != "" || upstream.Load() != nil { + t.Fatalf("no proxy set: %q", got) + } + t.Setenv("HTTPS_PROXY", "http://user:pw@127.0.0.1:3128") + t.Setenv("NO_PROXY", "registry.example.org,10.0.0.0/8") + if got := UseProxy(); got != "http://127.0.0.1:3128" { + t.Errorf("UseProxy = %q, want the address without credentials", got) + } + for addr, proxied := range map[string]bool{"www.example.com:443": true, "registry.example.org:443": false, "10.0.0.5:443": false} { + p, ok, err := proxyFor(addr) + if err != nil || ok != proxied { + t.Errorf("proxyFor(%s) = %v, %v, %v; want proxied %v", addr, p, ok, err, proxied) + } + } + if _, err := DialContext(t.Context(), "tcp", "10.0.0.5:443"); err == nil { + t.Error("a NO_PROXY private address was dialled directly") + } +} + +func TestOnlyHTTPProxies(t *testing.T) { + _, err := dialProxied(t.Context(), &url.URL{Scheme: "socks5", Host: "127.0.0.1:1080"}, "example.com:443") + if err == nil || !strings.Contains(err.Error(), "only http:// proxies") { + t.Errorf("socks5 proxy: %v", err) + } +} diff --git a/render/chrome.go b/render/chrome.go index 8b8c782..63133c6 100644 --- a/render/chrome.go +++ b/render/chrome.go @@ -3,8 +3,11 @@ package render import ( + "cmp" "os" "os/exec" + "path/filepath" + "slices" ) // macOSApps are the standard install paths, most common first. @@ -17,8 +20,13 @@ var macOSApps = []string{ // pathNames are looked up on PATH after the macOS apps. var pathNames = []string{"google-chrome", "google-chrome-stable", "chromium", "chromium-browser", "chrome"} +// playwrightBuilds are Playwright's Chromium builds inside its browsers +// folder, the newest last when sorted. +var playwrightBuilds = []string{"chromium-*/chrome-linux64/chrome", "chromium-*/chrome-linux/chrome"} + // FindChrome returns the Chrome to run: REFETCH_CHROME if set, then the -// macOS app paths (also under ~/Applications), then PATH. Empty means none. +// macOS app paths (also under ~/Applications), then PATH, then Playwright's +// Chromium. Empty means none. func FindChrome() string { if p := os.Getenv("REFETCH_CHROME"); p != "" { if isExec(p) { @@ -39,6 +47,25 @@ func FindChrome() string { return p } } + return playwrightChrome(home) +} + +func playwrightChrome(home string) string { + dirs := []string{"/ms-playwright", "/opt/pw-browsers", filepath.Join(home, ".cache", "ms-playwright")} + if p := os.Getenv("PLAYWRIGHT_BROWSERS_PATH"); p != "" && p != "0" { + dirs = append([]string{p}, dirs...) + } + for _, dir := range dirs { + for _, pattern := range playwrightBuilds { + found, _ := filepath.Glob(filepath.Join(dir, pattern)) + slices.SortFunc(found, func(a, b string) int { return cmp.Or(cmp.Compare(len(b), len(a)), cmp.Compare(b, a)) }) + for _, p := range found { + if isExec(p) { + return p + } + } + } + } return "" } diff --git a/render/chrome_test.go b/render/chrome_test.go index 51355bb..fa6ec4a 100644 --- a/render/chrome_test.go +++ b/render/chrome_test.go @@ -27,3 +27,20 @@ func TestFindChrome(t *testing.T) { t.Errorf("PATH lookup: %q", got) } } + +func TestPlaywrightChrome(t *testing.T) { + dir := t.TempDir() + for _, p := range []string{"chromium-999/chrome-linux/chrome", "chromium-1200/chrome-linux64/chrome", "chromium_headless_shell-1300/chrome-linux/headless_shell"} { + bin := filepath.Join(dir, p) + if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + } + t.Setenv("PLAYWRIGHT_BROWSERS_PATH", dir) + if got, want := playwrightChrome(t.TempDir()), filepath.Join(dir, "chromium-1200", "chrome-linux64", "chrome"); got != want { + t.Errorf("playwrightChrome = %q, want the newest full Chromium %q", got, want) + } +} diff --git a/render/render.go b/render/render.go index fb8f1fb..27e0e5a 100644 --- a/render/render.go +++ b/render/render.go @@ -29,7 +29,7 @@ const ( OutcomeSkippedOff = "skipped_off" // render = "off" OutcomeSkippedByCall = "skipped_by_call" // fetch_page(render: false) OutcomeNoChrome = "no_chrome" // no Chrome or Chromium found - OutcomeSkippedRoot = "skipped_root" // running as root, where Chrome's sandbox cannot run + OutcomeSkippedRoot = "skipped_root" // running as root outside Linux, where Chrome's sandbox cannot run ) // DefaultTimeout is render_timeout's default: Chrome start included. @@ -38,7 +38,7 @@ const DefaultTimeout = 15 * time.Second // Errors Render returns. var ( ErrNoChrome = errors.New("no Chrome or Chromium found") - ErrRoot = errors.New("running as root: headless Chrome needs its sandbox, which does not run as root") + ErrRoot = errors.New("running as root: headless Chrome needs its sandbox, which does not run as root (on Linux, refetch runs it as nobody instead)") ErrTimeout = errors.New("headless Chrome timed out") ) @@ -59,8 +59,10 @@ func (r *Renderer) Render(ctx context.Context, rawURL string) ([]byte, error) { return nil, ErrNoChrome } // Rendering untrusted pages without Chrome's sandbox would hand any - // renderer exploit root, so the step does not run as root at all. - if os.Geteuid() == 0 { + // renderer exploit root, so Chrome never runs as root: on Linux it runs + // as nobody instead, elsewhere the step does not run. + asRoot := os.Geteuid() == 0 + if asRoot && !canDropRoot { return nil, ErrRoot } timeout := cmp.Or(r.Timeout, DefaultTimeout) @@ -85,6 +87,11 @@ func (r *Renderer) Render(ctx context.Context, rawURL string) ([]byte, error) { cmd := exec.CommandContext(ctx, chrome, chromeArgs(ctx, chrome, profile, px.addr, rawURL, timeout)...) //nolint:gosec // the user's own Chrome, found by FindChrome or REFETCH_CHROME cmd.SysProcAttr = sysProcAttr() + if asRoot { + if err := runAsNobody(cmd, profile); err != nil { + return nil, err + } + } cmd.Cancel = func() error { return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) } // Chrome's helpers too cmd.WaitDelay = 2 * time.Second out := &capped{max: fetch.MaxBytes} diff --git a/render/sysproc_linux.go b/render/sysproc_linux.go index 7238c4f..57ae54b 100644 --- a/render/sysproc_linux.go +++ b/render/sysproc_linux.go @@ -1,9 +1,47 @@ package render -import "syscall" +import ( + "io/fs" + "os" + "os/exec" + "os/user" + "path/filepath" + "strconv" + "syscall" +) // sysProcAttr puts Chrome in its own process group, so a timeout kills its // helpers too, and has the kernel kill it if refetch itself dies. func sysProcAttr() *syscall.SysProcAttr { return &syscall.SysProcAttr{Setpgid: true, Pdeathsig: syscall.SIGKILL} } + +// canDropRoot is true where a root refetch can run Chrome as another user. +const canDropRoot = true + +// runAsNobody runs Chrome as the unprivileged nobody user, which keeps +// Chrome's sandbox on when refetch runs as root (in a container). The +// throwaway profile becomes nobody's, and Chrome's home with it. +func runAsNobody(cmd *exec.Cmd, profile string) error { + uid, gid := uint32(65534), uint32(65534) + if u, err := user.Lookup("nobody"); err == nil { + if n, err := strconv.ParseUint(u.Uid, 10, 32); err == nil { + uid = uint32(n) + } + if n, err := strconv.ParseUint(u.Gid, 10, 32); err == nil { + gid = uint32(n) + } + } + err := filepath.WalkDir(profile, func(p string, _ fs.DirEntry, err error) error { + if err != nil { + return err + } + return os.Lchown(p, int(uid), int(gid)) + }) + if err != nil { + return err + } + cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid, NoSetGroups: true} + cmd.Env = append(os.Environ(), "HOME="+profile, "XDG_CONFIG_HOME="+profile, "XDG_CACHE_HOME="+profile) + return nil +} diff --git a/render/sysproc_other.go b/render/sysproc_other.go index 0ce82dd..6a4ee3c 100644 --- a/render/sysproc_other.go +++ b/render/sysproc_other.go @@ -2,10 +2,17 @@ package render -import "syscall" +import ( + "os/exec" + "syscall" +) // sysProcAttr puts Chrome in its own process group, so a timeout kills its // helpers too. func sysProcAttr() *syscall.SysProcAttr { return &syscall.SysProcAttr{Setpgid: true} } + +const canDropRoot = false + +func runAsNobody(*exec.Cmd, string) error { return ErrRoot } From 515bf56f03bc613c91d253728fcf5fded45b0061 Mon Sep 17 00:00:00 2001 From: samgozman Date: Sat, 24 Oct 2026 08:45:38 +0400 Subject: [PATCH 2/5] fix: refuse numeric host spellings behind a proxy, and check a found Chromium's owner Behind use_proxy, a name whose last label starts with a digit (0x7f.1, 2130706433) is refused instead of being left to the proxy, which may read it as an address. A Playwright Chromium found by searching runs only if it and the folders above it belong to root or this user and nobody else can write to them; as root, the --version check also runs as nobody. Co-Authored-By: Claude Opus 5.5 (1M context) --- SECURITY.md | 7 +++++-- fetch/proxy.go | 5 +++++ fetch/proxy_test.go | 4 ++-- render/chrome.go | 28 +++++++++++++++++++++++++++- render/chrome_test.go | 18 ++++++++++++++++++ render/render.go | 15 ++++++++++++++- 6 files changed, 71 insertions(+), 6 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index ec36258..894866d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -12,8 +12,11 @@ injection can choose it. These guarantees are security boundaries: same check. Behind an upstream proxy (see below), the check runs before each connection instead. - **Chrome never runs as root.** When refetch runs as root on Linux, Chrome - runs as the `nobody` user, with its sandbox on; elsewhere the step does - not run as root at all. + (its `--version` check included) runs as the `nobody` user, with its + sandbox on; elsewhere the step does not run as root at all. A Playwright + Chromium that refetch finds by searching is used only if it and the + folders above it belong to root or to you and nobody else can write to + them. - **Only pinned binaries run.** The plugin shim downloads the binary for its version from this repo's GitHub release and checks the archive and the binary against the sha256 values pinned in the plugin. It checks the diff --git a/fetch/proxy.go b/fetch/proxy.go index d503ba9..3a09a76 100644 --- a/fetch/proxy.go +++ b/fetch/proxy.go @@ -124,6 +124,11 @@ func checkTarget(ctx context.Context, address string) error { if name == "localhost" || strings.HasSuffix(name, ".localhost") { return fmt.Errorf("%s is not a public address", host) } + // No top-level domain starts with a digit, so such a name is an address + // in another spelling (0x7f.1, 2130706433), which a proxy may read as one. + if last := name[strings.LastIndexByte(name, '.')+1:]; last == "" || last[0] >= '0' && last[0] <= '9' { + return fmt.Errorf("%s is not a host name refetch can check", host) + } ctx, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() ips, err := net.DefaultResolver.LookupNetIP(ctx, "ip", host) diff --git a/fetch/proxy_test.go b/fetch/proxy_test.go index 8e5222d..0df5263 100644 --- a/fetch/proxy_test.go +++ b/fetch/proxy_test.go @@ -95,8 +95,8 @@ func TestProxyTunnelsBothClients(t *testing.T) { func TestProxyStillRefusesNonPublicTargets(t *testing.T) { cp := withProxy(t, nil) - for _, addr := range []string{"127.0.0.1:80", "10.1.2.3:443", "[::1]:443", "localhost:80", "app.localhost:443"} { - if _, err := DialContext(t.Context(), "tcp", addr); err == nil || !strings.Contains(err.Error(), "not a public address") { + for _, addr := range []string{"127.0.0.1:80", "10.1.2.3:443", "[::1]:443", "localhost:80", "app.localhost:443", "0x7f.1:80", "2130706433:443", "127.1:80", "0177.0.0.1:80"} { + if _, err := DialContext(t.Context(), "tcp", addr); err == nil || !strings.Contains(err.Error(), "not a public address") && !strings.Contains(err.Error(), "not a host name refetch can check") { t.Errorf("dial %s through the proxy: %v", addr, err) } } diff --git a/render/chrome.go b/render/chrome.go index 63133c6..cf6bd97 100644 --- a/render/chrome.go +++ b/render/chrome.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "slices" + "syscall" ) // macOSApps are the standard install paths, most common first. @@ -60,7 +61,7 @@ func playwrightChrome(home string) string { found, _ := filepath.Glob(filepath.Join(dir, pattern)) slices.SortFunc(found, func(a, b string) int { return cmp.Or(cmp.Compare(len(b), len(a)), cmp.Compare(b, a)) }) for _, p := range found { - if isExec(p) { + if isExec(p) && ownedSafely(p) { return p } } @@ -69,6 +70,31 @@ func playwrightChrome(home string) string { return "" } +// ownedSafely reports that p and every folder above it belong to root or +// to this user and that nobody else can write to them, so a Chromium found +// by searching (not named by the user) was not planted by another account. +func ownedSafely(p string) bool { + euid := uint32(os.Geteuid()) //nolint:gosec // a uid is never negative + for { + fi, err := os.Lstat(p) //nolint:gosec // p is a Playwright build found under a fixed folder, or a folder above it + if err != nil { + return false + } + st, ok := fi.Sys().(*syscall.Stat_t) + // A sticky folder such as /tmp is fine above the build: others + // cannot replace what they do not own there. + sticky := fi.IsDir() && fi.Mode()&os.ModeSticky != 0 + if !ok || (st.Uid != 0 && st.Uid != euid) || (fi.Mode().Perm()&0o022 != 0 && !sticky) { + return false + } + parent := filepath.Dir(p) + if parent == p { + return true + } + p = parent + } +} + func isExec(p string) bool { fi, err := os.Stat(p) //nolint:gosec // p is the user's own REFETCH_CHROME or a fixed install path return err == nil && !fi.IsDir() && fi.Mode()&0o111 != 0 diff --git a/render/chrome_test.go b/render/chrome_test.go index fa6ec4a..760f0e3 100644 --- a/render/chrome_test.go +++ b/render/chrome_test.go @@ -44,3 +44,21 @@ func TestPlaywrightChrome(t *testing.T) { t.Errorf("playwrightChrome = %q, want the newest full Chromium %q", got, want) } } + +func TestPlaywrightChromeOthersCanWrite(t *testing.T) { + dir := t.TempDir() + bin := filepath.Join(dir, "chromium-1200", "chrome-linux64", "chrome") + if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Join(dir, "chromium-1200"), 0o777); err != nil { + t.Fatal(err) + } + t.Setenv("PLAYWRIGHT_BROWSERS_PATH", dir) + if got := playwrightChrome(t.TempDir()); got == bin { + t.Errorf("a Chromium in a folder others can write was used: %s", got) + } +} diff --git a/render/render.go b/render/render.go index 27e0e5a..d93593c 100644 --- a/render/render.go +++ b/render/render.go @@ -204,7 +204,20 @@ func Version(ctx context.Context, chrome string) string { } ctx, cancel := context.WithTimeout(ctx, 3*time.Second) defer cancel() - out, err := exec.CommandContext(ctx, chrome, "--version").Output() + cmd := exec.CommandContext(ctx, chrome, "--version") + if os.Geteuid() == 0 { // as root, even this runs only as nobody, in a throwaway home + home, err := os.MkdirTemp("", "refetch-version-") + if err != nil { + return "" + } + defer func() { _ = os.RemoveAll(home) }() + cmd.SysProcAttr = &syscall.SysProcAttr{} + if err := runAsNobody(cmd, home); err != nil { + versions[chrome] = "" + return "" + } + } + out, err := cmd.Output() v := "" if err == nil { v = chromeVersion.FindString(string(out)) From 4c62ee245936f88341fbf30ad66179dbdcfa9823 Mon Sep 17 00:00:00 2001 From: samgozman Date: Sat, 24 Oct 2026 09:07:13 +0400 Subject: [PATCH 3/5] fix(render): pass Linux lint, drop root's groups, and say when a container blocks Chrome's sandbox The profile walk goes through os.Root (gosec G122 fails CI's Linux lint). Chrome as nobody no longer keeps root's supplementary groups. As root, a Chrome that nobody cannot reach (in /root/.cache, say) is not picked. When the container blocks the user namespaces Chrome's sandbox needs, the step says so instead of "signal: aborted". codecov.yml holds the project to 85%, the cover-check line, and a change to 70% of its own lines. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 4 +++- codecov.yml | 15 +++++++++++++++ render/chrome.go | 24 +++++++++++++++++++++++- render/chrome_test.go | 19 +++++++++++++++++-- render/proxy_test.go | 2 +- render/render.go | 6 ++++++ render/render_test.go | 13 +++++++++++++ render/sysproc_linux.go | 13 +++++++++---- 8 files changed, 87 insertions(+), 9 deletions(-) create mode 100644 codecov.yml diff --git a/README.md b/README.md index 9b3220a..85a746f 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,9 @@ paths, then `google-chrome` or `chromium` on PATH, then Playwright's Chromium `~/.cache/ms-playwright`). refetch never downloads a browser. Chrome's sandbox does not run as root, so when refetch runs as root (in a container, for example), it runs Chrome as the `nobody` user on Linux and skips the -step elsewhere. +step elsewhere. Chrome's sandbox needs user namespaces, which Docker's +default seccomp profile blocks, so in a container the step needs one that +allows them (`--security-opt seccomp=unconfined` is the blunt way). To turn refetch off entirely, disable the plugin in `/plugin`. diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000..8e0897c --- /dev/null +++ b/codecov.yml @@ -0,0 +1,15 @@ +# CI's cover-check fails the build below 85%; Codecov reports against the +# same line instead of against the last commit. +coverage: + status: + project: + default: + target: 85% + # Some new code only runs as root on Linux (Chrome as nobody), which CI + # does not test, so a change is held to 70% of its own lines. + patch: + default: + target: 70% +comment: + layout: "condensed_header, diff, files" + require_changes: true diff --git a/render/chrome.go b/render/chrome.go index cf6bd97..1466d98 100644 --- a/render/chrome.go +++ b/render/chrome.go @@ -97,5 +97,27 @@ func ownedSafely(p string) bool { func isExec(p string) bool { fi, err := os.Stat(p) //nolint:gosec // p is the user's own REFETCH_CHROME or a fixed install path - return err == nil && !fi.IsDir() && fi.Mode()&0o111 != 0 + if err != nil || fi.IsDir() || fi.Mode()&0o111 == 0 { + return false + } + return !canDropRoot || os.Geteuid() != 0 || nobodyCanRun(p) +} + +// nobodyCanRun reports that any user can reach and run p: as root, Chrome +// runs as nobody, so one in a private folder such as /root/.cache would +// only fail later. +func nobodyCanRun(p string) bool { + p, err := filepath.EvalSymlinks(p) + for err == nil { + fi, statErr := os.Stat(p) //nolint:gosec // p is a Chrome found above, or a folder above it + if statErr != nil || fi.Mode().Perm()&0o001 == 0 { + return false + } + parent := filepath.Dir(p) + if parent == p { + return true + } + p = parent + } + return false } diff --git a/render/chrome_test.go b/render/chrome_test.go index 760f0e3..8a64162 100644 --- a/render/chrome_test.go +++ b/render/chrome_test.go @@ -6,8 +6,23 @@ import ( "testing" ) -func TestFindChrome(t *testing.T) { +// chromeDir is a temporary folder for fake Chromes. As root, refetch runs +// Chrome as nobody, who must be able to reach it. +func chromeDir(t *testing.T) string { + t.Helper() dir := t.TempDir() + if os.Geteuid() == 0 { + for _, d := range []string{dir, filepath.Dir(dir)} { + if err := os.Chmod(d, 0o755); err != nil { + t.Fatal(err) + } + } + } + return dir +} + +func TestFindChrome(t *testing.T) { + dir := chromeDir(t) bin := filepath.Join(dir, "chromium") if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { t.Fatal(err) @@ -29,7 +44,7 @@ func TestFindChrome(t *testing.T) { } func TestPlaywrightChrome(t *testing.T) { - dir := t.TempDir() + dir := chromeDir(t) for _, p := range []string{"chromium-999/chrome-linux/chrome", "chromium-1200/chrome-linux64/chrome", "chromium_headless_shell-1300/chrome-linux/headless_shell"} { bin := filepath.Join(dir, p) if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { diff --git a/render/proxy_test.go b/render/proxy_test.go index 8901396..fe915dd 100644 --- a/render/proxy_test.go +++ b/render/proxy_test.go @@ -103,7 +103,7 @@ func TestName(t *testing.T) { if got := Name(t.Context(), ""); got != "" { t.Errorf("no Chrome: %q", got) } - dir := t.TempDir() + dir := chromeDir(t) for file, want := range map[string]string{"chromium": "Chromium 151", "google-chrome": "Chrome 150", "broken-chrome": "Chrome"} { script := "#!/bin/sh\necho 'Google Chrome 150.0.7000.1'\n" switch file { diff --git a/render/render.go b/render/render.go index d93593c..bb6a759 100644 --- a/render/render.go +++ b/render/render.go @@ -40,6 +40,7 @@ var ( ErrNoChrome = errors.New("no Chrome or Chromium found") ErrRoot = errors.New("running as root: headless Chrome needs its sandbox, which does not run as root (on Linux, refetch runs it as nobody instead)") ErrTimeout = errors.New("headless Chrome timed out") + ErrSandbox = errors.New("headless Chrome cannot start its sandbox here: the container blocks the user namespaces it needs (Docker's default seccomp profile does)") ) // Renderer runs the user's Chrome headless, behind a local proxy on the @@ -96,6 +97,8 @@ func (r *Renderer) Render(ctx context.Context, rawURL string) ([]byte, error) { cmd.WaitDelay = 2 * time.Second out := &capped{max: fetch.MaxBytes} cmd.Stdout = out + stderr := &capped{max: 64 << 10} + cmd.Stderr = stderr err = cmd.Run() if cmd.Process != nil { _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) // helpers that outlived the browser @@ -104,6 +107,9 @@ func (r *Renderer) Render(ctx context.Context, rawURL string) ([]byte, error) { return nil, fmt.Errorf("%w after %s", ErrTimeout, timeout) } if err != nil && out.buf.Len() == 0 { + if bytes.Contains(stderr.buf.Bytes(), []byte("No usable sandbox")) { + return nil, ErrSandbox + } return nil, fmt.Errorf("headless Chrome failed: %w", err) } if out.buf.Len() == 0 { diff --git a/render/render_test.go b/render/render_test.go index 3b6d5a1..02053cd 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "sync/atomic" @@ -133,3 +134,15 @@ func TestCapped(t *testing.T) { t.Errorf("capped = %q", c.buf.String()) } } + +func TestNoUsableSandbox(t *testing.T) { + chrome := filepath.Join(chromeDir(t), "chrome") + script := "#!/bin/sh\necho 'FATAL:zygote_host_impl_linux.cc(128)] No usable sandbox!' >&2\nexit 134\n" + if err := os.WriteFile(chrome, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + r := &Renderer{Chrome: chrome, Timeout: 5 * time.Second, dial: loopbackDial} + if _, err := r.Render(t.Context(), "https://example.com/"); !errors.Is(err, ErrSandbox) { + t.Errorf("Chrome without a sandbox: %v", err) + } +} diff --git a/render/sysproc_linux.go b/render/sysproc_linux.go index 57ae54b..4d550a4 100644 --- a/render/sysproc_linux.go +++ b/render/sysproc_linux.go @@ -5,7 +5,6 @@ import ( "os" "os/exec" "os/user" - "path/filepath" "strconv" "syscall" ) @@ -32,16 +31,22 @@ func runAsNobody(cmd *exec.Cmd, profile string) error { gid = uint32(n) } } - err := filepath.WalkDir(profile, func(p string, _ fs.DirEntry, err error) error { + // os.Root keeps the walk inside the profile, even if a link appears. + root, err := os.OpenRoot(profile) + if err != nil { + return err + } + defer func() { _ = root.Close() }() + err = fs.WalkDir(root.FS(), ".", func(p string, _ fs.DirEntry, err error) error { if err != nil { return err } - return os.Lchown(p, int(uid), int(gid)) + return root.Lchown(p, int(uid), int(gid)) }) if err != nil { return err } - cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid, NoSetGroups: true} + cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid} // no supplementary groups: root's would stay otherwise cmd.Env = append(os.Environ(), "HOME="+profile, "XDG_CONFIG_HOME="+profile, "XDG_CACHE_HOME="+profile) return nil } From 1cc77304d8947cb3717be02a1586e18a48065ee1 Mon Sep 17 00:00:00 2001 From: samgozman Date: Sat, 24 Oct 2026 09:18:41 +0400 Subject: [PATCH 4/5] test: cover the proxy's error paths and the hand-over to nobody runAsNobody is split into nobody() and runAs(), so a test can hand a profile to its own user without root. New tests cover nobodyCanRun, a proxy that hangs up, is unreachable, has no port or sends bytes with its reply, and an unparsable HTTPS_PROXY. Co-Authored-By: Claude Opus 5.5 (1M context) --- fetch/proxy_test.go | 61 ++++++++++++++++++++++++++++++++++++ render/chrome_test.go | 26 +++++++++++++++ render/sysproc_linux.go | 18 ++++++++--- render/sysproc_linux_test.go | 37 ++++++++++++++++++++++ 4 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 render/sysproc_linux_test.go diff --git a/fetch/proxy_test.go b/fetch/proxy_test.go index 0df5263..5d87e68 100644 --- a/fetch/proxy_test.go +++ b/fetch/proxy_test.go @@ -1,6 +1,8 @@ package fetch import ( + "bufio" + "context" "io" "net" "net/http" @@ -146,3 +148,62 @@ func TestOnlyHTTPProxies(t *testing.T) { t.Errorf("socks5 proxy: %v", err) } } + +// rawProxy accepts one connection, reads the CONNECT request and answers +// with reply verbatim ("" closes without a word). +func rawProxy(t *testing.T, reply string) *url.URL { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = ln.Close() }) + go func() { + conn, err := ln.Accept() + if err != nil { + return + } + defer func() { _ = conn.Close() }() + _, _ = http.ReadRequest(bufio.NewReader(conn)) + _, _ = io.WriteString(conn, reply) + }() + return &url.URL{Scheme: "http", Host: ln.Addr().String()} +} + +func TestProxyReplies(t *testing.T) { + conn, err := dialProxied(t.Context(), rawProxy(t, "HTTP/1.1 200 Connection established\r\n\r\nearly bytes"), "refetch-test.invalid:443") + if err != nil { + t.Fatal(err) + } + b := make([]byte, 64) + n, _ := conn.Read(b) + _ = conn.Close() + if string(b[:n]) != "early bytes" { + t.Errorf("bytes sent with the proxy's reply: %q", b[:n]) + } + if _, err := dialProxied(t.Context(), rawProxy(t, ""), "refetch-test.invalid:443"); err == nil { + t.Error("a proxy that hung up was accepted") + } + if _, err := dialProxied(t.Context(), &url.URL{Scheme: "http", Host: "127.0.0.1:1"}, "refetch-test.invalid:443"); err == nil { + t.Error("an unreachable proxy was accepted") + } + ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second) + defer cancel() + if _, err := dialProxied(ctx, &url.URL{Scheme: "http", Host: "127.0.0.1"}, "refetch-test.invalid:443"); err == nil { + t.Error("a proxy without a port: port 80 answered as a proxy") + } + if _, err := dialProxied(t.Context(), &url.URL{Scheme: "http", Host: "127.0.0.1:1"}, "no-port"); err == nil { + t.Error("a target without a port was accepted") + } +} + +func TestUseProxyUnparsable(t *testing.T) { + t.Cleanup(func() { upstream.Store(nil) }) + for _, k := range []string{"https_proxy", "HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy"} { + t.Setenv(k, "") + } + t.Setenv("HTTPS_PROXY", "http://[::1") + if got := UseProxy(); got != "http://[::1" { + t.Errorf("an unparsable proxy is reported as %q", got) + } +} diff --git a/render/chrome_test.go b/render/chrome_test.go index 8a64162..45dd1bb 100644 --- a/render/chrome_test.go +++ b/render/chrome_test.go @@ -77,3 +77,29 @@ func TestPlaywrightChromeOthersCanWrite(t *testing.T) { t.Errorf("a Chromium in a folder others can write was used: %s", got) } } + +func TestNobodyCanRun(t *testing.T) { + // /tmp and everything above it are open to everyone; only this folder + // is changed. + dir, err := os.MkdirTemp("/tmp", "refetch-test-") + if err != nil { + t.Skip(err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + bin := filepath.Join(dir, "chrome") + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + if nobodyCanRun(bin) { + t.Errorf("%s is inside a private folder", bin) + } + if err := os.Chmod(dir, 0o755); err != nil { + t.Fatal(err) + } + if !nobodyCanRun(bin) { + t.Errorf("%s is open to everyone", bin) + } + if nobodyCanRun(filepath.Join(dir, "missing")) { + t.Error("a missing file can be run") + } +} diff --git a/render/sysproc_linux.go b/render/sysproc_linux.go index 4d550a4..a2dfc04 100644 --- a/render/sysproc_linux.go +++ b/render/sysproc_linux.go @@ -18,11 +18,16 @@ func sysProcAttr() *syscall.SysProcAttr { // canDropRoot is true where a root refetch can run Chrome as another user. const canDropRoot = true -// runAsNobody runs Chrome as the unprivileged nobody user, which keeps -// Chrome's sandbox on when refetch runs as root (in a container). The -// throwaway profile becomes nobody's, and Chrome's home with it. +// runAsNobody runs Chrome as nobody, which keeps its sandbox on when +// refetch runs as root (in a container). func runAsNobody(cmd *exec.Cmd, profile string) error { - uid, gid := uint32(65534), uint32(65534) + uid, gid := nobody() + return runAs(cmd, profile, uid, gid) +} + +// nobody is the nobody user's uid and gid, 65534 where it has no entry. +func nobody() (uid, gid uint32) { + uid, gid = 65534, 65534 if u, err := user.Lookup("nobody"); err == nil { if n, err := strconv.ParseUint(u.Uid, 10, 32); err == nil { uid = uint32(n) @@ -31,6 +36,11 @@ func runAsNobody(cmd *exec.Cmd, profile string) error { gid = uint32(n) } } + return uid, gid +} + +// runAs hands the profile, as its home, to the user cmd runs as. +func runAs(cmd *exec.Cmd, profile string, uid, gid uint32) error { // os.Root keeps the walk inside the profile, even if a link appears. root, err := os.OpenRoot(profile) if err != nil { diff --git a/render/sysproc_linux_test.go b/render/sysproc_linux_test.go new file mode 100644 index 0000000..f091244 --- /dev/null +++ b/render/sysproc_linux_test.go @@ -0,0 +1,37 @@ +package render + +import ( + "os" + "os/exec" + "path/filepath" + "slices" + "syscall" + "testing" +) + +// TestRunAs hands a profile to this test's own user, the one hand-over a +// test can make without root. +func TestRunAs(t *testing.T) { + if uid, gid := nobody(); uid == 0 || gid == 0 { + t.Errorf("nobody is %d:%d", uid, gid) + } + profile := t.TempDir() + if err := os.MkdirAll(filepath.Join(profile, "Default"), 0o700); err != nil { + t.Fatal(err) + } + uid, gid := uint32(os.Getuid()), uint32(os.Getgid()) + cmd := exec.Command("chrome") + cmd.SysProcAttr = &syscall.SysProcAttr{} + if err := runAs(cmd, profile, uid, gid); err != nil { + t.Fatal(err) + } + if c := cmd.SysProcAttr.Credential; c == nil || c.Uid != uid || c.Gid != gid || c.NoSetGroups || len(c.Groups) != 0 { + t.Errorf("credential %+v, want %d:%d with no supplementary groups", c, uid, gid) + } + if !slices.Contains(cmd.Env, "HOME="+profile) { + t.Errorf("HOME is not the profile: %v", cmd.Env) + } + if err := runAs(cmd, filepath.Join(profile, "missing"), uid, gid); err == nil { + t.Error("a missing profile was handed over") + } +} From d199d734d53da57105a50a4911bc57d4b750e095 Mon Sep 17 00:00:00 2001 From: samgozman Date: Sat, 24 Oct 2026 09:22:08 +0400 Subject: [PATCH 5/5] feat(doctor): name a Playwright Chromium that was skipped because others can write to it Playwright's Docker images make /ms-playwright writable by everyone, so refetch does not run that Chromium on its own. doctor now names it and the REFETCH_CHROME setting that uses it anyway. Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 5 ++++- cmd/refetch/doctor.go | 4 ++++ cmd/refetch/proxy_test.go | 29 +++++++++++++++++++++++++++++ render/chrome.go | 32 ++++++++++++++++++++++++++++---- render/chrome_test.go | 3 +++ 5 files changed, 68 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 85a746f..39299c6 100644 --- a/README.md +++ b/README.md @@ -206,7 +206,10 @@ sandbox does not run as root, so when refetch runs as root (in a container, for example), it runs Chrome as the `nobody` user on Linux and skips the step elsewhere. Chrome's sandbox needs user namespaces, which Docker's default seccomp profile blocks, so in a container the step needs one that -allows them (`--security-opt seccomp=unconfined` is the blunt way). +allows them (`--security-opt seccomp=unconfined` is the blunt way). refetch +skips a Playwright Chromium that other users can write to, as in +Playwright's Docker images; `refetch doctor` names it, and setting +`REFETCH_CHROME` to it uses it anyway. To turn refetch off entirely, disable the plugin in `/plugin`. diff --git a/cmd/refetch/doctor.go b/cmd/refetch/doctor.go index 23e012f..1801d11 100644 --- a/cmd/refetch/doctor.go +++ b/cmd/refetch/doctor.go @@ -193,6 +193,10 @@ func chromeCheck(ctx context.Context, cfg config.Config) check { if p := render.FindChrome(); p != "" { return check{true, "chrome", fmt.Sprintf("%s at %s; headless Chrome step on (timeout %s)", render.Name(ctx, p), p, cfg.RenderTimeout)} } + if p := render.UnsafePlaywrightChrome(); p != "" { + return check{false, "chrome", "Playwright's Chromium at " + p + " is skipped: other users can write to it or a folder above it.\n" + + "If this machine is yours alone (a container, say), set REFETCH_CHROME=" + p} + } return check{false, "chrome", "no Chrome or Chromium found, so the headless Chrome step is skipped.\n" + "Install Google Chrome, or set REFETCH_CHROME to a Chrome binary."} } diff --git a/cmd/refetch/proxy_test.go b/cmd/refetch/proxy_test.go index f7114a4..40736dc 100644 --- a/cmd/refetch/proxy_test.go +++ b/cmd/refetch/proxy_test.go @@ -1,10 +1,14 @@ package main import ( + "os" + "path/filepath" + "strings" "testing" "github.com/samgozman/refetch/config" "github.com/samgozman/refetch/fetch" + "github.com/samgozman/refetch/render" ) func TestNetworkCheck(t *testing.T) { @@ -33,3 +37,28 @@ func TestVersionOfADevelopmentCheckout(t *testing.T) { t.Errorf("REFETCH_BIN with a development checkout: %+v", c) } } + +func TestChromeCheckNamesAnUnsafePlaywright(t *testing.T) { + dir := t.TempDir() + bin := filepath.Join(dir, "chromium-1200", "chrome-linux64", "chrome") + if err := os.MkdirAll(filepath.Dir(bin), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Join(dir, "chromium-1200"), 0o777); err != nil { + t.Fatal(err) + } + t.Setenv("PLAYWRIGHT_BROWSERS_PATH", dir) + t.Setenv("REFETCH_CHROME", "") + t.Setenv("PATH", "") + t.Setenv("HOME", t.TempDir()) + if render.FindChrome() != "" { + t.Skip("a Chrome is installed in a fixed path here") + } + c := chromeCheck(t.Context(), config.Default()) + if c.ok || !strings.Contains(c.detail, "set REFETCH_CHROME="+bin) { + t.Errorf("chrome check: %+v", c) + } +} diff --git a/render/chrome.go b/render/chrome.go index 1466d98..bf4a4fd 100644 --- a/render/chrome.go +++ b/render/chrome.go @@ -52,6 +52,30 @@ func FindChrome() string { } func playwrightChrome(home string) string { + for _, p := range playwrightFound(home) { + if ownedSafely(p) { + return p + } + } + return "" +} + +// UnsafePlaywrightChrome is a Playwright Chromium that FindChrome passed +// over because others can write to it, as in Playwright's Docker images; +// "" when there is none. +func UnsafePlaywrightChrome() string { + home, _ := os.UserHomeDir() + for _, p := range playwrightFound(home) { + if !ownedSafely(p) { + return p + } + } + return "" +} + +// playwrightFound lists the Playwright Chromium builds, preferred first. +func playwrightFound(home string) []string { + var all []string dirs := []string{"/ms-playwright", "/opt/pw-browsers", filepath.Join(home, ".cache", "ms-playwright")} if p := os.Getenv("PLAYWRIGHT_BROWSERS_PATH"); p != "" && p != "0" { dirs = append([]string{p}, dirs...) @@ -61,13 +85,13 @@ func playwrightChrome(home string) string { found, _ := filepath.Glob(filepath.Join(dir, pattern)) slices.SortFunc(found, func(a, b string) int { return cmp.Or(cmp.Compare(len(b), len(a)), cmp.Compare(b, a)) }) for _, p := range found { - if isExec(p) && ownedSafely(p) { - return p + if isExec(p) { + all = append(all, p) } } } } - return "" + return all } // ownedSafely reports that p and every folder above it belong to root or @@ -76,7 +100,7 @@ func playwrightChrome(home string) string { func ownedSafely(p string) bool { euid := uint32(os.Geteuid()) //nolint:gosec // a uid is never negative for { - fi, err := os.Lstat(p) //nolint:gosec // p is a Playwright build found under a fixed folder, or a folder above it + fi, err := os.Lstat(p) if err != nil { return false } diff --git a/render/chrome_test.go b/render/chrome_test.go index 45dd1bb..c99cbf3 100644 --- a/render/chrome_test.go +++ b/render/chrome_test.go @@ -76,6 +76,9 @@ func TestPlaywrightChromeOthersCanWrite(t *testing.T) { if got := playwrightChrome(t.TempDir()); got == bin { t.Errorf("a Chromium in a folder others can write was used: %s", got) } + if got := UnsafePlaywrightChrome(); got != bin { + t.Errorf("UnsafePlaywrightChrome = %q, want %q", got, bin) + } } func TestNobodyCanRun(t *testing.T) {