p1: cap jq output, log refusals, fail contract without SDK

jq results are bounded (10k items / output cap) and return 400 when
exceeded. 401/403 log caller, host, and reason. make contract exits 1
unless firecrawl-py is present (SIFT_CONTRACT_SKIP_SDK=1 to skip).

Makefile

@@ -30,7 +30,10 @@ contract:
 	@if command -v python3 >/dev/null 2>&1; then \
 	  $(GO) build -tags sifttest -trimpath -ldflags "$(LDFLAGS)" -o bin/sift-test ./cmd/sift && \
 	  python3 tests/contract/sdk_test.py; \
-	else echo "python3 not found; SDK contract skipped"; fi
+	else \
+	  if [ "$$SIFT_CONTRACT_SKIP_SDK" = "1" ]; then echo "python3 missing; skipped"; \
+	  else echo "python3 required for make contract" >&2; exit 1; fi; \
+	fi
 
 install: build
 	install -d $(DESTDIR)$(PREFIX)/bin

internal/api/api.go

@@ -119,11 +119,13 @@ func (s *Server) caller(w http.ResponseWriter, r *http.Request) *auth.Caller {
 	tok := strings.TrimSpace(strings.TrimPrefix(h, "Bearer"))
 	tok = strings.TrimSpace(tok)
 	if tok == "" || s.Auth == nil {
+		s.Log.Warn("unauthorized", "caller", "-", "host", "-", "reason", "missing key")
 		writeErr(w, 401, "unauthorized", "")
 		return nil
 	}
 	c := s.Auth.LookupBearer(tok)
 	if c == nil {
+		s.Log.Warn("unauthorized", "caller", "-", "host", "-", "reason", "unknown key")
 		writeErr(w, 401, "unauthorized", "")
 		return nil
 	}
@@ -164,7 +166,7 @@ func (s *Server) scrape(w http.ResponseWriter, r *http.Request) {
 	s.scrapeN.Add(1)
 	res, cl, ferr := s.doFetch(r, c, req.URL, "", onlyMain)
 	if ferr != nil {
-		s.writeFetchErr(w, ferr)
+		s.writeFetchErr(w, c, req.URL, ferr)
 		return
 	}
 	md, htmlOut := cl.Markdown, ""
@@ -225,7 +227,7 @@ func (s *Server) v1fetch(w http.ResponseWriter, r *http.Request) {
 	onlyMain := true
 	res, cl, ferr := s.doFetch(r, c, req.URL, req.JQ, onlyMain)
 	if ferr != nil {
-		s.writeFetchErr(w, ferr)
+		s.writeFetchErr(w, c, req.URL, ferr)
 		return
 	}
 	content := cl.Markdown
@@ -235,7 +237,7 @@ func (s *Server) v1fetch(w http.ResponseWriter, r *http.Request) {
 	if mode == "raw" {
 		content = string(res.Body)
 		if len(content) > s.OutCap {
-			content = content[:s.OutCap]
+			content = clean.TruncateUTF8(content, s.OutCap)
 			cl.Trunc = true
 			cl.Warning = "truncated to output cap"
 		}
@@ -299,12 +301,21 @@ func (s *Server) doFetch(r *http.Request, c *auth.Caller, rawURL, jq string, onl
 		cl, cerr = clean.Text(res, s.OutCap)
 	}
 	if cerr != nil {
+		var tl *clean.TooLargeError
+		if errors.As(cerr, &tl) {
+			return nil, nil, &fetch.Error{Status: 400, Msg: tl.Error()}
+		}
 		return nil, nil, &fetch.Error{Status: 422, Msg: cerr.Error()}
 	}
 	return res, cl, nil
 }
 
-func (s *Server) writeFetchErr(w http.ResponseWriter, err error) {
+func (s *Server) writeFetchErr(w http.ResponseWriter, c *auth.Caller, rawURL string, err error) {
+	name := "-"
+	if c != nil {
+		name = c.Name
+	}
+	host := hostOf(rawURL)
 	var fe *fetch.Error
 	if errors.As(err, &fe) {
 		if fe.Status == 403 {
@@ -314,10 +325,15 @@ func (s *Server) writeFetchErr(w http.ResponseWriter, err error) {
 			fe.Status = 500
 			fe.Msg = "internal error"
 		}
+		if fe.Status == 401 || fe.Status == 403 {
+			s.Log.Warn("refused", "caller", name, "host", host, "reason", fe.Msg)
+		} else {
+			s.Log.Warn("fetch failed", "caller", name, "host", host, "reason", fe.Msg, "status", fe.Status)
+		}
 		writeErr(w, fe.Status, fe.Msg, fe.Hint)
 		return
 	}
-	s.Log.Error("internal", "err", err)
+	s.Log.Error("internal", "caller", name, "host", host, "err", err)
 	writeErr(w, 500, "internal error", "")
 }
 
@@ -349,13 +365,16 @@ func (s *Server) release(caller, rawURL string) {
 	key := caller + "|" + hostOf(rawURL)
 	s.perMu.Lock()
 	ch := s.per[key]
-	s.perMu.Unlock()
 	if ch != nil {
 		select {
 		case <-ch:
 		default:
 		}
+		if len(ch) == 0 {
+			delete(s.per, key)
 		}
+	}
+	s.perMu.Unlock()
 	select {
 	case <-s.global:
 	default:

internal/api/api_test.go

@@ -217,3 +217,17 @@ func TestFetcherUsesSameGuard(t *testing.T) {
 	_ = key
 	_ = tls.VersionTLS13
 }
+
+func TestJQTooLarge(t *testing.T) {
+	g := &guard.Guard{AllowPrefixes: []netip.Prefix{netip.MustParsePrefix("127.0.0.0/8")}}
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`0`))
+	}))
+	defer ts.Close()
+	s, key := setupAPI(t, g)
+	rec := postJSON(t, s, "/v1/fetch", key, map[string]any{"url": ts.URL, "jq": "range(20000)", "mode": "clean"})
+	if rec.Code != 400 || !strings.Contains(rec.Body.String(), "jq output too large") {
+		t.Fatalf("%d %s", rec.Code, rec.Body.String())
+	}
+}

internal/clean/clean.go

@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	readability "github.com/go-shiori/go-readability"
 	htmltomarkdown "github.com/JohannesKaufmann/html-to-markdown/v2"
@@ -140,6 +141,7 @@ func JSON(res *fetch.Resource, jqFilter string, outCap int) (*Result, error) {
 		}
 		iter := code.RunWithContext(ctx, v)
 		var acc []any
+		var running int
 		for {
 			x, ok := iter.Next()
 			if !ok {
@@ -149,6 +151,17 @@ func JSON(res *fetch.Resource, jqFilter string, outCap int) (*Result, error) {
 				return nil, fmt.Errorf("jq: %w", err)
 			}
 			acc = append(acc, x)
+			if len(acc) > 10_000 {
+				return nil, &TooLargeError{Msg: "jq output too large"}
+			}
+			b, err := json.Marshal(x)
+			if err != nil {
+				return nil, fmt.Errorf("jq: %w", err)
+			}
+			running += len(b)
+			if running > outCap {
+				return nil, &TooLargeError{Msg: "jq output too large"}
+			}
 		}
 		switch len(acc) {
 		case 0:
@@ -183,15 +196,33 @@ func Text(res *fetch.Resource, outCap int) (*Result, error) {
 	return capResult(out, outCap), nil
 }
 
+// TooLargeError is a jq result that exceeded item or byte caps.
+type TooLargeError struct{ Msg string }
+
+func (e *TooLargeError) Error() string { return e.Msg }
+
 func capResult(r *Result, capn int) *Result {
 	if len(r.Markdown) > capn {
-		r.Markdown = r.Markdown[:capn] + "\n…[truncated]"
+		r.Markdown = TruncateUTF8(r.Markdown, capn) + "\n…[truncated]"
 		r.Trunc = true
 		r.Warning = "truncated to output cap"
 	}
 	return r
 }
 
+func TruncateUTF8(s string, n int) string {
+	if n >= len(s) {
+		return s
+	}
+	if n < 0 {
+		return ""
+	}
+	for n > 0 && !utf8.RuneStart(s[n]) {
+		n--
+	}
+	return s[:n]
+}
+
 func stripDanger(s string) string {
 	doc, err := html.Parse(strings.NewReader(s))
 	if err != nil {

internal/fetch/fetch.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+	"sync"
 	"time"
 
 	"git.wntrmute.dev/kyle/sift/internal/guard"
@@ -60,6 +61,9 @@ type Fetcher struct {
 	UA      string
 	MaxBody int64
 	Timeout time.Duration
+
+	clientOnce sync.Once
+	client     *http.Client
 }
 
 func (f *Fetcher) maxBody() int64 {
@@ -83,7 +87,16 @@ func (f *Fetcher) ua() string {
 	return "sift/dev"
 }
 
-// Fetch GETs rawURL after Guard.CheckURL, using Guard.Client() from the same instance.
+func (f *Fetcher) httpClient() *http.Client {
+	f.clientOnce.Do(func() {
+		c := f.Guard.Client()
+		c.Timeout = f.timeout()
+		f.client = c
+	})
+	return f.client
+}
+
+// Fetch GETs rawURL after Guard.CheckURL, using a Client built once from the same Guard.
 func (f *Fetcher) Fetch(ctx context.Context, rawURL string) (*Resource, error) {
 	if f == nil || f.Guard == nil {
 		return nil, errf(500, "", "internal error")
@@ -103,10 +116,7 @@ func (f *Fetcher) Fetch(ctx context.Context, rawURL string) (*Resource, error) {
 	req.Header.Set("Accept", "text/html,application/xhtml+xml,application/json;q=0.9,text/plain;q=0.8,*/*;q=0.1")
 	// no cookies, no auth, no Referer
 
-	client := f.Guard.Client()
+	resp, err := f.httpClient().Do(req)
-	client.Timeout = f.timeout()
-
-	resp, err := client.Do(req)
 	if err != nil {
 		return nil, mapDial(err)
 	}
@@ -126,9 +136,6 @@ func (f *Fetcher) Fetch(ctx context.Context, rawURL string) (*Resource, error) {
 		final = resp.Request.URL.String()
 	}
 	ct := resp.Header.Get("Content-Type")
-	if i := strings.IndexByte(ct, ';'); i >= 0 {
-		// keep full header for charset; also store raw
-	}
 	res := &Resource{
 		URL:         rawURL,
 		FinalURL:    final,

tests/contract/sdk_test.py

@@ -26,6 +26,30 @@ class Handler(BaseHTTPRequestHandler):
         pass
 
     def do_GET(self):
+        if self.path.startswith("/forbid"):
+            body = b"nope"
+            self.send_response(403)
+            self.send_header("Content-Type", "text/plain")
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+            return
+        if self.path.startswith("/big"):
+            body = b"x" * (6 * 1024 * 1024)
+            self.send_response(200)
+            self.send_header("Content-Type", "text/plain")
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+            return
+        if self.path.startswith("/json"):
+            body = b'{"hello":"contract"}'
+            self.send_response(200)
+            self.send_header("Content-Type", "application/json")
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+            return
         body = b"<!doctype html><html><head><title>Contract</title></head><body><article><p>hello contract</p></article></body></html>"
         self.send_response(200)
         self.send_header("Content-Type", "text/html; charset=utf-8")
@@ -52,14 +76,14 @@ def main() -> int:
 
     tmp = Path(tempfile.mkdtemp(prefix="sift-contract-"))
     keys = tmp / "keys.toml"
-    keyfile = tmp / "key"
     raw = secrets.token_urlsafe(32)
-    keyfile.write_text(raw + "\n")
-    keyfile.chmod(0o600)
     digest = hashlib.sha256(raw.encode()).hexdigest()
-    keys.write_text(f'[[caller]]\nname = "contract"\nworkspace = "hermes-fleet"\nkey_sha256 = "{digest}"\n')
+    keys.write_text(
+        f'[[caller]]\nname = "contract"\nworkspace = "hermes-fleet"\nkey_sha256 = "{digest}"\n'
+    )
 
     listen = f"127.0.0.1:{free_port()}"
+    logf = tmp / "sift.log"
     proc = subprocess.Popen(
         [
             str(BIN),
@@ -71,8 +95,8 @@ def main() -> int:
             "-test-allow-prefix",
             "127.0.0.0/8",
         ],
-        stdout=subprocess.DEVNULL,
+        stdout=logf.open("w"),
-        stderr=subprocess.DEVNULL,
+        stderr=subprocess.STDOUT,
     )
     try:
         for _ in range(50):
@@ -98,40 +122,42 @@ def main() -> int:
         with urllib.request.urlopen(req, timeout=10) as resp:
             env = json.loads(resp.read())
         assert env.get("success") is True, env
-        md = env["data"]["markdown"]
-        assert "hello" in md.lower() or "contract" in md.lower(), md
-        assert env["data"]["metadata"]["title"]
 
-        # 403 guard
+        skip = os.environ.get("SIFT_CONTRACT_SKIP_SDK") == "1"
-        req = urllib.request.Request(
-            f"http://{listen}/v2/scrape",
-            data=json.dumps({"url": "http://10.0.0.5/"}).encode(),
-            headers={
-                "Authorization": f"Bearer {raw}",
-                "Content-Type": "application/json",
-            },
-            method="POST",
-        )
-        try:
-            urllib.request.urlopen(req, timeout=10)
-            print("expected 403", file=sys.stderr)
-            return 1
-        except urllib.error.HTTPError as e:
-            if e.code != 403:
-                print("want 403 got", e.code, file=sys.stderr)
-                return 1
-
-        # Real SDK if installed.
         try:
             from firecrawl import Firecrawl  # type: ignore
         except ImportError:
-            print("firecrawl-py not installed; JSON contract passed")
+            if skip:
+                print("SIFT_CONTRACT_SKIP_SDK=1; SDK not installed")
                 return 0
+            print("firecrawl-py==4.17.0 is required for make contract", file=sys.stderr)
+            print("install it, or set SIFT_CONTRACT_SKIP_SDK=1", file=sys.stderr)
+            return 1
 
         app = Firecrawl(api_url=f"http://{listen}", api_key=raw)
         doc = app.scrape(page, formats=["markdown"])
-        text = getattr(doc, "markdown", None) or (doc.get("markdown") if isinstance(doc, dict) else None)
+        text = getattr(doc, "markdown", None) or (
+            doc.get("markdown") if isinstance(doc, dict) else None
+        )
         assert text, doc
+
+        def scrape_err(url: str, key: str | None = None):
+            c = Firecrawl(api_url=f"http://{listen}", api_key=key if key is not None else raw)
+            try:
+                c.scrape(url, formats=["markdown"])
+                raise AssertionError(f"expected error for {url}")
+            except Exception as e:
+                return str(e)
+
+        msg = scrape_err("http://10.0.0.5/")
+        assert "blocked:" in msg, msg
+        msg = scrape_err(f"http://127.0.0.1:{fx_port}/forbid")
+        assert "hint: browser" in msg, msg
+        msg = scrape_err(f"http://127.0.0.1:{fx_port}/big")
+        assert "413" in msg or "too large" in msg, msg
+        msg = scrape_err(page, key="wrong")
+        assert "401" in msg or "Unauthorized" in msg or "unauthorized" in msg.lower(), msg
+
         print("firecrawl-py SDK contract passed")
         return 0
     finally: