feat(guided remediation): remediate unresolved dependency management …

…vulns (#1235)

Adds functionality to allow guided remediation to fix vulns in
`dependencyManagement` dependencies that do not appear in the resolved
dependency graph of the POM - useful for 'remediating' POMs without any
actual dependencies.

I've accomplished this by checking if each of the original management
dependencies (*excluding* those inherited from parents) appear in the
graph after the initial resolution. If they're missing, I add them to
the graph as direct dependencies (not resolving their transitive
dependencies).

This behaviour is disabled by default, and I've added a
`--maven-fix-management` flag to enable it. I was going to try combine
this and `--ignore-dev` into a `--groups` flag but it seemed like it
would be a bit too complicated.

cmd/osv-scanner/fix/main.go

@@ -11,6 +11,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/datasource"
 	"github.com/google/osv-scanner/internal/resolution/lockfile"
@@ -160,6 +161,11 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
 				Name:     "ignore-dev",
 				Usage:    "ignore vulnerabilities affecting only development dependencies",
 			},
+			&cli.BoolFlag{
+				Category: vulnCategory,
+				Name:     "maven-fix-management",
+				Usage:    "(pom.xml) also remediate vulnerabilities in dependencyManagement packages that do not appear in the resolved dependency graph",
+			},
 		},
 		Action: func(ctx *cli.Context) error {
 			var err error
@@ -233,6 +239,9 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
 
 	opts := osvFixOptions{
 		RemediationOptions: remediation.RemediationOptions{
+			ResolveOpts: resolution.ResolveOpts{
+				MavenManagement: ctx.Bool("maven-fix-management"),
+			},
 			IgnoreVulns:   ctx.StringSlice("ignore-vulns"),
 			ExplicitVulns: ctx.StringSlice("vulns"),
 			DevDeps:       !ctx.Bool("ignore-dev"),

cmd/osv-scanner/fix/model.go

@@ -201,8 +201,8 @@ type doRelockMsg struct {
 	err error
 }
 
-func doRelock(ctx context.Context, cl client.ResolutionClient, m manif.Manifest, matchFn func(resolution.ResolutionVuln) bool) tea.Msg {
-	res, err := resolution.Resolve(ctx, cl, m)
+func doRelock(ctx context.Context, cl client.ResolutionClient, m manif.Manifest, opts resolution.ResolveOpts, matchFn func(resolution.ResolutionVuln) bool) tea.Msg {
+	res, err := resolution.Resolve(ctx, cl, m, opts)
 	if err != nil {
 		return doRelockMsg{nil, err}
 	}
@@ -228,7 +228,7 @@ func doInitialRelock(ctx context.Context, opts osvFixOptions) tea.Msg {
 	}
 	opts.Client.PreFetch(ctx, m.Requirements, m.FilePath)
 
-	return doRelock(ctx, opts.Client, m, opts.MatchVuln)
+	return doRelock(ctx, opts.Client, m, opts.ResolveOpts, opts.MatchVuln)
 }
 
 // tui.ViewModel for showing non-interactive strings

cmd/osv-scanner/fix/noninteractive.go

@@ -130,7 +130,7 @@ func autoRelock(ctx context.Context, r reporter.Reporter, opts osvFixOptions, ma
 	}
 
 	opts.Client.PreFetch(ctx, manif.Requirements, manif.FilePath)
-	res, err := resolution.Resolve(ctx, opts.Client, manif)
+	res, err := resolution.Resolve(ctx, opts.Client, manif, opts.ResolveOpts)
 	if err != nil {
 		return err
 	}
@@ -295,7 +295,7 @@ func autoOverride(ctx context.Context, r reporter.Reporter, opts osvFixOptions,
 	}
 
 	opts.Client.PreFetch(ctx, manif.Requirements, manif.FilePath)
-	res, err := resolution.Resolve(ctx, opts.Client, manif)
+	res, err := resolution.Resolve(ctx, opts.Client, manif, opts.ResolveOpts)
 	if err != nil {
 		return err
 	}

cmd/osv-scanner/fix/state-relock-result.go

@@ -305,7 +305,7 @@ func (st *stateRelockResult) relaxChoice(m model) (model, tea.Cmd) {
 	st.currRes = nil
 
 	return m, func() tea.Msg {
-		return doRelock(m.ctx, m.cl, manifest, m.options.MatchVuln)
+		return doRelock(m.ctx, m.cl, manifest, m.options.ResolveOpts, m.options.MatchVuln)
 	}
 }
 

internal/remediation/__snapshots__/testhelpers_test.snap

@@ -3,6 +3,158 @@
 []
 ---
 
+[TestComputeOverridePatches/maven-management-only - 1]
+[
+  {
+    "Patch": {
+      "Deps": [
+        {
+          "Pkg": {
+            "System": 6,
+            "Name": "org.apache.commons:commons-configuration2"
+          },
+          "Type": {},
+          "OrigRequire": "",
+          "NewRequire": "2.10.1",
+          "OrigResolved": "2.8.0",
+          "NewResolved": "2.10.1"
+        }
+      ],
+      "EcosystemSpecific": null
+    },
+    "RemovedVulns": [
+      {
+        "ID": "GHSA-9w38-p64v-xpmv",
+        "AffectedNodes": [
+          16
+        ]
+      },
+      {
+        "ID": "GHSA-xjp4-hw94-mvp5",
+        "AffectedNodes": [
+          16
+        ]
+      }
+    ],
+    "AddedVulns": []
+  },
+  {
+    "Patch": {
+      "Deps": [
+        {
+          "Pkg": {
+            "System": 6,
+            "Name": "org.apache.shiro:shiro-web"
+          },
+          "Type": {},
+          "OrigRequire": "",
+          "NewRequire": "1.13.0",
+          "OrigResolved": "1.10.0",
+          "NewResolved": "1.13.0"
+        }
+      ],
+      "EcosystemSpecific": null
+    },
+    "RemovedVulns": [
+      {
+        "ID": "GHSA-hhw5-c326-822h",
+        "AffectedNodes": [
+          23
+        ]
+      },
+      {
+        "ID": "GHSA-pmhc-2g4f-85cg",
+        "AffectedNodes": [
+          23
+        ]
+      }
+    ],
+    "AddedVulns": []
+  },
+  {
+    "Patch": {
+      "Deps": [
+        {
+          "Pkg": {
+            "System": 6,
+            "Name": "org.apache.shiro:shiro-core"
+          },
+          "Type": {},
+          "OrigRequire": "",
+          "NewRequire": "1.13.0",
+          "OrigResolved": "1.10.0",
+          "NewResolved": "1.13.0"
+        }
+      ],
+      "EcosystemSpecific": null
+    },
+    "RemovedVulns": [
+      {
+        "ID": "GHSA-jc7h-c423-mpjc",
+        "AffectedNodes": [
+          22
+        ]
+      }
+    ],
+    "AddedVulns": []
+  },
+  {
+    "Patch": {
+      "Deps": [
+        {
+          "Pkg": {
+            "System": 6,
+            "Name": "org.apache.shiro:shiro-web"
+          },
+          "Type": {},
+          "OrigRequire": "",
+          "NewRequire": "1.12.0",
+          "OrigResolved": "1.10.0",
+          "NewResolved": "1.12.0"
+        }
+      ],
+      "EcosystemSpecific": null
+    },
+    "RemovedVulns": [
+      {
+        "ID": "GHSA-pmhc-2g4f-85cg",
+        "AffectedNodes": [
+          23
+        ]
+      }
+    ],
+    "AddedVulns": []
+  },
+  {
+    "Patch": {
+      "Deps": [
+        {
+          "Pkg": {
+            "System": 6,
+            "Name": "org.apache.thrift:libthrift"
+          },
+          "Type": {},
+          "OrigRequire": "",
+          "NewRequire": "0.14.0",
+          "OrigResolved": "0.13.0",
+          "NewResolved": "0.14.0"
+        }
+      ],
+      "EcosystemSpecific": null
+    },
+    "RemovedVulns": [
+      {
+        "ID": "GHSA-g2fg-mr77-6vrm",
+        "AffectedNodes": [
+          6
+        ]
+      }
+    ],
+    "AddedVulns": []
+  }
+]
+---
+
 [TestComputeOverridePatches/maven-zeppelin-server - 1]
 [
   {

internal/remediation/override.go

@@ -225,7 +225,7 @@ func overridePatchVulns(ctx context.Context, cl client.ResolutionClient, result
 			return nil, nil, err
 		}
 
-		result, err = resolution.Resolve(ctx, cl, newManif)
+		result, err = resolution.Resolve(ctx, cl, newManif, opts.ResolveOpts)
 		if err != nil {
 			return nil, nil, err
 		}

internal/remediation/override_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/google/osv-scanner/internal/remediation"
 	"github.com/google/osv-scanner/internal/remediation/upgrade"
+	"github.com/google/osv-scanner/internal/resolution"
 )
 
 func TestComputeOverridePatches(t *testing.T) {
@@ -35,6 +36,19 @@ func TestComputeOverridePatches(t *testing.T) {
 			manifestPath: "./fixtures/maven-classifier/pom.xml",
 			opts:         basicOpts,
 		},
+		{
+			name:         "maven-management-only",
+			universePath: "./fixtures/zeppelin-server/universe.yaml",
+			manifestPath: "./fixtures/zeppelin-server/parent/pom.xml",
+			opts: remediation.RemediationOptions{
+				ResolveOpts: resolution.ResolveOpts{
+					MavenManagement: true,
+				},
+				DevDeps:       true,
+				MaxDepth:      -1,
+				UpgradeConfig: upgrade.NewConfig(),
+			},
+		},
 		{
 			name:         "workaround-maven-guava-none-to-jre",
 			universePath: "./fixtures/override-workaround/universe.yaml",
@@ -64,7 +78,7 @@ func TestComputeOverridePatches(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			res, cl := parseRemediationFixture(t, tt.universePath, tt.manifestPath)
+			res, cl := parseRemediationFixture(t, tt.universePath, tt.manifestPath, tt.opts.ResolveOpts)
 			res.FilterVulns(tt.opts.MatchVuln)
 			p, err := remediation.ComputeOverridePatches(context.Background(), cl, res, tt.opts)
 			if err != nil {

internal/remediation/relax.go

@@ -105,7 +105,7 @@ func tryRelaxRemediate(
 		}
 
 		// re-resolve relaxed manifest
-		newRes, err = resolution.Resolve(ctx, cl, manif)
+		newRes, err = resolution.Resolve(ctx, cl, manif, opts.ResolveOpts)
 		if err != nil {
 			return nil, err
 		}

internal/remediation/relax_test.go

@@ -34,7 +34,7 @@ func TestComputeRelaxPatches(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			res, cl := parseRemediationFixture(t, tt.universePath, tt.manifestPath)
+			res, cl := parseRemediationFixture(t, tt.universePath, tt.manifestPath, tt.opts.ResolveOpts)
 			res.FilterVulns(tt.opts.MatchVuln)
 			p, err := remediation.ComputeRelaxPatches(context.Background(), cl, res, tt.opts)
 			if err != nil {

internal/remediation/remediation.go

@@ -40,6 +40,7 @@ func SupportsInPlace(l lockfile.LockfileIO) bool {
 }
 
 type RemediationOptions struct {
+	resolution.ResolveOpts
 	IgnoreVulns   []string // Vulnerability IDs to ignore
 	ExplicitVulns []string // If set, only consider these vulnerability IDs & ignore all others
 

internal/remediation/testhelpers_test.go

@@ -16,7 +16,7 @@ import (
 	"golang.org/x/exp/maps"
 )
 
-func parseRemediationFixture(t *testing.T, universePath, manifestPath string) (*resolution.ResolutionResult, client.ResolutionClient) {
+func parseRemediationFixture(t *testing.T, universePath, manifestPath string, opts resolution.ResolveOpts) (*resolution.ResolutionResult, client.ResolutionClient) {
 	t.Helper()
 
 	io, err := manifest.GetManifestIO(manifestPath)
@@ -37,7 +37,7 @@ func parseRemediationFixture(t *testing.T, universePath, manifestPath string) (*
 
 	cl := clienttest.NewMockResolutionClient(t, universePath)
 
-	res, err := resolution.Resolve(context.Background(), cl, m)
+	res, err := resolution.Resolve(context.Background(), cl, m, opts)
 	if err != nil {
 		t.Fatalf("Failed to resolve manifest: %v", err)
 	}