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/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) + } +} 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" 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")