From e35a80c2f8840519ec8f9ed7791d8665a3817c41 Mon Sep 17 00:00:00 2001 From: Michael Kedar Date: Wed, 22 Jan 2025 09:20:08 +1100 Subject: [PATCH] fix(guided remediation): remove `--relock-cmd` flag (#1517) `--relock-cmd` let you override the `npm install` command with an arbitrary command to regenerate the `package-lock.json` after doing a relax. We probably don't want to have that option in osv-scalibr when we migrate this there, so I've removed it now before the v2 release of osv-scanner. If someone actually needs this, they can run the command themselves outside of osv-scanner. --- cmd/osv-scanner/fix/main.go | 6 ------ cmd/osv-scanner/fix/noninteractive.go | 6 ++---- cmd/osv-scanner/fix/regen_lockfile.go | 8 +------- cmd/osv-scanner/fix/state-relock-result.go | 4 ++-- docs/guided-remediation.md | 5 +---- 5 files changed, 6 insertions(+), 23 deletions(-) diff --git a/cmd/osv-scanner/fix/main.go b/cmd/osv-scanner/fix/main.go index 7d9f81b7d5..e946b38676 100644 --- a/cmd/osv-scanner/fix/main.go +++ b/cmd/osv-scanner/fix/main.go @@ -53,7 +53,6 @@ type osvFixOptions struct { ManifestRW manifest.ReadWriter Lockfile string LockfileRW lockfile.ReadWriter - RelockCmd string NoIntroduce bool } @@ -91,10 +90,6 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command { Name: "maven-registry", Usage: "URL of the default Maven registry to fetch metadata", }, - &cli.StringFlag{ - Name: "relock-cmd", - Usage: "command to run to regenerate lockfile on disk after changing the manifest", - }, &cli.BoolFlag{ Name: "non-interactive", @@ -319,7 +314,6 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro }, Manifest: ctx.String("manifest"), Lockfile: ctx.String("lockfile"), - RelockCmd: ctx.String("relock-cmd"), NoIntroduce: ctx.Bool("no-introduce"), } diff --git a/cmd/osv-scanner/fix/noninteractive.go b/cmd/osv-scanner/fix/noninteractive.go index a77688c3e0..6c5a02cd88 100644 --- a/cmd/osv-scanner/fix/noninteractive.go +++ b/cmd/osv-scanner/fix/noninteractive.go @@ -182,7 +182,7 @@ func autoRelax(ctx context.Context, r *outputReporter, opts osvFixOptions, maxUp return err } - if opts.Lockfile != "" || opts.RelockCmd != "" { + if opts.Lockfile != "" { // 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") @@ -198,9 +198,7 @@ func autoRelax(ctx context.Context, r *outputReporter, opts osvFixOptions, maxUp 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 { diff --git a/cmd/osv-scanner/fix/regen_lockfile.go b/cmd/osv-scanner/fix/regen_lockfile.go index 13f8a14442..3c5b139275 100644 --- a/cmd/osv-scanner/fix/regen_lockfile.go +++ b/cmd/osv-scanner/fix/regen_lockfile.go @@ -4,7 +4,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" ) func regenerateLockfileCmd(opts osvFixOptions) (*exec.Cmd, error) { @@ -19,12 +18,7 @@ func regenerateLockfileCmd(opts osvFixOptions) (*exec.Cmd, error) { } // TODO: need to also remove node_modules/ in workspace packages - cmd := opts.RelockCmd - if cmd == "" { - cmd = "npm install --package-lock-only" - } - cmdParts := strings.Split(cmd, " ") - c := exec.Command(cmdParts[0], cmdParts[1:]...) //nolint:gosec + c := exec.Command("npm", "install", "--package-lock-only") c.Dir = dir return c, nil diff --git a/cmd/osv-scanner/fix/state-relock-result.go b/cmd/osv-scanner/fix/state-relock-result.go index eddab13b8c..1c9143e18a 100644 --- a/cmd/osv-scanner/fix/state-relock-result.go +++ b/cmd/osv-scanner/fix/state-relock-result.go @@ -514,7 +514,7 @@ func (st *stateRelockResult) write(m model) tea.Msg { return writeMsg{err} } - if m.options.Lockfile == "" && m.options.RelockCmd == "" { + if m.options.Lockfile == "" { // TODO: there's no user feedback to show this was successful return writeMsg{nil} } @@ -525,7 +525,7 @@ func (st *stateRelockResult) write(m model) tea.Msg { } return tea.ExecProcess(c, func(err error) tea.Msg { - if err != nil && m.options.RelockCmd == "" { + if err != nil { // try again with "--legacy-peer-deps" c, err := regenerateLockfileCmd(m.options) if err != nil { diff --git a/docs/guided-remediation.md b/docs/guided-remediation.md index 4deaea6083..84e8782279 100644 --- a/docs/guided-remediation.md +++ b/docs/guided-remediation.md @@ -683,10 +683,7 @@ The relaxation patches are presented in order of effectiveness, with patches tha If you wish to apply your current relock & relaxation changes, select the "Write" option to update your manifest file with the new requirements and regenerate your lockfile (if provided). {: .note } - -> The `package-lock.json` file is regenerated by first deleting the existing `package-lock.json` and `node_modules/` directory, then running `npm install --package-lock-only`. This recreates the lockfile but does not install the `node_modules/` dependencies. Run `npm ci` separately to install the dependencies. -> -> The `--relock-cmd` flag can be used to change the executed install command. +The `package-lock.json` file is regenerated by first deleting the existing `package-lock.json` and `node_modules/` directory, then running `npm install --package-lock-only`. This recreates the lockfile but does not install the `node_modules/` dependencies. Run `npm ci` separately to install the dependencies. ### Override dependency versions