From 711a47edd4f932d6589817ad247d2c8ba076d464 Mon Sep 17 00:00:00 2001 From: Haru <143938115+Gi-v@users.noreply.github.com> Date: Sun, 5 Jul 2026 12:43:48 +0000 Subject: [PATCH] Add per-ecosystem record counts to scan_summary (#52) --- README.md | 7 +- cmd/bumblebee/main.go | 10 ++- cmd/bumblebee/main_test.go | 58 +++++++++++++ internal/model/model.go | 117 ++++++++++++++++++++------- internal/scanner/scanner.go | 13 +++ internal/scanner/scanner_test.go | 135 +++++++++++++++++++++++++++++++ 6 files changed, 310 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 0776dc0..2eb0897 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,10 @@ lists every flag. Records are NDJSON, one per line. Diagnostics go to stderr as NDJSON. Each run ends with a `scan_summary` record; receivers use it to decide whether -to promote a run to current state. See [docs/transport.md](docs/transport.md) +to promote a run to current state. `scan_summary.counts` always includes +`package` and `finding`, and also includes one key for each ecosystem that +emitted package records (for example `npm`, `pypi`, `go`, etc.). See +[docs/transport.md](docs/transport.md) for HTTPS/file output and [docs/state-model.md](docs/state-model.md) for the receiver-side current-state model. @@ -287,4 +290,4 @@ catalog list and review guidance. ## License -Apache License 2.0. See [LICENSE](LICENSE). +Apache License 2.0. See [LICENSE](LICENSE). \ No newline at end of file diff --git a/cmd/bumblebee/main.go b/cmd/bumblebee/main.go index 476fe1a..9744472 100644 --- a/cmd/bumblebee/main.go +++ b/cmd/bumblebee/main.go @@ -296,10 +296,18 @@ func runScan(args []string) int { for _, r := range roots { summaryRoots = append(summaryRoots, model.SummaryRoot{Path: r.Path, Kind: r.Kind}) } - counts := map[string]int{ + // counts is deterministically ordered by model.Counts.MarshalJSON: + // "package", "finding", then each ecosystem in + // model.SupportedEcosystems() order (only keys actually present). + counts := model.Counts{ model.RecordTypePackage: res.RecordsEmitted, model.RecordTypeFinding: res.FindingsEmitted, } + for _, eco := range model.SupportedEcosystems() { + if n := res.RecordsByEcosystem[eco]; n > 0 { + counts[eco] = n + } + } if err := emitter.EmitSummary(model.ScanSummary{ SchemaVersion: model.SchemaVersion, ScannerName: model.ScannerName, diff --git a/cmd/bumblebee/main_test.go b/cmd/bumblebee/main_test.go index 30158a6..b776008 100644 --- a/cmd/bumblebee/main_test.go +++ b/cmd/bumblebee/main_test.go @@ -751,3 +751,61 @@ func TestRunRootsRejectsUnknownProfile(t *testing.T) { t.Fatalf("runRoots --profile=scheduled exit = %d, want 2 (unknown profile)", code) } } + +func writeMainTestFile(t *testing.T, path, body string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +// TestRunScanSummaryCountsDeterministicOrder is the end-to-end test for +// issue #52: it runs a real multi-ecosystem scan through runScan and +// asserts the on-disk scan_summary.counts JSON has package, finding, +// then ecosystem keys in model.SupportedEcosystemOrder — not Go's +// randomized native map order. +func TestRunScanSummaryCountsDeterministicOrder(t *testing.T) { + root := t.TempDir() + writeMainTestFile(t, filepath.Join(root, "proj", "package-lock.json"), + `{"lockfileVersion":3,"packages":{"node_modules/lodash":{"version":"4.17.21"}}}`) + writeMainTestFile(t, filepath.Join(root, "gomod", "go.sum"), + "github.com/example/foo v1.2.3 h1:abc=\ngithub.com/example/foo v1.2.3/go.mod h1:def=\n") + writeMainTestFile(t, filepath.Join(root, "rb", "Gemfile.lock"), + "GEM\n remote: https://rubygems.org/\n specs:\n rack (3.0.8)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n rack\n") + + out := filepath.Join(t.TempDir(), "out.ndjson") + code := runScan([]string{"--profile", "project", "--root", root, "--output", "file", "--output-file", out}) + if code != 0 { + t.Fatalf("runScan exit = %d", code) + } + data, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + var summaryLine string + for _, line := range strings.Split(string(data), "\n") { + if strings.Contains(line, `"record_type":"scan_summary"`) { + summaryLine = line + break + } + } + if summaryLine == "" { + t.Fatalf("no scan_summary line found in %s", out) + } + + wantOrder := []string{`"package"`, `"finding"`, `"npm"`, `"go"`, `"rubygems"`} + lastIdx := -1 + for _, key := range wantOrder { + idx := strings.Index(summaryLine, key+":") + if idx == -1 { + t.Fatalf("key %s not found in scan_summary: %s", key, summaryLine) + } + if idx < lastIdx { + t.Fatalf("key %s out of order (idx=%d, prev=%d) in scan_summary: %s", key, idx, lastIdx, summaryLine) + } + lastIdx = idx + } +} diff --git a/internal/model/model.go b/internal/model/model.go index 0ba4639..b1c4895 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -4,6 +4,7 @@ package model import ( "crypto/sha256" "encoding/hex" + "encoding/json" "sort" "strconv" "strings" @@ -204,37 +205,95 @@ type Finding struct { Evidence string `json:"evidence,omitempty"` } +// Counts is a per-record-type/per-ecosystem count map used in +// ScanSummary.Counts. It always marshals its keys in a deterministic +// order — "package", "finding", then each ecosystem in +// supportedEcosystemOrder (only keys actually present are emitted), +// then any remaining unrecognized keys sorted alphabetically — so +// scan_summary.counts key order is stable across runs and hosts +// regardless of Go's randomized native map iteration order. +type Counts map[string]int + +// MarshalJSON implements a deterministic key order for Counts. Standard +// map[string]int marshaling sorts keys alphabetically, which would put +// ecosystem keys (e.g. "go", "npm") out of the intended +// package/finding/ecosystem-order shape; this override fixes that. +func (c Counts) MarshalJSON() ([]byte, error) { + if c == nil { + return []byte("null"), nil + } + order := make([]string, 0, len(c)) + seen := make(map[string]bool, len(c)) + for _, k := range []string{RecordTypePackage, RecordTypeFinding} { + if _, ok := c[k]; ok { + order = append(order, k) + seen[k] = true + } + } + for _, k := range supportedEcosystemOrder { + if _, ok := c[k]; ok { + order = append(order, k) + seen[k] = true + } + } + var rest []string + for k := range c { + if !seen[k] { + rest = append(rest, k) + } + } + sort.Strings(rest) + order = append(order, rest...) + + var buf strings.Builder + buf.WriteByte('{') + for i, k := range order { + if i > 0 { + buf.WriteByte(',') + } + kb, err := json.Marshal(k) + if err != nil { + return nil, err + } + buf.Write(kb) + buf.WriteByte(':') + buf.WriteString(strconv.Itoa(c[k])) + } + buf.WriteByte('}') + return []byte(buf.String()), nil +} + // ScanSummary is a per-run terminator record emitted to the same sink as // package and finding records. Receivers should only promote a run to // current state after a matching scan_summary with status=complete has // arrived. type ScanSummary struct { - RecordType string `json:"record_type"` - RecordID string `json:"record_id"` - SchemaVersion string `json:"schema_version"` - ScannerName string `json:"scanner_name"` - ScannerVersion string `json:"scanner_version"` - RunID string `json:"run_id"` - ScanTime string `json:"scan_time"` - EndTime string `json:"end_time"` - Endpoint Endpoint `json:"endpoint"` - Profile string `json:"profile"` - Status string `json:"status"` - Roots []SummaryRoot `json:"roots,omitempty"` - Counts map[string]int `json:"counts,omitempty"` - PackageRecordsEmitted int `json:"package_records_emitted"` - PackageRecordsSuppressed int `json:"package_records_suppressed,omitempty"` - FindingsEmitted int `json:"findings_emitted"` - Duplicates int `json:"duplicates"` - DiagnosticsCount int `json:"diagnostics_count"` - FilesConsidered int `json:"files_considered"` - TimedOut bool `json:"timed_out"` - DurationMS int64 `json:"duration_ms"` - HTTPBatchesAttempted int `json:"http_batches_attempted,omitempty"` - HTTPBatchesSucceeded int `json:"http_batches_succeeded,omitempty"` - HTTPBatchesFailed int `json:"http_batches_failed,omitempty"` - HTTPLastStatus int `json:"http_last_status,omitempty"` - Error string `json:"error,omitempty"` + RecordType string `json:"record_type"` + RecordID string `json:"record_id"` + SchemaVersion string `json:"schema_version"` + ScannerName string `json:"scanner_name"` + ScannerVersion string `json:"scanner_version"` + RunID string `json:"run_id"` + ScanTime string `json:"scan_time"` + EndTime string `json:"end_time"` + Endpoint Endpoint `json:"endpoint"` + Profile string `json:"profile"` + Status string `json:"status"` + Roots []SummaryRoot `json:"roots,omitempty"` + Counts Counts `json:"counts,omitempty"` + PackageRecordsEmitted int `json:"package_records_emitted"` + PackageRecordsSuppressed int `json:"package_records_suppressed,omitempty"` + FindingsEmitted int `json:"findings_emitted"` + Duplicates int `json:"duplicates"` + DiagnosticsCount int `json:"diagnostics_count"` + FilesConsidered int `json:"files_considered"` + TimedOut bool `json:"timed_out"` + DurationMS int64 `json:"duration_ms"` + HTTPBatchesAttempted int `json:"http_batches_attempted,omitempty"` + HTTPBatchesSucceeded int `json:"http_batches_succeeded,omitempty"` + HTTPBatchesFailed int `json:"http_batches_failed,omitempty"` + HTTPLastStatus int `json:"http_last_status,omitempty"` + Error string `json:"error,omitempty"` } // SummaryRoot is one entry in ScanSummary.Roots — path plus the root kind @@ -368,7 +427,11 @@ func joinSorted(values []string) string { return joinWithUnitSeparator(sorted) } -func canonicalCounts(counts map[string]int) string { +// canonicalCounts renders counts in a stable, alphabetically-sorted form +// for hashing purposes only. This is independent of Counts.MarshalJSON's +// display order: the StableID hash just needs any deterministic order, +// not the human-facing package/finding/ecosystem order. +func canonicalCounts(counts Counts) string { if len(counts) == 0 { return "" } diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index d0de709..bc7ef28 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -125,6 +125,11 @@ type Result struct { Diagnostics int TimedOut bool Duration time.Duration + // RecordsByEcosystem counts, per ecosystem, the package records that + // were actually written to the records sink. It mirrors RecordEmitted + // semantics exactly: deduplicated/suppressed records and records + // skipped by --findings-only are not counted. + RecordsByEcosystem map[string]int } // Run executes one scan and returns aggregate counters. It blocks until @@ -148,6 +153,8 @@ func Run(ctx context.Context, cfg Config) (Result, error) { var findingsMu sync.Mutex var packageRecordsSuppressed int var suppressedMu sync.Mutex + recordsByEcosystem := make(map[string]int) + var recordsByEcosystemMu sync.Mutex var emitErr error var emitErrMu sync.Mutex setEmitErr := func(err error) { @@ -187,6 +194,9 @@ func Run(ctx context.Context, cfg Config) (Result, error) { setEmitErr(fmt.Errorf("emit package record: %w", err)) return } + recordsByEcosystemMu.Lock() + recordsByEcosystem[r.Ecosystem]++ + recordsByEcosystemMu.Unlock() } } if !written { @@ -501,6 +511,9 @@ func Run(ctx context.Context, cfg Config) (Result, error) { suppressedMu.Lock() res.PackageRecordsSuppressed = packageRecordsSuppressed suppressedMu.Unlock() + recordsByEcosystemMu.Lock() + res.RecordsByEcosystem = recordsByEcosystem + recordsByEcosystemMu.Unlock() if errors.Is(ctx.Err(), context.DeadlineExceeded) { res.TimedOut = true } diff --git a/internal/scanner/scanner_test.go b/internal/scanner/scanner_test.go index 31fb347..9216a58 100644 --- a/internal/scanner/scanner_test.go +++ b/internal/scanner/scanner_test.go @@ -145,6 +145,141 @@ func TestEndToEndScan(t *testing.T) { } } +// TestRecordsByEcosystem verifies that Result.RecordsByEcosystem counts +// package records per ecosystem using the same written==true semantics +// as RecordsEmitted: dedup-suppressed records and records skipped by +// --findings-only must not inflate the counts. +func TestRecordsByEcosystem(t *testing.T) { + root := t.TempDir() + + // npm: one lockfile with two packages. + writeFile(t, filepath.Join(root, "proj", "package-lock.json"), `{ + "lockfileVersion": 3, + "packages": { + "": {"name":"proj","version":"1.0.0"}, + "node_modules/lodash": {"version":"4.17.21","integrity":"sha512-a"}, + "node_modules/chalk": {"version":"5.3.0","integrity":"sha512-b"} + } +}`) + + // go: go.sum with two modules. + writeFile(t, filepath.Join(root, "gomod", "go.sum"), `github.com/example/foo v1.2.3 h1:abc= +github.com/example/foo v1.2.3/go.mod h1:def= +github.com/example/bar v0.0.0-20240101000000-abcdef h1:bar= +github.com/example/bar v0.0.0-20240101000000-abcdef/go.mod h1:bar2= +`) + + // rubygems: Gemfile.lock with three gems. + writeFile(t, filepath.Join(root, "rb", "Gemfile.lock"), `GEM + remote: https://rubygems.org/ + specs: + rack (3.0.8) + rails (7.1.2) + actionpack (= 7.1.2) + actionpack (7.1.2) + rack (>= 2.2.4) + +PLATFORMS + ruby + +DEPENDENCIES + rails + +BUNDLED WITH + 2.5.0 +`) + + run := func(em *output.Emitter, stdout *bytes.Buffer, findingsOnly bool) (Result, []model.Record) { + stdout.Reset() + res, err := Run(context.Background(), Config{ + Roots: []Root{{Path: root, Kind: model.RootKindProject}}, + Profile: model.ProfileProject, + MaxFileSize: 5 * 1024 * 1024, + Concurrency: 2, + FindingsOnly: findingsOnly, + BaseRecord: model.Record{ + SchemaVersion: model.SchemaVersion, + ScannerName: model.ScannerName, + ScannerVersion: "test", + RunID: "runtest", + ScanTime: time.Now().UTC().Format(time.RFC3339Nano), + }, + Emitter: em, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + var records []model.Record + for _, line := range bytes.Split(bytes.TrimSpace(stdout.Bytes()), []byte("\n")) { + if len(line) == 0 { + continue + } + var r model.Record + if err := json.Unmarshal(line, &r); err != nil { + t.Fatalf("bad ndjson line: %v: %s", err, line) + } + records = append(records, r) + } + return res, records + } + + want := map[string]int{ + model.EcosystemNPM: 2, // lodash + chalk + model.EcosystemGo: 2, // foo + bar + model.EcosystemRubyGems: 3, // rack + rails + actionpack + } + + stdout := &bytes.Buffer{} + em := output.New(stdout, &bytes.Buffer{}, "runtest") + + res, records := run(em, stdout, false) + if len(res.RecordsByEcosystem) != len(want) { + t.Fatalf("RecordsByEcosystem = %+v, want %+v", res.RecordsByEcosystem, want) + } + for eco, n := range want { + if res.RecordsByEcosystem[eco] != n { + t.Errorf("RecordsByEcosystem[%q] = %d, want %d", eco, res.RecordsByEcosystem[eco], n) + } + } + + // The emitted package records must match RecordsByEcosystem exactly. + gotByEco := map[string]int{} + for _, r := range records { + if r.RecordType == model.RecordTypePackage { + gotByEco[r.Ecosystem]++ + } + } + for eco, n := range want { + if gotByEco[eco] != n { + t.Errorf("emitted package records[%q] = %d, want %d", eco, gotByEco[eco], n) + } + } + + // Re-running the exact same scan against the SAME emitter must hit + // the emitter's within-run dedup for every record (identical source + // files, so identical record identities) and therefore must not add + // anything to RecordsByEcosystem. + dedupRes, _ := run(em, stdout, false) + if len(dedupRes.RecordsByEcosystem) != 0 { + t.Errorf("deduped re-scan RecordsByEcosystem = %+v, want empty", dedupRes.RecordsByEcosystem) + } + + // --findings-only must suppress package records entirely, so no + // ecosystem should accrue any counts even though the records are + // still walked and observed. + foStdout := &bytes.Buffer{} + foEm := output.New(foStdout, &bytes.Buffer{}, "runtest2") + foRes, foRecords := run(foEm, foStdout, true) + if len(foRes.RecordsByEcosystem) != 0 { + t.Errorf("findings-only RecordsByEcosystem = %+v, want empty", foRes.RecordsByEcosystem) + } + for _, r := range foRecords { + if r.RecordType == model.RecordTypePackage { + t.Errorf("findings-only scan emitted a package record: %+v", r) + } + } +} + // TestEndToEndScanClaudeJSONFileRoot verifies that a `.claude.json` // passed as a single-file root is visited by the walker, dispatched to // the Claude config parser, and yields records for both the top-level