From 6dce83bfaf0e4089d7647b79e30a24d867bcfc5b Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 15 Aug 2026 19:49:16 -0700 Subject: [PATCH 1/3] fix(runner): stream --ids sources and cap selection to the pinned set PLAN: HTTP and file --ids loaders used unbounded ReadAll. PR 45 capped HTTP at 256 KiB, which rejects the valid 1.3 MiB full SkillTrustBench JSONL list and left local files unbounded. DO: stream both sources through the existing line parser and reject more than 5520 unique IDs, the pinned SkillTrustBench full set. Signed-off-by: Sebastien Tardif --- cmd/clawscan/main.go | 2 +- docs/benchmarks.md | 4 ++- internal/runner/benchmark.go | 26 ++++++++++------ internal/runner/runner_test.go | 57 ++++++++++++++++++++++++++++++++++ skills/clawscan-cli/SKILL.md | 3 +- 5 files changed, 80 insertions(+), 12 deletions(-) diff --git a/cmd/clawscan/main.go b/cmd/clawscan/main.go index 86ac01d..ea38a3d 100644 --- a/cmd/clawscan/main.go +++ b/cmd/clawscan/main.go @@ -807,7 +807,7 @@ OpenClaw install policy: Benchmark command flags: --split Benchmark split. Defaults to benchmark for SkillTrustBench and eval_holdout for clawhub-security-signals. - --ids Run selected benchmark IDs from a text file or JSONL id source. SkillTrustBench only. + --ids Run selected benchmark IDs from a streamed text or JSONL source (max 5520 IDs). SkillTrustBench only. --limit Maximum benchmark rows to run. 0 means all rows. --offset Benchmark row offset. Defaults to 0. --predictions-output Write benchmark predictions JSONL. Defaults next to --output for clawhub-security-signals. diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 77682c0..2dfd08a 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -12,7 +12,9 @@ clawscan benchmark SkillTrustBench \ ``` Use `--ids ` with SkillTrustBench to run a fixed subset from a -plain text ID list or JSONL rows with an `id` field. +plain text ID list or JSONL rows with an `id` field. The loader streams the +source (file or HTTP) and accepts at most 5,520 unique IDs, the size of the +pinned SkillTrustBench full set. ## Available benchmarks diff --git a/internal/runner/benchmark.go b/internal/runner/benchmark.go index 1c9f115..3d38ade 100644 --- a/internal/runner/benchmark.go +++ b/internal/runner/benchmark.go @@ -40,6 +40,10 @@ const ( huggingFaceRowsEndpoint = "https://datasets-server.huggingface.co/rows" huggingFaceRowsPageSize = 100 huggingFaceRowsMaxAttempts = 6 + // maxSkillTrustBenchIDSelection is the pinned SkillTrustBench full set + // (5,520 cases). --ids is SkillTrustBench-only, so a valid selection + // cannot contain more unique IDs than that set. + maxSkillTrustBenchIDSelection = 5520 ) var huggingFaceRowsRetryDelay = 2 * time.Second @@ -312,11 +316,12 @@ func LoadBenchmarkIDSelection(source string) (BenchmarkIDSelection, error) { if source == "" { return BenchmarkIDSelection{}, errors.New("--ids source is required") } - data, err := readBenchmarkIDSource(source) + reader, err := openBenchmarkIDSource(source) if err != nil { return BenchmarkIDSelection{}, err } - ids, err := parseBenchmarkIDs(source, data) + defer reader.Close() + ids, err := parseBenchmarkIDs(source, reader) if err != nil { return BenchmarkIDSelection{}, err } @@ -329,28 +334,28 @@ func LoadBenchmarkIDSelection(source string) (BenchmarkIDSelection, error) { }, nil } -func readBenchmarkIDSource(source string) ([]byte, error) { +func openBenchmarkIDSource(source string) (io.ReadCloser, error) { if parsed, err := url.Parse(source); err == nil && (parsed.Scheme == "http" || parsed.Scheme == "https") { client := &http.Client{Timeout: 60 * time.Second} resp, err := client.Get(source) if err != nil { return nil, fmt.Errorf("read --ids source %s: %w", source, err) } - defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode > 299 { + resp.Body.Close() return nil, fmt.Errorf("read --ids source %s: HTTP %d", source, resp.StatusCode) } - return io.ReadAll(resp.Body) + return resp.Body, nil } - data, err := os.ReadFile(source) + file, err := os.Open(source) if err != nil { return nil, fmt.Errorf("read --ids source %s: %w", source, err) } - return data, nil + return file, nil } -func parseBenchmarkIDs(source string, data []byte) ([]string, error) { - scanner := bufio.NewScanner(strings.NewReader(string(data))) +func parseBenchmarkIDs(source string, reader io.Reader) ([]string, error) { + scanner := bufio.NewScanner(reader) scanner.Buffer(make([]byte, 1024), 1024*1024) var ids []string seen := map[string]bool{} @@ -370,6 +375,9 @@ func parseBenchmarkIDs(source string, data []byte) ([]string, error) { } seen[id] = true ids = append(ids, id) + if len(ids) > maxSkillTrustBenchIDSelection { + return nil, fmt.Errorf("--ids source %s exceeds the %d-id SkillTrustBench selection limit", source, maxSkillTrustBenchIDSelection) + } } if err := scanner.Err(); err != nil { return nil, fmt.Errorf("read --ids source %s: %w", source, err) diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index a6115cd..e3ef0e6 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -801,6 +801,63 @@ func TestLoadBenchmarkIDSelectionRejectsBadSources(t *testing.T) { } } +func TestLoadBenchmarkIDSelectionAcceptsJSONLLargerThan256KiB(t *testing.T) { + payload := oversizedBenchmarkIDJSONL(t, 400) + if len(payload) <= 256*1024 { + t.Fatalf("fixture is %d bytes, want more than 256 KiB", len(payload)) + } + + path := filepath.Join(t.TempDir(), "ids.jsonl") + if err := os.WriteFile(path, payload, 0o644); err != nil { + t.Fatal(err) + } + fileSelection, err := LoadBenchmarkIDSelection(path) + if err != nil { + t.Fatal(err) + } + if len(fileSelection.IDs) != 400 { + t.Fatalf("file ids = %d, want 400", len(fileSelection.IDs)) + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(payload) + })) + defer server.Close() + + httpSelection, err := LoadBenchmarkIDSelection(server.URL + "/ids.jsonl") + if err != nil { + t.Fatal(err) + } + if len(httpSelection.IDs) != 400 { + t.Fatalf("http ids = %d, want 400", len(httpSelection.IDs)) + } +} + +func TestLoadBenchmarkIDSelectionRejectsMoreIDsThanPinnedSet(t *testing.T) { + var body strings.Builder + for i := 0; i < maxSkillTrustBenchIDSelection+1; i++ { + fmt.Fprintf(&body, "case_%05d\n", i) + } + path := filepath.Join(t.TempDir(), "ids.txt") + if err := os.WriteFile(path, []byte(body.String()), 0o644); err != nil { + t.Fatal(err) + } + _, err := LoadBenchmarkIDSelection(path) + if err == nil || !strings.Contains(err.Error(), "5520-id") { + t.Fatalf("err = %v, want 5520-id selection limit", err) + } +} + +func oversizedBenchmarkIDJSONL(t *testing.T, count int) []byte { + t.Helper() + var body strings.Builder + pad := strings.Repeat("x", 700) + for i := 0; i < count; i++ { + fmt.Fprintf(&body, `{"id":"case_%05d","judgment":"normal","pad":"%s"}`+"\n", i, pad) + } + return []byte(body.String()) +} + func TestRunSkillTrustBenchBenchmarkRejectsMissingSelectedID(t *testing.T) { dir := t.TempDir() idsPath := filepath.Join(dir, "ids.txt") diff --git a/skills/clawscan-cli/SKILL.md b/skills/clawscan-cli/SKILL.md index c8a742e..2e0e24a 100644 --- a/skills/clawscan-cli/SKILL.md +++ b/skills/clawscan-cli/SKILL.md @@ -213,7 +213,8 @@ SkillTrustBench uses split `benchmark`. The first live run downloads and caches into temporary scan targets. Use `--ids ` with SkillTrustBench to run a fixed subset from a -plain text file with one ID per line or JSONL rows with an `id` field. `--ids` +plain text file with one ID per line or JSONL rows with an `id` field. The +source is streamed and may contain at most 5,520 unique IDs. `--ids` preserves source order, records `idsSource`, `idsCount`, and `idsSha256` in the artifact, and is mutually exclusive with `--limit` and `--offset`. From 86adc26753436579be4ef69dd1fe57434c631c3e Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 15 Aug 2026 20:06:56 -0700 Subject: [PATCH 2/3] fix(runner): bound retained --ids text, not just count The 5,520-id cap ran after each extracted id was stored. A hostile HTTP source could still retain thousands of unique megabyte-sized ids. Cap one id at 256 bytes and retained id text at 256 KiB. That still accepts the documented SkillTrustBench set. Signed-off-by: Sebastien Tardif --- internal/runner/benchmark.go | 16 ++++++++++++++++ internal/runner/runner_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/internal/runner/benchmark.go b/internal/runner/benchmark.go index 3d38ade..e0b0961 100644 --- a/internal/runner/benchmark.go +++ b/internal/runner/benchmark.go @@ -44,6 +44,14 @@ const ( // (5,520 cases). --ids is SkillTrustBench-only, so a valid selection // cannot contain more unique IDs than that set. maxSkillTrustBenchIDSelection = 5520 + // maxBenchmarkIDBytes caps one extracted id. Documented SkillTrustBench + // ids are case_NNNNN. The scanner still allows 1 MiB records, so this + // stops a hostile source from retaining megabyte-sized unique ids. + maxBenchmarkIDBytes = 256 + // maxBenchmarkIDSelectionBytes caps retained id text (not the JSONL + // stream). 256 KiB holds the 5,520-id set with headroom; it is not a + // file-size limit (the full JSONL is about 1.3 MiB). + maxBenchmarkIDSelectionBytes = 256 * 1024 ) var huggingFaceRowsRetryDelay = 2 * time.Second @@ -359,6 +367,7 @@ func parseBenchmarkIDs(source string, reader io.Reader) ([]string, error) { scanner.Buffer(make([]byte, 1024), 1024*1024) var ids []string seen := map[string]bool{} + retained := 0 lineNumber := 0 for scanner.Scan() { lineNumber++ @@ -373,8 +382,15 @@ func parseBenchmarkIDs(source string, reader io.Reader) ([]string, error) { if seen[id] { return nil, fmt.Errorf("--ids source %s line %d duplicates benchmark id %s", source, lineNumber, id) } + if len(id) > maxBenchmarkIDBytes { + return nil, fmt.Errorf("--ids source %s line %d exceeds the %d-byte benchmark id limit", source, lineNumber, maxBenchmarkIDBytes) + } + if retained+len(id) > maxBenchmarkIDSelectionBytes { + return nil, fmt.Errorf("--ids source %s exceeds the %d-byte retained-id budget", source, maxBenchmarkIDSelectionBytes) + } seen[id] = true ids = append(ids, id) + retained += len(id) if len(ids) > maxSkillTrustBenchIDSelection { return nil, fmt.Errorf("--ids source %s exceeds the %d-id SkillTrustBench selection limit", source, maxSkillTrustBenchIDSelection) } diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index e3ef0e6..187656d 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -833,6 +833,34 @@ func TestLoadBenchmarkIDSelectionAcceptsJSONLLargerThan256KiB(t *testing.T) { } } +func TestLoadBenchmarkIDSelectionRejectsOversizedRetainedIDs(t *testing.T) { + huge := strings.Repeat("a", maxBenchmarkIDBytes+1) + path := filepath.Join(t.TempDir(), "huge-id.txt") + if err := os.WriteFile(path, []byte(huge+"\n"), 0o644); err != nil { + t.Fatal(err) + } + _, err := LoadBenchmarkIDSelection(path) + if err == nil || !strings.Contains(err.Error(), "256-byte benchmark id limit") { + t.Fatalf("err = %v, want 256-byte benchmark id limit", err) + } + + var body strings.Builder + // 2000 IDs * 200 bytes is under the 5,520 count cap but over the + // retained-id budget (256 KiB). + chunk := strings.Repeat("b", 200) + for i := 0; i < 2000; i++ { + fmt.Fprintf(&body, "%s-%04d\n", chunk, i) + } + aggPath := filepath.Join(t.TempDir(), "agg-ids.txt") + if err := os.WriteFile(aggPath, []byte(body.String()), 0o644); err != nil { + t.Fatal(err) + } + _, err = LoadBenchmarkIDSelection(aggPath) + if err == nil || !strings.Contains(err.Error(), "262144-byte retained-id budget") { + t.Fatalf("err = %v, want 262144-byte retained-id budget", err) + } +} + func TestLoadBenchmarkIDSelectionRejectsMoreIDsThanPinnedSet(t *testing.T) { var body strings.Builder for i := 0; i < maxSkillTrustBenchIDSelection+1; i++ { From 6db87f2422471eb56db5fb82424ab6f7cd3c12b1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 15 Sep 2026 20:48:14 -0700 Subject: [PATCH 3/3] fix(runner): complete bounded benchmark ID loading Release whitespace-padded source lines, cover full-set streams and early HTTP rejection, and document all selection limits. Co-authored-by: Sebastien Tardif --- CHANGELOG.md | 5 +++ cmd/clawscan/main.go | 1 + docs/benchmarks.md | 6 +++- internal/runner/benchmark.go | 2 ++ internal/runner/runner_test.go | 65 +++++++++++++++++++++++++++++++--- skills/clawscan-cli/SKILL.md | 5 ++- 6 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..4f311a5 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,5 @@ +# Changelog + +## Unreleased + +- Fix unbounded memory use when loading benchmark `--ids` from files or HTTP, including whitespace-padded IDs; document selection limits and preserve full-set JSONL support. Thanks @SebTardif (#47). diff --git a/cmd/clawscan/main.go b/cmd/clawscan/main.go index ea38a3d..5aa19be 100644 --- a/cmd/clawscan/main.go +++ b/cmd/clawscan/main.go @@ -808,6 +808,7 @@ OpenClaw install policy: Benchmark command flags: --split Benchmark split. Defaults to benchmark for SkillTrustBench and eval_holdout for clawhub-security-signals. --ids Run selected benchmark IDs from a streamed text or JSONL source (max 5520 IDs). SkillTrustBench only. + Max 256 bytes per trimmed ID and 256 KiB total ID text, not source size; lines must be under 1 MiB. --limit Maximum benchmark rows to run. 0 means all rows. --offset Benchmark row offset. Defaults to 0. --predictions-output Write benchmark predictions JSONL. Defaults next to --output for clawhub-security-signals. diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 2dfd08a..3955784 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -14,7 +14,11 @@ clawscan benchmark SkillTrustBench \ Use `--ids ` with SkillTrustBench to run a fixed subset from a plain text ID list or JSONL rows with an `id` field. The loader streams the source (file or HTTP) and accepts at most 5,520 unique IDs, the size of the -pinned SkillTrustBench full set. +pinned SkillTrustBench full set. Each extracted ID may contain at most 256 +bytes, and all retained ID text together may contain at most 256 KiB (262,144 +bytes). Whitespace around IDs is trimmed. These limits apply to the extracted +IDs, not total source size: the full JSONL list may exceed 256 KiB. Individual +lines must be smaller than the parser's 1 MiB buffer limit. ## Available benchmarks diff --git a/internal/runner/benchmark.go b/internal/runner/benchmark.go index e0b0961..fb8d00d 100644 --- a/internal/runner/benchmark.go +++ b/internal/runner/benchmark.go @@ -388,6 +388,8 @@ func parseBenchmarkIDs(source string, reader io.Reader) ([]string, error) { if retained+len(id) > maxBenchmarkIDSelectionBytes { return nil, fmt.Errorf("--ids source %s exceeds the %d-byte retained-id budget", source, maxBenchmarkIDSelectionBytes) } + // Trimmed text IDs must not retain their potentially large source lines. + id = strings.Clone(id) seen[id] = true ids = append(ids, id) retained += len(id) diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 187656d..58d3394 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -802,7 +802,7 @@ func TestLoadBenchmarkIDSelectionRejectsBadSources(t *testing.T) { } func TestLoadBenchmarkIDSelectionAcceptsJSONLLargerThan256KiB(t *testing.T) { - payload := oversizedBenchmarkIDJSONL(t, 400) + payload := oversizedBenchmarkIDJSONL(t, maxSkillTrustBenchIDSelection) if len(payload) <= 256*1024 { t.Fatalf("fixture is %d bytes, want more than 256 KiB", len(payload)) } @@ -815,8 +815,8 @@ func TestLoadBenchmarkIDSelectionAcceptsJSONLLargerThan256KiB(t *testing.T) { if err != nil { t.Fatal(err) } - if len(fileSelection.IDs) != 400 { - t.Fatalf("file ids = %d, want 400", len(fileSelection.IDs)) + if len(fileSelection.IDs) != maxSkillTrustBenchIDSelection { + t.Fatalf("file ids = %d, want %d", len(fileSelection.IDs), maxSkillTrustBenchIDSelection) } server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -828,8 +828,63 @@ func TestLoadBenchmarkIDSelectionAcceptsJSONLLargerThan256KiB(t *testing.T) { if err != nil { t.Fatal(err) } - if len(httpSelection.IDs) != 400 { - t.Fatalf("http ids = %d, want 400", len(httpSelection.IDs)) + if !reflect.DeepEqual(httpSelection.IDs, fileSelection.IDs) || httpSelection.SHA256 != fileSelection.SHA256 { + t.Fatal("HTTP and file selections differ") + } +} + +func TestLoadBenchmarkIDSelectionReleasesWhitespacePadding(t *testing.T) { + path := filepath.Join(t.TempDir(), "padded-ids.txt") + file, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 64; i++ { + if _, err := fmt.Fprintf(file, "%s case_%05d\n", strings.Repeat(" ", 256*1024), i); err != nil { + file.Close() + t.Fatal(err) + } + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + runtime.GC() + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + selection, err := LoadBenchmarkIDSelection(path) + if err != nil { + t.Fatal(err) + } + runtime.GC() + runtime.ReadMemStats(&after) + runtime.KeepAlive(selection) + // The IDs occupy hundreds of bytes; retaining their padded source lines + // would keep more than 16 MiB live after collection. + if retained := int64(after.HeapAlloc) - int64(before.HeapAlloc); retained > 2*1024*1024 { + t.Fatalf("retained %d bytes for %d short IDs", retained, len(selection.IDs)) + } +} + +func TestLoadBenchmarkIDSelectionClosesRejectedHTTPStream(t *testing.T) { + closed := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, strings.Repeat("x", maxBenchmarkIDBytes+1)) + w.(http.Flusher).Flush() + select { + case <-r.Context().Done(): + close(closed) + case <-time.After(5 * time.Second): + } + })) + defer server.Close() + _, err := LoadBenchmarkIDSelection(server.URL) + if err == nil || !strings.Contains(err.Error(), "256-byte benchmark id limit") { + t.Fatalf("err = %v, want ID length rejection", err) + } + select { + case <-closed: + case <-time.After(5 * time.Second): + t.Fatal("rejected stream was not closed before reading the entire response") } } diff --git a/skills/clawscan-cli/SKILL.md b/skills/clawscan-cli/SKILL.md index 2e0e24a..d36079b 100644 --- a/skills/clawscan-cli/SKILL.md +++ b/skills/clawscan-cli/SKILL.md @@ -214,7 +214,10 @@ into temporary scan targets. Use `--ids ` with SkillTrustBench to run a fixed subset from a plain text file with one ID per line or JSONL rows with an `id` field. The -source is streamed and may contain at most 5,520 unique IDs. `--ids` +source is streamed and may contain at most 5,520 unique IDs, at most 256 bytes +per trimmed ID, and at most 256 KiB (262,144 bytes) of retained ID text. These +are selection limits, not a total source-size limit; JSONL sources may exceed +256 KiB. Individual lines must be smaller than the parser's 1 MiB buffer limit. `--ids` preserves source order, records `idsSource`, `idsCount`, and `idsSha256` in the artifact, and is mutually exclusive with `--limit` and `--offset`.