diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 4ab67556..b2141605 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -97,7 +97,8 @@ jobs: - name: ruby run: 'TestScan$/scan-bundler' - name: sbom - run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$' + run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$|TestScanSBOMExportOrigin$' + node: true - name: dotnet run: 'TestScan$/scan-nuget' dotnet: true diff --git a/.github/workflows/update-smoke-goldens.yml b/.github/workflows/update-smoke-goldens.yml index ade2b7d8..25b7d307 100644 --- a/.github/workflows/update-smoke-goldens.yml +++ b/.github/workflows/update-smoke-goldens.yml @@ -121,7 +121,8 @@ jobs: - name: cpp run: 'TestScan$/scan-cpp-conan' - name: sbom - run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$' + run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$|TestScanSBOMExportOrigin$' + node: true - name: plugin run: 'TestPluginWorkflows' - name: container diff --git a/dev-docs/ARCHITECTURE.md b/dev-docs/ARCHITECTURE.md index b556d462..f1923d32 100644 --- a/dev-docs/ARCHITECTURE.md +++ b/dev-docs/ARCHITECTURE.md @@ -594,6 +594,24 @@ Document identity is shared across formats: one generated UUIDv4 becomes both th Further identity and claim rules follow the same only-say-what-we-know principle. The project version comes from `--ref` or `git describe` and is stamped onto the primary component and first-party (main-module) components only — third-party versions are never touched, and no version is emitted when Git has nothing to say. The CycloneDX composition declaration is `complete` only for an unfiltered scan with no detector warnings; a `--scope` filter downgrades it to `incomplete` and degraded resolution to `unknown`. Vulnerability `recommendation` text is rendered only from enrichment-known fixed versions. Deprecated SPDX license identifiers are normalized to their current names token-wise inside expressions (`GPL-2.0` → `GPL-2.0-only`), leaving free-text license values untouched. Every SPDX package carries a `PrimaryPackagePurpose`; decode still prefers the `bomly:type=` comment so round-trips keep the richer domain types (workflow, action) that SPDX's vocabulary lacks. +### Decision: package origin is detector-asserted; SBOM export only projects it + +An SBOM should say where each package came from — SPDX `downloadLocation`, CycloneDX `distribution`/`vcs` references. The obvious place to derive that is export, which already sees every package: classify `Dependency.ResolvedURL` by shape and map it onto the format's fields. That was built and abandoned. It does not work, and the reason generalizes. + +`ResolvedURL` is not one kind of value. npm writes a registry tarball there, but also a local directory for link entries and a git remote for git specs. uv writes a repository, an archive, an index root, or an editable path, depending on the source stanza. Cargo writes a prefixed source string, Bundler writes the section's `remote:` (a gem server, a repository, or a directory), pub writes the pub server for hosted packages and a repository for git ones. Recovering the meaning downstream means guessing from the string, and every guess has an ecosystem-specific counterexample: an archive-extension check misclassifies real repositories whose names end in `.zip` or `.conda`; a fragment is a resolved commit in uv and cargo but a content checksum in Yarn Classic; a private registry root with a path is indistinguishable from a repository; and distinguishing an opaque token from a content hash is not decidable at all, because they have the same shape. Roughly twenty review rounds of layered special cases did not converge. + +Origin is therefore asserted where the meaning is known. Each detector reads its own lockfile's structured source fields and records at most one of an artifact URL or a repository URL plus resolved revision, on `sdk.Dependency.Origin` — a typed field on the shared contract, so an external plugin in its own module can assert origin exactly as a built-in detector does. (It began as CLI-internal metadata keys, which no plugin could reach; `bomly-sdk` v0.4.0 promoted it.) Registry and index roots are deliberately not representable: they describe an ecosystem's fetch configuration, not a package's provenance. + +One rule governs every published value, `sdk.NormalizeOriginURL`: absolute `http`/`https`, non-empty host, non-empty path, a usable port, no userinfo, host case and default ports canonicalized, output re-serialized from the parse rather than copied from input. The repository form additionally strips query and fragment (they carry the *requested* ref; the *resolved* one arrives separately from the detector's own field) and requires a non-empty path, because SPDX's `git+@` grammar has no query component and an empty path would make the `@` suffix re-parse as userinfo. This one function replaces the entire classifier: no archive-extension table, no credential-prefix list, no secret-shape heuristic. Local paths, `file:`, and ssh-style remotes fail the scheme or host check rather than a bespoke rule, and a credentialed URL fails the userinfo check. + +The invariant runs when a detector records a value, again at the JSON boundary in both directions, and again when export reads it through `Origin.Normalized()`. The second pass is not redundant: graphs also arrive from plugins and from hand-built callers, and export must not publish a location no built-in detector could have produced. Composition into a format's locator grammar stays in the encoders, so `Component.VCSURL` remains a plain URL and only SPDX builds the `git+…@…` form; CycloneDX external references have no revision slot, so a resolved commit survives an SPDX round trip and not a CycloneDX one. + +Where one package appears several times in a lockfile, its occurrences must agree before anything is published. `detectors.MergeOrigin`, applied wherever a detector folds a duplicate into an existing node, treats absence as compatible (an occurrence asserting nothing leaves an origin standing; one asserting something fills a gap) and treats two different assertions as cancelling. Picking a winner would make the output depend on traversal order rather than on the lockfile, and an SBOM that omits a location is honest where one that takes a side of a contradiction is not. Detector graph builds walk their inputs in sorted order for the same reason. + +The same rule holds wherever occurrences fold together, and the fold happens at several depths. Within one detector's graph the CLI reconciles: `node.AddNodeIfMissing`, cargo's node dedup, and `normalizeGraphPackageIdentity`, where two nodes collapse onto one canonical PURL. Across manifests the SDK's own graph merge reconciles, so the CLI no longer needs a pass of its own. A disagreement is recorded on the origin as `Disputed` rather than left as an absent value, because absence would let a later fold refill a location the project never agreed on. + +One consequence worth stating: origin does not appear in `scan`/`diff`/`explain` payloads, because those documents are built from explicit projections rather than from the SDK types. It is provenance for the SBOM, which is where users read it. (While origin rode on metadata, keeping it out took an explicit prefix filter in `output.cloneRefMetadata`; the typed field made that unnecessary.) + ## Build Modes Syft and Grype each support two build modes: diff --git a/docs/SBOM.md b/docs/SBOM.md index 7afca480..3be9e9b6 100644 --- a/docs/SBOM.md +++ b/docs/SBOM.md @@ -99,6 +99,82 @@ Both formats carry: knows fixed versions, each vulnerability carries a `recommendation` ("Upgrade to "). No guidance is invented when no fix is known. SPDX 2.3 has no equivalent field. +- Where each package came from, when its lockfile says: an exact download + location or a source repository. See "Where a package came from" below. + +### Where a package came from + +Each detector reports the origin of the packages it resolves, reading the field +its own lockfile records it in. Bomly does not infer origin from the shape of a +URL, because the same string means different things in different ecosystems. + +A detector reports one of two things, or nothing at all: + +| What the lockfile records | SPDX 2.3 | CycloneDX | +|---|---|---| +| The exact file the package was fetched from | `downloadLocation` | `distribution` external reference | +| The repository it was resolved from, and the commit when the lockfile pinned one | `downloadLocation` as `git+`, with `@` when pinned; `sourceInfo` when a download location is already taken by an artifact | `vcs` external reference (URL only) | +| Neither | `NOASSERTION` | no reference | + +CycloneDX external references have no field for a revision, so the commit a +detector resolved appears only in the SPDX form. + +What each ecosystem yields: + +- **npm, pnpm, yarn, bun** — the registry tarball recorded in the lockfile. + Yarn Classic appends the package checksum to that URL; it identifies + contents rather than a location, so it is dropped. pnpm v9 entries that + record only an integrity hash report nothing. +- **uv, poetry, pipenv, pip** — a repository plus the commit that was locked, + or a direct archive URL, depending on the recorded source type. +- **cargo, Bundler, SwiftPM, pub** — the repository and resolved commit for + git dependencies and source-control pins. SwiftPM and pub report the same + origin whether the build tool ran or Bomly read the committed file: the tools + do not print a commit, so it is read back from `Package.resolved` and + `pubspec.lock`. +- **Go modules, Maven, Gradle, NuGet, and the other detectors** — nothing yet; + their manifests do not record a per-package location. +- Packages found by Syft, and packages read from an ingested SBOM, carry no + detector origin. + +With `--enrich`, a package can also get a repository it has no lockfile claim +to. The OpenSSF Scorecard matcher resolves a canonical source repository from a +package's identity, and that repository is used whenever the detector reported +no repository of its own — including for the ecosystems listed above as +yielding nothing, for Syft-detected packages, and **for packages that already +have a download location**. An artifact and a repository answer different +questions (which file was fetched, where the source lives), so a package can +carry both: the artifact stays the SPDX `downloadLocation` and the CycloneDX +`distribution` reference, while the repository becomes the `vcs` reference and, +in SPDX, the package's source info. Only a detector-asserted repository +displaces it. + +It is a network lookup keyed on package identity rather than a claim any +manifest made, so it is weaker evidence, and no revision is attached: a +Scorecard repository names a project, not a resolved commit. Without +`--enrich`, or without the scorecard matcher selected, nothing of this kind +appears. + +Four kinds of value are never published, in any ecosystem: + +- **Registry and index roots** (`https://rubygems.org/`, `https://pub.dev`, the + crates.io index). They say where an ecosystem fetches from, not where a + package came from — and once out of context, a private server URL is + indistinguishable from a repository. +- **Local paths** — workspace members, editable installs, `file:` and `path:` + dependencies. These describe the machine that ran the scan. +- **Non-web locations** — `ssh://`, `git@host:org/repo`, and similar remotes + that name a transport rather than a fetchable address. +- **URLs carrying credentials.** A lockfile pointing at a private registry can + embed a token; publishing it in an SBOM would leak a live secret. + +Every origin a detector reports is an absolute `http`/`https` URL with a host, a +non-empty path, and no embedded credentials. Values are re-serialized from a +parse rather than copied from the lockfile, and the same check runs again at +export, so origin supplied by a plugin is held to the same rule as origin from a +built-in detector. SPDX then composes the validated repository URL into its +`git+@` locator form, which is the only place a revision +appears. ### Document identity @@ -165,14 +241,17 @@ registry (keyed by PURL): - Vulnerabilities — CycloneDX as a first-class `vulnerabilities` array (ratings, CWEs, advisories, `affects`); SPDX as `SECURITY`/`advisory` external references. - End-of-life status (CycloneDX `bomly:eol*` properties, SPDX package comment). +- A source repository resolved by the OpenSSF Scorecard matcher, used only when + the detector reported no repository of its own (see "Where a package came + from" above). Reachability annotations and other Bomly-specific metadata are emitted in the JSON output (`--json` or `--format json`), not in the standard SBOM formats. See [Output formats](OUTPUT_FORMATS.md). ### Preservation and conversion limits Bomly preserves component identity (including PURL), dependency edges, roots, -scope, package type, licenses, digests, CPEs, and the enrichment fields described -above when the destination format has an equivalent representation. Encoding is +scope, package type, licenses, digests, CPEs, and the enrichment fields +described above when the destination format has an equivalent representation. Encoding is deterministic when the scan timestamp and document identifiers are fixed. Some information necessarily becomes less specific during conversion: @@ -184,6 +263,10 @@ Some information necessarily becomes less specific during conversion: round trip. - Development scope maps to CycloneDX `excluded`; runtime scope maps to `required`. SPDX stores Bomly's normalized scope in the package comment. +- Package origin is written on export but not read back on ingest: scanning an + SBOM produces packages with no origin, so re-exporting that graph emits + `NOASSERTION` and no distribution or vcs reference. Origin comes from a + lockfile, and an ingested document is not one. - Bomly relationship confidence (`direct`, `transitive`, or `unknown`), source provenance, reachability analysis, policy findings, and run diagnostics are report data rather than portable SBOM fields. Use JSON when those distinctions diff --git a/docs/schemas/diff.md b/docs/schemas/diff.md index 7aded733..2aa2bb82 100644 --- a/docs/schemas/diff.md +++ b/docs/schemas/diff.md @@ -245,6 +245,7 @@ Complete reference for the `bomly diff` JSON output. |-------|------|-------------| | `algorithm` | `string` | | | `value` | `string` | | +| `subject` | `string` | | ### `EPSSScore` diff --git a/docs/schemas/diff.schema.json b/docs/schemas/diff.schema.json index c15348c9..77380653 100644 --- a/docs/schemas/diff.schema.json +++ b/docs/schemas/diff.schema.json @@ -348,6 +348,9 @@ "algorithm": { "type": "string" }, + "subject": { + "type": "string" + }, "value": { "type": "string" } diff --git a/docs/schemas/scan.md b/docs/schemas/scan.md index 5c53ea0a..0d84caaf 100644 --- a/docs/schemas/scan.md +++ b/docs/schemas/scan.md @@ -107,6 +107,7 @@ Complete reference for the `bomly scan` JSON output. |-------|------|-------------| | `algorithm` | `string` | | | `value` | `string` | | +| `subject` | `string` | | ### `EPSSScore` diff --git a/docs/schemas/scan.schema.json b/docs/schemas/scan.schema.json index 9437ae7b..c23e06c0 100644 --- a/docs/schemas/scan.schema.json +++ b/docs/schemas/scan.schema.json @@ -330,6 +330,9 @@ "algorithm": { "type": "string" }, + "subject": { + "type": "string" + }, "value": { "type": "string" } diff --git a/go.mod b/go.mod index c537345c..0d230dc0 100644 --- a/go.mod +++ b/go.mod @@ -114,7 +114,7 @@ require ( github.com/bodgit/plumbing v1.3.0 // indirect github.com/bodgit/sevenzip v1.6.1 // indirect github.com/bodgit/windows v1.0.1 // indirect - github.com/bomly-dev/bomly-sdk v0.3.0 + github.com/bomly-dev/bomly-sdk v0.4.0 github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/colorprofile v0.4.3 // indirect github.com/charmbracelet/harmonica v0.2.0 // indirect diff --git a/go.sum b/go.sum index 48b533e4..4b7ac967 100644 --- a/go.sum +++ b/go.sum @@ -259,6 +259,8 @@ github.com/bomly-dev/bomly-plugin-syft-detector v0.1.0 h1:izjWbFILBnjSrRyCwBQS2d github.com/bomly-dev/bomly-plugin-syft-detector v0.1.0/go.mod h1:uLBBHXzEJaNczZAyTIZjxfTSDoU+yLVC36qBE+vIAyk= github.com/bomly-dev/bomly-sdk v0.3.0 h1:JtC7qZ9yq3r4fUYyq7e/Os4f9wGMa4Qot8U0l9MepFA= github.com/bomly-dev/bomly-sdk v0.3.0/go.mod h1:yn1LBkoHG9gDBXKyRj0UNJo0BlXl8Bj9Ymb3WKLIh78= +github.com/bomly-dev/bomly-sdk v0.4.0 h1:C+3gOVF6KFZ8KOq4jV94snP+qn8s1QzxG8PhE30yDaI= +github.com/bomly-dev/bomly-sdk v0.4.0/go.mod h1:yn1LBkoHG9gDBXKyRj0UNJo0BlXl8Bj9Ymb3WKLIh78= github.com/bradleyjkemp/cupaloy/v2 v2.8.0 h1:any4BmKE+jGIaMpnU8YgH/I2LPiLBufr6oMMlVBbn9M= github.com/bradleyjkemp/cupaloy/v2 v2.8.0/go.mod h1:bm7JXdkRd4BHJk9HpwqAI8BoAY1lps46Enkdqw6aRX0= github.com/bufbuild/protocompile v0.14.1 h1:iA73zAf/fyljNjQKwYzUHD6AD4R8KMasmwa/FBatYVw= diff --git a/internal/detectors/cargo/detector.go b/internal/detectors/cargo/detector.go index 83d2c503..c2652704 100644 --- a/internal/detectors/cargo/detector.go +++ b/internal/detectors/cargo/detector.go @@ -304,7 +304,8 @@ func metadataGraphWithMembers(raw []byte, scopeFilter sdk.Scope) (*sdk.Graph, [] return nil, nil, fmt.Errorf("add root node: %w", err) } } - for id, pkg := range packagesByID { + for _, id := range sortedPackageIDs(packagesByID) { + pkg := packagesByID[id] node := packageNode(pkg, id, workspace) if err := addNodeIfMissing(g, node); err != nil { return nil, nil, err @@ -382,7 +383,7 @@ func packageNode(pkg metadataPackage, id string, workspace map[string]struct{}) source = sdk.DependencySourceWorkspace } } - return sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemRust, + node := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemRust, Name: pkg.Name, Version: pkg.Version, PackageManager: sdk.PackageManagerCargo, @@ -390,6 +391,8 @@ func packageNode(pkg metadataPackage, id string, workspace map[string]struct{}) Language: "rust", PURL: sdk.BuildPackageURL("cargo", "", pkg.Name, pkg.Version)}, Source: source, ResolvedURL: pkg.Source, }) + setCargoOrigin(node, pkg.Source) + return node } @@ -426,8 +429,25 @@ func sortedWorkspaceMembers(workspace map[string]struct{}) []string { return values } +// sortedPackageIDs orders cargo's package map so one lockfile always builds +// the same graph: map iteration would let two records of one crate decide the +// node differently per run. +func sortedPackageIDs(packages map[string]metadataPackage) []string { + ids := make([]string, 0, len(packages)) + for id := range packages { + ids = append(ids, id) + } + sort.Strings(ids) + return ids +} + func addNodeIfMissing(g *sdk.Graph, node *sdk.Dependency) error { - if _, ok := g.Node(node.ID); ok { + if existing, ok := g.Node(node.ID); ok { + // Cargo can resolve one crate name and version from two sources -- the + // same crate pulled from two git remotes, say. They share a PURL, so + // they are one node, and the node must not claim whichever source was + // visited first. + existing.Origin = sdk.ReconcileOrigin(existing.Origin, node.Origin) return nil } if err := g.AddNode(node); err != nil { diff --git a/internal/detectors/cargo/origin.go b/internal/detectors/cargo/origin.go new file mode 100644 index 00000000..6076a825 --- /dev/null +++ b/internal/detectors/cargo/origin.go @@ -0,0 +1,41 @@ +package cargo + +import ( + "net/url" + "strings" + + "github.com/bomly-dev/bomly-sdk" +) + +// setCargoOrigin records the repository a git-sourced crate was resolved from. +// Cargo writes one source string per package: "registry+"/"sparse+" name an +// index root rather than this crate's location, path and workspace members +// carry no source at all, and only "git+" identifies where the code came from. +func setCargoOrigin(node *sdk.Dependency, source string) { + trimmed := strings.TrimSpace(source) + if !strings.HasPrefix(trimmed, "git+") { + return + } + repository := strings.TrimPrefix(trimmed, "git+") + node.Origin = sdk.RepositoryOrigin(repository, cargoSourceRevision(repository)) +} + +// cargoSourceRevision returns the revision cargo locked. The URL fragment holds +// the resolved commit; the "rev", "tag", and "branch" query parameters hold +// what the manifest asked for, which is the weaker answer. +func cargoSourceRevision(repository string) string { + parsed, err := url.Parse(strings.TrimSpace(repository)) + if err != nil { + return "" + } + if parsed.Fragment != "" { + return parsed.Fragment + } + query := parsed.Query() + for _, key := range []string{"rev", "tag", "branch"} { + if value := strings.TrimSpace(query.Get(key)); value != "" { + return value + } + } + return "" +} diff --git a/internal/detectors/cargo/origin_test.go b/internal/detectors/cargo/origin_test.go new file mode 100644 index 00000000..a19dfd2e --- /dev/null +++ b/internal/detectors/cargo/origin_test.go @@ -0,0 +1,176 @@ +package cargo + +import ( + "testing" + + "github.com/bomly-dev/bomly-sdk" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// Cargo.lock records one source string per package. Only "git+" names where the +// code came from; the index prefixes name a registry, and path or workspace +// members carry no source at all. +func TestSetCargoOriginBySourcePrefix(t *testing.T) { + cases := []struct { + name string + source string + want sdk.PackageOrigin + }{ + { + name: "git dependency pins the resolved commit in the fragment", + source: "git+https://github.com/example/helper?rev=main#3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f", + want: sdk.PackageOrigin{Repository: "https://github.com/example/helper", Revision: "3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f"}, + }, + { + name: "requested tag is used when no commit was recorded", + source: "git+https://github.com/example/helper?tag=v1.2.3", + want: sdk.PackageOrigin{Repository: "https://github.com/example/helper", Revision: "v1.2.3"}, + }, + { + name: "branch dependency without a pin keeps the repository", + source: "git+https://github.com/example/helper", + want: sdk.PackageOrigin{Repository: "https://github.com/example/helper"}, + }, + {name: "crates.io index root", source: "registry+https://github.com/rust-lang/crates.io-index"}, + {name: "sparse index root", source: "sparse+https://index.crates.io/"}, + {name: "path or workspace member", source: ""}, + {name: "credentialed private git remote", source: "git+https://token:s3cret-value-here@git.corp/team/helper#4d5e6f70"}, + {name: "ssh git remote", source: "git+ssh://git@github.com/example/helper#5e6f7081"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + node := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Name: "helper", Version: "1.0.0"}}) + setCargoOrigin(node, tc.source) + if got := originOf(node); got != tc.want { + t.Fatalf("origin = %+v, want %+v", got, tc.want) + } + }) + } +} + +// Cargo can resolve one crate name and version from two sources -- the same +// crate pulled from two git remotes. They share a PURL, so they become one +// node, and that node must not claim whichever source was walked first. +func TestCargoDuplicateCrateSourcesCancelOrigin(t *testing.T) { + metadata := []byte(`{ + "packages": [ + {"id": "demo 0.1.0 (path+file:///w)", "name": "demo", "version": "0.1.0", "source": null, "dependencies": []}, + {"id": "helper 1.0.0 (git+https://github.com/a/helper#aaaa)", "name": "helper", "version": "1.0.0", "source": "git+https://github.com/a/helper#aaaabbbbccccddddeeeeffff0000111122223333", "dependencies": []}, + {"id": "helper 1.0.0 (git+https://github.com/b/helper#bbbb)", "name": "helper", "version": "1.0.0", "source": "git+https://github.com/b/helper#bbbbccccddddeeeeffff00001111222233334444", "dependencies": []} + ], + "workspace_members": ["demo 0.1.0 (path+file:///w)"], + "resolve": {"nodes": [], "root": "demo 0.1.0 (path+file:///w)"} + }`) + + // Repeat: the packages arrive in a map, so an order-dependent answer shows + // up as a different result between runs rather than a stable wrong one. + for range 25 { + graph, err := depGraphFromMetadata(metadata) + if err != nil { + t.Fatalf("depGraphFromMetadata() error = %v", err) + } + var checked int + graph.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name != "helper" { + return true + } + checked++ + if got := originOf(dep); !got.Empty() { + t.Fatalf("origin = %+v, want none: the crate resolved from two repositories", got) + } + return true + }) + if checked != 1 { + t.Fatalf("found %d helper nodes, want 1", checked) + } + } +} + +// Two records naming the same source still publish it. +func TestCargoDuplicateCrateSameSourceKeepsOrigin(t *testing.T) { + metadata := []byte(`{ + "packages": [ + {"id": "demo 0.1.0 (path+file:///w)", "name": "demo", "version": "0.1.0", "source": null, "dependencies": []}, + {"id": "helper 1.0.0 (git+https://github.com/a/helper#aaaa)", "name": "helper", "version": "1.0.0", "source": "git+https://github.com/a/helper#aaaabbbbccccddddeeeeffff0000111122223333", "dependencies": []}, + {"id": "helper 1.0.0 (git+https://github.com/a/helper#aaaa2)", "name": "helper", "version": "1.0.0", "source": "git+https://github.com/a/helper#aaaabbbbccccddddeeeeffff0000111122223333", "dependencies": []} + ], + "workspace_members": ["demo 0.1.0 (path+file:///w)"], + "resolve": {"nodes": [], "root": "demo 0.1.0 (path+file:///w)"} + }`) + + graph, err := depGraphFromMetadata(metadata) + if err != nil { + t.Fatalf("depGraphFromMetadata() error = %v", err) + } + want := sdk.PackageOrigin{ + Repository: "https://github.com/a/helper", + Revision: "aaaabbbbccccddddeeeeffff0000111122223333", + } + var checked int + graph.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name == "helper" { + checked++ + if got := originOf(dep); got != want { + t.Fatalf("origin = %+v, want %+v", got, want) + } + } + return true + }) + if checked != 1 { + t.Fatalf("found %d helper nodes, want 1", checked) + } +} + +// The lockfile path builds nodes through the same helper. +func TestCargoLockGraphCarriesOrigin(t *testing.T) { + lock := []byte(` +[[package]] +name = "demo" +version = "0.1.0" +dependencies = ["helper", "serde"] + +[[package]] +name = "helper" +version = "1.0.0" +source = "git+https://github.com/example/helper?rev=main#6f708192a3b4c5d6e7f8091a2b3c4d5e6f708192" + +[[package]] +name = "serde" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +`) + manifest := []byte("[package]\nname = \"demo\"\nversion = \"0.1.0\"\n[dependencies]\nhelper = { git = \"https://github.com/example/helper\" }\nserde = \"1\"\n") + + graph, err := depGraphFromLock(lock, manifest) + if err != nil { + t.Fatalf("depGraphFromLock() error = %v", err) + } + + helper, ok := graph.Node("helper@1.0.0") + if !ok { + t.Fatal("expected helper in graph") + } + want := sdk.PackageOrigin{Repository: "https://github.com/example/helper", Revision: "6f708192a3b4c5d6e7f8091a2b3c4d5e6f708192"} + if got := originOf(helper); got != want { + t.Fatalf("helper origin = %+v, want %+v", got, want) + } + serde, ok := graph.Node("serde@1.0.0") + if !ok { + t.Fatal("expected serde in graph") + } + if got := originOf(serde); !got.Empty() { + t.Fatalf("registry crate asserted an origin: %+v", got) + } +} diff --git a/internal/detectors/cargo/workspace.go b/internal/detectors/cargo/workspace.go index eebdcf13..1e600b29 100644 --- a/internal/detectors/cargo/workspace.go +++ b/internal/detectors/cargo/workspace.go @@ -181,7 +181,7 @@ func depGraphFromLockWorkspace(lockRaw []byte, rootManifest cargoManifest, membe pkgType = "application" source = sdk.DependencySourceWorkspace } - return sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemRust, + node := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemRust, Name: pkg.Name, Version: pkg.Version, PackageManager: sdk.PackageManagerCargo, @@ -189,6 +189,8 @@ func depGraphFromLockWorkspace(lockRaw []byte, rootManifest cargoManifest, membe Language: "rust", PURL: sdk.BuildPackageURL("cargo", "", pkg.Name, pkg.Version)}, Source: source, ResolvedURL: pkg.Source, }) + setCargoOrigin(node, pkg.Source) + return node } lockPackageFor := func(manifest cargoManifest) lockPackage { if pkg, ok := byName[manifest.Name]; ok && pkg.Version != "" { diff --git a/internal/detectors/node/bun/bun_lockfile_parser.go b/internal/detectors/node/bun/bun_lockfile_parser.go index 153eff8f..6e76fb20 100644 --- a/internal/detectors/node/bun/bun_lockfile_parser.go +++ b/internal/detectors/node/bun/bun_lockfile_parser.go @@ -141,6 +141,9 @@ func depGraphFromBunLockfile(projectPath string) (bunLockfileGraphs, error) { if _, exists := graph.Node(pkgNode.ID); exists { pkgNode = sdk.NewDependencyWithID("bun-package:"+key, dep) } + // Bun's tuple carries the registry tarball it fetched. Workspace + // members and git specs resolve to values the invariant rejects. + pkgNode.Origin = sdk.ArtifactOrigin(entry.resolved) if err := node.AddNodeIfMissing(graph, pkgNode); err != nil { return bunLockfileGraphs{}, err } diff --git a/internal/detectors/node/common.go b/internal/detectors/node/common.go index 1aa645a8..ad0ea390 100644 --- a/internal/detectors/node/common.go +++ b/internal/detectors/node/common.go @@ -9,6 +9,7 @@ import ( "io" "os" "path/filepath" + "sort" "time" "github.com/bomly-dev/bomly-cli/internal/logging" @@ -26,8 +27,12 @@ type BaseDetector struct { // NPMListNode is the npm list JSON node shape used by npm CLI and v1 package-lock parsing. type NPMListNode struct { - Name string `json:"name"` - Version string `json:"version"` + Name string `json:"name"` + Version string `json:"version"` + // Resolved is the tarball a package was installed from. Both sources of + // this shape record it: `npm ls --json` for registry packages, and the + // flat "dependencies" map of a v1 package-lock.json. + Resolved string `json:"resolved"` Dependencies map[string]*NPMListNode `json:"dependencies"` } @@ -163,7 +168,16 @@ func DepGraphFromNPMNode(root *NPMListNode) (*sdk.Graph, error) { current := stack[len(stack)-1] stack = stack[:len(stack)-1] - for depName, depNode := range current.deps { + depNames := make([]string, 0, len(current.deps)) + for depName := range current.deps { + depNames = append(depNames, depName) + } + // Walk in a fixed order: a map's iteration order would let two + // occurrences of one package decide the graph differently per run. + sort.Strings(depNames) + + for _, depName := range depNames { + depNode := current.deps[depName] if depNode == nil { continue } @@ -175,6 +189,7 @@ func DepGraphFromNPMNode(root *NPMListNode) (*sdk.Graph, error) { Name: name, Version: depNode.Version}, }) + node.Origin = sdk.ArtifactOrigin(depNode.Resolved) if err := AddNodeIfMissing(depsGraph, node); err != nil { return nil, err @@ -324,6 +339,7 @@ func splitYarnTreeName(value string) (string, string, error) { func AddNodeIfMissing(depsGraph *sdk.Graph, node *sdk.Dependency) error { if existing, ok := depsGraph.Node(node.ID); ok { existing.AddScope(node.PrimaryScope()) + existing.Origin = sdk.ReconcileOrigin(existing.Origin, node.Origin) return nil } if err := depsGraph.AddNode(node); err != nil { diff --git a/internal/detectors/node/npm/npm_lockfile_parser.go b/internal/detectors/node/npm/npm_lockfile_parser.go index 7828f1f5..5a80b7ed 100644 --- a/internal/detectors/node/npm/npm_lockfile_parser.go +++ b/internal/detectors/node/npm/npm_lockfile_parser.go @@ -247,6 +247,10 @@ func depGraphFromNPMLockfile(projectPath string) (npmLockfileGraphs, error) { pkg.Metadata = map[string]any{sdk.MetadataKeyNPM: meta} } pkgNode := sdk.NewDependency(pkg) + // npm records the registry tarball a package was installed from. + // Workspace members cleared ResolvedURL above (it names a local + // directory), and git or file specs are rejected by the invariant. + pkgNode.Origin = sdk.ArtifactOrigin(pkg.ResolvedURL) if entry.License != "" { sdk.SetDetectionLicenses(pkgNode, []sdk.PackageLicense{{Value: entry.License, Type: "declared"}}) } diff --git a/internal/detectors/node/npm/origin_test.go b/internal/detectors/node/npm/origin_test.go new file mode 100644 index 00000000..9d06487c --- /dev/null +++ b/internal/detectors/node/npm/origin_test.go @@ -0,0 +1,136 @@ +package npm + +import ( + "github.com/bomly-dev/bomly-sdk" + "os" + "path/filepath" + "testing" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// npm writes whatever it installed from into "resolved": a registry tarball, +// but also a git remote or a local path. Only the first is a location an SBOM +// can publish. +func TestNPMOriginByResolvedShape(t *testing.T) { + projectDir := t.TempDir() + lockfile := `{ + "name": "demo", + "version": "1.0.0", + "lockfileVersion": 3, + "packages": { + "": {"name": "demo", "version": "1.0.0", "dependencies": {"from-registry": "1.0.0", "from-git": "1.0.0", "from-file": "1.0.0", "from-private": "1.0.0"}}, + "node_modules/from-registry": {"version": "1.0.0", "resolved": "https://registry.npmjs.org/from-registry/-/from-registry-1.0.0.tgz"}, + "node_modules/from-git": {"version": "1.0.0", "resolved": "git+ssh://git@github.com/owner/repo.git#9f8e7d6c5b4a3928176554433221100ffeeddcc0"}, + "node_modules/from-file": {"version": "1.0.0", "resolved": "file:../vendor/from-file"}, + "node_modules/from-private": {"version": "1.0.0", "resolved": "https://build:s3cret-token-value@nexus.corp/repo/from-private-1.0.0.tgz"} + } + }` + if err := os.WriteFile(filepath.Join(projectDir, "package-lock.json"), []byte(lockfile), 0o644); err != nil { + t.Fatal(err) + } + + graphs, err := depGraphFromNPMLockfile(projectDir) + if err != nil { + t.Fatalf("depGraphFromNPMLockfile() error = %v", err) + } + + cases := []struct { + id string + want string + }{ + {id: "from-registry@1.0.0", want: "https://registry.npmjs.org/from-registry/-/from-registry-1.0.0.tgz"}, + {id: "from-git@1.0.0"}, // a git remote npm can reach, not a published location + {id: "from-file@1.0.0"}, // a path on the machine that ran npm install + {id: "from-private@1.0.0"}, // carries a credential + } + for _, tc := range cases { + node, ok := graphs.graph.Node(tc.id) + if !ok { + t.Fatalf("expected %s in graph", tc.id) + } + origin := originOf(node) + if origin.ArtifactURL != tc.want { + t.Errorf("%s artifact origin = %q, want %q", tc.id, origin.ArtifactURL, tc.want) + } + if origin.Repository != "" { + t.Errorf("%s asserted a repository %q", tc.id, origin.Repository) + } + // ResolvedURL is a separate contract (the scorecard matcher resolves + // repositories from it) and must keep carrying the raw lockfile value. + if node.ResolvedURL == "" { + t.Errorf("%s lost its ResolvedURL", tc.id) + } + } +} + +// A v1 lockfile repeats a package at every place it is installed, and the +// copies can disagree: one nested under a package pinned to a private mirror, +// one at the top level from the public registry. They share a name and version, +// so they become one graph node, and the node must not report whichever copy +// the traversal reached first. +func TestNPMv1DuplicateEntriesReconcileOrigin(t *testing.T) { + projectDir := t.TempDir() + lockfile := `{ + "name": "demo", + "version": "1.0.0", + "lockfileVersion": 1, + "dependencies": { + "a-first": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/a-first/-/a-first-1.0.0.tgz", + "dependencies": { + "disputed": {"version": "2.0.0", "resolved": "https://npm.corp/mirror/disputed/-/disputed-2.0.0.tgz"}, + "agreed": {"version": "3.0.0", "resolved": "https://registry.npmjs.org/agreed/-/agreed-3.0.0.tgz"} + } + }, + "z-last": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/z-last/-/z-last-1.0.0.tgz", + "dependencies": { + "disputed": {"version": "2.0.0", "resolved": "https://registry.npmjs.org/disputed/-/disputed-2.0.0.tgz"}, + "agreed": {"version": "3.0.0", "resolved": "https://registry.npmjs.org/agreed/-/agreed-3.0.0.tgz"} + } + } + } + }` + if err := os.WriteFile(filepath.Join(projectDir, "package-lock.json"), []byte(lockfile), 0o644); err != nil { + t.Fatal(err) + } + + // Repeat: the traversal walks maps, so an order-dependent result would + // show up as a different answer between runs rather than a stable wrong one. + for range 25 { + graphs, err := depGraphFromNPMLockfile(projectDir) + if err != nil { + t.Fatalf("depGraphFromNPMLockfile() error = %v", err) + } + + disputed, ok := graphs.graph.Node("disputed@2.0.0") + if !ok { + t.Fatal("expected disputed@2.0.0 in graph") + } + if origin := originOf(disputed); !origin.Empty() { + t.Fatalf("disputed origin = %+v, want none: its two copies name different locations", origin) + } + + agreed, ok := graphs.graph.Node("agreed@3.0.0") + if !ok { + t.Fatal("expected agreed@3.0.0 in graph") + } + const want = "https://registry.npmjs.org/agreed/-/agreed-3.0.0.tgz" + if got := originOf(agreed).ArtifactURL; got != want { + t.Fatalf("agreed origin = %q, want %q: its copies agree", got, want) + } + } +} diff --git a/internal/detectors/node/origin_integration_test.go b/internal/detectors/node/origin_integration_test.go new file mode 100644 index 00000000..d95a72be --- /dev/null +++ b/internal/detectors/node/origin_integration_test.go @@ -0,0 +1,137 @@ +package node_test + +import ( + "context" + "testing" + + "github.com/bomly-dev/bomly-cli/internal/detectors/node/bun" + "github.com/bomly-dev/bomly-cli/internal/detectors/node/npm" + "github.com/bomly-dev/bomly-cli/internal/detectors/node/pnpm" + "github.com/bomly-dev/bomly-cli/internal/detectors/node/yarn" + "github.com/bomly-dev/bomly-sdk" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// requireArtifactOrigin asserts a package asserts exactly the given artifact. +func requireArtifactOrigin(t *testing.T, g *sdk.Graph, name, version, want string) { + t.Helper() + origin := originOf(requirePackage(t, g, name, version)) + if origin.ArtifactURL != want { + t.Errorf("%s@%s artifact origin = %q, want %q", name, version, origin.ArtifactURL, want) + } + if origin.Repository != "" { + t.Errorf("%s@%s also asserted a repository %q", name, version, origin.Repository) + } +} + +// requireNoOrigin asserts a package publishes no location at all. +func requireNoOrigin(t *testing.T, g *sdk.Graph, name, version string) { + t.Helper() + if origin := originOf(requirePackage(t, g, name, version)); !origin.Empty() { + t.Errorf("%s@%s asserted an origin it should not have: %+v", name, version, origin) + } +} + +func TestNPMLockfileOriginIsTheRegistryTarball(t *testing.T) { + g, err := resolveLockfileGraph(t, npm.LockfileDetector{}, fixture("npm-v3")) + if err != nil { + t.Fatal(err) + } + requireArtifactOrigin(t, g, "lodash", "4.17.21", "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz") + requireArtifactOrigin(t, g, "jest", "29.7.0", "https://registry.npmjs.org/jest/-/jest-29.7.0.tgz") +} + +// A v1 lockfile has no packages map, so it resolves through the flat +// dependencies tree. It records the same tarball URLs, and must publish them. +func TestNPMLockfileV1OriginIsTheRegistryTarball(t *testing.T) { + g, err := resolveLockfileGraph(t, npm.LockfileDetector{}, fixture("npm-v1")) + if err != nil { + t.Fatal(err) + } + requireArtifactOrigin(t, g, "react", "18.2.0", "https://registry.npmjs.org/react/-/react-18.2.0.tgz") + requireArtifactOrigin(t, g, "loose-envify", "1.4.0", "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz") +} + +// Workspace members are local directories. npm records that directory as the +// member's "resolved" value, which must never reach an SBOM. +func TestNPMWorkspaceMembersAssertNoOrigin(t *testing.T) { + result, err := (npm.LockfileDetector{}).ResolveGraph(context.Background(), sdk.DetectionRequest{ProjectPath: fixture("npm-v3-workspaces")}) + if err != nil { + t.Fatal(err) + } + g, err := result.Graphs.ConsolidatedGraph() + if err != nil { + t.Fatal(err) + } + requireArtifactOrigin(t, g, "lodash", "4.17.21", "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz") + requireNoOrigin(t, g, "web", "0.2.0") + requireNoOrigin(t, g, "lib", "1.0.0") +} + +func TestPNPMLockfileOriginIsTheResolutionTarball(t *testing.T) { + g, err := resolveLockfileGraph(t, pnpm.LockfileDetector{}, fixture("pnpm-v5")) + if err != nil { + t.Fatal(err) + } + requireArtifactOrigin(t, g, "react", "18.2.0", "https://registry.npmjs.org/react/-/react-18.2.0.tgz") +} + +// pnpm v9 records only an integrity hash for registry packages. A hash is not a +// location, and the registry root is not this package's origin, so there is +// nothing to assert. +func TestPNPMIntegrityOnlyEntriesAssertNoOrigin(t *testing.T) { + result, err := (pnpm.LockfileDetector{}).ResolveGraph(context.Background(), sdk.DetectionRequest{ProjectPath: fixture("pnpm-v9-workspaces")}) + if err != nil { + t.Fatal(err) + } + g, err := result.Graphs.ConsolidatedGraph() + if err != nil { + t.Fatal(err) + } + requireNoOrigin(t, g, "lodash", "4.17.21") + requireNoOrigin(t, g, "shared-transitive", "2.0.0") +} + +// Yarn Classic appends the package checksum to the tarball URL as a fragment. +// It identifies the file's contents, not a location, so it is dropped. +func TestYarnClassicOriginDropsTheChecksumFragment(t *testing.T) { + g, err := resolveLockfileGraph(t, yarn.LockfileDetector{}, fixture("yarn-v1")) + if err != nil { + t.Fatal(err) + } + requireArtifactOrigin(t, g, "react", "18.2.0", "https://registry.npmjs.org/react/-/react-18.2.0.tgz") + requireArtifactOrigin(t, g, "js-tokens", "4.0.0", "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz") +} + +// Berry lockfiles record a resolution identity rather than a fetched URL. +func TestYarnBerryAssertsNoOrigin(t *testing.T) { + g, err := resolveLockfileGraph(t, yarn.LockfileDetector{}, fixture("yarn-berry")) + if err != nil { + t.Fatal(err) + } + requireNoOrigin(t, g, "react", "18.2.0") +} + +func TestBunLockfileOriginIsTheRegistryTarball(t *testing.T) { + result, err := (bun.LockfileDetector{}).ResolveGraph(context.Background(), sdk.DetectionRequest{ProjectPath: fixture("bun-v1-workspaces")}) + if err != nil { + t.Fatal(err) + } + g, err := result.Graphs.ConsolidatedGraph() + if err != nil { + t.Fatal(err) + } + requireArtifactOrigin(t, g, "is-number", "7.0.0", "https://registry.npmjs.org/is-number/-/is-number-7.0.0.tgz") + requireNoOrigin(t, g, "workspace:packages/lib", "") +} diff --git a/internal/detectors/node/pnpm/pnpm_lockfile_parser.go b/internal/detectors/node/pnpm/pnpm_lockfile_parser.go index 19100f8e..7a30d0a4 100644 --- a/internal/detectors/node/pnpm/pnpm_lockfile_parser.go +++ b/internal/detectors/node/pnpm/pnpm_lockfile_parser.go @@ -138,6 +138,10 @@ func depGraphFromPNPMLockfile(projectPath string) (pnpmLockfileGraphs, error) { if existing, ok := depsGraph.Node(pkgNode.ID); ok && existing.Type == sdk.PackageTypeApplication { pkgNode = sdk.NewDependencyWithID("pnpm-package:"+key, pkg) } + // pnpm records a tarball only when it resolved one; v9 lockfiles + // often carry just an integrity hash, and git or directory + // resolutions are not parsed, so those packages assert no origin. + pkgNode.Origin = sdk.ArtifactOrigin(entry.Resolution.Tarball) if entry.License != "" { sdk.SetDetectionLicenses(pkgNode, []sdk.PackageLicense{{Value: entry.License, Type: "declared"}}) } diff --git a/internal/detectors/node/yarn/yarn_lockfile_parser.go b/internal/detectors/node/yarn/yarn_lockfile_parser.go index c69024cc..70a33329 100644 --- a/internal/detectors/node/yarn/yarn_lockfile_parser.go +++ b/internal/detectors/node/yarn/yarn_lockfile_parser.go @@ -81,6 +81,10 @@ func depGraphFromYarnLockfile(projectPath string) (*sdk.Graph, error) { if existing, ok := depsGraph.Node(pkgNode.ID); ok && existing.Type == sdk.PackageTypeApplication { pkgNode = sdk.NewDependencyWithID(fmt.Sprintf("yarn-package:%d", idx), pkg) } + // Yarn Classic records the tarball it fetched, with the package + // checksum as a URL fragment the invariant strips. Berry entries + // carry no resolved location, and git specs are rejected. + pkgNode.Origin = sdk.ArtifactOrigin(entry.Resolved) if err := node.AddNodeIfMissing(depsGraph, pkgNode); err != nil { return "", err } diff --git a/internal/detectors/pub/detector.go b/internal/detectors/pub/detector.go index 5cf03c5d..6fa62e27 100644 --- a/internal/detectors/pub/detector.go +++ b/internal/detectors/pub/detector.go @@ -198,6 +198,11 @@ func packageNode(name string, pkg pubLockPackage) *sdk.Dependency { if resolved := resolvedURL(pkg.Description); resolved != "" { node.ResolvedURL = resolved } + if pubDependencySource(pkg.Source) == sdk.DependencySourceGit { + // A git package names its repository and the commit pub resolved. + // A hosted package's "url" is the pub server, and path is local. + node.Origin = sdk.RepositoryOrigin(descriptionString(pkg.Description, "url"), descriptionString(pkg.Description, "resolved-ref")) + } return node } diff --git a/internal/detectors/pub/origin_test.go b/internal/detectors/pub/origin_test.go new file mode 100644 index 00000000..0176c470 --- /dev/null +++ b/internal/detectors/pub/origin_test.go @@ -0,0 +1,272 @@ +package pub + +import ( + "os" + "path/filepath" + "testing" + + "github.com/bomly-dev/bomly-sdk" + "go.uber.org/zap" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// A pubspec.lock hosted package's description URL is the pub server, shared by +// every hosted package, and a path package is local. Only a git package names +// where its own code came from. +func TestPubOriginBySourceType(t *testing.T) { + lock := []byte(`packages: + collection: + dependency: transitive + description: + name: collection + sha256: abc + url: "https://pub.dev" + source: hosted + version: "1.18.0" + corp_widgets: + dependency: transitive + description: + name: corp_widgets + sha256: def + url: "https://dart.corp/internal/feed" + source: hosted + version: "3.1.0" + helper: + dependency: "direct main" + description: + url: "https://github.com/example/helper.git" + ref: main + resolved-ref: a3b4c5d6e7f8091a2b3c4d5e6f70819213243546 + path: "." + source: git + version: "2.0.0" + local_tools: + dependency: "direct dev" + description: + path: "../local_tools" + relative: true + source: path + version: "0.1.0" +`) + manifest := pubspec{ + Name: "demo", + Version: "1.0.0", + Dependencies: map[string]any{"helper": "any"}, + DevDependencies: map[string]any{"local_tools": "any"}, + } + + graph, err := depGraphFromLock(lock, manifest) + if err != nil { + t.Fatalf("depGraphFromLock() error = %v", err) + } + + cases := []struct { + id string + want sdk.PackageOrigin + }{ + {id: "collection@1.18.0"}, + // A self-hosted pub server's URL has a path, so nothing but the + // source kind distinguishes it from a repository URL. + {id: "corp_widgets@3.1.0"}, + {id: "helper@2.0.0", want: sdk.PackageOrigin{ + Repository: "https://github.com/example/helper.git", + Revision: "a3b4c5d6e7f8091a2b3c4d5e6f70819213243546", + }}, + {id: "local_tools@0.1.0"}, + } + for _, tc := range cases { + node, ok := graph.Node(tc.id) + if !ok { + t.Fatalf("expected %s in graph", tc.id) + } + if got := originOf(node); got != tc.want { + t.Errorf("%s origin = %+v, want %+v", tc.id, got, tc.want) + } + } +} + +// `dart pub deps --json` reports a name, version, and kind but not a package's +// source description, so the native path alone would export no origin for git +// dependencies. The descriptions are read back from pubspec.lock. +func TestPubNativeOriginIsReadFromPubspecLock(t *testing.T) { + workingDir := t.TempDir() + lock := `packages: + helper: + dependency: "direct main" + description: + url: "https://github.com/example/helper.git" + ref: main + resolved-ref: 1a2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d + path: "." + source: git + version: "2.0.0" + collection: + dependency: transitive + description: + name: collection + sha256: abc + url: "https://pub.dev" + source: hosted + version: "1.18.0" + local_tools: + dependency: "direct dev" + description: + path: "../local_tools" + relative: true + source: path + version: "0.1.0" +` + if err := os.WriteFile(filepath.Join(workingDir, "pubspec.lock"), []byte(lock), 0o644); err != nil { + t.Fatal(err) + } + + // `dart pub deps --json` reports a name, version, kind, and source per + // package, and no source description at all. + depsJSON := []byte(`{ + "root": "demo", + "packages": [ + {"name": "demo", "version": "1.0.0", "kind": "root", "source": "root", "dependencies": ["helper", "collection", "local_tools"]}, + {"name": "helper", "version": "2.0.0", "kind": "direct", "source": "git", "dependencies": []}, + {"name": "collection", "version": "1.18.0", "kind": "transitive", "source": "hosted", "dependencies": []}, + {"name": "local_tools", "version": "0.1.0", "kind": "dev", "source": "path", "dependencies": []} + ] + }`) + + g, err := nativeGraph(depsJSON, workingDir, zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + want := sdk.PackageOrigin{ + Repository: "https://github.com/example/helper.git", + Revision: "1a2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d", + } + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + origin := originOf(dep) + switch dep.Name { + case "helper": + checked++ + if origin != want { + t.Errorf("helper origin = %+v, want %+v", origin, want) + } + case "collection", "local_tools": + checked++ + if !origin.Empty() { + t.Errorf("%s asserted an origin: %+v", dep.Name, origin) + } + } + return true + }) + if checked != 3 { + t.Fatalf("checked %d packages, want 3", checked) + } +} + +// An override can point a package at a local path while pubspec.lock still +// describes the git dependency it replaced. The built code is local, so it must +// not be credited to that repository. +func TestPubOverriddenPackageIsNotCreditedToTheLockedRepository(t *testing.T) { + workingDir := t.TempDir() + lock := `packages: + helper: + dependency: "direct main" + description: + url: "https://github.com/example/helper.git" + ref: main + resolved-ref: 3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f + path: "." + source: git + version: "2.0.0" +` + if err := os.WriteFile(filepath.Join(workingDir, "pubspec.lock"), []byte(lock), 0o644); err != nil { + t.Fatal(err) + } + depsJSON := []byte(`{ + "root": "demo", + "packages": [ + {"name": "demo", "version": "1.0.0", "kind": "root", "source": "root", "dependencies": ["helper"]}, + {"name": "helper", "version": "2.0.0", "kind": "direct", "source": "path", "dependencies": []} + ] + }`) + + g, err := nativeGraph(depsJSON, workingDir, zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name != "helper" { + return true + } + checked++ + if got := originOf(dep); !got.Empty() { + t.Fatalf("overridden package origin = %+v, want none", got) + } + return true + }) + if checked != 1 { + t.Fatal("expected the overridden package in the graph") + } +} + +// A project with no pubspec.lock keeps the graph as it is. +func TestPubNativeOriginSurvivesMissingLock(t *testing.T) { + depsJSON := []byte(`{ + "root": "demo", + "packages": [ + {"name": "demo", "version": "1.0.0", "kind": "root", "source": "root", "dependencies": ["helper"]}, + {"name": "helper", "version": "2.0.0", "kind": "direct", "source": "git", "dependencies": []} + ] + }`) + + g, err := nativeGraph(depsJSON, t.TempDir(), zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name == "helper" { + checked++ + } + if got := originOf(dep); !got.Empty() { + t.Fatalf("%s origin = %+v, want none", dep.Name, got) + } + return true + }) + if checked != 1 { + t.Fatalf("found %d helper nodes, want 1", checked) + } +} + +// Loggers may be nil; a best-effort join must not be the thing that panics. +func TestPubNativeOriginToleratesNilLogger(t *testing.T) { + workingDir := t.TempDir() + depsJSON := []byte(`{"root":"demo","packages":[{"name":"demo","version":"1.0.0","kind":"root","source":"root","dependencies":[]}]}`) + if err := os.WriteFile(filepath.Join(workingDir, "pubspec.lock"), []byte("packages:\n helper:\n source: git\n"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := nativeGraph(depsJSON, workingDir, nil); err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + // An unparseable lock reaches the debug log, which is where a nil logger bites. + if err := os.WriteFile(filepath.Join(workingDir, "pubspec.lock"), []byte("packages: [oops"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := nativeGraph(depsJSON, workingDir, nil); err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } +} diff --git a/internal/detectors/pub/pub_native.go b/internal/detectors/pub/pub_native.go index 160c18cc..9d555d67 100644 --- a/internal/detectors/pub/pub_native.go +++ b/internal/detectors/pub/pub_native.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "path/filepath" "time" "github.com/bomly-dev/bomly-cli/internal/detectors" @@ -14,6 +15,7 @@ import ( logkit "github.com/bomly-dev/bomly-sdk/logkit" "github.com/bomly-dev/bomly-sdk/system" "go.uber.org/zap" + "gopkg.in/yaml.v3" ) // NativeDetector resolves Dart pub dependency graphs by running `dart pub deps --json`. @@ -75,7 +77,7 @@ func (d NativeDetector) ResolveGraph(_ context.Context, req sdk.DetectionRequest return sdk.DetectionResult{}, fmt.Errorf("dart pub deps: %w", err) } - g, err := depGraphFromPubDepsJSON(out.Bytes()) + g, err := nativeGraph(out.Bytes(), workingDir, logger) if err != nil { return sdk.DetectionResult{}, fmt.Errorf("parse dart pub deps output: %w", err) } @@ -85,6 +87,61 @@ func (d NativeDetector) ResolveGraph(_ context.Context, req sdk.DetectionRequest }, nil } +// nativeGraph builds the dependency graph for a native pub run: the tool's own +// output for structure, and the committed pubspec.lock for the package sources +// that output omits. +func nativeGraph(raw []byte, workingDir string, logger *zap.Logger) (*sdk.Graph, error) { + g, err := depGraphFromPubDepsJSON(raw) + if err != nil { + return nil, err + } + applyLockOrigins(g, workingDir, logger) + return g, nil +} + +// applyLockOrigins records where a native graph's git packages came from. +// `dart pub deps --json` reports a name, version, and kind but not a package's +// source description, so without this the default path would export no origin +// for git dependencies while the committed-file fallback exports the +// repository and the commit pub resolved. +// +// Best effort: a project with no readable pubspec.lock keeps the graph as it is. +func applyLockOrigins(g *sdk.Graph, workingDir string, logger *zap.Logger) { + if logger == nil { + logger = zap.NewNop() + } + raw, err := system.ReadRepositoryFile(filepath.Join(workingDir, "pubspec.lock")) + if err != nil { + return + } + var lock pubLock + if err := yaml.Unmarshal(raw, &lock); err != nil { + logger.Debug("pub: could not read pubspec.lock for origin", zap.Error(err)) + return + } + if len(lock.Packages) == 0 { + return + } + + recorded := 0 + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Source != sdk.DependencySourceGit { + // An override can point a package at a local path while the lock + // still describes the git dependency it replaced. What pub + // resolved for this build is the truth. + return true + } + pkg, ok := lock.Packages[dep.Name] + if !ok || pubDependencySource(pkg.Source) != sdk.DependencySourceGit { + return true + } + dep.Origin = sdk.RepositoryOrigin(descriptionString(pkg.Description, "url"), descriptionString(pkg.Description, "resolved-ref")) + recorded++ + return true + }) + logger.Debug(fmt.Sprintf("pub: recorded %d package origins from pubspec.lock", recorded)) +} + // FallbackDetector returns the configured fallback detector. func (d NativeDetector) FallbackDetector() sdk.Detector { return d.Fallback diff --git a/internal/detectors/python/common.go b/internal/detectors/python/common.go index cb13691e..71456702 100644 --- a/internal/detectors/python/common.go +++ b/internal/detectors/python/common.go @@ -217,6 +217,7 @@ func depGraphFromPipInspect(raw []byte, rootNode *sdk.Dependency, declared map[s Name: normalizePythonName(pkg.Metadata.Name), Version: pkg.Metadata.Version}, Source: pipInspectDependencySource(pkg.DirectURL), ResolvedURL: pipInspectResolvedURL(pkg.DirectURL), Metadata: sourceRevisionMetadata(pipInspectRevision(pkg.DirectURL)), }) + setPipInspectOrigin(node, pkg.DirectURL) if _, exists := nodesByName[node.Name]; !exists { nodesByName[node.Name] = node diff --git a/internal/detectors/python/origin.go b/internal/detectors/python/origin.go new file mode 100644 index 00000000..55f362d2 --- /dev/null +++ b/internal/detectors/python/origin.go @@ -0,0 +1,71 @@ +package python + +import ( + "strings" + + "github.com/bomly-dev/bomly-sdk" +) + +// Python lockfiles name a package's source explicitly, so each resolver can say +// what it resolved rather than leaving the shape of a URL to be guessed at. +// Index roots (PyPI, a private mirror) and local checkouts assert nothing: they +// describe how the environment was built, not where this package came from. + +// setUVOrigin records the origin uv resolved for a package. +func setUVOrigin(node *sdk.Dependency, source uvLockSource) { + switch { + case strings.TrimSpace(source.Git) != "": + // uv writes the resolved commit as the URL fragment and the + // requested ref as a query parameter; uvSourceRevision prefers the + // former, and the invariant drops both from the repository URL. + node.Origin = sdk.RepositoryOrigin(source.Git, uvSourceRevision(source)) + case strings.TrimSpace(source.URL) != "": + node.Origin = sdk.ArtifactOrigin(source.URL) + } +} + +// setPoetryOrigin records the origin poetry resolved for a package. +func setPoetryOrigin(node *sdk.Dependency, pkg *poetryLockPackage) { + switch strings.ToLower(strings.TrimSpace(pkg.Source.Type)) { + case "git": + // ResolvedReference is the commit poetry locked; Reference is the + // branch or tag that was asked for. + node.Origin = sdk.RepositoryOrigin(pkg.Source.URL, firstNonEmpty(pkg.Source.ResolvedReference, pkg.Source.Reference)) + case "url": + node.Origin = sdk.ArtifactOrigin(pkg.Source.URL) + } +} + +// setPipenvOrigin records the origin pipenv resolved for a package. +func setPipenvOrigin(node *sdk.Dependency, pkg pipfileLockPackage) { + switch { + case strings.TrimSpace(pkg.Git) != "": + node.Origin = sdk.RepositoryOrigin(pkg.Git, pkg.Ref) + case strings.TrimSpace(pkg.File) != "": + // "file" holds a remote archive for URL requirements and a local + // path for file:// ones; the invariant keeps only the former. + node.Origin = sdk.ArtifactOrigin(pkg.File) + } +} + +// setPipInspectOrigin records the origin recorded in an installed package's +// PEP 610 direct_url.json, which pip writes only for packages installed from a +// repository, an archive URL, or a local directory. +func setPipInspectOrigin(node *sdk.Dependency, directURL map[string]any) { + resolved := pipInspectResolvedURL(directURL) + if resolved == "" { + return + } + if vcsInfo, ok := directURL["vcs_info"].(map[string]any); ok { + vcs, _ := vcsInfo["vcs"].(string) + if strings.EqualFold(strings.TrimSpace(vcs), "git") { + node.Origin = sdk.RepositoryOrigin(resolved, pipInspectRevision(directURL)) + } + // Mercurial, Subversion, and Bazaar have no locator form here. + return + } + if _, ok := directURL["archive_info"]; ok { + node.Origin = sdk.ArtifactOrigin(resolved) + } + // dir_info marks a local directory install. +} diff --git a/internal/detectors/python/origin_test.go b/internal/detectors/python/origin_test.go new file mode 100644 index 00000000..689dc347 --- /dev/null +++ b/internal/detectors/python/origin_test.go @@ -0,0 +1,225 @@ +package python + +import ( + "os" + "path/filepath" + "testing" + + "github.com/bomly-dev/bomly-sdk" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// requireOrigin asserts the exact origin a named package asserts. +func requireOrigin(t *testing.T, graph *sdk.Graph, id string, want sdk.PackageOrigin) { + t.Helper() + node, ok := graph.Node(id) + if !ok { + t.Fatalf("expected %s in graph", id) + } + if got := originOf(node); got != want { + t.Errorf("%s origin = %+v, want %+v", id, got, want) + } +} + +func TestUVLockOriginBySourceType(t *testing.T) { + dir := t.TempDir() + lock := ` +version = 1 + +[[package]] +name = "project" +version = "0.1.0" +source = { editable = "." } + +[[package]] +name = "from-git" +version = "1.0.0" +source = { git = "https://github.com/example/from-git?rev=main#9f8e7d6c5b4a3928176554433221100ffeeddcc0" } + +[[package]] +name = "from-url" +version = "2.0.0" +source = { url = "https://files.pythonhosted.org/packages/ab/from_url-2.0.0-py3-none-any.whl" } + +[[package]] +name = "from-registry" +version = "3.0.0" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "from-path" +version = "4.0.0" +source = { path = "../vendor/from-path" } +` + path := filepath.Join(dir, "uv.lock") + if err := os.WriteFile(path, []byte(lock), 0o644); err != nil { + t.Fatal(err) + } + + graph, err := depGraphFromUVLock(path) + if err != nil { + t.Fatalf("depGraphFromUVLock() error = %v", err) + } + + // The fragment carries the commit uv resolved; the "rev" query carries + // what the manifest asked for. + requireOrigin(t, graph, "from-git@1.0.0", sdk.PackageOrigin{ + Repository: "https://github.com/example/from-git", + Revision: "9f8e7d6c5b4a3928176554433221100ffeeddcc0", + }) + requireOrigin(t, graph, "from-url@2.0.0", sdk.PackageOrigin{ + ArtifactURL: "https://files.pythonhosted.org/packages/ab/from_url-2.0.0-py3-none-any.whl", + }) + // An index root is not this package's origin, and a path is local. + requireOrigin(t, graph, "from-registry@3.0.0", sdk.PackageOrigin{}) + requireOrigin(t, graph, "from-path@4.0.0", sdk.PackageOrigin{}) +} + +func TestPoetryLockOriginBySourceType(t *testing.T) { + dir := t.TempDir() + lock := ` +[[package]] +name = "from-pypi" +version = "1.0.0" + +[[package]] +name = "from-git" +version = "2.0.0" +[package.source] +type = "git" +url = "https://github.com/example/from-git.git" +reference = "main" +resolved_reference = "0a1b2c3d4e5f60718293a4b5c6d7e8f901234567" + +[[package]] +name = "from-url" +version = "3.0.0" +[package.source] +type = "url" +url = "https://files.pythonhosted.org/packages/cd/from_url-3.0.0.tar.gz" + +[[package]] +name = "from-private-index" +version = "4.0.0" +[package.source] +type = "legacy" +url = "https://pypi.example.test/simple" +reference = "internal" + +[[package]] +name = "from-directory" +version = "5.0.0" +[package.source] +type = "directory" +url = "../vendor/from-directory" +` + lockPath := filepath.Join(dir, "poetry.lock") + if err := os.WriteFile(lockPath, []byte(lock), 0o644); err != nil { + t.Fatal(err) + } + + graph, err := depGraphFromPoetryLock(lockPath, dir) + if err != nil { + t.Fatalf("depGraphFromPoetryLock() error = %v", err) + } + + requireOrigin(t, graph, "from-pypi@1.0.0", sdk.PackageOrigin{}) + // resolved_reference is the commit poetry locked; reference is the branch. + requireOrigin(t, graph, "from-git@2.0.0", sdk.PackageOrigin{ + Repository: "https://github.com/example/from-git.git", + Revision: "0a1b2c3d4e5f60718293a4b5c6d7e8f901234567", + }) + requireOrigin(t, graph, "from-url@3.0.0", sdk.PackageOrigin{ + ArtifactURL: "https://files.pythonhosted.org/packages/cd/from_url-3.0.0.tar.gz", + }) + requireOrigin(t, graph, "from-private-index@4.0.0", sdk.PackageOrigin{}) + requireOrigin(t, graph, "from-directory@5.0.0", sdk.PackageOrigin{}) +} + +func TestPipfileLockOriginBySourceType(t *testing.T) { + dir := t.TempDir() + lock := `{ + "default": { + "from-pypi": {"version": "==1.0.0", "index": "pypi"}, + "from-git": {"git": "https://github.com/example/from-git.git", "ref": "1f2e3d4c5b6a79880912a3b4c5d6e7f809172635"}, + "from-archive": {"file": "https://files.pythonhosted.org/packages/ef/from_archive-2.0.0.tar.gz"}, + "from-local": {"file": "file:///workspace/wheels/from_local-3.0.0.whl"}, + "from-path": {"path": "../vendor/from-path"} + }, + "develop": {} + }` + path := filepath.Join(dir, "Pipfile.lock") + if err := os.WriteFile(path, []byte(lock), 0o644); err != nil { + t.Fatal(err) + } + + graph, err := depGraphFromPipfileLock(path, "demo") + if err != nil { + t.Fatalf("depGraphFromPipfileLock() error = %v", err) + } + + requireOrigin(t, graph, "from-pypi@1.0.0", sdk.PackageOrigin{}) + requireOrigin(t, graph, "from-git", sdk.PackageOrigin{ + Repository: "https://github.com/example/from-git.git", + Revision: "1f2e3d4c5b6a79880912a3b4c5d6e7f809172635", + }) + requireOrigin(t, graph, "from-archive", sdk.PackageOrigin{ + ArtifactURL: "https://files.pythonhosted.org/packages/ef/from_archive-2.0.0.tar.gz", + }) + requireOrigin(t, graph, "from-local", sdk.PackageOrigin{}) + requireOrigin(t, graph, "from-path", sdk.PackageOrigin{}) +} + +// pip records a PEP 610 direct_url.json for anything not installed from an +// index, distinguishing repositories, archives, and local directories. +func TestPipInspectOriginByDirectURLShape(t *testing.T) { + cases := []struct { + name string + directURL map[string]any + want sdk.PackageOrigin + }{ + {name: "index install", directURL: nil}, + { + name: "git checkout", + directURL: map[string]any{ + "url": "https://github.com/example/pkg.git", + "vcs_info": map[string]any{"vcs": "git", "commit_id": "2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e", "requested_revision": "main"}, + }, + want: sdk.PackageOrigin{Repository: "https://github.com/example/pkg.git", Revision: "2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e"}, + }, + { + name: "archive URL", + directURL: map[string]any{"url": "https://example.test/pkg-1.0.0-py3-none-any.whl", "archive_info": map[string]any{}}, + want: sdk.PackageOrigin{ArtifactURL: "https://example.test/pkg-1.0.0-py3-none-any.whl"}, + }, + { + name: "local directory", + directURL: map[string]any{"url": "file:///workspace/pkg", "dir_info": map[string]any{}}, + }, + { + name: "mercurial checkout", + directURL: map[string]any{"url": "https://hg.example.test/pkg", "vcs_info": map[string]any{"vcs": "hg", "commit_id": "9f8e7d6"}}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + node := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Name: "pkg", Version: "1.0.0"}}) + setPipInspectOrigin(node, tc.directURL) + if got := originOf(node); got != tc.want { + t.Fatalf("origin = %+v, want %+v", got, tc.want) + } + }) + } +} diff --git a/internal/detectors/python/pipenv.go b/internal/detectors/python/pipenv.go index 004928d5..a102f532 100644 --- a/internal/detectors/python/pipenv.go +++ b/internal/detectors/python/pipenv.go @@ -242,6 +242,7 @@ func addPipfileLockPackages(depsGraph *sdk.Graph, root *sdk.Dependency, packages Name: normalizedName, Version: strings.TrimPrefix(pkg.Version, "==")}, Source: pipfileDependencySource(pkg), ResolvedURL: pipfileResolvedURL(pkg), Metadata: sourceRevisionMetadata(pkg.Ref), Scopes: sdk.ScopesOf(scope), }) + setPipenvOrigin(node, pkg) if _, exists := depsGraph.Node(node.ID); !exists { if err := depsGraph.AddNode(node); err != nil { diff --git a/internal/detectors/python/poetrylock.go b/internal/detectors/python/poetrylock.go index f9480b77..a586f62d 100644 --- a/internal/detectors/python/poetrylock.go +++ b/internal/detectors/python/poetrylock.go @@ -87,6 +87,7 @@ func depGraphFromPoetryLock(lockPath, projectPath string) (*sdk.Graph, error) { Type: sdk.PackageTypePackage, PURL: sdk.BuildPackageURL("pypi", "", pkg.Name, pkg.Version)}, Source: poetryDependencySource(pkg.Source.Type), ResolvedURL: strings.TrimSpace(pkg.Source.URL), Metadata: sourceRevisionMetadata(firstNonEmpty(pkg.Source.ResolvedReference, pkg.Source.Reference)), }) + setPoetryOrigin(node, pkg) for _, group := range pkg.Groups { if group == "main" { diff --git a/internal/detectors/python/uvlock.go b/internal/detectors/python/uvlock.go index fa57b835..0a47f350 100644 --- a/internal/detectors/python/uvlock.go +++ b/internal/detectors/python/uvlock.go @@ -71,6 +71,7 @@ func depGraphFromUVLock(uvLockPath string) (*sdk.Graph, error) { Name: normalizePythonName(pkg.Name), Version: pkg.Version}, Source: uvDependencySource(pkg.Source), ResolvedURL: uvResolvedURL(pkg.Source), Metadata: sourceRevisionMetadata(uvSourceRevision(pkg.Source)), }) + setUVOrigin(node, pkg.Source) nodesByName[normalizePythonName(pkg.Name)] = node } diff --git a/internal/detectors/ruby/detector.go b/internal/detectors/ruby/detector.go index e8cee75a..560a2b80 100644 --- a/internal/detectors/ruby/detector.go +++ b/internal/detectors/ruby/detector.go @@ -488,13 +488,19 @@ func gemNode(spec lockSpec) *sdk.Dependency { if revision := strings.TrimSpace(spec.Revision); revision != "" { metadata = map[string]any{"source_revision": revision} } - return sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemRuby, + node := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemRuby, Name: strings.TrimSpace(spec.Name), Version: strings.TrimSpace(spec.Version), PackageManager: sdk.PackageManagerBundler, Type: "gem", Language: "ruby"}, Source: spec.Source, ResolvedURL: strings.TrimSpace(spec.ResolvedURL), Metadata: metadata, }) + if spec.Source == sdk.DependencySourceGit { + // A GIT section names the repository and the commit Bundler locked. + // A GEM section's remote is the gem server, and PATH is local. + node.Origin = sdk.RepositoryOrigin(spec.ResolvedURL, spec.Revision) + } + return node } diff --git a/internal/detectors/ruby/origin_test.go b/internal/detectors/ruby/origin_test.go new file mode 100644 index 00000000..5e550524 --- /dev/null +++ b/internal/detectors/ruby/origin_test.go @@ -0,0 +1,81 @@ +package ruby + +import ( + "github.com/bomly-dev/bomly-sdk" + "testing" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// A Gemfile.lock names its sources by section. GEM's remote is the gem server +// every gem in that section came from, not a per-gem location; PATH is a +// directory on the machine that ran bundle install. Only GIT identifies where +// a specific gem's code came from. +func TestBundlerOriginBySection(t *testing.T) { + raw := []byte(`GEM + remote: https://rubygems.org/ + specs: + rack (3.1.8) + +GEM + remote: https://gems.corp/private/feed/ + specs: + corp-auth (2.4.0) + +GIT + remote: https://github.com/example/helper.git + revision: 708192a3b4c5d6e7f8091a2b3c4d5e6f70819213 + specs: + helper (1.0.0) + +PATH + remote: ../local-gem + specs: + local-gem (0.1.0) + +DEPENDENCIES + corp-auth + helper! + local-gem! + rack +`) + + graph, err := depGraphFromLock(raw, nil) + if err != nil { + t.Fatalf("depGraphFromLock() error = %v", err) + } + + cases := []struct { + id string + want sdk.PackageOrigin + }{ + {id: "rack@3.1.8"}, + // A private gem server's remote has a path, so nothing but the + // section kind distinguishes it from a repository URL. + {id: "corp-auth@2.4.0"}, + {id: "helper@1.0.0", want: sdk.PackageOrigin{ + Repository: "https://github.com/example/helper.git", + Revision: "708192a3b4c5d6e7f8091a2b3c4d5e6f70819213", + }}, + {id: "local-gem@0.1.0"}, + } + for _, tc := range cases { + node, ok := graph.Node(tc.id) + if !ok { + t.Fatalf("expected %s in graph", tc.id) + } + if got := originOf(node); got != tc.want { + t.Errorf("%s origin = %+v, want %+v", tc.id, got, tc.want) + } + } +} diff --git a/internal/detectors/swiftpm/detector.go b/internal/detectors/swiftpm/detector.go index acb00c2d..77f1a520 100644 --- a/internal/detectors/swiftpm/detector.go +++ b/internal/detectors/swiftpm/detector.go @@ -275,6 +275,13 @@ func packageNode(pkg swiftPackage) *sdk.Dependency { Metadata: metadata, }) + if swiftDependencySource(pkg.SourceKind, pkg.Repository) == sdk.DependencySourceGit { + // Source-control pins name the repository and the commit SwiftPM + // resolved. Registry pins are identity-only, and local packages + // point at a checkout on this machine. + node.Origin = sdk.RepositoryOrigin(pkg.Repository, pkg.Revision) + } + // SwiftPM does not distinguish dev scope; all packages are runtime. node.AddScope(sdk.ScopeRuntime) return node diff --git a/internal/detectors/swiftpm/origin_test.go b/internal/detectors/swiftpm/origin_test.go new file mode 100644 index 00000000..031b3ea5 --- /dev/null +++ b/internal/detectors/swiftpm/origin_test.go @@ -0,0 +1,358 @@ +package swiftpm + +import ( + "os" + "path/filepath" + "testing" + + "github.com/bomly-dev/bomly-sdk" + "go.uber.org/zap" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// A Package.resolved pin says how SwiftPM obtained a package. Source-control +// pins name a repository and the commit that was resolved; registry pins are +// identity-only; local pins point at a checkout on this machine. +func TestSwiftPMOriginByPinKind(t *testing.T) { + resolved := []byte(`{ + "pins": [ + { + "identity": "swift-argument-parser", + "kind": "remoteSourceControl", + "location": "https://github.com/apple/swift-argument-parser.git", + "state": {"revision": "8192a3b4c5d6e7f8091a2b3c4d5e6f7081921324", "version": "1.3.0"} + }, + { + "identity": "internal-tools", + "kind": "registry", + "state": {"version": "2.0.0"} + }, + { + "identity": "local-helper", + "kind": "localSourceControl", + "location": "/Users/someone/src/local-helper", + "state": {"revision": "92a3b4c5d6e7f8091a2b3c4d5e6f708192132435", "version": "0.1.0"} + } + ], + "version": 2 + }`) + + graph, err := depGraphFromSwiftPM(resolved, nil) + if err != nil { + t.Fatalf("depGraphFromSwiftPM() error = %v", err) + } + + var checked int + for _, node := range graph.Nodes() { + origin := originOf(node) + switch node.Name { + case "swift-argument-parser": + checked++ + want := sdk.PackageOrigin{ + Repository: "https://github.com/apple/swift-argument-parser.git", + Revision: "8192a3b4c5d6e7f8091a2b3c4d5e6f7081921324", + } + if origin != want { + t.Errorf("%s origin = %+v, want %+v", node.Name, origin, want) + } + case "internal-tools", "local-helper": + checked++ + if !origin.Empty() { + t.Errorf("%s asserted an origin it should not have: %+v", node.Name, origin) + } + } + } + if checked != 3 { + t.Fatalf("checked %d pins, want 3", checked) + } +} + +// `swift package show-dependencies` reports a repository and a version but no +// revision, so the native path alone would export unpinned repositories while +// the committed-file fallback exports pinned ones. The pins are read back from +// Package.resolved and joined onto the graph. +func TestSwiftPMNativeOriginIsPinnedFromPackageResolved(t *testing.T) { + workingDir := t.TempDir() + resolved := `{ + "pins": [ + { + "identity": "swift-argument-parser", + "kind": "remoteSourceControl", + "location": "https://github.com/apple/swift-argument-parser.git", + "state": {"revision": "f8091a2b3c4d5e6f708192a3b4c5d6e7f8091a2b", "version": "1.3.0"} + }, + { + "identity": "local-helper", + "kind": "localSourceControl", + "location": "/Users/someone/src/local-helper", + "state": {"revision": "091a2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c", "version": "0.1.0"} + } + ], + "version": 2 + }` + if err := os.WriteFile(filepath.Join(workingDir, "Package.resolved"), []byte(resolved), 0o644); err != nil { + t.Fatal(err) + } + + // `swift package show-dependencies` reports a URL and a version per node, + // and no revision anywhere. + showDependencies := []byte(`{ + "name": "demo", + "url": "/workspace/demo", + "version": "unspecified", + "dependencies": [ + {"name": "swift-argument-parser", "url": "https://github.com/apple/swift-argument-parser.git", "version": "1.3.0", "dependencies": []}, + {"name": "local-helper", "url": "/Users/someone/src/local-helper", "version": "0.1.0", "dependencies": []} + ] + }`) + + g, err := nativeGraph(showDependencies, workingDir, zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + want := sdk.PackageOrigin{ + Repository: "https://github.com/apple/swift-argument-parser.git", + Revision: "f8091a2b3c4d5e6f708192a3b4c5d6e7f8091a2b", + } + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + origin := originOf(dep) + switch dep.Name { + case "swift-argument-parser": + checked++ + if origin != want { + t.Errorf("remote origin = %+v, want %+v", origin, want) + } + case "local-helper": + checked++ + // A local checkout is a path on this machine, pinned or not. + if !origin.Empty() { + t.Errorf("local package asserted an origin: %+v", origin) + } + } + return true + }) + if checked != 2 { + t.Fatalf("checked %d packages, want 2", checked) + } +} + +// `swift package edit --path ...` points a dependency at a local +// checkout, and Package.resolved keeps the pin that checkout replaced. The +// package being built is the local code, so it must not be credited to the +// remote repository and commit that pin still names. +func TestSwiftPMEditedPackageIsNotCreditedToItsFormerPin(t *testing.T) { + workingDir := t.TempDir() + resolved := `{ + "pins": [ + { + "identity": "helper", + "kind": "remoteSourceControl", + "location": "https://github.com/example/helper.git", + "state": {"revision": "2b3c4d5e6f708192a3b4c5d6e7f8091a2b3c4d5e", "version": "1.0.0"} + } + ], + "version": 2 + }` + if err := os.WriteFile(filepath.Join(workingDir, "Package.resolved"), []byte(resolved), 0o644); err != nil { + t.Fatal(err) + } + showDependencies := []byte(`{ + "name": "demo", + "url": "/workspace/demo", + "version": "unspecified", + "dependencies": [ + {"name": "helper", "url": "/workspace/helper", "version": "unspecified", "dependencies": []} + ] + }`) + + g, err := nativeGraph(showDependencies, workingDir, zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name != "helper" { + return true + } + checked++ + if got := originOf(dep); !got.Empty() { + t.Fatalf("edited package origin = %+v, want none: it is built from a local checkout", got) + } + return true + }) + if checked != 1 { + t.Fatal("expected the edited package in the graph") + } +} + +// A package can be built from a mirror while Package.resolved still pins the +// upstream host. The names match, so an identity fallback would credit the +// build to a repository it never fetched from. +func TestSwiftPMPinIsNotAttachedToADifferentRepository(t *testing.T) { + workingDir := t.TempDir() + resolved := `{ + "pins": [ + { + "identity": "helper", + "kind": "remoteSourceControl", + "location": "https://git.corp/team/helper.git", + "state": {"revision": "aaaabbbbccccddddeeeeffff0000111122223333", "version": "1.0.0"} + } + ], + "version": 2 + }` + if err := os.WriteFile(filepath.Join(workingDir, "Package.resolved"), []byte(resolved), 0o644); err != nil { + t.Fatal(err) + } + showDependencies := []byte(`{ + "name": "demo", + "url": "/workspace/demo", + "version": "unspecified", + "dependencies": [ + {"name": "helper", "url": "https://mirror.corp/team/helper.git", "version": "2.0.0", "dependencies": []} + ] + }`) + + g, err := nativeGraph(showDependencies, workingDir, zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.ResolvedURL == "" { + return true + } + checked++ + origin := originOf(dep) + if origin.Repository == "https://git.corp/team/helper.git" { + t.Fatalf("a package built from a mirror was credited to %q", origin.Repository) + } + if origin.Revision != "" { + t.Fatalf("origin = %+v, want no pin: the pinned repository is not this one", origin) + } + return true + }) + if checked != 1 { + t.Fatalf("checked %d packages, want 1", checked) + } +} + +// A project with no Package.resolved keeps whatever the native graph carried. +func TestSwiftPMNativeOriginSurvivesMissingPackageResolved(t *testing.T) { + showDependencies := []byte(`{ + "name": "demo", + "url": "/workspace/demo", + "version": "unspecified", + "dependencies": [ + {"name": "swift-argument-parser", "url": "https://github.com/apple/swift-argument-parser.git", "version": "1.3.0", "dependencies": []} + ] + }`) + + g, err := nativeGraph(showDependencies, t.TempDir(), zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + + want := sdk.PackageOrigin{Repository: "https://github.com/apple/swift-argument-parser.git"} + var checked int + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name == "swift-argument-parser" { + checked++ + if got := originOf(dep); got != want { + t.Fatalf("origin = %+v, want the unpinned repository %+v", got, want) + } + } + return true + }) + if checked != 1 { + t.Fatal("expected the remote package in the graph") + } +} + +// Loggers may be nil; a best-effort join must not be the thing that panics. +func TestSwiftPMNativeOriginToleratesNilLogger(t *testing.T) { + workingDir := t.TempDir() + if err := os.WriteFile(filepath.Join(workingDir, "Package.resolved"), []byte(`{"pins":[],"version":2}`), 0o644); err != nil { + t.Fatal(err) + } + if _, err := nativeGraph([]byte(`{"name":"demo","url":"/w","version":"unspecified","dependencies":[]}`), workingDir, nil); err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + // An unparseable file reaches the debug log, which is where a nil logger bites. + if err := os.WriteFile(filepath.Join(workingDir, "Package.resolved"), []byte("{not json"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := nativeGraph([]byte(`{"name":"demo","url":"/w","version":"unspecified","dependencies":[]}`), workingDir, nil); err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } +} + +// Repository paths are case-sensitive on a self-hosted host, so two packages +// differing only in path case are different repositories. Folding their keys +// together would attach one repository's pin to the other's package. +func TestSwiftPMRepositoryKeyPreservesPathCase(t *testing.T) { + if repositoryKey("https://git.corp/Team/Helper.git") == repositoryKey("https://git.corp/team/helper.git") { + t.Fatal("two repositories differing only in path case share a lookup key") + } + // Scheme and host are case-insensitive, so those spellings are one key. + if repositoryKey("HTTPS://Git.Corp/Team/Helper.git") != repositoryKey("https://git.corp/Team/Helper.git") { + t.Fatal("host casing should not change the key") + } + // The suffix and trailing slash still normalize away. + if repositoryKey("https://git.corp/Team/Helper/") != repositoryKey("https://git.corp/Team/Helper.git") { + t.Fatal("a trailing slash or .git suffix should not change the key") + } +} + +// A pin for a differently-cased path must not be attached to this package. +func TestSwiftPMNativeOriginDoesNotMatchAcrossPathCase(t *testing.T) { + workingDir := t.TempDir() + resolved := `{ + "pins": [ + { + "identity": "other-helper", + "kind": "remoteSourceControl", + "location": "https://git.corp/team/helper.git", + "state": {"revision": "5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f7081", "version": "1.0.0"} + } + ], + "version": 2 + }` + if err := os.WriteFile(filepath.Join(workingDir, "Package.resolved"), []byte(resolved), 0o644); err != nil { + t.Fatal(err) + } + showDependencies := []byte(`{ + "name": "demo", + "url": "/workspace/demo", + "version": "unspecified", + "dependencies": [ + {"name": "Helper", "url": "https://git.corp/Team/Helper.git", "version": "2.0.0", "dependencies": []} + ] + }`) + + g, err := nativeGraph(showDependencies, workingDir, zap.NewNop()) + if err != nil { + t.Fatalf("nativeGraph() error = %v", err) + } + g.WalkNodes(func(dep *sdk.Dependency) bool { + if origin := originOf(dep); origin.Revision == "5e6f708192a3b4c5d6e7f8091a2b3c4d5e6f7081" { + t.Fatalf("%s took a pin belonging to a differently-cased repository: %+v", dep.Name, origin) + } + return true + }) +} diff --git a/internal/detectors/swiftpm/swiftpm_native.go b/internal/detectors/swiftpm/swiftpm_native.go index eeefcdd6..ae44b6a8 100644 --- a/internal/detectors/swiftpm/swiftpm_native.go +++ b/internal/detectors/swiftpm/swiftpm_native.go @@ -6,6 +6,8 @@ import ( "encoding/json" "fmt" "io" + "net/url" + "strings" "time" "github.com/bomly-dev/bomly-cli/internal/detectors" @@ -75,7 +77,7 @@ func (d NativeDetector) ResolveGraph(_ context.Context, req sdk.DetectionRequest return sdk.DetectionResult{}, fmt.Errorf("swift package show-dependencies: %w", err) } - g, err := depGraphFromSwiftShowDeps(out.Bytes()) + g, err := nativeGraph(out.Bytes(), workingDir, logger) if err != nil { return sdk.DetectionResult{}, fmt.Errorf("parse swift show-dependencies output: %w", err) } @@ -85,6 +87,103 @@ func (d NativeDetector) ResolveGraph(_ context.Context, req sdk.DetectionRequest }, nil } +// nativeGraph builds the dependency graph for a native SwiftPM run: the tool's +// own output for structure, and the committed Package.resolved for the commits +// that output omits. +func nativeGraph(raw []byte, workingDir string, logger *zap.Logger) (*sdk.Graph, error) { + g, err := depGraphFromSwiftShowDeps(raw) + if err != nil { + return nil, err + } + applyResolvedOrigins(g, workingDir, logger) + return g, nil +} + +// applyResolvedOrigins pins the repositories in a native graph to the commits +// Package.resolved recorded. `swift package show-dependencies` reports a URL +// and a version but no revision, so without this the default path would export +// unpinned repositories while the committed-file fallback exports pinned ones. +// +// Best effort: a project with no readable Package.resolved keeps the origins +// the graph already carries. +func applyResolvedOrigins(g *sdk.Graph, workingDir string, logger *zap.Logger) { + if logger == nil { + logger = zap.NewNop() + } + raw, path, err := readFirstExisting(workingDir, []string{"Package.resolved", ".package.resolved", "project.xcworkspace/xcshareddata/swiftpm/Package.resolved"}) + if err != nil || len(raw) == 0 { + return + } + pins, err := parseResolved(raw) + if err != nil { + logger.Debug("swiftpm: could not read pins for origin", zap.String("path", path), zap.Error(err)) + return + } + if len(pins) == 0 { + return + } + + byRepository := make(map[string]swiftPackage, len(pins)) + for _, pin := range pins { + if key := repositoryKey(pin.Repository); key != "" { + byRepository[key] = pin + } + } + + pinned := 0 + g.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Source != sdk.DependencySourceGit { + // `swift package edit` replaces a dependency with a local + // checkout while Package.resolved keeps the pin it replaced. + // What the build resolved is the truth, so a local node is left + // alone rather than credited to the repository it stands in for. + return true + } + pin, ok := byRepository[repositoryKey(dep.ResolvedURL)] + if !ok { + // Matching by identity is only safe when the graph offers nothing + // better. A node that names a repository and did not match one has + // a repository the pins do not describe -- a mirror, say -- and + // crediting it to a same-named pin would name a location the build + // did not use. + if strings.TrimSpace(dep.ResolvedURL) != "" { + return true + } + if pin, ok = pins[dep.Name]; !ok { + return true + } + } + if pin.Revision == "" || swiftDependencySource(pin.SourceKind, pin.Repository) != sdk.DependencySourceGit { + return true + } + dep.Origin = sdk.RepositoryOrigin(pin.Repository, pin.Revision) + pinned++ + return true + }) + logger.Debug(fmt.Sprintf("swiftpm: pinned %d package origins from %s", pinned, path)) +} + +// repositoryKey normalizes a repository URL for matching a pin to a graph +// node: SwiftPM reports the same repository with and without a ".git" suffix +// and in either case. +func repositoryKey(repository string) string { + trimmed := strings.TrimSpace(repository) + if trimmed == "" { + return "" + } + parsed, err := url.Parse(trimmed) + if err != nil || parsed.Host == "" { + return strings.TrimSuffix(strings.TrimSuffix(trimmed, "/"), ".git") + } + parsed.Scheme = strings.ToLower(parsed.Scheme) + parsed.Host = strings.ToLower(parsed.Host) + // The path keeps its case: on a case-sensitive host "/Team/Helper" and + // "/team/helper" are different repositories, and folding them together + // would attach one repository's pin to the other's package. + parsed.Path = strings.TrimSuffix(strings.TrimSuffix(parsed.Path, "/"), ".git") + return parsed.String() +} + // FallbackDetector returns the configured fallback detector. func (d NativeDetector) FallbackDetector() sdk.Detector { return d.Fallback diff --git a/internal/engine/consolidation/enrichment.go b/internal/engine/consolidation/enrichment.go index fa572b4d..09c3c4fb 100644 --- a/internal/engine/consolidation/enrichment.go +++ b/internal/engine/consolidation/enrichment.go @@ -33,10 +33,16 @@ func normalizeGraphPackageIdentity(src *sdk.Graph) (*sdk.Graph, error) { if clone.ID == "" { return nil, fmt.Errorf("dependency %q has no canonical identity", node.QualifiedName()) } - if _, exists := normalized.Node(clone.ID); !exists { + if existing, exists := normalized.Node(clone.ID); !exists { if err := normalized.AddNode(clone); err != nil { return nil, fmt.Errorf("add normalized dependency %q: %w", clone.ID, err) } + } else { + // Two nodes normalizing to one identity are one package. Only one + // survives, so the discarded occurrence still gets a say about + // where the package came from -- otherwise a lockfile recording + // one package from two mirrors publishes whichever came first. + existing.Origin = sdk.ReconcileOrigin(existing.Origin, clone.Origin) } idMapping[node.ID] = clone.ID } diff --git a/internal/engine/consolidation/origin_test.go b/internal/engine/consolidation/origin_test.go new file mode 100644 index 00000000..070d64f5 --- /dev/null +++ b/internal/engine/consolidation/origin_test.go @@ -0,0 +1,200 @@ +package consolidation + +import ( + "fmt" + "testing" + + "github.com/bomly-dev/bomly-sdk" +) + +// originOf returns the origin a node publishes, or the zero value when it has +// none, so cases can compare plain structs. +func originOf(dep *sdk.Dependency) sdk.PackageOrigin { + if dep == nil { + return sdk.PackageOrigin{} + } + if origin := dep.Origin.Normalized(); origin != nil { + return *origin + } + return sdk.PackageOrigin{} +} + +// subprojectResult builds one manifest's detection result carrying a single +// package whose origin the caller chooses. +func subprojectResult(t *testing.T, relativePath, manifest, artifactURL string) sdk.DetectionResult { + t.Helper() + + g := sdk.New() + pkg := sdk.NewDependencyWithID("lodash@4.17.21", sdk.Dependency{Coordinates: sdk.Coordinates{ + Name: "lodash", Version: "4.17.21", Ecosystem: sdk.EcosystemNPM, PURL: "pkg:npm/lodash@4.17.21"}}) + if artifactURL != "" { + pkg.Origin = sdk.ArtifactOrigin(artifactURL) + } + if err := g.AddNode(pkg); err != nil { + t.Fatal(err) + } + return sdk.DetectionResult{ + SubprojectInfo: sdk.Subproject{ + ExecutionTarget: sdk.ExecutionTarget{Kind: sdk.ExecutionTargetWorkingDirectory, Location: "/repo"}, + RelativePath: relativePath, + PrimaryDetector: "npm-detector", + DetectedPackageManagers: []sdk.PackageManager{sdk.PackageManagerNPM}, + Ecosystem: sdk.EcosystemNPM, + }, + DetectorName: "npm-detector", + Graphs: sdk.SingleGraphContainer(g, sdk.ManifestMetadata{Path: manifest, Kind: "package-lock.json"}), + } +} + +// graphIDs lists node ids for failure messages. +func graphIDs(g *sdk.Graph) []string { + var ids []string + g.WalkNodes(func(dep *sdk.Dependency) bool { + ids = append(ids, dep.ID) + return true + }) + return ids +} + +// Each manifest is resolved on its own, so a package two subprojects share +// arrives as two nodes. Merging keeps one and drops the other, so the +// disagreement has to be settled while both are still visible. +func TestConsolidateGraphsSettlesOriginAcrossManifests(t *testing.T) { + const ( + public = "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz" + private = "https://npm.corp/mirror/lodash/-/lodash-4.17.21.tgz" + ) + + cases := []struct { + name string + left string + right string + want sdk.PackageOrigin + }{ + { + name: "subprojects agree", + left: public, + right: public, + want: sdk.PackageOrigin{ArtifactURL: public}, + }, + { + name: "one subproject resolved a private mirror", + left: public, + right: private, + }, + { + name: "one subproject recorded nothing", + left: public, + right: "", + want: sdk.PackageOrigin{ArtifactURL: public}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + consolidated, err := ConsolidateGraphs([]sdk.DetectionResult{ + subprojectResult(t, "apps/web", "apps/web/package-lock.json", tc.left), + subprojectResult(t, "services/api", "services/api/package-lock.json", tc.right), + }) + if err != nil { + t.Fatalf("ConsolidateGraphs() error = %v", err) + } + + merged, err := consolidated.Graphs.ConsolidatedGraph() + if err != nil { + t.Fatalf("ConsolidatedGraph() error = %v", err) + } + var node *sdk.Dependency + merged.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name == "lodash" { + node = dep + } + return true + }) + if node == nil { + t.Fatalf("expected lodash in the merged graph; ids present: %v", graphIDs(merged)) + } + if got := originOf(node); got != tc.want { + t.Fatalf("merged origin = %+v, want %+v", got, tc.want) + } + }) + } +} + +// One manifest can record a package twice with different locations -- a Bun +// lockfile listing one name and version from two mirrors. Both nodes normalize +// to one canonical identity and only one survives, so the disagreement has to +// be settled while both are still there. This is the single-manifest case, +// which never reaches cross-entry reconciliation. +func TestConsolidateGraphsSettlesOriginWithinOneManifest(t *testing.T) { + const ( + public = "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz" + private = "https://npm.corp/mirror/lodash/-/lodash-4.17.21.tgz" + ) + + cases := []struct { + name string + left string + right string + want sdk.PackageOrigin + }{ + {name: "entries agree", left: public, right: public, want: sdk.PackageOrigin{ArtifactURL: public}}, + {name: "entries disagree", left: public, right: private}, + {name: "one entry says nothing", left: public, right: "", want: sdk.PackageOrigin{ArtifactURL: public}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Distinct node IDs, one canonical PURL: what a lockfile parser + // produces when it disambiguates a duplicate package key. + g := sdk.New() + for i, artifactURL := range []string{tc.left, tc.right} { + pkg := sdk.NewDependencyWithID( + fmt.Sprintf("bun-package:lodash@4.17.21#%d", i), + sdk.Dependency{Coordinates: sdk.Coordinates{ + Name: "lodash", Version: "4.17.21", Ecosystem: sdk.EcosystemNPM, PURL: "pkg:npm/lodash@4.17.21"}}, + ) + if artifactURL != "" { + pkg.Origin = sdk.ArtifactOrigin(artifactURL) + } + if err := g.AddNode(pkg); err != nil { + t.Fatal(err) + } + } + + consolidated, err := ConsolidateGraphs([]sdk.DetectionResult{{ + SubprojectInfo: sdk.Subproject{ + ExecutionTarget: sdk.ExecutionTarget{Kind: sdk.ExecutionTargetWorkingDirectory, Location: "/repo"}, + RelativePath: ".", + PrimaryDetector: "bun-detector", + DetectedPackageManagers: []sdk.PackageManager{sdk.PackageManagerBun}, + Ecosystem: sdk.EcosystemNPM, + }, + DetectorName: "bun-detector", + Graphs: sdk.SingleGraphContainer(g, sdk.ManifestMetadata{Path: "bun.lock", Kind: "bun.lock"}), + }}) + if err != nil { + t.Fatalf("ConsolidateGraphs() error = %v", err) + } + + merged, err := consolidated.Graphs.ConsolidatedGraph() + if err != nil { + t.Fatalf("ConsolidatedGraph() error = %v", err) + } + var checked int + merged.WalkNodes(func(dep *sdk.Dependency) bool { + if dep.Name != "lodash" { + return true + } + checked++ + if got := originOf(dep); got != tc.want { + t.Fatalf("origin = %+v, want %+v", got, tc.want) + } + return true + }) + if checked != 1 { + t.Fatalf("found %d lodash nodes, want 1", checked) + } + }) + } +} diff --git a/internal/output/types.go b/internal/output/types.go index 2dbfcd06..fce71f41 100644 --- a/internal/output/types.go +++ b/internal/output/types.go @@ -262,6 +262,7 @@ func cloneAffectedSymbols(src []sdk.AffectedSymbol) []sdk.AffectedSymbol { return out } +// cloneRefMetadata copies package metadata for command output. func cloneRefMetadata(src map[string]any) map[string]any { if len(src) == 0 { return nil diff --git a/internal/sbom/cyclonedx.go b/internal/sbom/cyclonedx.go index 27fbe393..c33f7f03 100644 --- a/internal/sbom/cyclonedx.go +++ b/internal/sbom/cyclonedx.go @@ -47,6 +47,9 @@ func (c cycloneDXCodec) encodeJSON(doc *Document, opts EncodeOptions) ([]byte, e if props := cycloneDXEOLProperties(comp.EOL); len(props) > 0 { component.Properties = &props } + if refs := cycloneDXComponentReferences(comp); len(refs) > 0 { + component.ExternalReferences = &refs + } components = append(components, component) } bom.Components = &components @@ -248,6 +251,22 @@ func cycloneDXTools(names []string, primaryTool, toolVersion string) *cdx.ToolsC // cycloneDXSecurityReferences maps provenance contact fields onto external // references attached to the primary component. +// cycloneDXComponentReferences renders where a package came from: the exact +// file it was fetched from as a distribution reference, or the repository it +// was resolved from as a vcs reference. The repository is rendered as a plain +// URL -- CycloneDX external references carry no revision, so the commit a +// detector resolved is only expressible in the SPDX locator form. +func cycloneDXComponentReferences(component Component) []cdx.ExternalReference { + refs := make([]cdx.ExternalReference, 0, 2) + if artifact := strings.TrimSpace(component.ArtifactURL); artifact != "" { + refs = append(refs, cdx.ExternalReference{Type: cdx.ERTypeDistribution, URL: artifact}) + } + if repository := strings.TrimSpace(component.VCSURL); repository != "" { + refs = append(refs, cdx.ExternalReference{Type: cdx.ERTypeVCS, URL: repository}) + } + return refs +} + func cycloneDXSecurityReferences(p Provenance) []cdx.ExternalReference { refs := make([]cdx.ExternalReference, 0, 2) if contact := strings.TrimSpace(p.SecurityContact); contact != "" { diff --git a/internal/sbom/model.go b/internal/sbom/model.go index 31607005..c3f71a6b 100644 --- a/internal/sbom/model.go +++ b/internal/sbom/model.go @@ -135,6 +135,16 @@ type Component struct { Digests []Digest Vulnerabilities []Vulnerability EOL *EOL + + // Where the package came from. A detector asserts at most one of + // ArtifactURL and VCSURL; registry enrichment may then fill VCSURL from a + // resolved source repository when it is empty, so a component enriched + // under --enrich can carry both. VCSRevision only accompanies VCSURL. + // Both are plain absolute http(s) URLs; composing them into a format's + // locator grammar is the encoder's job. + ArtifactURL string + VCSURL string + VCSRevision string } // Dependency describes one package relationship list in the intermediate SBOM model. diff --git a/internal/sbom/origin_test.go b/internal/sbom/origin_test.go new file mode 100644 index 00000000..7084331f --- /dev/null +++ b/internal/sbom/origin_test.go @@ -0,0 +1,355 @@ +package sbom + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/bomly-dev/bomly-sdk" +) + +// originGraph builds a two-package graph whose nodes carry whatever origin +// metadata a case wants to exercise. +func originGraph(t *testing.T, mutate func(app, pkg *sdk.Dependency)) *sdk.Graph { + t.Helper() + + g := sdk.New() + app := sdk.NewDependencyRef("app", "1.0.0") + pkg := sdk.NewDependencyRef("react", "18.2.0") + mutate(app, pkg) + for _, n := range []*sdk.Dependency{app, pkg} { + if err := g.AddNode(n); err != nil { + t.Fatalf("add package %s: %v", n.ID, err) + } + } + if err := g.AddEdge(app.ID, pkg.ID); err != nil { + t.Fatalf("add edge: %v", err) + } + return g +} + +// spdxPackageByName decodes an SPDX document and returns one package object. +func spdxPackageByName(t *testing.T, raw []byte, name string) map[string]any { + t.Helper() + var doc struct { + Packages []map[string]any `json:"packages"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + t.Fatalf("decode SPDX: %v", err) + } + for _, pkg := range doc.Packages { + if pkg["name"] == name { + return pkg + } + } + t.Fatalf("package %q not found in SPDX document", name) + return nil +} + +// cycloneDXReferences decodes a CycloneDX document and returns one component's +// external references as type→URL pairs. +func cycloneDXReferences(t *testing.T, raw []byte, name string) map[string]string { + t.Helper() + var doc struct { + Components []struct { + Name string `json:"name"` + ExternalReferences []struct { + Type string `json:"type"` + URL string `json:"url"` + } `json:"externalReferences"` + } `json:"components"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + t.Fatalf("decode CycloneDX: %v", err) + } + for _, component := range doc.Components { + if component.Name != name { + continue + } + refs := make(map[string]string, len(component.ExternalReferences)) + for _, ref := range component.ExternalReferences { + refs[ref.Type] = ref.URL + } + return refs + } + t.Fatalf("component %q not found in CycloneDX document", name) + return nil +} + +func marshalBoth(t *testing.T, g *sdk.Graph) (spdxRaw, cdxRaw []byte) { + t.Helper() + opts := BuildOptions{DocumentName: "origin-test", ToolVersion: "test"} + spdxRaw, err := MarshalDepGraphJSON(g, TargetSPDX23JSON, opts, EncodeOptions{}) + if err != nil { + t.Fatalf("marshal SPDX: %v", err) + } + cdxRaw, err = MarshalDepGraphJSON(g, TargetCycloneDX17JSON, opts, EncodeOptions{}) + if err != nil { + t.Fatalf("marshal CycloneDX: %v", err) + } + return spdxRaw, cdxRaw +} + +func TestArtifactOriginIsPublishedInBothFormats(t *testing.T) { + const artifact = "https://registry.npmjs.org/react/-/react-18.2.0.tgz" + g := originGraph(t, func(_, pkg *sdk.Dependency) { + pkg.Origin = sdk.ArtifactOrigin(artifact) + }) + + spdxRaw, cdxRaw := marshalBoth(t, g) + + if got := spdxPackageByName(t, spdxRaw, "react")["downloadLocation"]; got != artifact { + t.Errorf("SPDX downloadLocation = %v, want %q", got, artifact) + } + if got := cycloneDXReferences(t, cdxRaw, "react")["distribution"]; got != artifact { + t.Errorf("CycloneDX distribution ref = %q, want %q", got, artifact) + } +} + +func TestRepositoryOriginIsPublishedInBothFormats(t *testing.T) { + const ( + repository = "https://github.com/facebook/react" + revision = "b4c5d6e7f8091a2b3c4d5e6f708192a3b4c5d6e7" + ) + g := originGraph(t, func(_, pkg *sdk.Dependency) { + pkg.Origin = sdk.RepositoryOrigin(repository, revision) + }) + + spdxRaw, cdxRaw := marshalBoth(t, g) + + // SPDX 2.3's version-control form carries the revision after "@". + want := "git+" + repository + "@" + revision + if got := spdxPackageByName(t, spdxRaw, "react")["downloadLocation"]; got != want { + t.Errorf("SPDX downloadLocation = %v, want %q", got, want) + } + // CycloneDX external references have no revision slot, so the reference is + // the plain repository URL. + if got := cycloneDXReferences(t, cdxRaw, "react")["vcs"]; got != repository { + t.Errorf("CycloneDX vcs ref = %q, want %q", got, repository) + } + if got := cycloneDXReferences(t, cdxRaw, "react")["distribution"]; got != "" { + t.Errorf("repository origin also emitted a distribution ref: %q", got) + } +} + +func TestUnpinnedRepositoryOriginOmitsTheRevisionSuffix(t *testing.T) { + const repository = "https://github.com/facebook/react" + g := originGraph(t, func(_, pkg *sdk.Dependency) { + pkg.Origin = sdk.RepositoryOrigin(repository, "") + }) + + spdxRaw, _ := marshalBoth(t, g) + + if got := spdxPackageByName(t, spdxRaw, "react")["downloadLocation"]; got != "git+"+repository { + t.Errorf("SPDX downloadLocation = %v, want %q", got, "git+"+repository) + } +} + +// A package whose detector asserted nothing must say so, not guess. +func TestPackageWithoutOriginKeepsNOASSERTION(t *testing.T) { + g := originGraph(t, func(_, _ *sdk.Dependency) {}) + + spdxRaw, cdxRaw := marshalBoth(t, g) + + if got := spdxPackageByName(t, spdxRaw, "react")["downloadLocation"]; got != "NOASSERTION" { + t.Errorf("SPDX downloadLocation = %v, want NOASSERTION", got) + } + if refs := cycloneDXReferences(t, cdxRaw, "react"); len(refs) != 0 { + t.Errorf("CycloneDX emitted references for a package with no origin: %v", refs) + } +} + +// A hand-built origin can reach export from a plugin or a caller that never +// went through the constructors. Export re-validates rather than trusting it, +// so a bad value is dropped instead of published. +func TestExportRevalidatesOrigin(t *testing.T) { + hostile := []struct { + name string + origin *sdk.PackageOrigin + }{ + {name: "credentialed artifact", origin: &sdk.PackageOrigin{ArtifactURL: "https://build:s3cret-token-value@nexus.corp/repo/react-18.2.0.tgz"}}, + {name: "local path", origin: &sdk.PackageOrigin{ArtifactURL: "/Users/someone/src/project/react.tgz"}}, + {name: "file url", origin: &sdk.PackageOrigin{Repository: "file:///Users/someone/src/react"}}, + {name: "registry root", origin: &sdk.PackageOrigin{ArtifactURL: "https://registry.npmjs.org/"}}, + {name: "revision breaking the locator grammar", origin: &sdk.PackageOrigin{ + Repository: "https://github.com/facebook/react", Revision: "main@evil.test/x", + }}, + {name: "a recorded disagreement publishes nothing", origin: &sdk.PackageOrigin{ + Disputed: true, ArtifactURL: "https://registry.npmjs.org/react/-/react-18.2.0.tgz", + }}, + } + + for _, tc := range hostile { + t.Run(tc.name, func(t *testing.T) { + g := originGraph(t, func(_, pkg *sdk.Dependency) { + pkg.Origin = tc.origin + }) + + spdxRaw, cdxRaw := marshalBoth(t, g) + + download, _ := spdxPackageByName(t, spdxRaw, "react")["downloadLocation"].(string) + refs := cycloneDXReferences(t, cdxRaw, "react") + published := []string{download, refs["distribution"], refs["vcs"]} + for _, value := range published { + for _, forbidden := range []string{"s3cret", "@nexus.corp", "/Users/", "file://", "evil.test"} { + if strings.Contains(value, forbidden) { + t.Fatalf("published %q, which contains %q", value, forbidden) + } + } + } + // The revision case keeps a valid repository; the rest publish nothing. + if tc.name == "revision breaking the locator grammar" { + if download != "git+https://github.com/facebook/react" { + t.Fatalf("downloadLocation = %q, want the repository without a revision", download) + } + return + } + if download != "NOASSERTION" { + t.Fatalf("downloadLocation = %q, want NOASSERTION", download) + } + }) + } +} + +// The scorecard matcher resolves a canonical source repository during +// enrichment. It fills the gap when a lockfile named no repository, and never +// overrides one the detector asserted. +func TestScorecardRepositoryFillsTheOriginGap(t *testing.T) { + const purl = "pkg:npm/react@18.2.0" + + build := func(t *testing.T, detectorRepository string) []byte { + t.Helper() + g := sdk.New() + react := sdk.NewDependencyWithID("react@18.2.0", sdk.Dependency{Coordinates: sdk.Coordinates{ + Name: "react", Version: "18.2.0", PURL: purl, Ecosystem: "npm"}}) + if detectorRepository != "" { + react.Origin = sdk.RepositoryOrigin(detectorRepository, "") + } + if err := g.AddNode(react); err != nil { + t.Fatalf("add node: %v", err) + } + registry := sdk.NewPackageRegistry() + pkg := registry.Ensure(purl) + pkg.Name, pkg.Version, pkg.Matched = "react", "18.2.0", true + pkg.Scorecard = &sdk.PackageScorecard{Source: "api.scorecard.dev", Repository: "github.com/facebook/react"} + + raw, err := MarshalDepGraphJSON(g, TargetCycloneDX17JSON, BuildOptions{Registry: registry}, EncodeOptions{}) + if err != nil { + t.Fatalf("marshal CycloneDX: %v", err) + } + return raw + } + + t.Run("fills the gap", func(t *testing.T) { + if got := cycloneDXReferences(t, build(t, ""), "react")["vcs"]; got != "https://github.com/facebook/react" { + t.Errorf("vcs ref = %q, want the scorecard repository as an https URL", got) + } + }) + + t.Run("never overrides the detector", func(t *testing.T) { + const asserted = "https://github.com/facebook/react-fork" + if got := cycloneDXReferences(t, build(t, asserted), "react")["vcs"]; got != asserted { + t.Errorf("vcs ref = %q, want the detector-asserted repository %q", got, asserted) + } + }) + + // An artifact says where the file was fetched; a repository says where the + // source lives. They are complementary, so an enriched package can carry + // both -- the repository as a vcs reference in CycloneDX, and as source + // info in SPDX, whose single download location the artifact holds. + t.Run("a repository accompanies an artifact", func(t *testing.T) { + g := sdk.New() + react := sdk.NewDependencyWithID("react@18.2.0", sdk.Dependency{Coordinates: sdk.Coordinates{ + Name: "react", Version: "18.2.0", PURL: purl, Ecosystem: "npm"}}) + react.Origin = sdk.ArtifactOrigin("https://registry.npmjs.org/react/-/react-18.2.0.tgz") + if err := g.AddNode(react); err != nil { + t.Fatal(err) + } + registry := sdk.NewPackageRegistry() + pkg := registry.Ensure(purl) + pkg.Name, pkg.Version, pkg.Matched = "react", "18.2.0", true + pkg.Scorecard = &sdk.PackageScorecard{Source: "api.scorecard.dev", Repository: "github.com/facebook/react"} + + opts := BuildOptions{Registry: registry} + cdxRaw, err := MarshalDepGraphJSON(g, TargetCycloneDX17JSON, opts, EncodeOptions{}) + if err != nil { + t.Fatal(err) + } + spdxRaw, err := MarshalDepGraphJSON(g, TargetSPDX23JSON, opts, EncodeOptions{}) + if err != nil { + t.Fatal(err) + } + + refs := cycloneDXReferences(t, cdxRaw, "react") + if refs["distribution"] != "https://registry.npmjs.org/react/-/react-18.2.0.tgz" { + t.Errorf("distribution ref = %q, want the detector artifact", refs["distribution"]) + } + if refs["vcs"] != "https://github.com/facebook/react" { + t.Errorf("vcs ref = %q, want the scorecard repository", refs["vcs"]) + } + + spdxPkg := spdxPackageByName(t, spdxRaw, "react") + if spdxPkg["downloadLocation"] != "https://registry.npmjs.org/react/-/react-18.2.0.tgz" { + t.Errorf("downloadLocation = %v, want the artifact", spdxPkg["downloadLocation"]) + } + if got, _ := spdxPkg["sourceInfo"].(string); got != "Source repository: git+https://github.com/facebook/react" { + t.Errorf("sourceInfo = %q, want the repository", got) + } + }) + + // With no artifact, the repository is the download location, so repeating + // it as source info would say the same thing twice. + t.Run("a repository alone is not repeated as source info", func(t *testing.T) { + g := originGraph(t, func(_, pkg *sdk.Dependency) { + pkg.Origin = sdk.RepositoryOrigin("https://github.com/facebook/react", "") + }) + spdxRaw, _ := marshalBoth(t, g) + spdxPkg := spdxPackageByName(t, spdxRaw, "react") + if got, found := spdxPkg["sourceInfo"]; found && got != "" { + t.Errorf("sourceInfo = %v, want none: the repository is the download location", got) + } + }) + + t.Run("absent without enrichment", func(t *testing.T) { + g := originGraph(t, func(_, _ *sdk.Dependency) {}) + _, cdxRaw := marshalBoth(t, g) + if refs := cycloneDXReferences(t, cdxRaw, "react"); len(refs) != 0 { + t.Errorf("unenriched export emitted references: %v", refs) + } + }) +} + +// Origin is written on export and not read back on ingest: it describes what a +// lockfile said, and an ingested document is not a lockfile. Scanning an SBOM +// therefore yields packages with no origin, and re-exporting that graph says +// NOASSERTION rather than repeating a claim Bomly did not resolve itself. +// +// This pins the documented limitation; preserving third-party origin across +// ingest is tracked separately. +func TestOriginIsNotReadBackFromAnIngestedDocument(t *testing.T) { + g := originGraph(t, func(_, pkg *sdk.Dependency) { + pkg.Origin = sdk.RepositoryOrigin("https://github.com/facebook/react", "d6e7f8091a2b3c4d5e6f708192a3b4c5d6e7f809") + }) + + exported, _ := marshalBoth(t, g) + if got := spdxPackageByName(t, exported, "react")["downloadLocation"]; got == "NOASSERTION" { + t.Fatal("precondition failed: the first export carried no origin") + } + + ingested, _, err := UnmarshalAutoJSON(exported) + if err != nil { + t.Fatalf("ingest SPDX: %v", err) + } + reingestedGraph, err := ToGraph(ingested) + if err != nil { + t.Fatalf("to graph: %v", err) + } + reexported, err := MarshalDepGraphJSON(reingestedGraph, TargetSPDX23JSON, BuildOptions{DocumentName: "origin-test", ToolVersion: "test"}, EncodeOptions{}) + if err != nil { + t.Fatalf("re-export SPDX: %v", err) + } + + if got := spdxPackageByName(t, reexported, "react")["downloadLocation"]; got != "NOASSERTION" { + t.Fatalf("re-exported downloadLocation = %v, want NOASSERTION; if origin now survives ingest, docs/SBOM.md must say so", got) + } +} diff --git a/internal/sbom/spdx23.go b/internal/sbom/spdx23.go index cff750e4..5196d388 100644 --- a/internal/sbom/spdx23.go +++ b/internal/sbom/spdx23.go @@ -44,13 +44,14 @@ func (spdx23Codec) encodeJSON(doc *Document, opts EncodeOptions) ([]byte, error) PackageName: c.NameOrID(), PackageSPDXIdentifier: spdxID, PackageVersion: c.Version, - PackageDownloadLocation: "NOASSERTION", + PackageDownloadLocation: spdxDownloadLocation(c), FilesAnalyzed: false, PackageComment: spdxPackageComment(c), PackageLicenseDeclared: spdxLicenseValue(c.Licenses), PackageLicenseConcluded: spdxLicenseValue(c.Licenses), PackageCopyrightText: spdxCopyrightValue(c.Copyright), PackageChecksums: spdxChecksums(c.Digests), + PackageSourceInfo: spdxSourceInfo(c), PackageExternalReferences: spdxExternalReferences(c), PrimaryPackagePurpose: spdxPrimaryPackagePurpose(c.Type), } @@ -428,6 +429,52 @@ func spdxCopyrightValue(value string) string { return value } +// spdxSourceInfoPrefix labels the repository recorded in PackageSourceInfo, +// which SPDX defines as free text. +const spdxSourceInfoPrefix = "Source repository: " + +// spdxDownloadLocation renders where a package came from. SPDX requires the +// field, so a package whose detector asserted nothing keeps NOASSERTION rather +// than a guess. +func spdxDownloadLocation(component Component) string { + if artifact := strings.TrimSpace(component.ArtifactURL); artifact != "" { + return artifact + } + if locator := spdxVCSLocator(component); locator != "" { + return locator + } + return "NOASSERTION" +} + +// spdxVCSLocator renders a repository in SPDX 2.3's version-control form, +// "+://[@]". The grammar has no query +// component and no room for anything but the revision after "@", which is why +// the origin invariant strips both before a URL gets here. +func spdxVCSLocator(component Component) string { + repository := strings.TrimSpace(component.VCSURL) + if repository == "" { + return "" + } + locator := "git+" + repository + if revision := strings.TrimSpace(component.VCSRevision); revision != "" { + locator += "@" + revision + } + return locator +} + +// spdxSourceInfo records the source repository when it is not already the +// download location. A package downloaded as an artifact still has a +// repository worth naming, and SPDX has one download location per package, so +// the repository goes here rather than being dropped. +func spdxSourceInfo(component Component) string { + repository := strings.TrimSpace(component.VCSURL) + if repository == "" || strings.TrimSpace(component.ArtifactURL) == "" { + // With no artifact, the repository is the download location already. + return "" + } + return spdxSourceInfoPrefix + spdxVCSLocator(component) +} + func spdxExternalReferences(component Component) []*v23.PackageExternalReference { refs := make([]*v23.PackageExternalReference, 0, 1+len(component.CPEs)+len(component.Vulnerabilities)) if purl := strings.TrimSpace(component.PURL); purl != "" { diff --git a/internal/sbom/transform.go b/internal/sbom/transform.go index 891d4f4a..103681f1 100644 --- a/internal/sbom/transform.go +++ b/internal/sbom/transform.go @@ -47,6 +47,7 @@ func FromDepGraph(g *sdk.Graph, opts BuildOptions) (*Document, error) { Licenses: componentLicenses(sdk.DetectionLicenses(pkg)), Digests: componentDigests(pkg.Digests), } + applyOrigin(&component, pkg.Origin.Normalized()) enrichComponentFromRegistry(&component, opts.Registry, pkg.PURL) components = append(components, component) depsByRef[pkg.ID] = nil @@ -263,6 +264,36 @@ func uniqueToolNames(values []string) []string { // enrichComponentFromRegistry folds matching-stage data resolved by PURL onto a // component: registry-learned licenses (preferred over detection-time when // present), CPEs, digests, vulnerabilities, and EOL. registry may be nil. +// scorecardRepositoryURL renders a scorecard repository, which is a canonical +// host/owner/name identifier with no scheme, as a URL. It is held to the same +// invariant as detector-asserted origins. +func scorecardRepositoryURL(scorecard *sdk.PackageScorecard) (string, bool) { + if scorecard == nil { + return "", false + } + repository := strings.TrimSpace(scorecard.Repository) + if repository == "" { + return "", false + } + if !strings.Contains(repository, "://") { + repository = "https://" + repository + } + return sdk.NormalizeOriginURL(repository, true) +} + +// applyOrigin projects the origin a detector asserted onto a component. The +// value arrives already validated -- Normalized applies the SDK's rule and +// returns nothing when a location does not survive it -- so there is nothing to +// decide here: export publishes what detection resolved, or nothing. +func applyOrigin(component *Component, origin *sdk.PackageOrigin) { + if origin == nil { + return + } + component.ArtifactURL = origin.ArtifactURL + component.VCSURL = origin.Repository + component.VCSRevision = origin.Revision +} + func enrichComponentFromRegistry(component *Component, registry *sdk.PackageRegistry, purl string) { if component == nil || registry == nil || purl == "" { return @@ -283,6 +314,12 @@ func enrichComponentFromRegistry(component *Component, registry *sdk.PackageRegi if len(pkg.Vulnerabilities) > 0 { component.Vulnerabilities = vulnerabilitiesFromPackage(pkg.EcosystemName(), pkg.Vulnerabilities) } + if repository, ok := scorecardRepositoryURL(pkg.Scorecard); ok && component.VCSURL == "" { + // The scorecard matcher resolved a canonical source repository for + // this package. A detector-asserted repository is the stronger claim + // (it came from the lockfile), so this only fills a gap. + component.VCSURL = repository + } if pkg.EOL != nil { component.EOL = &EOL{ EOL: pkg.EOL.EOL, diff --git a/test/smoke/smoke_test.go b/test/smoke/smoke_test.go index bcd0d6ab..7becafdb 100644 --- a/test/smoke/smoke_test.go +++ b/test/smoke/smoke_test.go @@ -12,11 +12,13 @@ package smoke import ( + "bytes" "encoding/json" "fmt" "os" "os/exec" "path/filepath" + "regexp" "runtime" "strings" "testing" @@ -546,6 +548,108 @@ func TestScanSBOMSyftJSONRejected(t *testing.T) { } } +// TestScanSBOMExportOrigin drives a real npm project end to end and asserts on +// the exported SBOM bytes rather than a golden: SPDX and CycloneDX documents +// carry a namespace, serial number, timestamp, and tool version that change on +// every run, and normalizeJSON targets Bomly's scan document, not SBOM output. +// +// The properties asserted here are the ones that matter and that unit tests +// cannot reach: a real lockfile from a real repository produces real download +// locations, and nothing about the machine that ran the scan leaks into them. +func TestScanSBOMExportOrigin(t *testing.T) { + t.Parallel() + requireTool(t, "npm") + + outputDir := t.TempDir() + spdxPath := filepath.Join(outputDir, "out.spdx.json") + cdxPath := filepath.Join(outputDir, "out.cdx.json") + + _, stderr, code := runBomly(t, + "scan", "--url", "https://github.com/bomly-dev/example-javascript-npm", "--ref", "v1.0.0", + "--detectors", "npm", "--format", "json", + "-o", "spdx="+spdxPath, "-o", "cyclonedx="+cdxPath, + ) + if code != 0 { + t.Fatalf("bomly exited %d\nstderr:\n%s", code, stderr) + } + + spdxRaw, err := os.ReadFile(spdxPath) + if err != nil { + t.Fatalf("read SPDX output: %v", err) + } + cdxRaw, err := os.ReadFile(cdxPath) + if err != nil { + t.Fatalf("read CycloneDX output: %v", err) + } + + var spdxDoc struct { + Packages []struct { + Name string `json:"name"` + DownloadLocation string `json:"downloadLocation"` + } `json:"packages"` + } + if err := json.Unmarshal(spdxRaw, &spdxDoc); err != nil { + t.Fatalf("decode SPDX output: %v", err) + } + var cdxDoc struct { + Components []struct { + Name string `json:"name"` + ExternalReferences []struct { + Type string `json:"type"` + URL string `json:"url"` + } `json:"externalReferences"` + } `json:"components"` + } + if err := json.Unmarshal(cdxRaw, &cdxDoc); err != nil { + t.Fatalf("decode CycloneDX output: %v", err) + } + + // npm records a registry tarball for every installed package, so a real + // scan must produce real download locations, not a document of NOASSERTION. + downloads := 0 + for _, pkg := range spdxDoc.Packages { + if strings.HasPrefix(pkg.DownloadLocation, "https://") { + downloads++ + } + } + if downloads == 0 { + t.Fatalf("no SPDX package carried a download location; got %d packages", len(spdxDoc.Packages)) + } + distributions := 0 + for _, component := range cdxDoc.Components { + for _, ref := range component.ExternalReferences { + if ref.Type == "distribution" && strings.HasPrefix(ref.URL, "https://") { + distributions++ + } + } + } + if distributions == 0 { + t.Fatalf("no CycloneDX component carried a distribution reference; got %d components", len(cdxDoc.Components)) + } + + // Nothing about the machine that ran the scan may reach the documents. + // The clone directory is the specific hazard: it is a real path that a + // detector could pass through verbatim. + cloneMarkers := []string{"file://", `:"/home/`, `:"/Users/`, `:"/var/folders/`, `:"/private/`} + for _, output := range []struct { + name string + raw []byte + }{{"SPDX", spdxRaw}, {"CycloneDX", cdxRaw}} { + for _, marker := range cloneMarkers { + if bytes.Contains(output.raw, []byte(marker)) { + t.Errorf("%s output contains %q, which points at the scanning machine", output.name, marker) + } + } + // A URL of the form scheme://userinfo@host would carry a credential. + if credentialedURL.Match(output.raw) { + t.Errorf("%s output contains a URL with embedded credentials", output.name) + } + } +} + +// credentialedURL matches an http(s) URL carrying userinfo before its host. +var credentialedURL = regexp.MustCompile(`https?://[^"/\s]*@`) + // --------------------------------------------------------------------------- // Helpers // ---------------------------------------------------------------------------