From fa4c6d04beba8974f24010261d63ccf6d2d1810d Mon Sep 17 00:00:00 2001 From: matiasdaloia <66310421+matiasdaloia@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:08:22 +0100 Subject: [PATCH 1/3] fix(scanoss): the file-contents id is the scan's file_hash, not an MD5 /v3/file-contents/{id} takes whatever file_hash the scan reported for the matched OSS file, today a 16-hex CRC64. The comments and parameter names said MD5. --- pkg/scanoss/filecontents.go | 20 +++++++++++--------- pkg/scanoss/filecontents_test.go | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/scanoss/filecontents.go b/pkg/scanoss/filecontents.go index 6d78f7a..839774f 100644 --- a/pkg/scanoss/filecontents.go +++ b/pkg/scanoss/filecontents.go @@ -31,25 +31,27 @@ import ( // serviceFileContents is the raw source-content endpoint (v3). Unlike the decoration // services it takes no components and returns plain text, not JSON: the file's bytes, -// optionally sliced to a line range. The content id is the file's MD5 — for a snippet -// match, the matched OSS file's hash (FileResult.FileHash). +// optionally sliced to a line range. The content id is whatever file_hash the scan reported for +// the matched OSS file (FileResult.FileHash), passed through unchanged; today that is a 16-hex +// CRC64, not an MD5. var serviceFileContents = Service{Name: "file.contents", endpoint: "/v3/file-contents/"} // ContentsAPI is the raw file-content surface. It serves the source of a matched OSS -// file by its MD5, which the snippet classifier needs to score a match. +// file by the file_hash the scan reported for it, which the snippet classifier needs to +// score a match. It needs an API key with file-contents access. type ContentsAPI interface { - // File returns the raw text of the file with the given MD5. start and end are a + // File returns the raw text of the file with the given hash. start and end are a // 1-based inclusive line range; pass 0 for either to leave it at the default (line - // 1 / the last line), so File(ctx, md5, 0, 0) returns the whole file. - File(ctx context.Context, md5 string, start, end int) ([]byte, error) + // 1 / the last line), so File(ctx, hash, 0, 0) returns the whole file. + File(ctx context.Context, hash string, start, end int) ([]byte, error) } type contentsService struct{ c *Client } var _ ContentsAPI = contentsService{} -// File fetches the raw content of the file identified by md5. See ContentsAPI.File. -func (s contentsService) File(ctx context.Context, md5 string, start, end int) ([]byte, error) { +// File fetches the raw content of the file identified by hash. See ContentsAPI.File. +func (s contentsService) File(ctx context.Context, hash string, start, end int) ([]byte, error) { var q url.Values if start > 0 || end > 0 { q = url.Values{} @@ -60,5 +62,5 @@ func (s contentsService) File(ctx context.Context, md5 string, start, end int) ( q.Set("end", strconv.Itoa(end)) } } - return s.c.get(ctx, serviceFileContents.endpoint+url.PathEscape(md5), q) + return s.c.get(ctx, serviceFileContents.endpoint+url.PathEscape(hash), q) } diff --git a/pkg/scanoss/filecontents_test.go b/pkg/scanoss/filecontents_test.go index 93480dd..b64c94e 100644 --- a/pkg/scanoss/filecontents_test.go +++ b/pkg/scanoss/filecontents_test.go @@ -30,7 +30,7 @@ import ( "testing" ) -// File GETs /v3/file-contents/{md5} and returns the raw body, with no line-range query when +// File GETs /v3/file-contents/{file_hash} and returns the raw body, with no line-range query when // start and end are both 0. func TestContentsFileWholeFile(t *testing.T) { const body = "line one\nline two\nline three\n" From ab0974223bbacbdac292dc7ee9b1fc0042cbc1df Mon Sep 17 00:00:00 2001 From: matiasdaloia <66310421+matiasdaloia@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:08:22 +0100 Subject: [PATCH 2/3] feat(sbom): remove false-positive snippets; carry the verdict in CycloneDX Inventory.RemoveFalsePositiveSnippets drops the snippet matches the classifier proposed as false positives and any component left with no evidence. It never touches file matches, declared or transitive evidence, unscored or real snippets, or a match a bom.identify rule claimed, so the dependency graph keeps every component it names. An advisory loses the PURLs that named only removed components, and goes when none is left, so CycloneDX never emits a vulnerability that affects nothing. CycloneDX occurrences have no properties, so a scored snippet appends 'classifier p=<0.00> model=' to additionalContext, after the ranges. --- pkg/sbom/cyclonedx.go | 21 +++- pkg/sbom/inventory.go | 3 +- pkg/sbom/snippetfilter.go | 94 ++++++++++++++ pkg/sbom/snippetfilter_test.go | 218 +++++++++++++++++++++++++++++++++ 4 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 pkg/sbom/snippetfilter.go create mode 100644 pkg/sbom/snippetfilter_test.go diff --git a/pkg/sbom/cyclonedx.go b/pkg/sbom/cyclonedx.go index 8e8d263..539450a 100644 --- a/pkg/sbom/cyclonedx.go +++ b/pkg/sbom/cyclonedx.go @@ -297,7 +297,8 @@ func cdxAcknowledgement(ack LicenseAcknowledgement) cdx.LicenseAcknowledgement { // cycloneDXOccurrences turns matched files into evidence occurrences. Snippet matches // carry the start line and the full matched ranges in additionalContext (CycloneDX -// occurrences have no native range field). Transitive evidence is left out: the component +// occurrences have no native range field), followed by the classifier's verdict when the +// match was scored, since occurrences have no properties either. Transitive evidence is left out: the component // does not occur in that manifest, its lockfile pulls it in, and the dependencies section is // where that relationship is stated. func cycloneDXOccurrences(files []FileEvidence) []cdx.EvidenceOccurrence { @@ -312,9 +313,14 @@ func cycloneDXOccurrences(files []FileEvidence) []cdx.EvidenceOccurrence { start := f.InputLineRanges[0].StartLine o.Line = &start } - if ctx := rangesContext(f.InputLineRanges, f.OssLineRanges); ctx != "" { - o.AdditionalContext = ctx + ctx := rangesContext(f.InputLineRanges, f.OssLineRanges) + if cls := classificationContext(f.SnippetClassification); cls != "" { + if ctx != "" { + ctx += "; " + } + ctx += cls } + o.AdditionalContext = ctx } occurrences = append(occurrences, o) } @@ -335,6 +341,15 @@ func rangesContext(input, oss []LineRange) string { return strings.Join(parts, "; ") } +// classificationContext renders a snippet-classifier result as short, stable text, e.g. +// "classifier false_positive p=0.91 model=lr-d51f2e100211". It is "" for an unscored match. +func classificationContext(c *SnippetClassification) string { + if c == nil { + return "" + } + return fmt.Sprintf("classifier %s p=%.2f model=%s", c.Verdict, c.Probability, c.ModelVersion) +} + // joinRanges renders ranges as a comma-separated "start-end" list. func joinRanges(ranges []LineRange) string { parts := make([]string, 0, len(ranges)) diff --git a/pkg/sbom/inventory.go b/pkg/sbom/inventory.go index 61abbca..46280ac 100644 --- a/pkg/sbom/inventory.go +++ b/pkg/sbom/inventory.go @@ -314,7 +314,8 @@ func lowerMatchIndex(a, b *int) bool { // SnippetClassification is the snippet-classifier's answer for one snippet match: whether the // reported OSS and local line ranges correspond, as a thresholded verdict plus the probability -// behind it. It is a proposal for a human to confirm, not a filtering decision. +// behind it. It is a proposal for a human to confirm; removing what it proposes is a separate, +// opt-in step (Inventory.RemoveFalsePositiveSnippets). type SnippetClassification struct { Verdict string `json:"verdict"` // "false_positive" | "real" Probability float64 `json:"probability"` // P(false positive), 0..1 diff --git a/pkg/sbom/snippetfilter.go b/pkg/sbom/snippetfilter.go new file mode 100644 index 0000000..e779924 --- /dev/null +++ b/pkg/sbom/snippetfilter.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright (c) 2026, SCANOSS + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package sbom + +import "slices" + +// VerdictFalsePositive is the SnippetClassification verdict for a match whose reported ranges do +// not correspond. +const VerdictFalsePositive = "false_positive" + +// RemoveFalsePositiveSnippets drops every snippet match the classifier proposed as a false +// positive, and returns how many it dropped. A component left with no evidence goes with them. +// +// Only that evidence is touched: file matches, declared and transitive evidence, snippet matches +// that were not scored or were scored real stay, and so does a match a bom.identify rule claimed, +// since a person already confirmed it. Because declared and transitive evidence always stays, a +// component the dependency graph names keeps an entry, so Dependencies never points at a removed +// component. Vulnerabilities do: an advisory names components by PURL, so a PURL that named only +// removed components is dropped from it, and an advisory left naming nothing is dropped too. +func (inv *Inventory) RemoveFalsePositiveSnippets() int { + var removed int + var gone []Component + kept := inv.Components[:0] + for _, c := range inv.Components { + before := len(c.Evidence) + c.Evidence = slices.DeleteFunc(c.Evidence, isFalsePositiveSnippet) + removed += before - len(c.Evidence) + if before > 0 && len(c.Evidence) == 0 { + gone = append(gone, c) + continue + } + kept = append(kept, c) + } + inv.Components = kept + if len(gone) > 0 { + inv.Vulnerabilities = pruneVulnerabilities(inv.Vulnerabilities, gone, inv.Components) + } + return removed +} + +// isFalsePositiveSnippet reports whether e is a snippet match the classifier proposed as a false +// positive and no bom.identify rule claimed. +func isFalsePositiveSnippet(e FileEvidence) bool { + return e.MatchType == "snippet" && !e.Identified && + e.SnippetClassification != nil && e.SnippetClassification.Verdict == VerdictFalsePositive +} + +// pruneVulnerabilities removes from each advisory the PURLs that resolved to a removed component +// and to no remaining one, resolving as the SBOM formats do: by versioned ref or bare PURL. An +// advisory whose PURLs all went is dropped. A PURL that resolved to nothing before is left alone: +// it is not this filter's to judge. +func pruneVulnerabilities(vulns []Vulnerability, gone, remaining []Component) []Vulnerability { + keys := func(comps []Component) map[string]bool { + m := make(map[string]bool, len(comps)*2) + for _, c := range comps { + m[c.Ref()] = true + m[c.Purl] = true + } + return m + } + goneKeys, liveKeys := keys(gone), keys(remaining) + + out := vulns[:0] + for _, v := range vulns { + had := len(v.Purls) + v.Purls = slices.DeleteFunc(v.Purls, func(p string) bool { return goneKeys[p] && !liveKeys[p] }) + if had > 0 && len(v.Purls) == 0 { + continue + } + out = append(out, v) + } + return out +} diff --git a/pkg/sbom/snippetfilter_test.go b/pkg/sbom/snippetfilter_test.go new file mode 100644 index 0000000..6f982d5 --- /dev/null +++ b/pkg/sbom/snippetfilter_test.go @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright (c) 2026, SCANOSS + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package sbom + +import ( + "slices" + "strings" + "testing" +) + +func scored(verdict string, p float64) *SnippetClassification { + return &SnippetClassification{Verdict: verdict, Probability: p, ModelVersion: "lr-test"} +} + +func snippetAt(path string, c *SnippetClassification) FileEvidence { + return FileEvidence{ + Path: path, MatchType: "snippet", SnippetClassification: c, + InputLineRanges: []LineRange{{StartLine: 1, EndLine: 9}}, OssLineRanges: []LineRange{{StartLine: 1, EndLine: 9}}, + } +} + +// filterInventory has one component of each kind the filter must tell apart. +func filterInventory() Inventory { + return Inventory{ + Components: []Component{ + // Only false-positive snippets: removed whole. + {Purl: "pkg:github/acme/gone", Version: "1.0.0", Evidence: []FileEvidence{ + snippetAt("src/a.c", scored(VerdictFalsePositive, 0.91)), + snippetAt("src/b.c", scored(VerdictFalsePositive, 0.80)), + }}, + // A file match, a real snippet, an unscored snippet and a false positive: only the last goes. + {Purl: "pkg:github/acme/mixed", Version: "2.0.0", Evidence: []FileEvidence{ + {Path: "src/whole.c", MatchType: "file"}, + snippetAt("src/real.c", scored("real", 0.10)), + snippetAt("src/unscored.c", nil), + snippetAt("src/fp.c", scored(VerdictFalsePositive, 0.95)), + }}, + // A false positive a bom.identify rule claimed: a person confirmed it, so it stays. + {Purl: "pkg:github/acme/identified", Identified: true, Evidence: []FileEvidence{ + func() FileEvidence { + e := snippetAt("src/claimed.c", scored(VerdictFalsePositive, 0.99)) + e.Identified = true + return e + }(), + }}, + // Declared in a manifest and also matched by a false-positive snippet: the graph names + // it, and its declared evidence keeps it. + {Purl: "pkg:npm/left-pad", Version: "1.3.0", Evidence: []FileEvidence{ + {Path: "package.json", MatchType: "declared"}, + snippetAt("vendor/left-pad.js", scored(VerdictFalsePositive, 0.90)), + }}, + }, + Vulnerabilities: []Vulnerability{ + {ID: "CVE-GONE", Purls: []string{"pkg:github/acme/gone@1.0.0"}}, + {ID: "CVE-GONE-BARE", Purls: []string{"pkg:github/acme/gone"}}, + {ID: "CVE-BOTH", Purls: []string{"pkg:github/acme/gone", "pkg:github/acme/mixed"}}, + {ID: "CVE-LEFTPAD", Purls: []string{"pkg:npm/left-pad"}}, + {ID: "CVE-ELSEWHERE", Purls: []string{"pkg:npm/not-in-inventory"}}, + }, + Dependencies: []DependencyRoot{{ + Manifest: "package.json", Lockfile: "package-lock.json", Ecosystem: "npm", Status: GraphComplete, + Direct: []string{"pkg:npm/left-pad@1.3.0"}, + }}, + } +} + +func pathsOf(c *Component) []string { + var out []string + for _, e := range c.Evidence { + out = append(out, e.Path) + } + return out +} + +func findByPurl(inv Inventory, purl string) *Component { + for i := range inv.Components { + if inv.Components[i].Purl == purl { + return &inv.Components[i] + } + } + return nil +} + +func TestRemoveFalsePositiveSnippets(t *testing.T) { + inv := filterInventory() + if got := inv.RemoveFalsePositiveSnippets(); got != 4 { + t.Errorf("removed = %d, want 4 (gone x2, mixed x1, left-pad x1)", got) + } + + if findByPurl(inv, "pkg:github/acme/gone") != nil { + t.Error("a component left with no evidence must be removed") + } + mixed := findByPurl(inv, "pkg:github/acme/mixed") + if mixed == nil { + t.Fatal("mixed was removed; it still has evidence") + } + if got, want := pathsOf(mixed), []string{"src/whole.c", "src/real.c", "src/unscored.c"}; !slices.Equal(got, want) { + t.Errorf("mixed evidence = %v, want %v: file matches, real and unscored snippets stay", got, want) + } + if c := findByPurl(inv, "pkg:github/acme/identified"); c == nil || len(c.Evidence) != 1 { + t.Errorf("identified = %+v, want its claimed snippet kept", c) + } + leftPad := findByPurl(inv, "pkg:npm/left-pad") + if leftPad == nil { + t.Fatal("left-pad was removed; its declared evidence stays") + } + if got := pathsOf(leftPad); !slices.Equal(got, []string{"package.json"}) { + t.Errorf("left-pad evidence = %v, want only the manifest", got) + } + if got := inv.DirectMembers(inv.Dependencies[0]); !slices.Equal(got, []string{"pkg:npm/left-pad@1.3.0"}) { + t.Errorf("graph direct members = %v, want left-pad still resolved", got) + } + + var ids []string + for _, v := range inv.Vulnerabilities { + ids = append(ids, v.ID) + if v.ID == "CVE-BOTH" && !slices.Equal(v.Purls, []string{"pkg:github/acme/mixed"}) { + t.Errorf("CVE-BOTH purls = %v, want only the surviving component", v.Purls) + } + } + if want := []string{"CVE-BOTH", "CVE-LEFTPAD", "CVE-ELSEWHERE"}; !slices.Equal(ids, want) { + t.Errorf("vulnerabilities = %v, want %v: an advisory naming only removed components goes, others stay", ids, want) + } +} + +// A removed component must leave no trace in any rendered format: no component, no advisory +// pointing at it, no dependency edge. +func TestRemoveFalsePositiveSnippetsLeavesNoDanglingReference(t *testing.T) { + inv := filterInventory() + inv.RemoveFalsePositiveSnippets() + + raw, err := NewRawDocument(inv, RawMetadata{}).Marshal() + if err != nil { + t.Fatal(err) + } + cyclonedx, err := Generate(inv, FormatCycloneDX) + if err != nil { + t.Fatal(err) + } + spdx, err := Generate(inv, FormatSPDX) + if err != nil { + t.Fatal(err) + } + for name, out := range map[string]string{"raw": raw, "cyclonedx": cyclonedx, "spdx": spdx} { + if strings.Contains(out, "acme/gone") { + t.Errorf("%s output still names the removed component", name) + } + if !strings.Contains(out, "acme/mixed") { + t.Errorf("%s output lost a component that kept evidence", name) + } + } + + bom := decodeBOM(t, cyclonedx) + for _, v := range *bom.Vulnerabilities { + if v.ID == "CVE-ELSEWHERE" { + continue // never resolved to a component; not the filter's to judge + } + if v.Affects == nil || len(*v.Affects) == 0 { + t.Errorf("CycloneDX vulnerability %s affects nothing", v.ID) + } + } +} + +func TestRemoveFalsePositiveSnippetsWithNothingToRemove(t *testing.T) { + inv := Inventory{Components: []Component{ + {Purl: "pkg:x/a", Evidence: []FileEvidence{snippetAt("a.c", scored("real", 0.2)), snippetAt("b.c", nil)}}, + {Purl: "pkg:x/no-evidence"}, + }} + if got := inv.RemoveFalsePositiveSnippets(); got != 0 { + t.Errorf("removed = %d, want 0", got) + } + if len(inv.Components) != 2 || len(inv.Components[0].Evidence) != 2 { + t.Errorf("inventory changed: %+v", inv.Components) + } +} + +// CycloneDX occurrences have no properties, so a scored snippet carries its verdict in +// additionalContext, after the ranges. +func TestCycloneDXOccurrenceCarriesClassification(t *testing.T) { + ev := snippetAt("src/b.c", &SnippetClassification{Verdict: VerdictFalsePositive, Probability: 0.9132, ModelVersion: "lr-d51f2e100211"}) + ev.InputLineRanges = []LineRange{{StartLine: 12, EndLine: 48}} + ev.OssLineRanges = []LineRange{{StartLine: 100, EndLine: 136}} + unscored := snippetAt("src/c.c", nil) + inv := Inventory{Components: []Component{{Purl: "pkg:x/y", Version: "1", Evidence: []FileEvidence{ev, unscored}}}} + + out, err := Generate(inv, FormatCycloneDX) + if err != nil { + t.Fatal(err) + } + occ := *(*decodeBOM(t, out).Components)[0].Evidence.Occurrences + if got, want := occ[0].AdditionalContext, "input lines 12-48; oss lines 100-136; classifier false_positive p=0.91 model=lr-d51f2e100211"; got != want { + t.Errorf("scored additionalContext = %q, want %q", got, want) + } + if got, want := occ[1].AdditionalContext, "input lines 1-9; oss lines 1-9"; got != want { + t.Errorf("unscored additionalContext = %q, want %q", got, want) + } +} From eb85481e9f9b7a9c17c1582691c7c395332c194f Mon Sep 17 00:00:00 2001 From: matiasdaloia <66310421+matiasdaloia@users.noreply.github.com> Date: Wed, 28 Oct 2026 17:08:22 +0100 Subject: [PATCH 3/3] feat(scan): --classify-threshold, --classify-filter and a classifier summary --classify-snippets now works with every output format. --classify-threshold (default 0.7, strictly between 0 and 1) and --classify-filter need --classify-snippets, and a bad value is refused before any upload. Classification runs before enrichment, so a component the filter removes is never sent to the decoration services. After the scan, stderr gets one line saying how many snippet matches were scored, proposed as false positives, removed and left unscored, and when the API refused file contents with 401/403, so a run that scored nothing no longer looks like a run that found nothing. Docs: CHANGELOG (covering --classify-snippets too), CLIENT_HELP and README. --- CHANGELOG.md | 21 ++++ CLIENT_HELP.md | 60 ++++++++- README.md | 2 +- cmd/classify_test.go | 139 +++++++++++++++++++++ cmd/scan.go | 117 +++++++++++++++--- pkg/scanpipeline/classify_test.go | 170 ++++++++++++++++++++++++++ pkg/scanpipeline/scanpipeline.go | 85 ++++++++++--- pkg/snippetmatch/snippetmatch.go | 80 ++++++++---- pkg/snippetmatch/snippetmatch_test.go | 51 ++++++-- 9 files changed, 653 insertions(+), 72 deletions(-) create mode 100644 cmd/classify_test.go create mode 100644 pkg/scanpipeline/classify_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f7407b3..0af525b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **Snippet classification on `scan`**, opt-in. `--classify-snippets` scores each snippet match + with a local classifier (github.com/scanoss/snippets-classifier) and proposes whether it is a + false positive, recording the verdict, the probability and the model version. It downloads + each matched OSS file from `/v3/file-contents/`, so it needs an API key with file-contents + access and counts against the quota. It works with every format: `raw` adds + `snippet_classification` to the evidence, CycloneDX appends it to the occurrence's + `additionalContext`, and SPDX, which has no per-file data, does not carry it. + `--classify-threshold` (default 0.7, strictly between 0 and 1) sets the probability at or above + which a match is proposed. `--classify-filter` removes the proposed false positives, and any + component and advisory left with nothing; it never removes file matches, dependencies, + unscored matches or `bom.identify` claims. Both need `--classify-snippets`, and a bad value is + rejected before anything is uploaded. A summary on stderr gives how many matches were scored, + proposed, removed and left unscored, and says when the API refused file contents (401/403). + At 0.7 the classifier proposes for a person to confirm: on 8 public application repositories + (876 snippet matches) its false-positive precision was about 0.71 and recall 0.90, so + filtering drops some real matches. Use a higher threshold such as 0.9 to filter, and do not use + filtering in place of review. SDK: `scanpipeline.Options.FilterFalsePositives`, + `Result.Classification` and `sbom.Inventory.RemoveFalsePositiveSnippets`. + ## [0.15.0] - 2026-10-23 ### Added diff --git a/CLIENT_HELP.md b/CLIENT_HELP.md index 4359c98..8dc1116 100644 --- a/CLIENT_HELP.md +++ b/CLIENT_HELP.md @@ -166,7 +166,9 @@ default 0), `--max-size` (0 = unlimited), `--gitignore` (true), `--max-components` (a positive integer or `all`; unset = the server's defaults), `--skip-headers` (**default true**, `--skip-headers=false` to disable), `--skip-headers-limit` -(0 = no limit). +(0 = no limit), `--classify-snippets`, `--classify-threshold` (0.7), +`--classify-filter` (see [Snippet classification](#snippet-classification---classify-snippets); +`scan ` only). `-i, --identify` and `-n, --ignore` name a component list; see [BOM rules](#bom-rules). `--skip-headers` applies while fingerprinting, so @@ -432,6 +434,59 @@ CycloneDX and SPDX carry the component order but not `match_index`: a CycloneDX `evidence.occurrences` entry admits only `location`, `line`, `offset`, `symbol`, `additionalContext` and `bom-ref`, and SPDX Lite has no per-file evidence. The per-file order is `raw`-only. +### Snippet classification (`--classify-snippets`) + +A snippet match can be a false positive: the scanner reports a local range and an OSS range that +do not correspond. `--classify-snippets` scores every snippet match with a local classifier and +proposes whether it is one. It is **off by default** and costs API calls: scoring needs the +matched OSS file, which the CLI downloads from `/v3/file-contents/` once per distinct file. That +needs an API key with file-contents access, and each download counts against the key's quota. It +needs the scanned files on disk, so it applies to `scan ` only. + +```bash +# Propose false positives for a person to review +scanoss-cli scan ./my-project --api-key "$SCANOSS_API_KEY" --classify-snippets + +# Remove the likely ones, with a stricter threshold +scanoss-cli scan ./my-project --api-key "$SCANOSS_API_KEY" --classify-snippets \ + --classify-threshold 0.9 --classify-filter +``` + +- `--classify-threshold` is the probability of a false positive at or above which a match is + proposed as one, strictly between 0 and 1 (default `0.7`). Any other value is rejected before + anything is uploaded. +- `--classify-filter` removes the snippet matches proposed as false positives, and a component + left with no evidence goes with them, along with any advisory that named only that component. + File matches, declared and transitive dependencies, unscored snippet matches and matches a + `bom.identify` rule claimed are never removed. +- Both are rejected without `--classify-snippets`. + +Each scored match records the verdict (`false_positive` or `real`), the probability and the model +version: + +- `raw` adds `snippet_classification` to the snippet's evidence: + `{"verdict": "false_positive", "probability": 0.91, "model_version": "lr-d51f2e100211"}`. +- `cyclonedx` appends it to the occurrence's `additionalContext`, after the ranges: + `input lines 12-48; oss lines 100-136; classifier false_positive p=0.91 model=lr-d51f2e100211`. +- `spdx` has no per-file data, so the verdict is not carried. `--classify-filter` still applies. + +A match that could not be scored (no OSS file hash, an unreadable local file, a failed download) +carries no verdict and is never filtered. After the scan a summary on stderr says how many were +scored, proposed and removed, and how many could not be scored: + +``` +Snippet classifier (model lr-d51f2e100211, threshold 0.7): scored 52 of 56 snippet matches, 6 proposed as false positives; 4 could not be scored +``` + +When the API answers 401 or 403 for file contents, the summary says so: the key lacks +file-contents access, and nothing was scored. + +> **Review, do not trust blindly.** At `0.7` the classifier proposes false positives for a person +> to confirm. On 8 public application repositories (876 snippet matches) it reached a +> false-positive precision of about 0.71 and recall of 0.90 at `0.7`: roughly 3 in 10 proposed +> matches were real. `--classify-filter` therefore drops some real matches; use a higher +> threshold such as `0.9` when filtering, and never use filtering as a substitute for review. + ## Resuming a scan (`results`) Retrieve the results of a scan by the id printed during `scan` (works after a @@ -701,6 +756,9 @@ with `--settings`. It carries BOM context and file-skip rules. | `--chunk-size` (scan) | `1048576` | WFP upload block size (bytes) | | `--poll-interval` (scan) | `2s` | Scan status poll cadence | | `--max-components` (scan) | unset | Not sent; the server's default applies | +| `--classify-snippets` (scan) | `false` | No snippet match is scored; no file contents are fetched | +| `--classify-threshold` (scan) | `0.7` | Strictly between 0 and 1; needs `--classify-snippets` | +| `--classify-filter` (scan) | `false` | Nothing is removed; needs `--classify-snippets` | | `--min-size` / `--max-size` | `0` / `0` | Literal minimum; `0` max means unlimited | | `--chunk-size` (decoration) | `10` | PURLs per request | | `--workers` (decoration) | `5` | Max concurrent requests | diff --git a/README.md b/README.md index 5083c8c..bd2b3fd 100644 --- a/README.md +++ b/README.md @@ -103,7 +103,7 @@ Stdout stays reserved for results, so logs never corrupt `--output` or piped JSO | Command | Purpose | |---------|---------| -| `scan ` | Fingerprint a folder/file, scan against the SCANOSS v3 API, and output results (`--format raw`/`spdx`/`cyclonedx`; opt into dependency/vuln/license/crypto/geo layers with `--include`). | +| `scan ` | Fingerprint a folder/file, scan against the SCANOSS v3 API, and output results (`--format raw`/`spdx`/`cyclonedx`; opt into dependency/vuln/license/crypto/geo layers with `--include`; opt into flagging likely false-positive snippet matches with `--classify-snippets`). | | `scan wfp ` | Scan a pre-generated WFP file (no fingerprinting). | | `wfp ` | Generate WFP fingerprints only (no upload). | | `results ` | Resume or poll a scan by its id. | diff --git a/cmd/classify_test.go b/cmd/classify_test.go new file mode 100644 index 0000000..68d14bf --- /dev/null +++ b/cmd/classify_test.go @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright (c) 2026, SCANOSS + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package cmd + +import ( + "errors" + "strings" + "testing" + + "github.com/scanoss/scanoss.go/pkg/scanpipeline" + "github.com/scanoss/scanoss.go/pkg/snippetmatch" +) + +// A threshold outside (0, 1) is refused before anything is fingerprinted or uploaded. +func TestScanRejectsClassifyThresholdOutOfRange(t *testing.T) { + for _, bad := range []string{"0", "1", "-0.1", "1.5", "NaN"} { + t.Run("value="+bad, func(t *testing.T) { + var rec scanRequests + srv := rec.server(t) + err := runScanDirCmd(t, srv.URL, "--classify-snippets", "--classify-threshold="+bad) + if err == nil || !strings.Contains(err.Error(), "--classify-threshold") { + t.Fatalf("error = %v, want one naming --classify-threshold", err) + } + if n := len(rec.queries); n != 0 { + t.Errorf("server saw %d uploads, want 0", n) + } + }) + } +} + +// --classify-threshold and --classify-filter do nothing without --classify-snippets, so they are +// refused rather than ignored in silence. +func TestScanClassifyFlagsRequireClassifySnippets(t *testing.T) { + for _, args := range [][]string{ + {"--classify-threshold", "0.9"}, + {"--classify-filter"}, + } { + t.Run(args[0], func(t *testing.T) { + var rec scanRequests + srv := rec.server(t) + err := runScanDirCmd(t, srv.URL, args...) + if err == nil || !strings.Contains(err.Error(), args[0]+" requires --classify-snippets") { + t.Fatalf("error = %v, want %q", err, args[0]+" requires --classify-snippets") + } + if n := len(rec.queries); n != 0 { + t.Errorf("server saw %d uploads, want 0", n) + } + }) + } +} + +// Classification is no longer raw-only: every format accepts it, with a valid threshold and the +// filter. +func TestScanClassifyAcceptsEveryFormat(t *testing.T) { + for _, format := range []string{"raw", "cyclonedx", "spdx"} { + t.Run(format, func(t *testing.T) { + var rec scanRequests + srv := rec.server(t) + if err := runScanDirCmd(t, srv.URL, "-f", format, "--classify-snippets", "--classify-threshold", "0.9", "--classify-filter"); err != nil { + t.Fatal(err) + } + if len(rec.queries) == 0 { + t.Error("server saw no upload") + } + }) + } +} + +func TestClassificationSummary(t *testing.T) { + base := snippetmatch.Summary{ModelVersion: "lr-d51f2e100211", Threshold: 0.7} + with := func(f func(*scanpipeline.ClassifySummary)) scanpipeline.ClassifySummary { + c := scanpipeline.ClassifySummary{Summary: base} + f(&c) + return c + } + for _, tc := range []struct { + name string + in scanpipeline.ClassifySummary + filtered bool + want string + }{ + { + name: "partly scored", + in: with(func(c *scanpipeline.ClassifySummary) { c.Matches, c.Scored, c.FalsePositives = 56, 52, 6 }), + want: "Snippet classifier (model lr-d51f2e100211, threshold 0.7): scored 52 of 56 snippet matches, 6 proposed as false positives; 4 could not be scored", + }, + { + name: "filtered, all scored", + in: with(func(c *scanpipeline.ClassifySummary) { c.Matches, c.Scored, c.FalsePositives, c.Removed = 10, 10, 3, 3 }), + filtered: true, + want: "Snippet classifier (model lr-d51f2e100211, threshold 0.7): scored 10 of 10 snippet matches, 3 proposed as false positives, 3 removed", + }, + { + name: "file contents denied", + in: with(func(c *scanpipeline.ClassifySummary) { + c.Matches, c.DeniedStatus, c.Threshold = 5, 403, 0.9 + }), + filtered: true, + want: "Snippet classifier (model lr-d51f2e100211, threshold 0.9): scored 0 of 5 snippet matches, 0 proposed as false positives, 0 removed; 5 could not be scored (the API refused file contents with HTTP 403: the API key needs file-contents access)", + }, + { + name: "no snippets", + in: with(func(*scanpipeline.ClassifySummary) {}), + want: "Snippet classifier (model lr-d51f2e100211, threshold 0.7): no snippet matches to score", + }, + { + name: "could not start", + in: scanpipeline.ClassifySummary{Err: errors.New("bad model")}, + want: "Snippet classifier could not start, no snippet match was scored: bad model", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := classificationSummary(tc.in, tc.filtered); got != tc.want { + t.Errorf("got %q\nwant %q", got, tc.want) + } + }) + } +} diff --git a/cmd/scan.go b/cmd/scan.go index 2e14634..d6ba1c9 100644 --- a/cmd/scan.go +++ b/cmd/scan.go @@ -29,6 +29,7 @@ import ( "log/slog" "os" "path/filepath" + "strconv" "strings" "sync" @@ -45,6 +46,7 @@ import ( "github.com/scanoss/scanoss.go/pkg/scanoss" "github.com/scanoss/scanoss.go/pkg/scanpipeline" "github.com/scanoss/scanoss.go/pkg/settings" + "github.com/scanoss/scanoss.go/pkg/snippetmatch" "github.com/scanoss/scanoss.go/pkg/wfp" ) @@ -323,7 +325,10 @@ Examples: scanoss-cli scan wfp fingerprints.wfp # Save WFP fingerprints before sending to the API - scanoss-cli scan ./my-project --save-wfp fingerprints.wfp`, + scanoss-cli scan ./my-project --save-wfp fingerprints.wfp + + # Propose likely false-positive snippet matches for review (fetches the matched OSS files) + scanoss-cli scan ./my-project --classify-snippets`, Args: cobra.MaximumNArgs(1), RunE: runScan, } @@ -373,9 +378,12 @@ func init() { addSkipHeaderFlags(scanCmd) // Snippet classification (path scan only: it needs the scanned files on disk). Off by default - // because scoring fetches the matched OSS files over the network. raw format only — SPDX and - // CycloneDX have nowhere to carry the verdict. - scanCmd.Flags().Bool("classify-snippets", false, "Score each snippet match with the local classifier and annotate whether it looks like a false positive (raw format only)") + // because scoring fetches the matched OSS files over the network. raw carries the verdict as + // snippet_classification, CycloneDX in the occurrence's additionalContext; SPDX-lite has no + // per-file data to carry it, though --classify-filter still applies there. + scanCmd.Flags().Bool("classify-snippets", false, "Score each snippet match with the local classifier and annotate whether it looks like a false positive (fetches the matched OSS files; needs file-contents access)") + scanCmd.Flags().Float64("classify-threshold", snippetmatch.DefaultThreshold, "P(false positive) at or above which --classify-snippets proposes a match as a false positive (strictly between 0 and 1)") + scanCmd.Flags().Bool("classify-filter", false, "Remove the snippet matches --classify-snippets proposes as false positives, and components left with no evidence") // The BOM rule flags are declared on both, since either can apply them: `scan wfp` cannot // fingerprint, but it still post-processes a result. @@ -434,9 +442,9 @@ func runScan(cmd *cobra.Command, args []string) error { allFolders, _ := cmd.Flags().GetBool("all-folders") applyGitignore, _ := cmd.Flags().GetBool("gitignore") allHidden, _ := cmd.Flags().GetBool("all-hidden") - classifySnippets, _ := cmd.Flags().GetBool("classify-snippets") - if classifySnippets && outputFormat != config.FormatRaw { - warnf("--classify-snippets only annotates the raw format; ignoring it for %q output", outputFormat) + classify, err := resolveClassifyFlags(cmd) + if err != nil { + return err } // Settings drive file filtering and the BOM rules, which are applied SDK-side, post-scan, @@ -484,23 +492,28 @@ func runScan(cmd *cobra.Command, args []string) error { } result, err := scanpipeline.Run(ctx, scanpipeline.Options{ - Client: client, - Services: servicesFor(layers), - SourceDeclared: layers.Has(LayerDeps), - SourcePath: targetPath, - Threads: threads, - ScanFilters: collectOpts, - DependencyFilters: depOpts, - ScanOptions: scanTuning(cmd, scanSettings, prog, components), - WFPOptions: fingerprintOptions(skipHeaders, skipHeadersLimit), - WFPWriter: wfpWriterOrNil(wfpOut), - OnProgress: prog.layer, - ClassifySnippets: classifySnippets && outputFormat == config.FormatRaw, + Client: client, + Services: servicesFor(layers), + SourceDeclared: layers.Has(LayerDeps), + SourcePath: targetPath, + Threads: threads, + ScanFilters: collectOpts, + DependencyFilters: depOpts, + ScanOptions: scanTuning(cmd, scanSettings, prog, components), + WFPOptions: fingerprintOptions(skipHeaders, skipHeadersLimit), + WFPWriter: wfpWriterOrNil(wfpOut), + OnProgress: prog.layer, + ClassifySnippets: classify.enabled, + ClassifyThreshold: classify.threshold, + FilterFalsePositives: classify.filter, }) prog.finish() if err != nil { return renderAPIError(fmt.Errorf("scan failed: %w", err)) } + if c := result.Classification; c != nil { + reportClassification(*c, classify.filter) + } // Close before claiming success: a buffered write error can surface only here, // and "saved" must not be printed over an incomplete file. if wfpOut != nil { @@ -518,6 +531,72 @@ func runScan(cmd *cobra.Command, args []string) error { return nil } +// classifyFlags is what the snippet-classifier flags asked for. +type classifyFlags struct { + enabled bool + threshold float64 + filter bool +} + +// resolveClassifyFlags reads the snippet-classifier flags and refuses a bad combination before +// anything is fingerprinted or uploaded: a threshold outside (0, 1), or --classify-threshold or +// --classify-filter without --classify-snippets, which would otherwise do nothing in silence. +func resolveClassifyFlags(cmd *cobra.Command) (classifyFlags, error) { + var f classifyFlags + f.enabled, _ = cmd.Flags().GetBool("classify-snippets") + f.threshold, _ = cmd.Flags().GetFloat64("classify-threshold") + f.filter, _ = cmd.Flags().GetBool("classify-filter") + if !f.enabled { + for _, name := range []string{"classify-threshold", "classify-filter"} { + if cmd.Flags().Changed(name) { + return f, usageError(cmd, "--%s requires --classify-snippets", name) + } + } + return f, nil + } + // Written so NaN fails too. + if !(f.threshold > 0 && f.threshold < 1) { + return f, fmt.Errorf("--classify-threshold %v: must be strictly between 0 and 1", f.threshold) + } + return f, nil +} + +// reportClassification prints the snippet-classifier summary: a warning when some matches went +// unscored or the classifier could not run, so a run that scored nothing cannot pass for one that +// found nothing; an info line otherwise. +func reportClassification(c scanpipeline.ClassifySummary, filtered bool) { + line := classificationSummary(c, filtered) + if c.Err != nil || c.Scored < c.Matches { + warnf("%s", line) + return + } + infof("%s", line) +} + +// classificationSummary renders the snippet-classifier summary line, e.g. +// "Snippet classifier (model lr-d51f2e100211, threshold 0.7): scored 52 of 56 snippet matches, +// 6 proposed as false positives; 4 could not be scored". filtered adds how many were removed. +func classificationSummary(c scanpipeline.ClassifySummary, filtered bool) string { + if c.Err != nil { + return fmt.Sprintf("Snippet classifier could not start, no snippet match was scored: %v", c.Err) + } + head := fmt.Sprintf("Snippet classifier (model %s, threshold %s)", c.ModelVersion, strconv.FormatFloat(c.Threshold, 'g', -1, 64)) + if c.Matches == 0 { + return head + ": no snippet matches to score" + } + line := fmt.Sprintf("%s: scored %d of %d snippet matches, %d proposed as false positives", head, c.Scored, c.Matches, c.FalsePositives) + if filtered { + line += fmt.Sprintf(", %d removed", c.Removed) + } + if unscored := c.Matches - c.Scored; unscored > 0 { + line += fmt.Sprintf("; %d could not be scored", unscored) + } + if c.DeniedStatus != 0 { + line += fmt.Sprintf(" (the API refused file contents with HTTP %d: the API key needs file-contents access)", c.DeniedStatus) + } + return line +} + // wfpWriterOrNil keeps a nil *os.File from becoming a non-nil io.Writer: an interface // holding a typed nil pointer is not nil, so the pipeline would write to it and panic. func wfpWriterOrNil(f *os.File) io.Writer { diff --git a/pkg/scanpipeline/classify_test.go b/pkg/scanpipeline/classify_test.go new file mode 100644 index 0000000..e070ca0 --- /dev/null +++ b/pkg/scanpipeline/classify_test.go @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright (c) 2026, SCANOSS + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package scanpipeline + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/scanoss/scanoss.go/pkg/filter" + "github.com/scanoss/scanoss.go/pkg/scanoss" +) + +// The local file and the OSS file a snippet match pairs share no informative line: the +// classifier proposes it as a false positive. +const ( + classifyLocal = "func handler(w http.ResponseWriter, r *http.Request) {\n\tuser := loadUser(r)\n\tif user == nil {\n\t\thttp.Error(w, \"forbidden\", 403)\n\t\treturn\n\t}\n\trender(w, user.Profile)\n}\n" + classifyOSS = "def parse_config(path):\n with open(path) as fh:\n data = yaml.safe_load(fh)\n validate(data)\n return Config(**data)\n" +) + +// classifyServer answers the scan with one snippet match of handler.go against pkg:github/acme/fp, +// serves file contents with contentsStatus, and records the bodies of the vulnerability requests. +type classifyServer struct { + contentsStatus int + mu sync.Mutex + vulnBodies []string +} + +func (s *classifyServer) start(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasPrefix(r.URL.Path, "/v3/file-contents/"): + if s.contentsStatus != http.StatusOK { + http.Error(w, `{"error":"forbidden"}`, s.contentsStatus) + return + } + _, _ = w.Write([]byte(classifyOSS)) + case strings.Contains(r.URL.Path, "/vulnerabilities"): + body, _ := io.ReadAll(r.Body) + s.mu.Lock() + s.vulnBodies = append(s.vulnBodies, string(body)) + s.mu.Unlock() + _, _ = w.Write([]byte(`{"components":[]}`)) + case r.Method == http.MethodPost: + w.WriteHeader(http.StatusAccepted) + default: + _, _ = w.Write([]byte(`{"scan_id":"s1","status":"completed","phase":"done","result":{ + "files":[{"path":"handler.go","match_type":"snippet","file_hash":"0123456789abcdef","matches":[{"url_hash":"h1", + "input_line_ranges":[{"start_line":1,"end_line":8}],"oss_line_ranges":[{"start_line":1,"end_line":5}]}]}], + "components":{"h1":{"purls":["pkg:github/acme/fp"],"version":"1.0.0"}}}}`)) + } + })) + t.Cleanup(srv.Close) + return srv +} + +func runClassify(t *testing.T, srvURL string, filterFP bool, services []scanoss.Service) Result { + t.Helper() + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "handler.go"), []byte(classifyLocal), 0o644); err != nil { + t.Fatal(err) + } + res, err := Run(context.Background(), Options{ + Client: mustNewClient(t, srvURL), + Services: services, + SourcePath: root, + ScanFilters: filter.Scanning(nil), + ScanOptions: []scanoss.ScanOption{scanoss.WithPollInterval(10 * time.Millisecond)}, + ClassifySnippets: true, + FilterFalsePositives: filterFP, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if res.Classification == nil { + t.Fatal("no classification summary for a run that asked for one") + } + return res +} + +// Without the filter, the match is scored and kept. +func TestRunClassifiesWithoutRemoving(t *testing.T) { + s := &classifyServer{contentsStatus: http.StatusOK} + res := runClassify(t, s.start(t).URL, false, nil) + + c := res.Classification + if c.Matches != 1 || c.Scored != 1 || c.FalsePositives != 1 || c.Removed != 0 || c.DeniedStatus != 0 { + t.Errorf("summary = %+v, want 1 match scored as a false positive, none removed", *c) + } + comp := findComponent(res.Inventory, "pkg:github/acme/fp") + if comp == nil || comp.Evidence[0].SnippetClassification == nil { + t.Fatalf("component = %+v, want it kept with its classification", comp) + } +} + +// With the filter, the false positive goes, its component with it, and the component is never +// sent to the decoration services. +func TestRunFiltersFalsePositivesBeforeEnrichment(t *testing.T) { + s := &classifyServer{contentsStatus: http.StatusOK} + res := runClassify(t, s.start(t).URL, true, []scanoss.Service{scanoss.ServiceVulnerabilities}) + + if c := res.Classification; c.Removed != 1 { + t.Errorf("removed = %d, want 1", c.Removed) + } + if comp := findComponent(res.Inventory, "pkg:github/acme/fp"); comp != nil { + t.Errorf("component left with no evidence must be removed, got %+v", comp) + } + s.mu.Lock() + defer s.mu.Unlock() + for _, body := range s.vulnBodies { + if strings.Contains(body, "acme/fp") { + t.Errorf("a removed component was sent for vulnerabilities: %s", body) + } + } +} + +// A key without file-contents access scores nothing; the summary says why rather than only +// counting failures. +func TestRunReportsFileContentsDenied(t *testing.T) { + for _, status := range []int{http.StatusUnauthorized, http.StatusForbidden} { + s := &classifyServer{contentsStatus: status} + res := runClassify(t, s.start(t).URL, true, nil) + + c := res.Classification + if c.DeniedStatus != status || c.Scored != 0 || c.Matches != 1 || c.Removed != 0 { + t.Errorf("status %d: summary = %+v, want DeniedStatus %d, nothing scored or removed", status, *c, status) + } + if findComponent(res.Inventory, "pkg:github/acme/fp") == nil { + t.Errorf("status %d: an unscored match must never be filtered", status) + } + } +} + +// Any other failure is counted as unscored but is not reported as an access problem. +func TestRunDoesNotReportOtherFailuresAsDenied(t *testing.T) { + s := &classifyServer{contentsStatus: http.StatusNotFound} + res := runClassify(t, s.start(t).URL, false, nil) + if c := res.Classification; c.DeniedStatus != 0 || c.Scored != 0 { + t.Errorf("summary = %+v, want an unscored match and no access denial", *c) + } +} diff --git a/pkg/scanpipeline/scanpipeline.go b/pkg/scanpipeline/scanpipeline.go index f6bab7f..b7dd926 100644 --- a/pkg/scanpipeline/scanpipeline.go +++ b/pkg/scanpipeline/scanpipeline.go @@ -37,9 +37,11 @@ import ( "errors" "fmt" "io" + "net/http" "os" "path/filepath" "sync" + "sync/atomic" scanossapi "github.com/scanoss/scanoss.api-sdk" @@ -99,13 +101,32 @@ type Options struct { // ClassifySnippets asks for each snippet match in the inventory to be scored by the local // snippet classifier, which proposes whether the match is a false positive (its reported OSS // and local line ranges do not correspond). The verdict lands on each match's - // FileEvidence.SnippetClassification; nothing is removed. It is opt-in because scoring fetches - // the matched OSS files over the network, so a bare scan pays nothing for it. + // FileEvidence.SnippetClassification; nothing is removed unless FilterFalsePositives says so. + // It is opt-in because scoring fetches the matched OSS files over the network, so a bare scan + // pays nothing for it. ClassifySnippets bool // ClassifyThreshold overrides the classifier's decision threshold (P(false positive) at or // above which a match is proposed as a false positive). 0 uses snippetmatch.DefaultThreshold // (0.7), tuned to propose for human confirmation. ClassifyThreshold float64 + // FilterFalsePositives removes the snippet matches the classifier proposed as false positives, + // and any component left with no evidence (see sbom.Inventory.RemoveFalsePositiveSnippets). + // Ignored unless ClassifySnippets is set. + FilterFalsePositives bool +} + +// ClassifySummary is what snippet classification did, for the caller to report: the +// classifier's own tally plus what the pipeline did around it. +type ClassifySummary struct { + snippetmatch.Summary + + // Removed is how many snippet matches FilterFalsePositives removed. + Removed int + // DeniedStatus is the HTTP status (401 or 403) the API answered a file-contents request with, + // or 0. Every match needs its OSS file, so a key without file-contents access scores nothing. + DeniedStatus int + // Err is set when the classifier could not start; nothing was scored. + Err error } // Result is the outcome of Run: the gathered inventory, the files that could not be @@ -128,6 +149,10 @@ type Result struct { // inventory with no licences because the project declares none from one with no licences // because every request failed. Nil when everything asked for arrived. EnrichError error + + // Classification reports the snippet classification. Nil unless Options.ClassifySnippets was + // set and the scan produced an inventory. + Classification *ClassifySummary } // Run executes the full pipeline over Options.SourcePath: collect the files (applying the @@ -347,32 +372,38 @@ func Run(ctx context.Context, opts Options) (Result, error) { inv := scansource.Inventory(scanResult, scansource.WithIdentified(bomReport.IsIdentified)) inv.Add(declared.Components...) inv.Dependencies = declared.Dependencies - enrichErr := Enricher{Client: opts.Client, Services: opts.Services, Reporter: r}.Enrich(ctx, &inv) - // Snippet classification runs last, over the assembled inventory: it needs the matched OSS - // files, which it fetches by hash. Opt-in, and non-fatal like enrichment — a match that could - // not be scored is simply left unannotated. + // Snippet classification runs over the assembled inventory, before enrichment: a component the + // filter removes is then never sent to the decoration services, so no advisory or licence is + // gathered for it. Opt-in, and non-fatal like enrichment — a match that could not be scored is + // simply left unannotated, and the summary says how many. + var classification *ClassifySummary if opts.ClassifySnippets { - classifySnippets(ctx, opts, scanRoot, &inv, emit) + classification = classifySnippets(ctx, opts, scanRoot, &inv, emit) } + enrichErr := Enricher{Client: opts.Client, Services: opts.Services, Reporter: r}.Enrich(ctx, &inv) + return Result{ - Inventory: inv, - ProcessErrors: procErrors, - EnrichError: enrichErr, + Inventory: inv, + ProcessErrors: procErrors, + EnrichError: enrichErr, + Classification: classification, }, nil } // classifySnippets scores the inventory's snippet matches in place, reporting progress under -// LayerClassify. It is best-effort: a construction failure or a partly-failed run is logged, not -// returned, because a scan whose matches could not all be scored is still a usable scan. -func classifySnippets(ctx context.Context, opts Options, scanRoot string, inv *sbom.Inventory, emit func(Progress)) { +// LayerClassify, and removes the proposed false positives when asked to. It is best-effort: a +// construction failure or a partly-failed run is reported in the summary, not returned, because +// a scan whose matches could not all be scored is still a usable scan. +func classifySnippets(ctx context.Context, opts Options, scanRoot string, inv *sbom.Inventory, emit func(Progress)) *ClassifySummary { emit(Progress{Layer: LayerClassify, Status: StatusRunning, Total: 0}) // The fetch stage reports done/total as OSS files arrive; remember the total so completion // can fill the bar (a StatusCompleted with Total 0 would leave it aborted). var total int + fetcher := &deniedRecorder{api: opts.Client.Contents} annotator, err := snippetmatch.New(snippetmatch.Options{ - Fetcher: opts.Client.Contents, + Fetcher: fetcher, ScanRoot: scanRoot, Threshold: opts.ClassifyThreshold, Workers: opts.Threads, @@ -384,13 +415,35 @@ func classifySnippets(ctx context.Context, opts Options, scanRoot string, inv *s if err != nil { logging.Warn("snippet classification could not start", "err", err) emit(Progress{Layer: LayerClassify, Status: StatusFailed}) - return + return &ClassifySummary{Err: err} } - if err := annotator.Annotate(ctx, inv); err != nil { + sum, err := annotator.Annotate(ctx, inv) + if err != nil { // Partial failures are already logged per match inside Annotate; this names the tally. logging.Warn("snippet classification incomplete", "err", err) } + out := &ClassifySummary{Summary: sum, DeniedStatus: int(fetcher.status.Load())} + if opts.FilterFalsePositives { + out.Removed = inv.RemoveFalsePositiveSnippets() + } emit(Progress{Layer: LayerClassify, Status: StatusCompleted, Done: total, Total: total}) + return out +} + +// deniedRecorder passes file-contents requests through and remembers the first 401 or 403, so +// the summary can say the API key lacks file-contents access instead of only counting failures. +type deniedRecorder struct { + api scanoss.ContentsAPI + status atomic.Int32 +} + +func (d *deniedRecorder) File(ctx context.Context, hash string, start, end int) ([]byte, error) { + b, err := d.api.File(ctx, hash, start, end) + var se *scanoss.StatusError + if errors.As(err, &se) && (se.StatusCode == http.StatusUnauthorized || se.StatusCode == http.StatusForbidden) { + d.status.CompareAndSwap(0, int32(se.StatusCode)) + } + return b, err } // Enricher is what the requested layers are gathered with: a client to ask, the layers to ask diff --git a/pkg/snippetmatch/snippetmatch.go b/pkg/snippetmatch/snippetmatch.go index 842499c..61145d3 100644 --- a/pkg/snippetmatch/snippetmatch.go +++ b/pkg/snippetmatch/snippetmatch.go @@ -33,6 +33,7 @@ // every match that shares them. // // Annotation only proposes: it sets FileEvidence.SnippetClassification and removes nothing. +// Removing what it proposes is the caller's choice (see sbom.Inventory.RemoveFalsePositiveSnippets). package snippetmatch import ( @@ -56,11 +57,12 @@ const DefaultThreshold = snippetclassifier.DefaultThreshold // DefaultWorkers bounds the concurrent OSS content fetches when Options.Workers is unset. const DefaultWorkers = 8 -// Fetcher retrieves the raw content of an OSS file by its MD5. A 1-based inclusive line range -// may be given; 0 for either bound leaves it at the default (line 1 / the last line), so -// (md5, 0, 0) is the whole file. scanoss.ContentsAPI satisfies this interface. +// Fetcher retrieves the raw content of an OSS file by the file_hash the scan reported for it. A +// 1-based inclusive line range may be given; 0 for either bound leaves it at the default (line 1 +// / the last line), so (hash, 0, 0) is the whole file. scanoss.ContentsAPI satisfies this +// interface. type Fetcher interface { - File(ctx context.Context, md5 string, start, end int) ([]byte, error) + File(ctx context.Context, hash string, start, end int) ([]byte, error) } // Options configures an Annotator. @@ -124,25 +126,41 @@ func New(opts Options) (*Annotator, error) { // ModelVersion identifies the model the Annotator scores with. func (a *Annotator) ModelVersion() string { return a.inner.ModelVersion() } +// Threshold is the P(false positive) at or above which the Annotator proposes a false positive. +func (a *Annotator) Threshold() float64 { return a.inner.Threshold() } + +// Summary tallies one Annotate run, for the caller to report: how many snippet matches the +// inventory held, how many were scored, and how many of those were proposed as false positives. +// Matches - Scored is how many could not be scored, for whatever reason (no OSS hash or line +// ranges, an unreadable local file, a failed fetch). +type Summary struct { + ModelVersion string + Threshold float64 + Matches int // snippet matches in the inventory + Scored int // matches that received a classification + FalsePositives int // scored matches proposed as false positives +} + // job is one snippet match to score: where its result goes and what it needs. type job struct { ev *sbom.FileEvidence path string // local file path, relative to scanRoot - hash string // OSS file MD5 + hash string // OSS file hash (FileEvidence.FileHash) spans []snippetclassifier.Span } // Annotate scores every snippet match in inv in place, setting FileEvidence.SnippetClassification -// on each match it could score. It is non-fatal: a match whose local file is unreadable or whose -// OSS content cannot be fetched is left unannotated and logged. The returned error names how many -// matches could not be scored, for the caller to surface; it does not mean the inventory is -// unusable. +// on each match it could score, and returns the tally. It is non-fatal: a match whose local file +// is unreadable or whose OSS content cannot be fetched is left unannotated and logged. The +// returned error names how many matches could not be scored, for the caller to surface; it does +// not mean the inventory is unusable. // // The inventory is scored, not filtered: no match is removed, whatever its verdict. -func (a *Annotator) Annotate(ctx context.Context, inv *sbom.Inventory) error { +func (a *Annotator) Annotate(ctx context.Context, inv *sbom.Inventory) (Summary, error) { + sum := Summary{ModelVersion: a.ModelVersion(), Threshold: a.Threshold(), Matches: countSnippets(inv)} jobs := collectJobs(inv) if len(jobs) == 0 { - return nil + return sum, unscoredErr(sum) } // Fetch each distinct OSS file once. Several matches often point at the same OSS file @@ -156,29 +174,19 @@ func (a *Annotator) Annotate(ctx context.Context, inv *sbom.Inventory) error { // Score. Each job reads only from the read-only caches, and the classifier is safe for // concurrent use, so the CPU-bound scoring fans out too. A worst-case OSS file takes ~1s. - var skipped int var mu sync.Mutex a.forEach(jobs, func(j job) { lb, ok := local[j.path] if !ok { - mu.Lock() - skipped++ - mu.Unlock() return } ob, ok := oss[j.hash] if !ok { - mu.Lock() - skipped++ - mu.Unlock() return } res, err := a.inner.Classify(snippetclassifier.Match{Local: lb, OSS: ob, Spans: j.spans}) if err != nil { logging.Warn("snippet classification failed", "path", j.path, "err", err) - mu.Lock() - skipped++ - mu.Unlock() return } j.ev.SnippetClassification = &sbom.SnippetClassification{ @@ -186,14 +194,40 @@ func (a *Annotator) Annotate(ctx context.Context, inv *sbom.Inventory) error { Probability: res.Probability, ModelVersion: res.ModelVersion, } + mu.Lock() + sum.Scored++ + if res.Verdict == snippetclassifier.VerdictFalsePositive { + sum.FalsePositives++ + } + mu.Unlock() }) - if skipped > 0 { - return fmt.Errorf("snippetmatch: %d of %d snippet matches could not be classified", skipped, len(jobs)) + return sum, unscoredErr(sum) +} + +// unscoredErr names how many of the inventory's snippet matches went unscored, or is nil when +// every one was scored. +func unscoredErr(sum Summary) error { + if skipped := sum.Matches - sum.Scored; skipped > 0 { + return fmt.Errorf("snippetmatch: %d of %d snippet matches could not be classified", skipped, sum.Matches) } return nil } +// countSnippets counts every snippet match in inv, scorable or not, so a match the classifier +// could not even attempt (no OSS hash, no paired ranges) still counts as unscored. +func countSnippets(inv *sbom.Inventory) int { + var n int + for _, c := range inv.Components { + for _, ev := range c.Evidence { + if ev.MatchType == "snippet" { + n++ + } + } + } + return n +} + // collectJobs gathers every snippet match that can be scored: one with an OSS file hash and at // least one paired local/OSS line range. The evidence pointer is kept so the verdict lands back // on the inventory. diff --git a/pkg/snippetmatch/snippetmatch_test.go b/pkg/snippetmatch/snippetmatch_test.go index 79b6af7..5c5ecdf 100644 --- a/pkg/snippetmatch/snippetmatch_test.go +++ b/pkg/snippetmatch/snippetmatch_test.go @@ -75,27 +75,27 @@ type fakeFetcher struct { err error // when set, every File call fails } -func (f *fakeFetcher) File(_ context.Context, md5 string, _, _ int) ([]byte, error) { +func (f *fakeFetcher) File(_ context.Context, hash string, _, _ int) ([]byte, error) { f.mu.Lock() defer f.mu.Unlock() if f.calls == nil { f.calls = map[string]int{} } - f.calls[md5]++ + f.calls[hash]++ if f.err != nil { return nil, f.err } - c, ok := f.content[md5] + c, ok := f.content[hash] if !ok { return nil, errors.New("not found") } return c, nil } -func (f *fakeFetcher) callCount(md5 string) int { +func (f *fakeFetcher) callCount(hash string) int { f.mu.Lock() defer f.mu.Unlock() - return f.calls[md5] + return f.calls[hash] } // writeLocal writes name under root with the given content and returns name (relative), as it @@ -137,7 +137,7 @@ func TestAnnotateIdenticalIsReal(t *testing.T) { if err != nil { t.Fatal(err) } - if err := a.Annotate(context.Background(), inv); err != nil { + if _, err := a.Annotate(context.Background(), inv); err != nil { t.Fatalf("Annotate: %v", err) } got := inv.Components[0].Evidence[0].SnippetClassification @@ -159,7 +159,7 @@ func TestAnnotateUnrelatedIsFalsePositive(t *testing.T) { inv := invWith(snippetEvidence(path, "h1", 8)) a, _ := New(Options{Fetcher: f, ScanRoot: root}) - if err := a.Annotate(context.Background(), inv); err != nil { + if _, err := a.Annotate(context.Background(), inv); err != nil { t.Fatalf("Annotate: %v", err) } got := inv.Components[0].Evidence[0].SnippetClassification @@ -177,7 +177,7 @@ func TestOSSFetchIsDedupedByHash(t *testing.T) { inv := invWith(snippetEvidence(p1, "shared", 10), snippetEvidence(p2, "shared", 10)) a, _ := New(Options{Fetcher: f, ScanRoot: root}) - if err := a.Annotate(context.Background(), inv); err != nil { + if _, err := a.Annotate(context.Background(), inv); err != nil { t.Fatalf("Annotate: %v", err) } if n := f.callCount("shared"); n != 1 { @@ -197,7 +197,7 @@ func TestMissingLocalFileIsSkipped(t *testing.T) { inv := invWith(snippetEvidence("ghost.go", "h1", 10)) a, _ := New(Options{Fetcher: f, ScanRoot: root}) - err := a.Annotate(context.Background(), inv) + _, err := a.Annotate(context.Background(), inv) if err == nil { t.Fatal("want an error naming the skipped match") } @@ -213,7 +213,7 @@ func TestFetchErrorIsNonFatalAndSkips(t *testing.T) { inv := invWith(snippetEvidence(path, "h1", 10)) a, _ := New(Options{Fetcher: f, ScanRoot: root}) - if err := a.Annotate(context.Background(), inv); err == nil { + if _, err := a.Annotate(context.Background(), inv); err == nil { t.Fatal("want an error when OSS content cannot be fetched") } if inv.Components[0].Evidence[0].SnippetClassification != nil { @@ -231,7 +231,7 @@ func TestNonSnippetEvidenceIsIgnored(t *testing.T) { ) a, _ := New(Options{Fetcher: f, ScanRoot: root}) - if err := a.Annotate(context.Background(), inv); err != nil { + if _, err := a.Annotate(context.Background(), inv); err != nil { t.Fatalf("Annotate: %v", err) } if n := fetched.Load(); n != 0 { @@ -249,7 +249,7 @@ func TestNoSnippetsMakesNoFetch(t *testing.T) { f := &countingFetcher{onCall: func() { fetched.Add(1) }} inv := invWith() // no evidence at all a, _ := New(Options{Fetcher: f, ScanRoot: t.TempDir()}) - if err := a.Annotate(context.Background(), inv); err != nil { + if _, err := a.Annotate(context.Background(), inv); err != nil { t.Fatalf("Annotate: %v", err) } if fetched.Load() != 0 { @@ -257,6 +257,33 @@ func TestNoSnippetsMakesNoFetch(t *testing.T) { } } +// The summary counts every snippet match in the inventory, scorable or not, so a match that +// was never attempted still shows up as unscored. +func TestAnnotateSummaryCounts(t *testing.T) { + root := t.TempDir() + realPath := writeLocal(t, root, "sum.go", realCode) + fpPath := writeLocal(t, root, "handler.go", unrelatedLocal) + f := &fakeFetcher{content: map[string][]byte{"real": []byte(realCode), "fp": []byte(unrelatedOSS)}} + noHash := snippetEvidence(realPath, "", 10) + inv := invWith( + snippetEvidence(realPath, "real", 10), + snippetEvidence(fpPath, "fp", 8), + snippetEvidence("ghost.go", "real", 10), // local file missing + noHash, // nothing to fetch + sbom.FileEvidence{Path: "whole.go", FileHash: "real", MatchType: "file"}, + ) + + a, _ := New(Options{Fetcher: f, ScanRoot: root, Threshold: 0.6}) + sum, err := a.Annotate(context.Background(), inv) + if err == nil { + t.Error("want an error naming the unscored matches") + } + want := Summary{ModelVersion: a.ModelVersion(), Threshold: 0.6, Matches: 4, Scored: 2, FalsePositives: 1} + if sum != want { + t.Errorf("summary = %+v, want %+v", sum, want) + } +} + func TestNewRequiresFetcher(t *testing.T) { if _, err := New(Options{}); err == nil { t.Fatal("New must reject a nil Fetcher")