Skip to content

Commit

Permalink
Guided Remediation: Add dependency relaxation & re-resolution (#765)
Browse files Browse the repository at this point in the history
The remediation part of guided remediation :)
Adds the functionality to attempt to fix vulnerabilities in a manifest
by relaxing the requirements of its direct dependencies.

I've simplified & refactored `tryRelaxRemediate` (née `RelaxResolve`),
so PTAL at that.

I still need to migrate the part that uses this to make the list of
possible 'patches' to the manifest (hence the `//nolint:unused`
everywhere), but I didn't want to keep bringing more and more bits into
one PR.
  • Loading branch information
michaelkedar authored Jan 24, 2024
1 parent 44254c8 commit 251b676
Show file tree
Hide file tree
Showing 6 changed files with 309 additions and 1 deletion.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.21.5
require (
deps.dev/api/v3alpha v0.0.0-20240109042716-00b51ef52ece
deps.dev/util/resolve v0.0.0-20240109042716-00b51ef52ece
deps.dev/util/semver v0.0.0-20240109040450-1e316b822bc4
github.com/BurntSushi/toml v1.3.2
github.com/CycloneDX/cyclonedx-go v0.8.0
github.com/gkampitakis/go-snaps v0.4.12
Expand Down Expand Up @@ -32,7 +33,6 @@ require (

require (
dario.cat/mergo v1.0.0 // indirect
deps.dev/util/semver v0.0.0-20240109040450-1e316b822bc4 // indirect
github.com/Microsoft/go-winio v0.6.1 // indirect
github.com/ProtonMail/go-crypto v0.0.0-20230923063757-afb1ddc0824c // indirect
github.com/anchore/go-struct-converter v0.0.0-20230627203149-c72ef8859ca9 // indirect
Expand Down
85 changes: 85 additions & 0 deletions internal/remediation/relax.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package remediation

import (
"context"
"errors"
"slices"

"deps.dev/util/resolve"
"github.com/google/osv-scanner/internal/remediation/relaxer"
"github.com/google/osv-scanner/internal/resolution"
)

//nolint:unused
var errRelaxRemediateImpossible = errors.New("cannot fix vulns by relaxing")

//nolint:unused
func tryRelaxRemediate(
ctx context.Context,
cl resolve.Client,
orig *resolution.ResolutionResult,
vulnIDs []string,
opts RemediationOptions,
) (*resolution.ResolutionResult, error) {
relaxer, err := relaxer.GetRelaxer(orig.Manifest.System())
if err != nil {
return nil, err
}

newRes := orig
toRelax := reqsToRelax(newRes, vulnIDs, opts)
for len(toRelax) > 0 {
// Try relaxing all necessary requirements
manif := newRes.Manifest.Clone()
for _, idx := range toRelax {
rv := manif.Requirements[idx]
// If we'd need to relax a package we want to avoid changing, we cannot fix the vuln
if slices.Contains(opts.AvoidPkgs, rv.Name) {
return nil, errRelaxRemediateImpossible
}
newVer, ok := relaxer.Relax(ctx, cl, rv, opts.AllowMajor)
if !ok {
return nil, errRelaxRemediateImpossible
}
manif.Requirements[idx] = newVer
}

// re-resolve relaxed manifest
newRes, err = resolution.Resolve(ctx, cl, manif)
if err != nil {
return nil, err
}
toRelax = reqsToRelax(newRes, vulnIDs, opts)
}

return newRes, nil
}

//nolint:unused
func reqsToRelax(res *resolution.ResolutionResult, vulnIDs []string, opts RemediationOptions) []int {
toRelax := make(map[resolve.VersionKey]string)
for _, v := range res.Vulns {
// Don't do a full opts.MatchVuln() since we know we don't need to check every condition
if !slices.Contains(vulnIDs, v.Vulnerability.ID) || (!opts.DevDeps && v.DevOnly) {
continue
}
// Only relax dependencies if their chain length is less than MaxDepth
for _, ch := range v.ProblemChains {
if opts.MaxDepth <= 0 || len(ch.Edges) <= opts.MaxDepth {
vk, req := ch.DirectDependency()
toRelax[vk] = req
}
}
}

// Find the index into the Manifest.Requirements of each that needs to be relaxed
reqIdxs := make([]int, 0, len(toRelax))
for vk, req := range toRelax {
idx := slices.IndexFunc(res.Manifest.Requirements, func(rv resolve.RequirementVersion) bool {
return rv.PackageKey == vk.PackageKey && rv.Version == req
})
reqIdxs = append(reqIdxs, idx)
}

return reqIdxs
}
119 changes: 119 additions & 0 deletions internal/remediation/relaxer/npm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package relaxer

import (
"context"
"slices"

"deps.dev/util/resolve"
"deps.dev/util/semver"
)

type NPMRelaxer struct{}

func (r NPMRelaxer) Relax(ctx context.Context, cl resolve.Client, req resolve.RequirementVersion, allowMajor bool) (resolve.RequirementVersion, bool) {
c, err := semver.NPM.ParseConstraint(req.Version)
if err != nil {
// The specified version is not a valid semver constraint
// Check if it's a version tag (usually 'latest') by seeing if there are matching versions
vks, err := cl.MatchingVersions(ctx, req.VersionKey)
if err != nil || len(vks) == 0 { // no matches, cannot relax
return req, false
}
// Use the first matching version (there should only be one) as a pinned version
c, err = semver.NPM.ParseConstraint(vks[0].Version)
if err != nil {
return req, false
}
}

// Get all the concrete versions of the package
allVKs, err := cl.Versions(ctx, req.PackageKey)
if err != nil {
return req, false
}
var vers []string
for _, vk := range allVKs {
if vk.VersionType == resolve.Concrete {
vers = append(vers, vk.Version)
}
}
slices.SortFunc(vers, semver.NPM.Compare)

// Find the versions on either side of the upper boundary of the requirement
var lastIdx int // highest version matching constraint
var nextIdx int = -1 // next version outside of range, preferring non-prerelease
nextIsPre := true // if the next version is a prerelease version
for lastIdx = len(vers) - 1; lastIdx >= 0; lastIdx-- {
v, err := semver.NPM.Parse(vers[lastIdx])
if err != nil {
continue
}
if c.MatchVersion(v) { // found the upper bound, stop iterating
break
}

// Want to prefer non-prerelease versions, so only select one if we haven't seen any non-prerelease versions
if !v.IsPrerelease() || nextIsPre {
nextIdx = lastIdx
nextIsPre = v.IsPrerelease()
}
}

// Didn't find any higher versions of the package
if nextIdx == -1 {
return req, false
}

// No versions match the existing constraint, something is wrong
if lastIdx == -1 {
return req, false
}

// Our desired relaxation ordering is
// 1.2.3 -> 1.2.* -> 1.*.* -> 2.*.* -> 3.*.* -> ...
// But we want to use npm-like version specifiers e.g.
// 1.2.3 -> ~1.2.4 -> ^1.4.5 -> ^2.6.7 -> ^3.8.9 -> ...
// using the latest versions of the ranges

cmpVer := vers[lastIdx]
_, diff, _ := semver.NPM.Difference(cmpVer, vers[nextIdx])
if diff == semver.DiffMajor {
if !allowMajor {
return req, false
}
// Want to step only one major version at a time
// Instead of looking for a difference larger than major,
// we want to look for a major version bump from the first next version
cmpVer = vers[nextIdx]
diff = semver.DiffMinor
}

// Find the highest version with the same difference
best := vers[nextIdx]
for i := nextIdx + 1; i < len(vers); i++ {
_, d, err := semver.NPM.Difference(cmpVer, vers[i])
if err != nil {
continue
}
// DiffMajor < DiffMinor < DiffPatch < DiffPrerelease
// So if d is less than the original diff, it represents a larger change
if d < diff {
break
}
ver, err := semver.NPM.Parse(vers[i])
if err != nil {
continue
}
if !ver.IsPrerelease() || nextIsPre {
best = vers[i]
}
}

if diff == semver.DiffPatch {
req.Version = "~" + best
} else {
req.Version = "^" + best
}

return req, true
}
34 changes: 34 additions & 0 deletions internal/remediation/relaxer/relaxer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package relaxer

import (
"context"
"errors"

"deps.dev/util/resolve"
)

// A RequirementRelaxer provides an ecosystem-specific method for 'relaxing' the
// specified versions of dependencies for vulnerability remediation.
// Relaxing involves incrementally widening and bumping the version specifiers
// of the requirement to allow more recent versions to be selected during
// dependency resolution.
// It has access to the available versions of a package via a resolve client.
//
// e.g. in a semver-like ecosystem, relaxation could follow the sequence:
// 1.2.3 -> 1.2.* -> 1.*.* -> 2.*.* -> 3.*.* -> ...
type RequirementRelaxer interface {
// Relax attempts to relax import requirement.
// Returns the newly relaxed import and true it was successful.
// If unsuccessful, it returns the original import and false.
Relax(ctx context.Context, cl resolve.Client, req resolve.RequirementVersion, allowMajor bool) (resolve.RequirementVersion, bool)
}

func GetRelaxer(ecosystem resolve.System) (RequirementRelaxer, error) {
// TODO: is using ecosystem fine, or should this be per manifest?
switch ecosystem { //nolint:exhaustive
case resolve.NPM:
return NPMRelaxer{}, nil
default:
return nil, errors.New("unsupported ecosystem")
}
}
64 changes: 64 additions & 0 deletions internal/remediation/remediation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package remediation

import (
"slices"

"github.com/google/osv-scanner/internal/resolution"
)

type RemediationOptions struct {
IgnoreVulns []string // Vulnerability IDs to ignore
ExplicitVulns []string // If set, only consider these vulnerability IDs & ignore all others

DevDeps bool // Whether to consider vulnerabilities in dev dependencies
MinSeverity float64 // Minimum vulnerability CVSS score to consider
MaxDepth int // Maximum depth of dependency to consider vulnerabilities for (e.g. 1 for direct only)

AvoidPkgs []string // Names of direct dependencies to avoid upgrading
AllowMajor bool // Whether to allow changes to major versions of direct dependencies
}

func (opts RemediationOptions) MatchVuln(v resolution.ResolutionVuln) bool {
if slices.Contains(opts.IgnoreVulns, v.Vulnerability.ID) {
return false
}

if len(opts.ExplicitVulns) > 0 && !slices.Contains(opts.ExplicitVulns, v.Vulnerability.ID) {
return false
}

if !opts.DevDeps && v.DevOnly {
return false
}

return opts.matchSeverity(v) && opts.matchDepth(v)
}

func (opts RemediationOptions) matchSeverity(v resolution.ResolutionVuln) bool {
// TODO
return true
}

func (opts RemediationOptions) matchDepth(v resolution.ResolutionVuln) bool {
if opts.MaxDepth <= 0 {
return true
}

if len(v.ProblemChains)+len(v.NonProblemChains) == 0 {
panic("vulnerability with no dependency chains")
}

for _, ch := range v.ProblemChains {
if len(ch.Edges) <= opts.MaxDepth {
return true
}
}

for _, ch := range v.NonProblemChains {
if len(ch.Edges) <= opts.MaxDepth {
return true
}
}

return false
}
6 changes: 6 additions & 0 deletions internal/resolution/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ func (res *ResolutionResult) computeVulns(ctx context.Context, cl resolve.Client
}
rv.DevOnly = rv.DevOnly && ChainIsDev(chain, res.Manifest)
}
if len(rv.ProblemChains) == 0 {
// There has to be at least one problem chain for the vulnerability to appear.
// If our heuristic couldn't determine any, treat them all as problematic.
rv.ProblemChains = rv.NonProblemChains
rv.NonProblemChains = nil
}
res.Vulns = append(res.Vulns, rv)
}

Expand Down

0 comments on commit 251b676

Please sign in to comment.