Add package dependency completness field #3402

wagoodman · 2024-10-31T16:16:59Z

Description

Adds a field that describes the completeness of a package's direct dependencies:

complete: the package has all of its direct dependencies resolved and related to this package.
incomplete: indicates that the package is known to not have all of its direct dependencies listed. This is reserved for cases where we know there are a non-zero number of dependencies for a package, but we are not listing them intentionally or because we are unable to resolve them.
complete-with-indirect: a superset of complete -- indicates that the package has all of its direct dependencies resolved as well as some or all of indirect dependencies. What is notable about this is that direct and indirect dependencies are linked directly to this package and are not separable (you cannot distinguish between a direct and indirect dependency from the perspective of this package).
unknown: indicates that the completeness of the dependencies cannot be considered positively complete or incomplete. This should be used when the dependency resolution mechanism is not well understood, the set of dependencies is unknowable, or no attempt has been made to resolve dependencies (no assertion).

In addition to adding this new field, all catalogers were updated to raise up accurate values for this field:

Cataloger	Completeness	Comment
alpm-db-cataloger	complete
apk-db-cataloger	complete
binary-classifier-cataloger	unknown	classifiers are limited to identifying package identities, but have no information about dependencies (though there may be dependencies via shared libs and other mechanisms [static lib at compile time])
cargo-auditable-binary-cataloger	unknown
cocoapods-cataloger	unknown
conan-cataloger	conanfile.txt: unknown, conan.lock: complete
conan-info-cataloger	complete
dart-pubspec-lock-cataloger	unknown
dotnet-deps-cataloger	complete
dotnet-portable-executable-cataloger	unknown
dpkg-db-cataloger	complete
elf-binary-package-cataloger	unknown	though we can look for shared libs, we cannot see static dependencies nor dynamic dependencies using dlopen. This means that, even in cases where the dep info is actually complete, we can't programmatically determine that.
elixir-mix-lock-cataloger	unknown
erlang-otp-application-cataloger	unknown
erlang-rebar-lock-cataloger	unknown
github-action-workflow-usage-cataloger	complete	the only known exception to this is shared workflows
github-actions-usage-cataloger	unknown
go-module-binary-cataloger	complete-with-indirect / unknown	the main module gets the `complete-with-indirect` but all dependencies get `unknown`
go-module-file-cataloger	unknown	there is no main module discovered to make relationships for
graalvm-native-image-cataloger	unknown	or if anything is decoded from the internal SBOM, then that value is used
haskell-cataloger	unknown
java-archive-cataloger	complete / unknown	all packages are assumed to be unknown unless searching for transitive dependencies configuration option has been enabled
java-gradle-lockfile-cataloger	unknown
java-jvm-cataloger	unknown
java-pom-cataloger	unknown
javascript-lock-cataloger	unknown
javascript-package-cataloger	unknown	we find all dependency nodes but are not crafting relationships
linux-kernel-cataloger	unknown
lua-rock-cataloger	unknown
nix-store-cataloger	unknown
opam-cataloger	unknown
php-composer-installed-cataloger	unknown
php-composer-lock-cataloger	unknown
php-pecl-serialized-cataloger	unknown
portage-cataloger	unknown
python-installed-package-cataloger	complete	there is a `WithResolvingProcessors(wheelEggRelationships)` on the cataloger
python-package-cataloger	requirements.txt: unknown, setup.py: unknown, poetry.lock: complete, Pipfile.lock: unknown
r-package-cataloger	unknown
rpm-archive-cataloger	unknown
rpm-db-cataloger	complete
ruby-gemfile-cataloger	unknown
ruby-gemspec-cataloger	unknown
ruby-installed-gemspec-cataloger	unknown
rust-cargo-lock-cataloger	complete
sbom-cataloger	unknown
swift-package-manager-cataloger	unknown
swipl-pack-cataloger	unknown
wordpress-plugins-cataloger	unknown

Closes Supply "depth" information when including relationships #3010

Type of change

New feature (non-breaking change which adds functionality)

Checklist:

I have added unit tests that cover changed behavior
I have tested my code in common scenarios and confirmed there are no regressions
I have added comments to my code, particularly in hard-to-understand sections

Signed-off-by: Alex Goodman <[email protected]>

willmurphyscode · 2024-11-15T14:59:22Z

@wagoodman I wanted to make sure I understand why the rust-cargo-lock-cataloger is labeled as "complete" not as "mixed".

It seems like as we make a set of dependency relationships more complete, it passes from complete, to mixed, and back to complete, which is surprising:

It finds all the direct dependencies and nothing else -> "complete"
It finds all the direct and some or all indirect dependencies, but you can't tell how direct the dependencies are -> "mixed"
It finds all direct and indirect dependencies, and you can tell which ones are direct -> "complete"

It seems weird to me that a cataloger can effectively get promoted from complete to mixed, and then from mixed to complete, by adding relationship data.

When I first read the descriptions above, I thought, "rust isn't complete, it's mixed, because it includes transitive dependencies."

Am I understanding correctly here?

wagoodman · 2024-11-15T15:08:20Z

Here each node in the cargo.lock is describing only direct dependencies so this is complete

Signed-off-by: Alex Goodman <[email protected]>

syft/pkg/cataloger/golang/parse_go_mod.go

willmurphyscode · 2024-11-20T13:42:21Z

Cross posting #3010 (comment) so that it gets seen here.

Signed-off-by: Alex Goodman <[email protected]>

willmurphyscode · 2024-12-02T12:12:08Z

syft/format/syftjson/model/package.go

+	Language     pkg.Language               `json:"language"`
+	CPEs         cpes                       `json:"cpes"`
+	PURL         string                     `json:"purl"`
+	Dependencies pkg.DependencyCompleteness `json:"dependencies"`


Is this the right name for this field? licenses is the actual licenses, cpes is the actual CPEs, locations is the actual locations, but dependencies is the completeness and directness of the dependencies. What about dependencyCompleteness or something? I hate to make it that much longer, but I also hate to have dependencies be the only plural noun on here that's no a slice of noun.

willmurphyscode · 2024-12-02T12:19:04Z

syft/format/syftjson/to_syft_model.go

-		CPEs:      cpes,
-		PURL:      p.PURL,
-		Metadata:  p.Metadata,
+		Name:         p.Name,


I was expecting to see syft/format/... for other formats. Do SPDX and CDX have anywhere to put dependency completeness?

For SPDX only in 3.x (which we don't support yet), for Cyclone there is support for it in the format spec for versions we support -- I'll update the PR to map appropriately.

Did this change get missed? I don't see a diff in the cyclonedx and cyclonedx json formatters, and I expected to. (I think it's also ok to do this as a follow up PR, since the current PR is large.)

syft/pkg/cataloger/binary/elf_package.go

syft/pkg/cataloger/cpp/package.go

syft/pkg/cataloger/dart/package.go

syft/pkg/cataloger/dart/parse_pubspec_lock_test.go

willmurphyscode · 2024-12-02T14:47:37Z

syft/pkg/cataloger/dotnet/parse_dotnet_portable_executable.go

-		PURL:      portableExecutablePackageURL(name, version),
-		Metadata:  metadata,
+		PURL:      portableExecutablePackageURL(name, ver),
+		// by nature PE metadata does not have any dependency information, thus we are forced to claim incomplete


Just to clarify the difference between unknown and incomplete one more time:

Dependency is known to be incomplete or known to be missing entirely -> Incomplete

It is not knowable whether dependency data is complete -> unknown.

Is "PE file does not include dependency information, so this package's dependencies are known to be incomplete." a good summary of the comment here then?

basic definitions:

incomplete: the package does not have all of its direct dependencies resolved.
unknown: indicates when dependency resolution mechanism for this package is not well understood.

I've struggled through most of development on a good firm definition for unknown vs incomplete.

Is "PE file does not include dependency information, so this package's dependencies are known to be incomplete." a good summary of the comment here then?

It seems like one, but I'm stuck on "dependencies are known to be incomplete", which isn't quite true.

Do you think we should extend unknown to really mean "no assertion" (for any reason), in which case may things listed as incomplete in the interim would switch to unknown.

I've adjusted the definitions based off of our offline discussions:

incomplete: indicates that the package is known to not have all of its direct dependencies listed.

unknown: indicates that the completeness of the dependencies cannot be considered positively complete or incomplete. This should be used when the dependency resolution mechanism is not well understood, the set of dependencies is unknowable, or no attempt has been made to resolve dependencies (no assertion).

That also means that I don't think we have a case yet where we would list 'incomplete'.

syft/pkg/cataloger/elixir/package.go

syft/pkg/cataloger/githubactions/parse_composite_action_test.go

syft/pkg/cataloger/golang/package.go

willmurphyscode · 2024-12-02T16:21:13Z

syft/pkg/cataloger/java/parse_pom_xml.go

+			relationships = append(relationships, artifact.Relationship{ //nolint:gocritic // we intentionally want to use the reference to the package which will still be mutated and finalized to a value later
+				// both the to and from packages may be mutated based on the resolved dependencies, so we need references to these values
+				// to ensure the nodes used are consistent with the final state of the packages.
+				// note: it is VITAL that these references are replaced with values by the caller of this function before using


These kind of comments always make me nervous. Is there something we can do to make sure this happens? What if we return something that represents instructions for building an []artifact.Relationships in the caller, so that the caller can't forget and return a []artifact.Relationships with unexpected types in it?

yes that's what the finalizeRelationships call at the final return of the cataloger (a chokepoint)

syft/syft/pkg/cataloger/java/parse_pom_xml.go

Lines 84 to 97 in 5b7ec60

return finalizePackages(pkgs), finalizeRelationships(relationships), errs

}

func finalizeRelationships(relationships []artifact.Relationship) []artifact.Relationship {

for i := range relationships {

if f, ok := relationships[i].From.(*pkg.Package); ok {

relationships[i].From = *f

}

if t, ok := relationships[i].To.(*pkg.Package); ok {

relationships[i].To = *t

}

}

return relationships

}

I see that. I think I'd rather the type system remember that finalizeRelationships needs to be called. For example, you could have a local struct unfinalizedRelationship and return a slice of those, and then finalize relationship can take a slice of unfinalizedRelationship and return a slice of a artifact.Relationship. That way if someone tries to use this function without finalizing the relationships, the compiler won't let them.

syft/pkg/cataloger/python/parse_wheel_egg.go

syft/pkg/cataloger/redhat/parse_rpm_manifest.go

willmurphyscode · 2024-12-03T15:56:17Z

syft/pkg/cataloger/rust/package.go

@@ -46,6 +47,8 @@ func newPackageFromAudit(dep *rustaudit.Package, locations ...file.Location) pkg
 		Language:  pkg.Rust,
 		Type:      pkg.RustPkg,
 		Locations: file.NewLocationSet(locations...),
+		// no attempt is made by the parser function to resolve dependencies
+		Dependencies: pkg.IncompleteDependencies,


The rust auditable binary cataloger is a lot like the Go binary cataloger. I think this cataloger can be changed to emit a "main" package and CompleteWithIndirect. Or do we not trust the completeness of this metadata if it's present?

I'm trying not to change the cataloger logic in this PR -- what you're describing would be a good follow up enhancement for this cataloger

willmurphyscode · 2024-12-03T16:34:14Z

syft/pkg/package.go

+	Type         Type                   `cyclonedx:"type"`                   // the package type (e.g. Npm, Yarn, Python, Rpm, Deb, etc)
+	CPEs         []cpe.CPE              `hash:"ignore"`                      // all possible Common Platform Enumerators (note: this is NOT included in the definition of the ID since all fields on a CPE are derived from other fields)
+	PURL         string                 `hash:"ignore"`                      // the Package URL (see https://github.com/package-url/purl-spec)
+	Dependencies DependencyCompleteness `hash:"ignore"`                      // the completeness of the dependency information for this package


I think I said this above, but I don't think Dependencies is the right name for this field. Licenses is of type LicenseSet, which is a set of licenses. CPEs is a slice of cpe.CPE. Locations is a LocationSet. Dependencies is an enum about how complete the dependencies are. It seems to break an established pattern.

Signed-off-by: Alex Goodman <[email protected]>

willmurphyscode · 2024-12-06T16:03:03Z

syft/pkg/cataloger/rust/cataloger_test.go

+			Locations:    file.NewLocationSet(file.NewVirtualLocation("/hello-auditable", "/hello-auditable")),
+			Language:     pkg.Rust,
+			Type:         pkg.RustPkg,
+			Dependencies: pkg.UnknownDependencyCompleteness,


Does this need to change now that #3500 is merged?

willmurphyscode · 2024-12-06T16:03:59Z

syft/pkg/cataloger/rust/package.go

@@ -46,6 +47,8 @@ func newPackageFromAudit(dep *rustaudit.Package, locations ...file.Location) pkg
 		Language:  pkg.Rust,
 		Type:      pkg.RustPkg,
 		Locations: file.NewLocationSet(locations...),
+		// no attempt is made by the parser function to resolve dependencies


I think the attempt is made now that #3500 is merged.

I'll handle this on the rebase

willmurphyscode · 2024-12-06T16:05:42Z

syft/pkg/cataloger/swipl/parse_pack_test.go

@@ -9,17 +9,18 @@ import (
 	"github.com/anchore/syft/syft/pkg/cataloger/internal/pkgtest"
 )

-func TestParsePackPackage(t *testing.T) {
+func xTestParsePackPackage(t *testing.T) {


Is committing this x intentional?

great catch, that's a typo

github-actions bot added the json-schema Changes the json schema label Oct 31, 2024

wagoodman force-pushed the note-dep-quality branch 2 times, most recently from bb1b51b to 141d19a Compare October 31, 2024 16:27

wagoodman marked this pull request as ready for review October 31, 2024 16:28

wagoodman requested a review from a team November 15, 2024 14:44

add package dependency quality notes

5b7ec60

Signed-off-by: Alex Goodman <[email protected]>

wagoodman force-pushed the note-dep-quality branch from 141d19a to 17abccb Compare November 15, 2024 14:51

update formatter and json schema

ad2abfa

Signed-off-by: Alex Goodman <[email protected]>

wagoodman force-pushed the note-dep-quality branch from 17abccb to ad2abfa Compare November 15, 2024 16:21

wagoodman commented Nov 15, 2024

View reviewed changes

syft/pkg/cataloger/golang/parse_go_mod.go Outdated Show resolved Hide resolved

wagoodman mentioned this pull request Nov 20, 2024

Supply "depth" information when including relationships #3010

Open

rename mixed enum and fix golang

b642131

Signed-off-by: Alex Goodman <[email protected]>