Skip to content

Commit

Permalink
upgrade-config flag
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelkedar committed Aug 20, 2024
1 parent 0ecee7c commit 16f103f
Show file tree
Hide file tree
Showing 14 changed files with 245 additions and 97 deletions.
73 changes: 67 additions & 6 deletions cmd/osv-scanner/fix/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"io"
"os"
"path/filepath"
"strings"

"deps.dev/util/resolve"
"github.com/google/osv-scanner/internal/remediation"
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution/client"
"github.com/google/osv-scanner/internal/resolution/lockfile"
"github.com/google/osv-scanner/internal/resolution/manifest"
Expand Down Expand Up @@ -110,16 +112,23 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
Value: -1,
},

&cli.StringSliceFlag{
Category: upgradeCategory,
Name: "upgrade-config",
Usage: "the allowed package upgrades, in the format `[package-name:]level`. If package-name is omitted, level is applied to all packages. level must be one of (major, minor, patch, none).",
DefaultText: "major",
},
&cli.BoolFlag{
// TODO: allow for finer control e.g. specific packages, major/minor/patch
Category: upgradeCategory,
Name: "disallow-major-upgrades",
Usage: "disallow major version changes to dependencies",
Hidden: true,
},
&cli.StringSliceFlag{
Category: upgradeCategory,
Name: "disallow-package-upgrades",
Usage: "list of packages to disallow version changes",
Hidden: true,
},

&cli.IntFlag{
Expand Down Expand Up @@ -160,20 +169,75 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
}
}

func parseUpgradeConfig(ctx *cli.Context, r reporter.Reporter) upgrade.Config {
config := upgrade.NewConfig()

if ctx.IsSet("disallow-major-upgrades") {
r.Warnf("WARNING: `--disallow-major-upgrades` flag is deprecated, use `--upgrade-config minor` instead\n")
if ctx.Bool("disallow-major-upgrades") {
config.SetDefault(upgrade.Minor)
} else {
config.SetDefault(upgrade.Major)
}
}
if ctx.IsSet("disallow-package-upgrades") {
r.Warnf("WARNING: `--disallow-package-upgrades` flag is deprecated, use `--upgrade-config PKG:none` instead\n")
for _, pkg := range ctx.StringSlice("disallow-package-upgrades") {
config.Set(pkg, upgrade.None)
}
}

for _, spec := range ctx.StringSlice("upgrade-config") {
idx := strings.LastIndex(spec, ":")
if idx == 0 {
r.Warnf("WARNING: `--upgrade-config %s` - skipping empty package name\n", spec)
continue
}
pkg := ""
levelStr := spec
if idx > 0 {
pkg = spec[:idx]
levelStr = spec[idx+1:]
}
var level upgrade.Level
switch levelStr {
case "major":
level = upgrade.Major
case "minor":
level = upgrade.Minor
case "patch":
level = upgrade.Patch
case "none":
level = upgrade.None
default:
r.Warnf("WARNING: `--upgrade-config %s` - invalid level string '%s'\n", spec, levelStr)
continue
}
if config.Set(pkg, level) { // returns true if was previously set
r.Warnf("WARNING: `--upgrade-config %s` - config for package specified multiple times\n", spec)
}
}

return config
}

func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, error) {
if !ctx.IsSet("manifest") && !ctx.IsSet("lockfile") {
return nil, errors.New("manifest or lockfile is required")
}

// TODO: This isn't what the reporter is designed for.
// Only using r.Infof()/r.Warnf()/r.Errorf() to print to stdout/stderr.
r := reporter.NewTableReporter(stdout, stderr, reporter.InfoLevel, false, 0)

opts := osvFixOptions{
RemediationOptions: remediation.RemediationOptions{
IgnoreVulns: ctx.StringSlice("ignore-vulns"),
ExplicitVulns: ctx.StringSlice("vulns"),
DevDeps: !ctx.Bool("ignore-dev"),
MinSeverity: ctx.Float64("min-severity"),
MaxDepth: ctx.Int("max-depth"),
AvoidPkgs: ctx.StringSlice("disallow-package-upgrades"),
AllowMajor: !ctx.Bool("disallow-major-upgrades"),
UpgradeConfig: parseUpgradeConfig(ctx, r),
},
Manifest: ctx.String("manifest"),
Lockfile: ctx.String("lockfile"),
Expand Down Expand Up @@ -241,9 +305,6 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
return nil, interactiveMode(ctx.Context, opts)
}

// TODO: This isn't what the reporter is designed for.
// Only using r.Infof() and r.Errorf() to print to stdout & stderr respectively.
r := reporter.NewTableReporter(stdout, stderr, reporter.InfoLevel, false, 0)
maxUpgrades := ctx.Int("apply-top")

strategy := ctx.String("strategy")
Expand Down
13 changes: 6 additions & 7 deletions internal/remediation/in_place.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"deps.dev/util/resolve"
"deps.dev/util/resolve/dep"
"deps.dev/util/semver"
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution"
"github.com/google/osv-scanner/internal/resolution/client"
lf "github.com/google/osv-scanner/internal/resolution/lockfile"
Expand Down Expand Up @@ -135,17 +136,15 @@ func ComputeInPlacePatches(ctx context.Context, cl client.ResolutionClient, grap
continue
}
// Consider vulns affecting packages we don't want to change unfixable
if slices.Contains(opts.AvoidPkgs, vk.Name) {
if opts.UpgradeConfig.Get(vk.Name) == upgrade.None {
result.Unfixable = append(result.Unfixable, vuln)
continue
}
newVK, err := findFixedVersion(ctx, cl, vk.PackageKey, func(newVK resolve.VersionKey) bool {
// Check if this is a disallowed major version bump
if !opts.AllowMajor {
_, diff, err := vk.Semver().Difference(vk.Version, newVK.Version)
if err != nil || diff == semver.DiffMajor {
return false
}
// Check if this is a disallowed version bump
_, diff, err := vk.Semver().Difference(vk.Version, newVK.Version)
if err != nil || !opts.UpgradeConfig.Get(vk.Name).Allows(diff) {
return false
}
// Check if dependent packages are still satisfied by new version
ok, err := vkDependentConstraint[vk].Match(newVK.Version)
Expand Down
7 changes: 4 additions & 3 deletions internal/remediation/in_place_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"deps.dev/util/resolve"
"github.com/google/osv-scanner/internal/remediation"
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution"
"github.com/google/osv-scanner/internal/resolution/client"
"github.com/google/osv-scanner/internal/resolution/clienttest"
Expand Down Expand Up @@ -111,9 +112,9 @@ func TestComputeInPlacePatches(t *testing.T) {
t.Parallel()

basicOpts := remediation.RemediationOptions{
DevDeps: true,
MaxDepth: -1,
AllowMajor: true,
DevDeps: true,
MaxDepth: -1,
UpgradeConfig: upgrade.NewConfig(),
}

tests := []struct {
Expand Down
12 changes: 5 additions & 7 deletions internal/remediation/override.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (

"deps.dev/util/resolve"
"deps.dev/util/resolve/dep"
"deps.dev/util/semver"
"github.com/google/osv-scanner/internal/manifest"
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution"
"github.com/google/osv-scanner/internal/resolution/client"
resolutionmanifest "github.com/google/osv-scanner/internal/resolution/manifest"
Expand Down Expand Up @@ -169,7 +169,7 @@ func overridePatchVulns(ctx context.Context, cl client.ResolutionClient, result
// For each VersionKey, try fix as many of the vulns affecting it as possible.
for vk, vulnerabilities := range vkVulns {
// Consider vulns affecting packages we don't want to change unfixable
if slices.Contains(opts.AvoidPkgs, vk.Name) {
if opts.UpgradeConfig.Get(vk.Name) == upgrade.None {
continue
}

Expand All @@ -182,11 +182,9 @@ func overridePatchVulns(ctx context.Context, cl client.ResolutionClient, result

// Find the minimal greater version that fixes as many vulnerabilities as possible.
for _, ver := range versions {
if !opts.AllowMajor {
// Major version updates are not allowed - break if we've encountered a major update.
if _, diff, _ := vk.System.Semver().Difference(vk.Version, ver.Version); diff == semver.DiffMajor {
break
}
// Break if we've encountered a disallowed version update.
if _, diff, _ := vk.System.Semver().Difference(vk.Version, ver.Version); !opts.UpgradeConfig.Get(vk.Name).Allows(diff) {
break
}

// Count the remaining known vulns that affect this version.
Expand Down
7 changes: 4 additions & 3 deletions internal/remediation/override_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@ import (
"testing"

"github.com/google/osv-scanner/internal/remediation"
"github.com/google/osv-scanner/internal/remediation/upgrade"
)

func TestComputeOverridePatches(t *testing.T) {
t.Parallel()

basicOpts := remediation.RemediationOptions{
DevDeps: true,
MaxDepth: -1,
AllowMajor: true,
DevDeps: true,
MaxDepth: -1,
UpgradeConfig: upgrade.NewConfig(),
}

tests := []struct {
Expand Down
5 changes: 3 additions & 2 deletions internal/remediation/relax.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"deps.dev/util/resolve"
"github.com/google/osv-scanner/internal/remediation/relax"
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution"
"github.com/google/osv-scanner/internal/resolution/client"
)
Expand Down Expand Up @@ -93,10 +94,10 @@ func tryRelaxRemediate(
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) {
if opts.UpgradeConfig.Get(rv.Name) == upgrade.None {
return nil, errRelaxRemediateImpossible
}
newVer, ok := relaxer.Relax(ctx, cl, rv, opts.AllowMajor)
newVer, ok := relaxer.Relax(ctx, cl, rv, opts.UpgradeConfig)
if !ok {
return nil, errRelaxRemediateImpossible
}
Expand Down
20 changes: 16 additions & 4 deletions internal/remediation/relax/npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,17 @@ import (

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

type NpmRelaxer struct{}

func (r NpmRelaxer) Relax(ctx context.Context, cl resolve.Client, req resolve.RequirementVersion, allowMajor bool) (resolve.RequirementVersion, bool) {
func (r NpmRelaxer) Relax(ctx context.Context, cl resolve.Client, req resolve.RequirementVersion, config upgrade.Config) (resolve.RequirementVersion, bool) {
configLevel := config.Get(req.Name)
if configLevel == upgrade.None {
return req, false
}

c, err := semver.NPM.ParseConstraint(req.Version)
if err != nil {
// The specified version is not a valid semver constraint
Expand Down Expand Up @@ -77,10 +83,10 @@ func (r NpmRelaxer) Relax(ctx context.Context, cl resolve.Client, req resolve.Re

cmpVer := vers[lastIdx]
_, diff, _ := semver.NPM.Difference(cmpVer, vers[nextIdx])
if !configLevel.Allows(diff) {
return req, false
}
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
Expand All @@ -95,6 +101,12 @@ func (r NpmRelaxer) Relax(ctx context.Context, cl resolve.Client, req resolve.Re
if err != nil {
continue
}

// If we've exceeded our allowed upgrade level, stop looking.
if !configLevel.Allows(d) {
break
}

// DiffMajor < DiffMinor < DiffPatch < DiffPrerelease
// So if d is less than the original diff, it represents a larger change
if d < diff {
Expand Down
Loading

0 comments on commit 16f103f

Please sign in to comment.