Guided remediation: Interactive mode TUI

cmd/osv-scanner/fix/interactive.go

@@ -6,8 +6,6 @@ import (
 	tea "github.com/charmbracelet/bubbletea"
 )
 
-// TODO: this file is too big
-
 // TODO: currently, it's impossible to undo commands
 // Need to think about how to support this
 

cmd/osv-scanner/fix/main.go

@@ -3,6 +3,7 @@ package fix
 import (
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"
 
 	"github.com/google/osv-scanner/internal/remediation"
@@ -12,6 +13,7 @@ import (
 	"github.com/google/osv-scanner/pkg/depsdev"
 	"github.com/google/osv-scanner/pkg/reporter"
 	"github.com/urfave/cli/v2"
+	"golang.org/x/term"
 )
 
 const (
@@ -67,10 +69,9 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
 			},
 
 			&cli.BoolFlag{
-				Name:   "non-interactive",
-				Usage:  "run in the non-interactive mode",
-				Value:  true, //!term.IsTerminal(int(os.Stdin.Fd())), // Default to non-interactive if not being run in a terminal
-				Hidden: true, // TODO: un-hide when interactive mode is added
+				Name:  "non-interactive",
+				Usage: "run in the non-interactive mode",
+				Value: !term.IsTerminal(int(os.Stdin.Fd())), // Default to non-interactive if not being run in a terminal
 			},
 			&cli.StringFlag{
 				Category: autoModeCategory,
@@ -223,7 +224,7 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
 	}
 
 	if !ctx.Bool("non-interactive") {
-		return nil, fmt.Errorf("not implemented")
+		return nil, interactiveMode(ctx.Context, opts)
 	}
 
 	// TODO: This isn't what the reporter is designed for.

cmd/osv-scanner/fix/model.go

@@ -20,9 +20,10 @@ import (
 )
 
 type model struct {
-	ctx           context.Context         // Context, mostly used in sos functions
+	//nolint:containedctx
+	ctx           context.Context         // Context, mostly used in deps.dev functions
 	options       osvFixOptions           // options, from command line
-	cl            client.ResolutionClient // graph client used for sos functions
+	cl            client.ResolutionClient // graph client used for deps.dev functions
 	lockfileGraph *resolve.Graph
 
 	termWidth  int // width of the whole terminal
@@ -111,6 +112,7 @@ func (m *model) getBorderStyles() (lipgloss.Style, lipgloss.Style) {
 		m.infoViewStyle.BorderForeground(tui.ColorDisabled)
 		m.mainViewStyle.UnsetBorderForeground()
 	}
+
 	return m.mainViewStyle, m.infoViewStyle
 }
 
@@ -163,9 +165,9 @@ func (m model) View() string {
 }
 
 type modelState interface {
-	Init(model) tea.Cmd
-	Update(model, tea.Msg) (tea.Model, tea.Cmd)
-	View(model) string
+	Init(m model) tea.Cmd
+	Update(m model, msg tea.Msg) (tea.Model, tea.Cmd)
+	View(m model) string
 	Resize(w, h int)
 
 	InfoView() string
@@ -180,16 +182,6 @@ type inPlaceResolutionMsg struct {
 }
 
 func doInPlaceResolution(ctx context.Context, cl client.ResolutionClient, opts osvFixOptions) tea.Msg {
-	mf, err := osvLockfile.OpenLocalDepFile(opts.Manifest)
-	if err != nil {
-		return inPlaceResolutionMsg{err: err}
-	}
-	defer mf.Close()
-	m, err := opts.ManifestRW.Read(mf)
-	if err != nil {
-		return inPlaceResolutionMsg{err: err}
-	}
-
 	lf, err := osvLockfile.OpenLocalDepFile(opts.Lockfile)
 	if err != nil {
 		return inPlaceResolutionMsg{err: err}
@@ -199,9 +191,8 @@ func doInPlaceResolution(ctx context.Context, cl client.ResolutionClient, opts o
 	if err != nil {
 		return inPlaceResolutionMsg{err: err}
 	}
-
-	cl.PreFetch(ctx, m.Requirements, m.FilePath)
 	res, err := remediation.ComputeInPlacePatches(ctx, cl, g, opts.RemediationOptions)
+
 	return inPlaceResolutionMsg{res, g, err}
 }
 
@@ -211,8 +202,18 @@ type doRelockMsg struct {
 }
 
 func doRelock(ctx context.Context, cl client.ResolutionClient, m manif.Manifest, matchFn func(resolution.ResolutionVuln) bool) tea.Msg {
-	res, err := computeRelockVulns(ctx, cl, m, matchFn)
-	return doRelockMsg{res, err}
+	res, err := resolution.Resolve(ctx, cl, m)
+	if err != nil {
+		return doRelockMsg{nil, err}
+	}
+
+	if err := cl.WriteCache(m.FilePath); err != nil {
+		return doRelockMsg{nil, err}
+	}
+
+	res.FilterVulns(matchFn)
+
+	return doRelockMsg{res, nil}
 }
 
 func doInitialRelock(ctx context.Context, opts osvFixOptions) tea.Msg {
@@ -226,6 +227,7 @@ func doInitialRelock(ctx context.Context, opts osvFixOptions) tea.Msg {
 		return doRelockMsg{err: err}
 	}
 	opts.Client.PreFetch(ctx, m.Requirements, m.FilePath)
+
 	return doRelock(ctx, opts.Client, m, opts.MatchVuln)
 }
 
@@ -245,6 +247,7 @@ func resolutionErrorView(res *resolution.ResolutionResult, errs []resolution.Res
 	s := strings.Builder{}
 	s.WriteString("The following errors were encountered during resolution which may impact results:\n")
 	s.WriteString(resolutionErrorString(res, errs))
+
 	return infoStringView(s.String())
 }
 

cmd/osv-scanner/fix/noninteractive.go

@@ -3,6 +3,7 @@ package fix
 import (
 	"context"
 	"fmt"
+	"os"
 	"slices"
 	"strings"
 
@@ -136,7 +137,8 @@ func autoRelock(ctx context.Context, r reporter.Reporter, opts osvFixOptions, ma
 		return nil
 	}
 
-	depPatches, nFixed, nUnfixable := autoChooseRelockPatches(allPatches, maxUpgrades)
+	depPatches, nFixed := autoChooseRelockPatches(allPatches, maxUpgrades)
+	nUnfixable := len(relockUnfixableVulns(allPatches))
 	r.Infof("Can fix %d/%d matching vulnerabilities by changing %d dependencies\n", nFixed, totalVulns, len(depPatches))
 	for _, p := range depPatches {
 		r.Infof("UPGRADED-PACKAGE: %s,%s,%s\n", p.Pkg.Name, p.OrigRequire, p.NewRequire)
@@ -155,31 +157,44 @@ func autoRelock(ctx context.Context, r reporter.Reporter, opts osvFixOptions, ma
 		// We only recreate the lockfile if we know a lockfile already exists
 		// or we've been given a command to run.
 		r.Infof("Shelling out to regenerate lockfile...\n")
-		return regenerateLockfile(r, opts)
+		cmd, err := regenerateLockfileCmd(opts)
+		if err != nil {
+			return err
+		}
+		// ideally I'd have the reporter's stdout/stderr here...
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+		r.Infof("Executing `%s`...\n", cmd)
+		err = cmd.Run()
+		if err == nil {
+			return nil
+		}
+		if opts.RelockCmd != "" {
+			return err
+		}
+		r.Warnf("Install failed. Trying again with `--legacy-peer-deps`...\n")
+		cmd, err = regenerateLockfileCmd(opts)
+		if err != nil {
+			return err
+		}
+		cmd.Args = append(cmd.Args, "--legacy-peer-deps")
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+
+		return cmd.Run()
 	}
 
 	return nil
 }
 
-// returns the top {maxUpgrades} compatible patches, the number of vulns fixed, and the number unfixable vulns
+// returns the top {maxUpgrades} compatible patches, and the number of vulns fixed
 // if maxUpgrades is < 0, do as many patches as possible
-func autoChooseRelockPatches(diffs []resolution.ResolutionDiff, maxUpgrades int) ([]manifest.DependencyPatch, int, int) {
-	// Treat every original vulnerability as unfixable until we see a patch that removes it
-	unfixableVulnIDs := make(map[string]struct{})
-	for _, v := range diffs[0].Original.Vulns {
-		unfixableVulnIDs[v.Vulnerability.ID] = struct{}{}
-	}
-
+func autoChooseRelockPatches(diffs []resolution.ResolutionDiff, maxUpgrades int) ([]manifest.DependencyPatch, int) {
 	var patches []manifest.DependencyPatch
 	pkgChanged := make(map[resolve.VersionKey]bool) // dependencies we've already applied a patch to
 	numFixed := 0
 
 	for _, diff := range diffs {
-		// remove all the vulns fixed by the patch from the set of unfixables
-		for _, v := range diff.RemovedVulns {
-			delete(unfixableVulnIDs, v.Vulnerability.ID)
-		}
-
 		// If we are not picking any more patches, or this patch is incompatible with existing patches, skip adding it to the patch list.
 		// A patch is incompatible if any of its changed packages have already been changed by an existing patch.
 		if maxUpgrades == 0 || slices.ContainsFunc(diff.Deps, func(dp manifest.DependencyPatch) bool {
@@ -197,7 +212,30 @@ func autoChooseRelockPatches(diffs []resolution.ResolutionDiff, maxUpgrades int)
 		maxUpgrades--
 	}
 
-	return patches, numFixed, len(unfixableVulnIDs)
+	return patches, numFixed
+}
+
+func relockUnfixableVulns(diffs []resolution.ResolutionDiff) []*resolution.ResolutionVuln {
+	if len(diffs) == 0 {
+		return nil
+	}
+	// find every vuln ID fixed in any patch
+	fixableVulnIDs := make(map[string]struct{})
+	for _, diff := range diffs {
+		for _, v := range diff.RemovedVulns {
+			fixableVulnIDs[v.Vulnerability.ID] = struct{}{}
+		}
+	}
+
+	// select only vulns that aren't fixed in any patch
+	var unfixable []*resolution.ResolutionVuln
+	for i, v := range diffs[0].Original.Vulns {
+		if _, ok := fixableVulnIDs[v.Vulnerability.ID]; !ok {
+			unfixable = append(unfixable, &diffs[0].Original.Vulns[i])
+		}
+	}
+
+	return unfixable
 }
 
 func resolutionErrorString(res *resolution.ResolutionResult, errs []resolution.ResolutionError) string {

cmd/osv-scanner/fix/regen_lockfile.go

@@ -5,19 +5,17 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
-
-	"github.com/google/osv-scanner/pkg/reporter"
 )
 
-func regenerateLockfile(r reporter.Reporter, opts osvFixOptions) error {
+func regenerateLockfileCmd(opts osvFixOptions) (*exec.Cmd, error) {
 	// TODO: this is npm-specific and hacky
 	// delete existing package-lock & node_modules directory to force npm to do a clean install
 	dir := filepath.Dir(opts.Manifest)
 	if err := os.RemoveAll(filepath.Join(dir, "package-lock.json")); err != nil {
-		return err
+		return nil, err
 	}
 	if err := os.RemoveAll(filepath.Join(dir, "node_modules")); err != nil {
-		return err
+		return nil, err
 	}
 	// TODO: need to also remove node_modules/ in workspace packages
 
@@ -28,20 +26,6 @@ func regenerateLockfile(r reporter.Reporter, opts osvFixOptions) error {
 	cmdParts := strings.Split(cmd, " ")
 	c := exec.Command(cmdParts[0], cmdParts[1:]...) //nolint:gosec
 	c.Dir = dir
-	// ideally I'd have the reporter's stdout/stderr here...
-	c.Stdout = os.Stdout
-	c.Stderr = os.Stderr
-	r.Infof("Executing `%s`...\n", cmd)
-	err := c.Run()
-	if err != nil && opts.RelockCmd == "" {
-		r.Warnf("Install failed. Trying again with `--legacy-peer-deps`...\n")
-		cmdParts = append(cmdParts, "--legacy-peer-deps")
-		c := exec.Command(cmdParts[0], cmdParts[1:]...) //nolint:gosec
-		c.Dir = dir
-		c.Stdout = os.Stdout
-		c.Stderr = os.Stderr
-		err = c.Run()
-	}
 
-	return err
+	return c, nil
 }

cmd/osv-scanner/fix/state-choose-in-place-patches.go

@@ -140,6 +140,7 @@ func (st *stateChooseInPlacePatches) updateTableRows(m model) {
 }
 
 func (st *stateChooseInPlacePatches) toggleSelection(idx int) {
+	// TODO: Prevent selection of multiple (incompatible) patches for same package version
 	i := st.patchIdx[idx]
 	st.stateInPlace.selectedChanges[i] = !st.stateInPlace.selectedChanges[i]
 }
@@ -148,6 +149,7 @@ func (st *stateChooseInPlacePatches) currentInfoView() (view tui.ViewModel, canF
 	if c := st.table.Cursor(); c > 0 && c < len(st.table.Rows())-1 {
 		return st.vulnsInfos[c-1], true
 	}
+
 	return emptyInfoView, false
 }