Keep the Chrome sandbox on by default

kage launched Chrome with --no-sandbox unconditionally, which turns off the browser's main security boundary for every run, including ordinary desktop use where the sandbox works fine. Since kage renders pages from the open web, a renderer exploit could then reach the host. Reported in #10. Keep the sandbox on by default and drop it only where it genuinely cannot initialize: inside a container, or when running as root (Chrome refuses to start a sandbox as root). Containers are detected from IN_DOCKER or the /.dockerenv marker, and there kage also sets --disable-dev-shm-usage because the default 64 MB /dev/shm is too small for the renderer on large pages. Whenever the sandbox is dropped kage says so on stderr, so it is never silent. Thanks to Dimitrios Prasakis for the report and to the commenter on Hacker News who suggested the IN_DOCKER opt-in.
2026-06-15 12:24:55 +07:00
parent 01e75b87ec
commit dab6c11ea8
8 changed files with 486 additions and 16 deletions
@@ -6,6 +6,34 @@ All notable changes to kage are recorded here. The format follows

 ## [Unreleased]

+### Security
+
+- Chrome now keeps its sandbox on by default. It was previously launched with
+  `--no-sandbox` unconditionally, which removed Chrome's main line of defense
+  when rendering pages from the open web (reported in #10). The sandbox is now
+  dropped only where it genuinely cannot run: inside a container, or when
+  running as root. `KAGE_NO_SANDBOX=1` forces it off and `KAGE_NO_SANDBOX=0`
+  forces it on, and the choice is logged so it is never silent.
+
+### Added
+
+- Container-aware Chrome flags. kage detects a container from the `IN_DOCKER`
+  environment variable or a `/.dockerenv` marker and, only there, drops the
+  sandbox and adds `--disable-dev-shm-usage` (the default 64 MB `/dev/shm` is
+  too small for Chrome on large pages). Outside a container the faster shared
+  memory is left in place.
+- Asset downloads retry on a transient failure (a 403/429, a 5xx, or a network
+  blip) with a short backoff, recovering files that bot-protection rejects on
+  the first request of a burst. Permanent failures (404, 401, …) are not
+  retried.
+
+### Changed
+
+- Clearer crawl error reporting. Each failure is logged with a classified
+  reason (`HTTP 403 Forbidden`, `timed out`, …), the URL, and the page that
+  referenced it, and the end-of-run summary lists what went wrong instead of
+  printing only a count.
+
 ## [0.1.1] - 2026-06-14

 ### Added
@@ -2,6 +2,7 @@ package asset

 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -18,6 +19,7 @@ type Downloader struct {
 	Client    *http.Client
 	UserAgent string
 	MaxBytes  int64 // per-asset cap; 0 = unlimited
+	Retries   int   // extra attempts for a transient failure (0 = try once)
 }

 // NewDownloader builds a Downloader with a sane client and the given timeout.
@@ -26,6 +28,10 @@ func NewDownloader(userAgent string, timeout time.Duration, maxBytes int64) *Dow
 		Client:    &http.Client{Timeout: timeout},
 		UserAgent: userAgent,
 		MaxBytes:  maxBytes,
+		// A few sites (and the bot-protection in front of them) reject the first
+		// request of a burst with a 403 or 429 but serve a retry fine, so give
+		// transient failures a couple of extra tries before giving up.
+		Retries: 3,
 	}
 }

@@ -36,9 +42,55 @@ type Result struct {
 	IsCSS       bool
 }

+// StatusError reports a non-2xx HTTP response. It carries the code so callers
+// can render a clear message ("HTTP 403 Forbidden") and decide whether a retry
+// is worthwhile, without the URL baked in (the caller already has it).
+type StatusError struct {
+	Code int
+}
+
+func (e *StatusError) Error() string {
+	if t := http.StatusText(e.Code); t != "" {
+		return fmt.Sprintf("HTTP %d %s", e.Code, t)
+	}
+	return fmt.Sprintf("HTTP %d", e.Code)
+}
+
 // Get fetches u, sending referer as the Referer header. It reads at most
 // MaxBytes and reports whether the body is CSS (so the caller can rewrite it).
+// A transient failure (a 403/429/5xx or a network blip) is retried with a short
+// backoff up to Retries times.
 func (d *Downloader) Get(ctx context.Context, u *url.URL, referer string) (*Result, error) {
+	attempts := d.Retries + 1
+	if attempts < 1 {
+		attempts = 1
+	}
+	var lastErr error
+	for i := 0; i < attempts; i++ {
+		if err := ctx.Err(); err != nil {
+			return nil, err
+		}
+		if i > 0 {
+			select {
+			case <-ctx.Done():
+				return nil, ctx.Err()
+			case <-time.After(backoff(i)):
+			}
+		}
+		res, err := d.try(ctx, u, referer)
+		if err == nil {
+			return res, nil
+		}
+		lastErr = err
+		if !transient(err) {
+			break
+		}
+	}
+	return nil, lastErr
+}
+
+// try performs a single fetch attempt.
+func (d *Downloader) try(ctx context.Context, u *url.URL, referer string) (*Result, error) {
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
 	if err != nil {
 		return nil, err
@@ -55,7 +107,7 @@ func (d *Downloader) Get(ctx context.Context, u *url.URL, referer string) (*Resu
 	}
 	defer func() { _ = resp.Body.Close() }()
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return nil, fmt.Errorf("status %d for %s", resp.StatusCode, u)
+		return nil, &StatusError{Code: resp.StatusCode}
 	}
 	var r io.Reader = resp.Body
 	if d.MaxBytes > 0 {
@@ -73,6 +125,34 @@ func (d *Downloader) Get(ctx context.Context, u *url.URL, referer string) (*Resu
 	}, nil
 }

+// backoff returns the pause before retry attempt i (1-based): 500ms, 1s, 2s, …
+func backoff(i int) time.Duration {
+	d := 500 * time.Millisecond << (i - 1)
+	if max := 5 * time.Second; d > max {
+		d = max
+	}
+	return d
+}
+
+// transient reports whether an error is worth retrying. Bot-protection statuses
+// (403/429), request-timeout and too-early (408/425), and 5xx server errors are
+// transient; other 4xx (404, 401, 410, …) are permanent. A network error is
+// retried, but a cancelled or expired context is not.
+func transient(err error) bool {
+	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+		return false
+	}
+	var se *StatusError
+	if errors.As(err, &se) {
+		switch se.Code {
+		case http.StatusForbidden, http.StatusRequestTimeout, http.StatusTooEarly, http.StatusTooManyRequests:
+			return true
+		}
+		return se.Code >= 500
+	}
+	return true
+}
+
 // isCSS reports whether a response is a stylesheet, by content-type or by a
 // .css path when the server sends no useful type.
 func isCSS(contentType string, u *url.URL) bool {
@@ -0,0 +1,112 @@
+package asset
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+func TestStatusErrorMessage(t *testing.T) {
+	cases := map[int]string{
+		403: "HTTP 403 Forbidden",
+		404: "HTTP 404 Not Found",
+		999: "HTTP 999",
+	}
+	for code, want := range cases {
+		if got := (&StatusError{Code: code}).Error(); got != want {
+			t.Errorf("StatusError{%d} = %q; want %q", code, got, want)
+		}
+	}
+}
+
+func TestGetRetriesTransientThenSucceeds(t *testing.T) {
+	var hits int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// 403 on the first try (like bot-protection), then serve the file.
+		if atomic.AddInt32(&hits, 1) == 1 {
+			w.WriteHeader(http.StatusForbidden)
+			return
+		}
+		w.Header().Set("Content-Type", "text/css")
+		_, _ = w.Write([]byte("body{}"))
+	}))
+	defer srv.Close()
+
+	d := NewDownloader("kage-test", 5*time.Second, 0)
+	u, _ := url.Parse(srv.URL + "/style.css")
+	res, err := d.Get(context.Background(), u, "")
+	if err != nil {
+		t.Fatalf("Get after retry: %v", err)
+	}
+	if !res.IsCSS || string(res.Body) != "body{}" {
+		t.Errorf("unexpected result: css=%v body=%q", res.IsCSS, res.Body)
+	}
+	if hits < 2 {
+		t.Errorf("expected a retry; server saw %d hits", hits)
+	}
+}
+
+func TestGetDoesNotRetryPermanent(t *testing.T) {
+	var hits int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&hits, 1)
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	defer srv.Close()
+
+	d := NewDownloader("kage-test", 5*time.Second, 0)
+	u, _ := url.Parse(srv.URL + "/missing.png")
+	_, err := d.Get(context.Background(), u, "")
+
+	var se *StatusError
+	if !errors.As(err, &se) || se.Code != 404 {
+		t.Fatalf("got %v; want StatusError 404", err)
+	}
+	if hits != 1 {
+		t.Errorf("404 should not be retried; server saw %d hits", hits)
+	}
+}
+
+func TestGetGivesUpAfterRetries(t *testing.T) {
+	var hits int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&hits, 1)
+		w.WriteHeader(http.StatusTooManyRequests)
+	}))
+	defer srv.Close()
+
+	d := NewDownloader("kage-test", 5*time.Second, 0)
+	d.Retries = 2
+	u, _ := url.Parse(srv.URL + "/rate.css")
+	_, err := d.Get(context.Background(), u, "")
+
+	var se *StatusError
+	if !errors.As(err, &se) || se.Code != 429 {
+		t.Fatalf("got %v; want StatusError 429", err)
+	}
+	if hits != 3 { // 1 try + 2 retries
+		t.Errorf("expected 3 attempts, server saw %d", hits)
+	}
+}
+
+func TestTransientClassification(t *testing.T) {
+	transientCodes := []int{403, 408, 425, 429, 500, 502, 503}
+	for _, c := range transientCodes {
+		if !transient(&StatusError{Code: c}) {
+			t.Errorf("status %d should be transient", c)
+		}
+	}
+	for _, c := range []int{400, 401, 404, 410} {
+		if transient(&StatusError{Code: c}) {
+			t.Errorf("status %d should be permanent", c)
+		}
+	}
+	if transient(context.Canceled) {
+		t.Error("context.Canceled should not be transient")
+	}
+}
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"os"
 	"runtime"
+	"strings"
 	"sync"
 	"time"

@@ -130,9 +131,25 @@ func (p *Pool) getBrowser() (*rod.Browser, error) {
 		l := launcher.New().
 			Headless(p.opts.Headless).
 			Set("disable-blink-features", "AutomationControlled").
-			Set("disable-dev-shm-usage", "").
-			Set("no-sandbox", "").
 			Set("disable-gpu", "")
+
+		// Chrome's sandbox is the main line of defense when rendering pages from
+		// the open web, so kage keeps it on by default (issue #10). It is dropped
+		// only where it genuinely cannot initialize: inside a container, or when
+		// running as root, where Chrome otherwise refuses to start. The decision
+		// is logged so it is never silent.
+		if off, reason := disableSandbox(); off {
+			l = l.Set("no-sandbox", "")
+			warnSandboxDisabled(reason)
+		}
+
+		// In a container, the default /dev/shm is only 64 MB, too small for
+		// Chrome's renderer on large pages, so steer it to a temp file instead.
+		// Outside a container /dev/shm is roomy and faster, so leave it alone.
+		if inContainer() {
+			l = l.Set("disable-dev-shm-usage", "")
+		}
+
 		if bin := p.chromeBin(); bin != "" {
 			l = l.Bin(bin)
 		}
@@ -225,6 +242,80 @@ func systemChromeCandidates() []string {
 	}
 }

+// disableSandbox decides whether Chrome should launch without its sandbox, with
+// a short reason for the log. The secure default is to keep the sandbox on; it
+// is dropped only where it cannot run: inside a container, or when running as
+// root (Chrome refuses to start a sandbox as root).
+func disableSandbox() (off bool, reason string) {
+	if inContainer() {
+		return true, "container"
+	}
+	if isRoot() {
+		return true, "root"
+	}
+	return false, ""
+}
+
+// warnSandboxDisabled prints why the sandbox was turned off, so dropping a
+// security boundary is always visible rather than silent.
+func warnSandboxDisabled(reason string) {
+	switch reason {
+	case "container":
+		fmt.Fprintln(os.Stderr, "kage: container detected, Chrome sandbox disabled")
+	case "root":
+		fmt.Fprintln(os.Stderr, "kage: running as root, Chrome sandbox disabled (run as a non-root user to keep it on)")
+	}
+}
+
+// inContainer reports whether kage is running inside a container, where Chrome
+// needs container-specific flags. It honors IN_DOCKER (set it in your image)
+// and the /.dockerenv marker that Docker writes into every container.
+//
+// Keeping the sandbox on by default and dropping it only here was prompted by
+// Dimitrios Prasakis (issue #10); the IN_DOCKER opt-in was suggested on Hacker
+// News (https://news.ycombinator.com/item?id=48534865). Thanks to both.
+func inContainer() bool {
+	if envTrue("IN_DOCKER") {
+		return true
+	}
+	if _, err := os.Stat("/.dockerenv"); err == nil {
+		return true
+	}
+	return false
+}
+
+// isRoot reports whether the process runs as the superuser. On Windows
+// os.Geteuid returns -1, so this is false there.
+func isRoot() bool {
+	return os.Geteuid() == 0
+}
+
+// envTrue reports whether the named environment variable is set to a truthy
+// value.
+func envTrue(name string) bool {
+	v, ok := envBool(name)
+	return ok && v
+}
+
+// envBool parses a boolean-ish environment variable. It returns ok=false when
+// the variable is unset or empty. "1", "true", "yes", "on" are true and "0",
+// "false", "no", "off" are false (case-insensitive); any other non-empty value
+// counts as true, so IN_DOCKER=docker reads as set.
+func envBool(name string) (val, ok bool) {
+	s := strings.TrimSpace(os.Getenv(name))
+	if s == "" {
+		return false, false
+	}
+	switch strings.ToLower(s) {
+	case "1", "true", "yes", "on":
+		return true, true
+	case "0", "false", "no", "off":
+		return false, true
+	default:
+		return true, true
+	}
+}
+
 // settle waits for the network to go quiet for d, recovering from any rod
 // panic and capping the wait so a chatty page can never hang the worker.
 func settle(page *rod.Page, d time.Duration) {
@@ -4,6 +4,7 @@ import (
 	"context"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -17,6 +18,70 @@ func TestLookChromeReadsEnv(t *testing.T) {
 	}
 }

+func TestEnvBool(t *testing.T) {
+	cases := []struct {
+		in      string
+		set     bool
+		wantVal bool
+		wantOk  bool
+	}{
+		{"", false, false, false},
+		{"1", true, true, true},
+		{"true", true, true, true},
+		{"TRUE", true, true, true},
+		{"yes", true, true, true},
+		{"on", true, true, true},
+		{"0", true, false, true},
+		{"false", true, false, true},
+		{"off", true, false, true},
+		{"no", true, false, true},
+		{"docker", true, true, true},   // any other non-empty value is true
+		{"  true  ", true, true, true}, // trimmed
+	}
+	for _, c := range cases {
+		if c.set {
+			t.Setenv("KAGE_TEST_BOOL", c.in)
+		} else {
+			_ = os.Unsetenv("KAGE_TEST_BOOL")
+		}
+		val, ok := envBool("KAGE_TEST_BOOL")
+		if val != c.wantVal || ok != c.wantOk {
+			t.Errorf("envBool(%q) = (%v, %v); want (%v, %v)", c.in, val, ok, c.wantVal, c.wantOk)
+		}
+	}
+}
+
+func TestDisableSandboxDefaultKeepsItOn(t *testing.T) {
+	// Not in a container and not root, the sandbox stays on. (When the test
+	// itself runs as root, e.g. some CI containers, "root" is the honest
+	// reason; accept that rather than asserting a false negative.)
+	t.Setenv("IN_DOCKER", "")
+	off, reason := disableSandbox()
+	if isRoot() || inContainer() {
+		if !off {
+			t.Errorf("disableSandbox() = false as root/container; want true")
+		}
+		return
+	}
+	if off {
+		t.Errorf("disableSandbox() = true (%q) on a normal host; want sandbox kept on", reason)
+	}
+}
+
+func TestInContainerHonorsEnv(t *testing.T) {
+	t.Setenv("IN_DOCKER", "1")
+	if !inContainer() {
+		t.Errorf("inContainer() = false with IN_DOCKER=1; want true")
+	}
+}
+
+func TestDisableSandboxContainer(t *testing.T) {
+	t.Setenv("IN_DOCKER", "true")
+	if off, reason := disableSandbox(); !off || reason != "container" {
+		t.Errorf("in container: got (%v, %q); want (true, container)", off, reason)
+	}
+}
+
 func TestRenderCapturesFinalDOM(t *testing.T) {
 	if testing.Short() {
 		t.Skip("render test drives Chrome; skipped under -short")
@@ -186,9 +186,27 @@ func printSummary(res clone.Result) {
 		styleAccent.Render("assets"), res.Assets)
 	if res.PageErrors+res.AssetErrors > 0 {
 		fmt.Fprintf(os.Stderr, "  %s %d\n", styleErr.Render("errors"), res.PageErrors+res.AssetErrors)
+		printFailures(res)
 	}
 	if res.Skipped > 0 {
 		fmt.Fprintf(os.Stderr, "  %s %d\n", styleWarn.Render("skipped"), res.Skipped)
 	}
 	fmt.Fprintf(os.Stderr, "  open %s\n", styleAccent.Render("kage serve "+res.OutDir))
 }
+
+// printFailures lists what went wrong, grouped reason and URL, so the error
+// count is actionable instead of opaque. The list is capped during the crawl;
+// when it overflows, say how many more there were.
+func printFailures(res clone.Result) {
+	total := res.PageErrors + res.AssetErrors
+	for _, f := range res.Failures {
+		line := fmt.Sprintf("    %s  %s", styleErr.Render(f.Reason), f.URL)
+		fmt.Fprintln(os.Stderr, line)
+		if f.Referer != "" {
+			fmt.Fprintln(os.Stderr, styleDim.Render("      referenced by "+f.Referer))
+		}
+	}
+	if more := total - int64(len(res.Failures)); more > 0 {
+		fmt.Fprintln(os.Stderr, styleDim.Render(fmt.Sprintf("    ... and %d more", more)))
+	}
+}
@@ -2,6 +2,7 @@ package clone

 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -165,7 +166,7 @@ func (c *Cloner) Run(ctx context.Context) (Result, error) {
 		}
 	}

-	res := Result{Progress: c.stats.snapshot(), OutDir: c.outRoot}
+	res := Result{Progress: c.stats.snapshot(), OutDir: c.outRoot, Failures: c.stats.recordedFailures()}
 	if ctx.Err() != nil {
 		return res, ctx.Err()
 	}
@@ -236,15 +237,13 @@ func (c *Cloner) processPage(ctx context.Context, j pageItem) {

 	res, err := c.pool.Render(ctx, j.u.String())
 	if err != nil {
-		c.stats.pageErrors.Add(1)
-		c.logf("page error %s: %v", j.u, err)
+		c.failPage(j.u.String(), fmt.Errorf("render: %w", err))
 		return
 	}

 	root, err := html.Parse(strings.NewReader(res.HTML))
 	if err != nil {
-		c.stats.pageErrors.Add(1)
-		c.logf("parse error %s: %v", j.u, err)
+		c.failPage(j.u.String(), fmt.Errorf("parse: %w", err))
 		return
 	}

@@ -275,12 +274,11 @@ func (c *Cloner) processPage(ctx context.Context, j pageItem) {

 	var buf strings.Builder
 	if err := html.Render(&buf, root); err != nil {
-		c.stats.pageErrors.Add(1)
+		c.failPage(j.u.String(), fmt.Errorf("render html: %w", err))
 		return
 	}
 	if err := c.writeFile(localFile, []byte(buf.String())); err != nil {
-		c.stats.pageErrors.Add(1)
-		c.logf("write error %s: %v", localFile, err)
+		c.failPage(j.u.String(), fmt.Errorf("write %s: %w", localFile, err))
 		return
 	}
 	c.front.markVisited(key)
@@ -295,8 +293,7 @@ func (c *Cloner) processAsset(ctx context.Context, j assetItem) {
 	}
 	res, err := c.dl.Get(ctx, j.u, j.referer)
 	if err != nil {
-		c.stats.assetErrors.Add(1)
-		c.logf("asset error %s: %v", j.u, err)
+		c.failAsset(j.u.String(), j.referer, err)
 		return
 	}

@@ -312,13 +309,54 @@ func (c *Cloner) processAsset(ctx context.Context, j assetItem) {
 		body = asset.RewriteCSS(body, j.u, cssSink)
 	}
 	if err := c.writeFile(localFile, body); err != nil {
-		c.stats.assetErrors.Add(1)
-		c.logf("write error %s: %v", localFile, err)
+		c.failAsset(j.u.String(), j.referer, fmt.Errorf("write %s: %w", localFile, err))
 		return
 	}
 	c.stats.assets.Add(1)
 }

+// failAsset records and logs a failed asset, naming the page that referenced it
+// so a 403 or 404 is traceable back to where it came from. The reason is
+// classified (HTTP status, timeout, or other) for a readable line.
+func (c *Cloner) failAsset(u, referer string, err error) {
+	c.stats.assetErrors.Add(1)
+	reason := classifyError(err)
+	c.stats.recordFailure(Failure{Kind: "asset", URL: u, Referer: referer, Reason: reason})
+	if referer != "" {
+		c.logf("asset error: %s\n    %s\n    referenced by %s", reason, u, referer)
+	} else {
+		c.logf("asset error: %s\n    %s", reason, u)
+	}
+}
+
+// failPage records and logs a failed page.
+func (c *Cloner) failPage(u string, err error) {
+	c.stats.pageErrors.Add(1)
+	reason := classifyError(err)
+	c.stats.recordFailure(Failure{Kind: "page", URL: u, Reason: reason})
+	c.logf("page error: %s\n    %s", reason, u)
+}
+
+// classifyError turns an error into a short, human-readable reason for the log
+// and the final report: an HTTP status with its name, a timeout, a cancellation,
+// or the underlying message otherwise.
+func classifyError(err error) string {
+	if err == nil {
+		return ""
+	}
+	var se *asset.StatusError
+	if errors.As(err, &se) {
+		return se.Error()
+	}
+	switch {
+	case errors.Is(err, context.DeadlineExceeded):
+		return "timed out"
+	case errors.Is(err, context.Canceled):
+		return "cancelled"
+	}
+	return err.Error()
+}
+
 // enqueuePage offers a page URL to the frontier, honouring the visited set, the
 // depth cap, and the page budget. It reports whether the page was newly queued.
 func (c *Cloner) enqueuePage(ctx context.Context, u *url.URL, depth int) bool {
@@ -1,6 +1,14 @@
 package clone

-import "sync/atomic"
+import (
+	"sync"
+	"sync/atomic"
+)
+
+// maxRecordedFailures caps how many individual failures Run keeps for the final
+// report, so a huge broken site cannot grow the slice without bound. The error
+// counters still count every failure.
+const maxRecordedFailures = 100

 // stats are the live counters of a run, read by the CLI's progress ticker.
 type stats struct {
@@ -9,6 +17,34 @@ type stats struct {
 	pageErrors  atomic.Int64
 	assetErrors atomic.Int64
 	skipped     atomic.Int64 // robots-disallowed or out of budget
+
+	muFail   sync.Mutex
+	failures []Failure
+}
+
+// Failure is one thing that went wrong, kept for the end-of-run report so the
+// errors are visible as a list rather than only as a count.
+type Failure struct {
+	Kind    string // "page" or "asset"
+	URL     string
+	Referer string // the page that referenced it, when known
+	Reason  string // e.g. "HTTP 403 Forbidden"
+}
+
+func (s *stats) recordFailure(f Failure) {
+	s.muFail.Lock()
+	if len(s.failures) < maxRecordedFailures {
+		s.failures = append(s.failures, f)
+	}
+	s.muFail.Unlock()
+}
+
+func (s *stats) recordedFailures() []Failure {
+	s.muFail.Lock()
+	defer s.muFail.Unlock()
+	out := make([]Failure, len(s.failures))
+	copy(out, s.failures)
+	return out
 }

 // Progress is a snapshot of a run for display.
@@ -34,4 +70,6 @@ func (s *stats) snapshot() Progress {
 type Result struct {
 	Progress
 	OutDir string
+	// Failures is a capped sample of what went wrong, for the final report.
+	Failures []Failure
 }