From 46abd2cc0034aec1116b7a592ac8f0cdfc0f436c Mon Sep 17 00:00:00 2001 From: coresoftware dev Date: Wed, 30 Sep 2026 14:30:30 +0200 Subject: [PATCH 1/2] feat(scan): classify snippet matches as false positives (opt-in) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrate github.com/scanoss/snippets-classifier to score each snippet match on whether its reported OSS and local line ranges correspond. The classifier is offline, but scoring needs the matched OSS file, which is not local — so a new pipeline stage fetches it and annotates the result. - pkg/scanoss: Contents.File fetches raw source by MD5 via GET /v3/file-contents/{md5} (whole file or a line range). - pkg/snippetmatch: an Annotator that reads each local file, fetches every distinct OSS file once (cache by hash, bounded concurrency), classifies, and sets FileEvidence.SnippetClassification. Non-fatal: a match it cannot score is left unannotated. - sbom: SnippetClassification {verdict, probability, model_version} on FileEvidence, rendered in the raw output. - scanpipeline: opt-in ClassifySnippets stage after enrichment, with a "classify" progress layer. - cmd: --classify-snippets flag (path scan, raw format only). The classifier only proposes (threshold 0.7); it removes nothing. snippets-classifier is a public, MIT licensed module (v0.2.0), so the build needs no GOPRIVATE or token. Co-Authored-By: Claude Opus 5.5 --- cmd/scan.go | 12 + go.mod | 1 + go.sum | 2 + pkg/sbom/inventory.go | 14 ++ pkg/scanoss/client.go | 4 + pkg/scanoss/filecontents.go | 64 +++++ pkg/scanoss/filecontents_test.go | 90 +++++++ pkg/scanpipeline/progress.go | 1 + pkg/scanpipeline/scanpipeline.go | 50 ++++ pkg/snippetmatch/snippetmatch.go | 333 ++++++++++++++++++++++++++ pkg/snippetmatch/snippetmatch_test.go | 271 +++++++++++++++++++++ 11 files changed, 842 insertions(+) create mode 100644 pkg/scanoss/filecontents.go create mode 100644 pkg/scanoss/filecontents_test.go create mode 100644 pkg/snippetmatch/snippetmatch.go create mode 100644 pkg/snippetmatch/snippetmatch_test.go diff --git a/cmd/scan.go b/cmd/scan.go index c0297a9..2e14634 100644 --- a/cmd/scan.go +++ b/cmd/scan.go @@ -188,6 +188,8 @@ func layerLabel(service string) string { return "Geoprovenance" case scanoss.ServiceDependencies.Name: return "Resolving dependencies" + case scanpipeline.LayerClassify: + return "Classifying snippets" default: return service } @@ -370,6 +372,11 @@ func init() { scanCmd.Flags().Bool("all-hidden", false, "Include hidden files and folders, version-control metadata included") 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)") + // The BOM rule flags are declared on both, since either can apply them: `scan wfp` cannot // fingerprint, but it still post-processes a result. addBOMRuleFlags(scanCmd) @@ -427,6 +434,10 @@ 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) + } // Settings drive file filtering and the BOM rules, which are applied SDK-side, post-scan, // via WithBOM. --identify and --ignore contribute to the same BOM. @@ -484,6 +495,7 @@ func runScan(cmd *cobra.Command, args []string) error { WFPOptions: fingerprintOptions(skipHeaders, skipHeadersLimit), WFPWriter: wfpWriterOrNil(wfpOut), OnProgress: prog.layer, + ClassifySnippets: classifySnippets && outputFormat == config.FormatRaw, }) prog.finish() if err != nil { diff --git a/go.mod b/go.mod index 30874f3..7e0081d 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/google/uuid v1.6.0 github.com/pelletier/go-toml/v2 v2.4.3 github.com/scanoss/scanoss.api-sdk v0.31.0 + github.com/scanoss/snippets-classifier v0.2.0 github.com/spdx/tools-golang v0.5.7 github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index 27fedb4..b9c73fd 100644 --- a/go.sum +++ b/go.sum @@ -56,6 +56,8 @@ github.com/sagikazarmark/locafero v0.11.0 h1:1iurJgmM9G3PA/I+wWYIOw/5SyBtxapeHDc github.com/sagikazarmark/locafero v0.11.0/go.mod h1:nVIGvgyzw595SUSUE6tvCp3YYTeHs15MvlmU87WwIik= github.com/scanoss/scanoss.api-sdk v0.31.0 h1:bQGD9w0cRnoSL9Zf2UjULs5Nr8OnJ9xr9emQOucYCi8= github.com/scanoss/scanoss.api-sdk v0.31.0/go.mod h1:+NvJCBBPVCd1gMgMfNYEigYG0uxr/GKosF+uaxzLtR4= +github.com/scanoss/snippets-classifier v0.2.0 h1:GU0dMPWZ7ilGOYBCKsTlNPP3T7DVc0eq9Qf9cFUbvxo= +github.com/scanoss/snippets-classifier v0.2.0/go.mod h1:7Tv4u7c/KVJBf6z/i4zSVIls3pALdzp+78BuCMO3Hs0= github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 h1:+jumHNA0Wrelhe64i8F6HNlS8pkoyMv5sreGx2Ry5Rw= github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8/go.mod h1:3n1Cwaq1E1/1lhQhtRK2ts/ZwZEhjcQeJQ1RuC6Q/8U= github.com/spdx/tools-golang v0.5.7 h1:+sWcKGnhwp3vLdMqPcLdA6QK679vd86cK9hQWH3AwCg= diff --git a/pkg/sbom/inventory.go b/pkg/sbom/inventory.go index af339b8..61abbca 100644 --- a/pkg/sbom/inventory.go +++ b/pkg/sbom/inventory.go @@ -291,6 +291,11 @@ type FileEvidence struct { // by path and the components are not in any file's order, so a consumer rebuilding "the // candidates of this file" ranks them by this field, not by where it found them. MatchIndex *int `json:"match_index,omitempty"` + + // SnippetClassification is the local snippet-classifier's verdict on whether this snippet + // match is a false positive (its reported ranges do not correspond). Populated only when + // classification was requested and the match could be scored; nil otherwise. + SnippetClassification *SnippetClassification `json:"snippet_classification,omitempty"` } // IsPrimary reports whether this evidence is its file's primary match (MatchIndex 0). @@ -307,6 +312,15 @@ func lowerMatchIndex(a, b *int) bool { return b == nil || *a < *b } +// 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. +type SnippetClassification struct { + Verdict string `json:"verdict"` // "false_positive" | "real" + Probability float64 `json:"probability"` // P(false positive), 0..1 + ModelVersion string `json:"model_version"` // model that produced the score, e.g. "lr-d51f2e100211" +} + // Vulnerability is one known vulnerability affecting one or more components, in a // format independent of any decoration API wire type. type Vulnerability struct { diff --git a/pkg/scanoss/client.go b/pkg/scanoss/client.go index 678ed24..cfcd2dc 100644 --- a/pkg/scanoss/client.go +++ b/pkg/scanoss/client.go @@ -84,6 +84,9 @@ type Client struct { Components ComponentsAPI Dependencies DependencyAPI + // Contents service (raw source retrieval by file hash). Wired in New. + Contents ContentsAPI + // Scan service (batch WFP scanning). Wired in New. Scan ScanAPI } @@ -131,6 +134,7 @@ func New(cfg Config) (*Client, error) { c.Copyright = copyrightService{c} c.Components = componentsService{c} c.Dependencies = dependencyService{c} + c.Contents = contentsService{c} c.Scan = scanService{c} return c, nil } diff --git a/pkg/scanoss/filecontents.go b/pkg/scanoss/filecontents.go new file mode 100644 index 0000000..6d78f7a --- /dev/null +++ b/pkg/scanoss/filecontents.go @@ -0,0 +1,64 @@ +// 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 scanoss + +import ( + "context" + "net/url" + "strconv" +) + +// 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). +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. +type ContentsAPI interface { + // File returns the raw text of the file with the given MD5. 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) +} + +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) { + var q url.Values + if start > 0 || end > 0 { + q = url.Values{} + if start > 0 { + q.Set("start", strconv.Itoa(start)) + } + if end > 0 { + q.Set("end", strconv.Itoa(end)) + } + } + return s.c.get(ctx, serviceFileContents.endpoint+url.PathEscape(md5), q) +} diff --git a/pkg/scanoss/filecontents_test.go b/pkg/scanoss/filecontents_test.go new file mode 100644 index 0000000..93480dd --- /dev/null +++ b/pkg/scanoss/filecontents_test.go @@ -0,0 +1,90 @@ +// 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 scanoss + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" +) + +// File GETs /v3/file-contents/{md5} 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" + var gotMethod, gotPath, gotQuery string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod, gotPath, gotQuery = r.Method, r.URL.Path, r.URL.RawQuery + _, _ = w.Write([]byte(body)) + })) + defer srv.Close() + + client := mustNew(t, Config{APIURL: srv.URL}) + got, err := client.Contents.File(context.Background(), "abc123", 0, 0) + if err != nil { + t.Fatalf("File: %v", err) + } + if gotMethod != http.MethodGet || gotPath != "/v3/file-contents/abc123" { + t.Errorf("got %s %s, want GET /v3/file-contents/abc123", gotMethod, gotPath) + } + if gotQuery != "" { + t.Errorf("query = %q, want empty for a whole-file fetch", gotQuery) + } + if string(got) != body { + t.Errorf("body = %q, want %q", got, body) + } +} + +// A line range is passed through as start/end query params. +func TestContentsFileLineRange(t *testing.T) { + var gotStart, gotEnd string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotStart = r.URL.Query().Get("start") + gotEnd = r.URL.Query().Get("end") + _, _ = w.Write([]byte("x")) + })) + defer srv.Close() + + client := mustNew(t, Config{APIURL: srv.URL}) + if _, err := client.Contents.File(context.Background(), "abc123", 10, 42); err != nil { + t.Fatalf("File: %v", err) + } + if gotStart != "10" || gotEnd != "42" { + t.Errorf("range params = start:%q end:%q, want 10/42", gotStart, gotEnd) + } +} + +// A non-2xx status becomes an error (a missing file is a 404). +func TestContentsFileNotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"error":{"code":"FILE_NOT_FOUND"}}`, http.StatusNotFound) + })) + defer srv.Close() + + client := mustNew(t, Config{APIURL: srv.URL}) + if _, err := client.Contents.File(context.Background(), "missing", 0, 0); err == nil { + t.Fatal("want an error for a 404") + } +} diff --git a/pkg/scanpipeline/progress.go b/pkg/scanpipeline/progress.go index 4da090d..9d26e1a 100644 --- a/pkg/scanpipeline/progress.go +++ b/pkg/scanpipeline/progress.go @@ -64,6 +64,7 @@ const ( LayerManifests = "manifests" // local: parsing dependency manifests LayerUpload = "upload" // remote: uploading the WFP LayerScan = "scan" // remote: the server-side scan + LayerClassify = "classify" // remote+local: scoring snippet matches with the classifier ) // scanPasses is how many equal segments the scan's progress is divided into, one per pass that can diff --git a/pkg/scanpipeline/scanpipeline.go b/pkg/scanpipeline/scanpipeline.go index fb961ae..f6bab7f 100644 --- a/pkg/scanpipeline/scanpipeline.go +++ b/pkg/scanpipeline/scanpipeline.go @@ -50,6 +50,7 @@ import ( "github.com/scanoss/scanoss.go/pkg/sbom/depsource" "github.com/scanoss/scanoss.go/pkg/sbom/scansource" "github.com/scanoss/scanoss.go/pkg/scanoss" + "github.com/scanoss/scanoss.go/pkg/snippetmatch" "github.com/scanoss/scanoss.go/pkg/wfp" ) @@ -94,6 +95,17 @@ type Options struct { // OnProgress receives every layer's progress. Optional; nil reports nothing. The pipeline runs // layers concurrently, so it must be safe for concurrent use. OnProgress func(Progress) + + // 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. + 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 } // Result is the outcome of Run: the gathered inventory, the files that could not be @@ -336,6 +348,14 @@ func Run(ctx context.Context, opts Options) (Result, error) { 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. + if opts.ClassifySnippets { + classifySnippets(ctx, opts, scanRoot, &inv, emit) + } + return Result{ Inventory: inv, ProcessErrors: procErrors, @@ -343,6 +363,36 @@ func Run(ctx context.Context, opts Options) (Result, error) { }, 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)) { + 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 + annotator, err := snippetmatch.New(snippetmatch.Options{ + Fetcher: opts.Client.Contents, + ScanRoot: scanRoot, + Threshold: opts.ClassifyThreshold, + Workers: opts.Threads, + OnProgress: func(done, t int) { + total = t + emit(Progress{Layer: LayerClassify, Status: StatusRunning, Done: done, Total: t}) + }, + }) + if err != nil { + logging.Warn("snippet classification could not start", "err", err) + emit(Progress{Layer: LayerClassify, Status: StatusFailed}) + return + } + if err := annotator.Annotate(ctx, inv); err != nil { + // Partial failures are already logged per match inside Annotate; this names the tally. + logging.Warn("snippet classification incomplete", "err", err) + } + emit(Progress{Layer: LayerClassify, Status: StatusCompleted, Done: total, Total: total}) +} + // Enricher is what the requested layers are gathered with: a client to ask, the layers to ask // for, and who to report progress to. Run builds one from its Options; a caller that already has // an inventory — from a scan result, or parsed from an existing SBOM — builds one directly. diff --git a/pkg/snippetmatch/snippetmatch.go b/pkg/snippetmatch/snippetmatch.go new file mode 100644 index 0000000..842499c --- /dev/null +++ b/pkg/snippetmatch/snippetmatch.go @@ -0,0 +1,333 @@ +// 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 snippetmatch annotates a scan inventory's snippet matches with the local +// snippet-classifier's verdict on whether each match is a false positive — a match whose +// reported OSS line range does not correspond to its reported local range. +// +// The classifier itself (github.com/scanoss/snippets-classifier) is offline and needs no +// state, but scoring a match needs three inputs: the scanned (local) file, the matched OSS +// file, and the paired line ranges. The inventory carries the ranges and the OSS file's hash; +// this package reads the local file from disk and fetches the OSS file's content by hash. The +// fetch is the only network cost, so unique OSS hashes are downloaded once and reused across +// every match that shares them. +// +// Annotation only proposes: it sets FileEvidence.SnippetClassification and removes nothing. +package snippetmatch + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "sync" + + snippetclassifier "github.com/scanoss/snippets-classifier" + + "github.com/scanoss/scanoss.go/internal/logging" + "github.com/scanoss/scanoss.go/pkg/sbom" +) + +// DefaultThreshold is the P(false positive) at or above which a match is proposed as a false +// positive. It is the classifier's own default (0.7), tuned to propose for human confirmation. +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. +type Fetcher interface { + File(ctx context.Context, md5 string, start, end int) ([]byte, error) +} + +// Options configures an Annotator. +type Options struct { + // Fetcher supplies OSS file content by hash. Required. + Fetcher Fetcher + // ScanRoot is the directory the inventory's file paths are relative to. Required for a + // directory scan; a match whose local file cannot be read is skipped, not failed. + ScanRoot string + // Threshold overrides the classifier's decision threshold. 0 uses DefaultThreshold. + Threshold float64 + // Model, when non-empty, replaces the embedded model (a per-organisation model in the same + // JSON shape). Empty uses the embedded default. + Model []byte + // Workers bounds concurrent OSS content fetches. <1 uses DefaultWorkers. + Workers int + // OnProgress, when set, is called as OSS files are fetched (done out of total unique + // hashes). It must be safe for concurrent use. + OnProgress func(done, total int) +} + +// Annotator scores an inventory's snippet matches. Build one with New and reuse it. +type Annotator struct { + fetcher Fetcher + scanRoot string + workers int + inner *snippetclassifier.Classifier + onProgress func(done, total int) +} + +// New builds an Annotator from opts. It errors when no Fetcher is given or the model cannot be +// loaded. +func New(opts Options) (*Annotator, error) { + if opts.Fetcher == nil { + return nil, errors.New("snippetmatch: a Fetcher is required") + } + var copts []snippetclassifier.Option + if opts.Threshold != 0 { + copts = append(copts, snippetclassifier.WithThreshold(opts.Threshold)) + } + if len(opts.Model) > 0 { + copts = append(copts, snippetclassifier.WithModel(opts.Model)) + } + inner, err := snippetclassifier.New(copts...) + if err != nil { + return nil, fmt.Errorf("snippetmatch: %w", err) + } + workers := opts.Workers + if workers < 1 { + workers = DefaultWorkers + } + return &Annotator{ + fetcher: opts.Fetcher, + scanRoot: opts.ScanRoot, + workers: workers, + inner: inner, + onProgress: opts.OnProgress, + }, nil +} + +// ModelVersion identifies the model the Annotator scores with. +func (a *Annotator) ModelVersion() string { return a.inner.ModelVersion() } + +// 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 + 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. +// +// The inventory is scored, not filtered: no match is removed, whatever its verdict. +func (a *Annotator) Annotate(ctx context.Context, inv *sbom.Inventory) error { + jobs := collectJobs(inv) + if len(jobs) == 0 { + return nil + } + + // Fetch each distinct OSS file once. Several matches often point at the same OSS file + // (different local files matching the same upstream source), and the download is the only + // network cost, so it is shared. + oss := a.fetchOSS(ctx, uniqueHashes(jobs)) + + // Local files are read from disk once per distinct path. Cheap next to the network, so this + // stays sequential. + local := a.readLocal(uniquePaths(jobs)) + + // 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{ + Verdict: string(res.Verdict), + Probability: res.Probability, + ModelVersion: res.ModelVersion, + } + }) + + if skipped > 0 { + return fmt.Errorf("snippetmatch: %d of %d snippet matches could not be classified", skipped, len(jobs)) + } + return nil +} + +// 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. +func collectJobs(inv *sbom.Inventory) []job { + var jobs []job + for ci := range inv.Components { + evs := inv.Components[ci].Evidence + for ei := range evs { + ev := &evs[ei] + if ev.MatchType != "snippet" || ev.FileHash == "" { + continue + } + spans := spansOf(ev.InputLineRanges, ev.OssLineRanges) + if len(spans) == 0 { + continue + } + jobs = append(jobs, job{ev: ev, path: ev.Path, hash: ev.FileHash, spans: spans}) + } + } + return jobs +} + +// spansOf pairs the local and OSS line ranges index by index, as the scanner reports them in +// parallel. A range present on only one side has no counterpart and is dropped. +func spansOf(local, oss []sbom.LineRange) []snippetclassifier.Span { + n := min(len(local), len(oss)) + if n == 0 { + return nil + } + spans := make([]snippetclassifier.Span, 0, n) + for i := 0; i < n; i++ { + spans = append(spans, snippetclassifier.Span{ + LocalStart: local[i].StartLine, LocalEnd: local[i].EndLine, + OSSStart: oss[i].StartLine, OSSEnd: oss[i].EndLine, + }) + } + return spans +} + +// fetchOSS downloads the whole content of each hash, concurrently and bounded by a.workers. +// The classifier's global features (matched_inf_ratio, log_longest_run) read the entire OSS +// file, so the whole file is fetched — not just the matched window. A fetch that fails leaves +// its hash out of the map and is logged. +func (a *Annotator) fetchOSS(ctx context.Context, hashes []string) map[string][]byte { + out := make(map[string][]byte, len(hashes)) + var mu sync.Mutex + var done int + total := len(hashes) + + sem := make(chan struct{}, a.workers) + var wg sync.WaitGroup + for _, h := range hashes { + wg.Add(1) + sem <- struct{}{} + go func(hash string) { + defer wg.Done() + defer func() { <-sem }() + content, err := a.fetcher.File(ctx, hash, 0, 0) + mu.Lock() + if err != nil { + logging.Warn("fetching OSS file content failed", "hash", hash, "err", err) + } else { + out[hash] = content + } + done++ + d := done + mu.Unlock() + if a.onProgress != nil { + a.onProgress(d, total) + } + }(h) + } + wg.Wait() + return out +} + +// readLocal reads each distinct local file once, resolving it against the scan root. An +// unreadable file is left out of the map and logged. +func (a *Annotator) readLocal(paths []string) map[string][]byte { + out := make(map[string][]byte, len(paths)) + for _, p := range paths { + full := p + if a.scanRoot != "" && !filepath.IsAbs(p) { + full = filepath.Join(a.scanRoot, p) + } + content, err := os.ReadFile(full) + if err != nil { + logging.Warn("reading local file for snippet classification failed", "path", p, "err", err) + continue + } + out[p] = content + } + return out +} + +// forEach runs fn over every job, bounded by a.workers. Jobs only read the shared caches and +// write to their own evidence pointer, so no locking is needed inside fn beyond what fn does. +func (a *Annotator) forEach(jobs []job, fn func(job)) { + sem := make(chan struct{}, a.workers) + var wg sync.WaitGroup + for _, j := range jobs { + wg.Add(1) + sem <- struct{}{} + go func(j job) { + defer wg.Done() + defer func() { <-sem }() + fn(j) + }(j) + } + wg.Wait() +} + +func uniqueHashes(jobs []job) []string { + seen := make(map[string]struct{}, len(jobs)) + var out []string + for _, j := range jobs { + if _, ok := seen[j.hash]; ok { + continue + } + seen[j.hash] = struct{}{} + out = append(out, j.hash) + } + return out +} + +func uniquePaths(jobs []job) []string { + seen := make(map[string]struct{}, len(jobs)) + var out []string + for _, j := range jobs { + if _, ok := seen[j.path]; ok { + continue + } + seen[j.path] = struct{}{} + out = append(out, j.path) + } + return out +} diff --git a/pkg/snippetmatch/snippetmatch_test.go b/pkg/snippetmatch/snippetmatch_test.go new file mode 100644 index 0000000..79b6af7 --- /dev/null +++ b/pkg/snippetmatch/snippetmatch_test.go @@ -0,0 +1,271 @@ +// 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 snippetmatch + +import ( + "context" + "errors" + "os" + "path/filepath" + "sync" + "sync/atomic" + "testing" + + "github.com/scanoss/scanoss.go/pkg/sbom" +) + +// realCode is a block whose lines correspond to themselves: local == OSS over the same range is +// the clearest "real" match. +const realCode = `func Sum(xs []int) int { + total := 0 + for _, x := range xs { + total += x + } + if total < 0 { + panic("negative total") + } + return total +} +` + +// unrelatedLocal / unrelatedOSS share no informative line: a wrong candidate assignment. +const unrelatedLocal = `func handler(w http.ResponseWriter, r *http.Request) { + user := loadUser(r) + if user == nil { + http.Error(w, "forbidden", 403) + return + } + render(w, user.Profile) +} +` + +const unrelatedOSS = `def parse_config(path): + with open(path) as fh: + data = yaml.safe_load(fh) + validate(data) + return Config(**data) +` + +// fakeFetcher serves OSS content from a map and counts calls per hash. +type fakeFetcher struct { + mu sync.Mutex + content map[string][]byte + calls map[string]int + err error // when set, every File call fails +} + +func (f *fakeFetcher) File(_ context.Context, md5 string, _, _ int) ([]byte, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.calls == nil { + f.calls = map[string]int{} + } + f.calls[md5]++ + if f.err != nil { + return nil, f.err + } + c, ok := f.content[md5] + if !ok { + return nil, errors.New("not found") + } + return c, nil +} + +func (f *fakeFetcher) callCount(md5 string) int { + f.mu.Lock() + defer f.mu.Unlock() + return f.calls[md5] +} + +// writeLocal writes name under root with the given content and returns name (relative), as it +// would appear in a scan result. +func writeLocal(t *testing.T, root, name, content string) string { + t.Helper() + full := filepath.Join(root, name) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return name +} + +// snippetEvidence builds one snippet FileEvidence covering the whole [1,lines] range on both sides. +func snippetEvidence(path, hash string, lines int) sbom.FileEvidence { + return sbom.FileEvidence{ + Path: path, + FileHash: hash, + MatchType: "snippet", + InputLineRanges: []sbom.LineRange{{StartLine: 1, EndLine: lines}}, + OssLineRanges: []sbom.LineRange{{StartLine: 1, EndLine: lines}}, + } +} + +func invWith(evs ...sbom.FileEvidence) *sbom.Inventory { + return &sbom.Inventory{Components: []sbom.Component{{Purl: "pkg:x/y", Evidence: evs}}} +} + +func TestAnnotateIdenticalIsReal(t *testing.T) { + root := t.TempDir() + path := writeLocal(t, root, "sum.go", realCode) + f := &fakeFetcher{content: map[string][]byte{"h1": []byte(realCode)}} + inv := invWith(snippetEvidence(path, "h1", 10)) + + a, err := New(Options{Fetcher: f, ScanRoot: root}) + if err != nil { + t.Fatal(err) + } + if err := a.Annotate(context.Background(), inv); err != nil { + t.Fatalf("Annotate: %v", err) + } + got := inv.Components[0].Evidence[0].SnippetClassification + if got == nil { + t.Fatal("no classification set") + } + if got.Verdict != "real" { + t.Fatalf("verdict = %q (p=%.3f), want real", got.Verdict, got.Probability) + } + if got.ModelVersion == "" { + t.Error("model version not recorded") + } +} + +func TestAnnotateUnrelatedIsFalsePositive(t *testing.T) { + root := t.TempDir() + path := writeLocal(t, root, "handler.go", unrelatedLocal) + f := &fakeFetcher{content: map[string][]byte{"h1": []byte(unrelatedOSS)}} + inv := invWith(snippetEvidence(path, "h1", 8)) + + a, _ := New(Options{Fetcher: f, ScanRoot: root}) + if err := a.Annotate(context.Background(), inv); err != nil { + t.Fatalf("Annotate: %v", err) + } + got := inv.Components[0].Evidence[0].SnippetClassification + if got == nil || got.Verdict != "false_positive" { + t.Fatalf("classification = %+v, want false_positive", got) + } +} + +func TestOSSFetchIsDedupedByHash(t *testing.T) { + root := t.TempDir() + p1 := writeLocal(t, root, "a.go", realCode) + p2 := writeLocal(t, root, "b.go", realCode) + f := &fakeFetcher{content: map[string][]byte{"shared": []byte(realCode)}} + // Two matches in two files against the same OSS file hash. + 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 { + t.Fatalf("Annotate: %v", err) + } + if n := f.callCount("shared"); n != 1 { + t.Fatalf("OSS hash fetched %d times, want 1 (cache by hash)", n) + } + for i, ev := range inv.Components[0].Evidence { + if ev.SnippetClassification == nil { + t.Errorf("evidence %d not classified", i) + } + } +} + +func TestMissingLocalFileIsSkipped(t *testing.T) { + root := t.TempDir() + f := &fakeFetcher{content: map[string][]byte{"h1": []byte(realCode)}} + // No file written for "ghost.go". + inv := invWith(snippetEvidence("ghost.go", "h1", 10)) + + a, _ := New(Options{Fetcher: f, ScanRoot: root}) + err := a.Annotate(context.Background(), inv) + if err == nil { + t.Fatal("want an error naming the skipped match") + } + if inv.Components[0].Evidence[0].SnippetClassification != nil { + t.Fatal("unreadable local file must not be annotated") + } +} + +func TestFetchErrorIsNonFatalAndSkips(t *testing.T) { + root := t.TempDir() + path := writeLocal(t, root, "sum.go", realCode) + f := &fakeFetcher{err: errors.New("boom")} + inv := invWith(snippetEvidence(path, "h1", 10)) + + a, _ := New(Options{Fetcher: f, ScanRoot: root}) + 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 { + t.Fatal("a match with no OSS content must not be annotated") + } +} + +func TestNonSnippetEvidenceIsIgnored(t *testing.T) { + root := t.TempDir() + var fetched atomic.Int32 + f := &countingFetcher{onCall: func() { fetched.Add(1) }} + inv := invWith( + sbom.FileEvidence{Path: "whole.go", FileHash: "h1", MatchType: "file"}, + sbom.FileEvidence{Path: "pkg.json", MatchType: "declared"}, + ) + + a, _ := New(Options{Fetcher: f, ScanRoot: root}) + if err := a.Annotate(context.Background(), inv); err != nil { + t.Fatalf("Annotate: %v", err) + } + if n := fetched.Load(); n != 0 { + t.Fatalf("fetched %d OSS files for non-snippet evidence, want 0", n) + } + for _, ev := range inv.Components[0].Evidence { + if ev.SnippetClassification != nil { + t.Error("non-snippet evidence must not be classified") + } + } +} + +func TestNoSnippetsMakesNoFetch(t *testing.T) { + var fetched atomic.Int32 + 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 { + t.Fatalf("Annotate: %v", err) + } + if fetched.Load() != 0 { + t.Fatal("empty inventory must trigger no fetch") + } +} + +func TestNewRequiresFetcher(t *testing.T) { + if _, err := New(Options{}); err == nil { + t.Fatal("New must reject a nil Fetcher") + } +} + +type countingFetcher struct{ onCall func() } + +func (c *countingFetcher) File(_ context.Context, _ string, _, _ int) ([]byte, error) { + c.onCall() + return []byte("x"), nil +} From 8ec29c698a930f3d7d4cacca753d44e84deb1046 Mon Sep 17 00:00:00 2001 From: Matias Daloia <66310421+matiasdaloia@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:51:06 +0100 Subject: [PATCH 2/2] feat(scan): classifier threshold, filtering, CycloneDX verdict and summary (#129) * 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. * 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. * 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/sbom/cyclonedx.go | 21 ++- pkg/sbom/inventory.go | 3 +- pkg/sbom/snippetfilter.go | 94 +++++++++++ pkg/sbom/snippetfilter_test.go | 218 ++++++++++++++++++++++++++ pkg/scanoss/filecontents.go | 20 +-- pkg/scanoss/filecontents_test.go | 2 +- pkg/scanpipeline/classify_test.go | 170 ++++++++++++++++++++ pkg/scanpipeline/scanpipeline.go | 85 ++++++++-- pkg/snippetmatch/snippetmatch.go | 80 +++++++--- pkg/snippetmatch/snippetmatch_test.go | 51 ++++-- 15 files changed, 997 insertions(+), 86 deletions(-) create mode 100644 cmd/classify_test.go create mode 100644 pkg/sbom/snippetfilter.go create mode 100644 pkg/sbom/snippetfilter_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/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")