From 0d96838ee4adcb9078e314967a7b5b6000f11da8 Mon Sep 17 00:00:00 2001
From: Tigah <88289044+TBX3D@users.noreply.github.com>
Date: Wed, 22 Jul 2026 12:35:18 -0700
Subject: [PATCH] fix(scan): bound crawl breadth with a page budget (#269)

Crawl only capped recursion depth, not breadth per level. A link-heavy page
(pagination, faceted search) drives fetch count toward branching^depth with no
ceiling: a depth-3 crawl of a page linking 40 fresh urls issued 1641 real
fetches.

Cap total fetches at a page budget via an atomic counter in an OnRequest hook
that aborts once exceeded, and add a Truncated flag on CrawlResult so callers
can tell a run was cut short.
---
 internal/scan/crawl.go             | 28 ++++++++++-
 internal/scan/crawl_budget_test.go | 75 ++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 internal/scan/crawl_budget_test.go

diff --git a/internal/scan/crawl.go b/internal/scan/crawl.go
index 3bbaba1..55f908e 100644
--- a/internal/scan/crawl.go
+++ b/internal/scan/crawl.go
@@ -17,6 +17,7 @@ import (
 	"net/url"
 	"sort"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/gocolly/colly/v2"
@@ -26,9 +27,15 @@ import (
 	"github.com/vmfunc/sif/internal/output"
 )
 
+// maxCrawlPages bounds total fetches independent of depth: MaxDepth caps
+// recursion depth but not breadth, so a link-heavy page (pagination, faceted
+// search) otherwise grows request count as branching^depth with no ceiling.
+const maxCrawlPages = 500
+
 // CrawlResult holds the deduped set of urls discovered by the spider.
 type CrawlResult struct {
-	URLs []string `json:"urls"`
+	URLs      []string `json:"urls"`
+	Truncated bool     `json:"truncated,omitempty"` // hit maxCrawlPages before the spider ran out of links
 }
 
 func (r *CrawlResult) ResultType() string { return "crawl" }
@@ -74,6 +81,11 @@ func Crawl(targetURL string, depth int, timeout time.Duration, logdir string) (*
 	var mu sync.Mutex
 	seen := make(map[string]struct{})
 
+	// visited caps the total pages fetched (see maxCrawlPages); atomic since
+	// colly's callbacks can fire from multiple goroutines.
+	var visited int64
+	var truncated int32
+
 	record := func(raw string) {
 		if raw == "" {
 			return
@@ -94,6 +106,15 @@ func Crawl(targetURL string, depth int, timeout time.Duration, logdir string) (*
 		mu.Unlock()
 	}
 
+	// count every request toward the budget (the seed included) and abort once
+	// it is exceeded, before the dial, so a link-heavy target can't run away.
+	collector.OnRequest(func(r *colly.Request) {
+		if atomic.AddInt64(&visited, 1) > maxCrawlPages {
+			atomic.StoreInt32(&truncated, 1)
+			r.Abort()
+		}
+	})
+
 	// links drive recursion; scripts/forms are recorded but not followed.
 	collector.OnHTML("a[href]", func(e *colly.HTMLElement) {
 		link := e.Request.AbsoluteURL(e.Attr("href"))
@@ -120,7 +141,10 @@ func Crawl(targetURL string, depth int, timeout time.Duration, logdir string) (*
 	}
 	collector.Wait()
 
-	result := &CrawlResult{URLs: sortedKeys(seen)}
+	result := &CrawlResult{URLs: sortedKeys(seen), Truncated: atomic.LoadInt32(&truncated) != 0}
+	if result.Truncated {
+		log.Warn("crawl hit the %d-page budget and stopped early; results are partial", maxCrawlPages)
+	}
 
 	log.Complete(len(result.URLs), "urls")
 	return result, nil
diff --git a/internal/scan/crawl_budget_test.go b/internal/scan/crawl_budget_test.go
new file mode 100644
index 0000000..bd7f750
--- /dev/null
+++ b/internal/scan/crawl_budget_test.go
@@ -0,0 +1,75 @@
+package scan
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+// TestCrawlBudgetCapsUnboundedBreadth guards against the request count Crawl
+// issues growing without bound. MaxDepth only caps recursion depth; without a
+// breadth budget, a page that hands back many fresh links per level (ordinary
+// pagination/faceted-search shapes, not just a deliberate trap) drives the
+// fetch count to branching^depth. This pins the total at maxCrawlPages instead.
+func TestCrawlBudgetCapsUnboundedBreadth(t *testing.T) {
+	const branching = 40 // links per page
+	var reqCount int64
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/robots.txt", func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	})
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt64(&reqCount, 1)
+		// every page hands back `branching` brand-new, never-before-seen paths
+		// keyed off the requesting path, so colly's exact-url dedupe can't
+		// collapse them - the pagination/calendar-trap shape.
+		prefix := r.URL.Path
+		if prefix == "/" {
+			prefix = ""
+		}
+		for i := 0; i < branching; i++ {
+			fmt.Fprintf(w, `<a href="%s/%d">x</a>`, prefix, i)
+		}
+	})
+
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	// depth=3 would otherwise fetch 1 + branching + branching^2 = 1641 pages;
+	// the budget must cut that off at maxCrawlPages regardless of depth.
+	result, err := Crawl(srv.URL, 3, 5*time.Second, "")
+	if err != nil {
+		t.Fatalf("Crawl: %v", err)
+	}
+
+	got := atomic.LoadInt64(&reqCount)
+	t.Logf("depth=3 branching=%d -> %d requests (budget %d), truncated=%v",
+		branching, got, maxCrawlPages, result.Truncated)
+
+	if got > maxCrawlPages {
+		t.Fatalf("expected the page budget to cap requests at %d, got %d", maxCrawlPages, got)
+	}
+	if !result.Truncated {
+		t.Fatalf("expected Truncated=true once the budget is hit, got false")
+	}
+}
+
+// TestCrawlBudgetDoesNotTruncateSmallSites is the negative case: an ordinary
+// small site well under the budget crawls to completion and reports
+// Truncated=false, so the new cap doesn't regress normal runs.
+func TestCrawlBudgetDoesNotTruncateSmallSites(t *testing.T) {
+	srv := crawlSite(t)
+	defer srv.Close()
+
+	result, err := Crawl(srv.URL, 2, 2*time.Second, "")
+	if err != nil {
+		t.Fatalf("Crawl: %v", err)
+	}
+	if result.Truncated {
+		t.Fatalf("small site should not hit the page budget, got Truncated=true (urls=%v)", result.URLs)
+	}
+}