diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f311a5..eec7e53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,4 +2,5 @@ ## Unreleased +- Preserve every scanner report when sanitized target, profile, or custom scanner names collide with each other or generated numeric suffixes. - 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/docs/scanners.md b/docs/scanners.md index dfd3c80..f4c0157 100644 --- a/docs/scanners.md +++ b/docs/scanners.md @@ -17,6 +17,12 @@ clawscan scanners clawscan scanners skillspector ``` +When writing a results bundle with `--output`, each scanner result's +`outputPath` points to its raw JSON evidence beside the main artifact. Target, +profile, and scanner names are sanitized for filesystem use; colliding paths +receive numeric suffixes so every report is preserved. Read the recorded +`outputPath` instead of reconstructing a filename from a target or scanner ID. + ## Profile scanner configuration A trusted config can mix built-in scanner IDs with user-defined command diff --git a/internal/runner/output_bundle_test.go b/internal/runner/output_bundle_test.go new file mode 100644 index 0000000..95460a8 --- /dev/null +++ b/internal/runner/output_bundle_test.go @@ -0,0 +1,84 @@ +package runner + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" +) + +func TestWriteRunTargetsResultBundlePreservesCollidingEvidence(t *testing.T) { + tests := []struct { + name string + targets []string + profiles []string + scanners []string + }{ + { + name: "target suffix used after collision", + targets: []string{"skills/a", "skills/a!", "skills/a-2", "skills/a"}, + scanners: []string{"clawscan-static"}, + }, + { + name: "target suffix used before collision", + targets: []string{"skills/a-2", "skills/a", "skills/a!", "skills/a"}, + scanners: []string{"clawscan-static"}, + }, + { + name: "profile names", + targets: []string{"skills/a", "skills/a", "skills/a-2"}, + profiles: []string{"review", "review!", "review"}, + scanners: []string{"clawscan-static"}, + }, + { + name: "scanner names", + targets: []string{"skills/a"}, + scanners: []string{"custom", "custom-", "custom--", "custom-2"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + batch := BatchArtifact{SchemaVersion: "clawscan-batch-v1"} + for i, target := range tt.targets { + artifact := Artifact{ + SchemaVersion: "clawscan-run-v1", + Target: Target{Kind: "skill", Input: target, ResolvedPath: filepath.Join(dir, target)}, + Scanners: map[string]ScannerResult{}, + } + if len(tt.profiles) != 0 { + artifact.Profile = tt.profiles[i] + } + for _, scanner := range tt.scanners { + artifact.Scanners[scanner] = ScannerResult{ + Status: "completed", + Raw: json.RawMessage(fmt.Sprintf(`{"run":%d,"scanner":%q}`, i, scanner)), + } + } + batch.Runs = append(batch.Runs, artifact) + } + out := filepath.Join(dir, "artifact.json") + if err := WriteRunTargetsResultBundle(out, RunTargetsResult{Batch: &batch}); err != nil { + t.Fatal(err) + } + seen := map[string]bool{} + for _, run := range batch.Runs { + for scanner, result := range run.Scanners { + if seen[result.OutputPath] { + t.Errorf("reused evidence path %q", result.OutputPath) + } + seen[result.OutputPath] = true + raw, err := os.ReadFile(filepath.Join(dir, result.OutputPath)) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(raw, result.Raw) { + t.Errorf("evidence for %s/%s = %s, want %s", run.Target.Input, scanner, raw, result.Raw) + } + } + } + }) + } +} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index db26c35..7cdea9a 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -738,6 +738,7 @@ func writeScannerOutputFiles(spec outputBundleSpec, artifacts []*Artifact) error scanners = append(scanners, scanner) } sort.Strings(scanners) + usedScannerPaths := map[string]int{} for _, scanner := range scanners { result := artifact.Scanners[scanner] if len(result.Raw) == 0 { @@ -745,7 +746,8 @@ func writeScannerOutputFiles(spec outputBundleSpec, artifacts []*Artifact) error artifact.Scanners[scanner] = result continue } - relPath := filepath.ToSlash(filepath.Join(spec.PathPrefix, runPath, safeOutputPathSegment(scanner)+".json")) + scannerPath := uniqueOutputPath(safeOutputPathSegment(scanner), usedScannerPaths) + relPath := filepath.ToSlash(filepath.Join(spec.PathPrefix, runPath, scannerPath+".json")) absPath := filepath.Join(spec.RootDir, filepath.FromSlash(relPath)) if err := os.MkdirAll(filepath.Dir(absPath), 0o755); err != nil { return err @@ -807,12 +809,20 @@ func targetOutputPath(input string) string { } func uniqueOutputPath(base string, used map[string]int) string { - used[base]++ - if used[base] == 1 { + if used[base] == 0 { + used[base] = 1 return base } dir, file := path.Split(base) - return dir + file + "-" + strconv.Itoa(used[base]) + for { + used[base]++ + candidate := dir + file + "-" + strconv.Itoa(used[base]) + if used[candidate] == 0 { + // Reserve generated names too: another input may already use a suffix. + used[candidate] = 1 + return candidate + } + } } func safeOutputPathSegment(value string) string {