From 8e3a6cd339dd8c6d59a77d411fb8507181aef1f6 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Tue, 22 Sep 2026 22:02:38 -0700 Subject: [PATCH] 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 | 5 ++- internal/api/api.go | 31 ++++++++++--- internal/api/api_test.go | 14 ++++++ internal/clean/clean.go | 33 +++++++++++++- internal/fetch/fetch.go | 23 ++++++---- tests/contract/sdk_test.py | 90 ++++++++++++++++++++++++-------------- 6 files changed, 148 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index 3a8e13c..7ceb911 100644 --- a/Makefile +++ b/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 diff --git a/internal/api/api.go b/internal/api/api.go index 701365e..91668a7 100644 --- a/internal/api/api.go +++ b/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: diff --git a/internal/api/api_test.go b/internal/api/api_test.go index d2ff96d..65c33b6 100644 --- a/internal/api/api_test.go +++ b/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()) + } +} diff --git a/internal/clean/clean.go b/internal/clean/clean.go index 3c227ed..fcb8e9d 100644 --- a/internal/clean/clean.go +++ b/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 { diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go index 81921e0..02b2ffd 100644 --- a/internal/fetch/fetch.go +++ b/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() - client.Timeout = f.timeout() - - resp, err := client.Do(req) + resp, err := f.httpClient().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, diff --git a/tests/contract/sdk_test.py b/tests/contract/sdk_test.py index 51af18e..c8a4b49 100644 --- a/tests/contract/sdk_test.py +++ b/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"Contract

hello contract

" 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, - stderr=subprocess.DEVNULL, + stdout=logf.open("w"), + 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 - 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. + skip = os.environ.get("SIFT_CONTRACT_SKIP_SDK") == "1" try: from firecrawl import Firecrawl # type: ignore except ImportError: - print("firecrawl-py not installed; JSON contract passed") - return 0 + 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: