Skip to content

Commit

Permalink
fix(guided remediation): error on --data-source=native for Maven (#…
Browse files Browse the repository at this point in the history
…1180)

Add a check for the ecosystem & error for unimplemented native registry
clients.
  • Loading branch information
michaelkedar authored Aug 14, 2024
1 parent 8fea3eb commit 0ecee7c
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 22 deletions.
60 changes: 38 additions & 22 deletions cmd/osv-scanner/fix/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"path/filepath"

"deps.dev/util/resolve"
"github.com/google/osv-scanner/internal/remediation"
"github.com/google/osv-scanner/internal/resolution/client"
"github.com/google/osv-scanner/internal/resolution/lockfile"
Expand Down Expand Up @@ -96,7 +97,7 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
return errors.New("override strategy requires manifest file")
}
default:
return fmt.Errorf("unsupported strategy \"%s\" - must be one of: in-place, relock", s)
return fmt.Errorf("unsupported strategy \"%s\" - must be one of: in-place, relock, override", s)
}

return nil
Expand Down Expand Up @@ -182,27 +183,15 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
},
}

switch ctx.String("data-source") {
case "deps.dev":
cl, err := client.NewDepsDevClient(depsdev.DepsdevAPI)
if err != nil {
return nil, err
}
opts.Client.DependencyClient = cl
case "native":
// TODO: determine ecosystem & client from manifest/lockfile
var workDir string
// Prefer to use the manifest's directory if available.
if opts.Manifest != "" {
workDir = filepath.Dir(opts.Manifest)
} else {
workDir = filepath.Dir(opts.Lockfile)
}
cl, err := client.NewNpmRegistryClient(workDir)
system := resolve.UnknownSystem

if opts.Lockfile != "" {
rw, err := lockfile.GetLockfileIO(opts.Lockfile)
if err != nil {
return nil, err
}
opts.Client.DependencyClient = cl
opts.LockfileRW = rw
system = rw.System()
}

if opts.Manifest != "" {
Expand All @@ -211,14 +200,41 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
return nil, err
}
opts.ManifestRW = rw
// Prefer the manifest's system over the lockfile's.
// TODO: make sure they match
system = rw.System()
}

if opts.Lockfile != "" {
rw, err := lockfile.GetLockfileIO(opts.Lockfile)
switch ctx.String("data-source") {
case "deps.dev":
cl, err := client.NewDepsDevClient(depsdev.DepsdevAPI)
if err != nil {
return nil, err
}
opts.LockfileRW = rw
opts.Client.DependencyClient = cl
case "native":
switch system {
case resolve.NPM:
var workDir string
// Prefer to use the manifest's directory if available.
if opts.Manifest != "" {
workDir = filepath.Dir(opts.Manifest)
} else {
workDir = filepath.Dir(opts.Lockfile)
}
cl, err := client.NewNpmRegistryClient(workDir)
if err != nil {
return nil, err
}
opts.Client.DependencyClient = cl
case resolve.Maven:
// TODO: MavenRegistryClient
fallthrough
case resolve.UnknownSystem:
fallthrough
default:
return nil, fmt.Errorf("native data-source currently unsupported for %s ecosystem", system.String())
}
}

if !ctx.Bool("non-interactive") {
Expand Down
2 changes: 2 additions & 0 deletions internal/resolution/lockfile/lockfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ type DependencyPatch struct {
}

type LockfileIO interface {
// System returns which ecosystem this LockfileIO is for.
System() resolve.System
// Read parses a lockfile into a resolved graph
Read(file lockfile.DepFile) (*resolve.Graph, error)
// Write applies the DependencyPatches to the lockfile, with minimal changes to the file.
Expand Down
2 changes: 2 additions & 0 deletions internal/resolution/lockfile/npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (

type NpmLockfileIO struct{}

func (NpmLockfileIO) System() resolve.System { return resolve.NPM }

type npmNodeModule struct {
NodeID resolve.NodeID
Parent *npmNodeModule
Expand Down
2 changes: 2 additions & 0 deletions internal/resolution/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ type ManifestPatch struct {
}

type ManifestIO interface {
// System returns which ecosystem this ManifestIO is for.
System() resolve.System
// Read parses a manifest file into a Manifest, possibly recursively following references to other local manifest files
Read(file lockfile.DepFile) (Manifest, error)
// Write applies the ManifestPatch to the manifest, with minimal changes to the file.
Expand Down
2 changes: 2 additions & 0 deletions internal/resolution/manifest/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type MavenManifestIO struct {
datasource.MavenRegistryAPIClient
}

func (MavenManifestIO) System() resolve.System { return resolve.Maven }

func NewMavenManifestIO() MavenManifestIO {
return MavenManifestIO{
MavenRegistryAPIClient: *datasource.NewMavenRegistryAPIClient(datasource.MavenCentral),
Expand Down
2 changes: 2 additions & 0 deletions internal/resolution/manifest/npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ func npmRequirementKey(requirement resolve.RequirementVersion) RequirementKey {

type NpmManifestIO struct{}

func (NpmManifestIO) System() resolve.System { return resolve.NPM }

type PackageJSON struct {
Name string `json:"name"`
Version string `json:"version"`
Expand Down

0 comments on commit 0ecee7c

Please sign in to comment.