sonatype-nexus-community / ahab

ahab is a tool to check for vulnerabilities in your apt, apk, or yum powered operating systems, powered by Sonatype OSS Index.
Apache License 2.0
66 stars 17 forks source link

Cleanup references to os #29

Open zendern opened 4 years ago

zendern commented 4 years ago

Thanks for creating an issue! Please fill out this form so we can be sure to have all the information we need, and to minimize back and forth.

This is much less important now that auto detection is a thing but I would like to possibly do the following things.

  1. deprecate the -os option
  2. add a new -pm --package-manager option that allows for you to pass in yum, dkpg, apt, dnf, etc etc.
  3. update all code paths that refer to os to no longer do that

See these comments here https://github.com/sonatype-nexus-community/ahab/blob/4279556b8f0d862fe1cee966df3bb2e78a19252b/packages/detector.go#L40

cc @bhamail / @DarthHater / @ken-duck

deadlysyn commented 4 years ago

If we don't already have WIP for this, I can give it a shot. I am interested in learning more about Ahab to see if I can plug it into my containerized pipelines.

zendern commented 4 years ago

@deadlysyn go for it. Im not aware on anyone working this yet. Please reach out with any questions or open a draft PR if you get that far and we can work through it together. 👍

deadlysyn commented 3 years ago

i've gone through chase/iq.go (mostly) and s/os/package-manager along with related cleanup...wanted to get your thoughts on a couple questions.

context:

the former os or operating value got passed into package methods. that looked like:

// package.go
type IPackage interface {
    ExtractPurlsFromProjectList(string) []string
}

// apt.go, etc...
type Apt struct {
    ProjectList parse.ProjectList
}

func (a Apt) ExtractPurlsFromProjectList(operating string) (purls []string) {
    for _, s := range a.ProjectList.Projects {
        var purl = fmt.Sprintf("pkg:deb/%s/%s@%s", operating, s.Name, s.Version)
        purls = append(purls, purl)
    }
    return
}

passing operating into ExtractPurlsFromProjectList makes sense to satisfy the interface, and looks normal in the apt example (or dnf/yum) but apk.go does not seem to use it at all:

// apk.go
type Apk struct {
    ProjectList parse.ProjectList
}

func (a Apk) ExtractPurlsFromProjectList(operating string) (purls []string) {
    for _, s := range a.ProjectList.Projects {
        var purl = fmt.Sprintf("pkg:alpine/%s@%s", s.Name, s.Version)
        purls = append(purls, purl)
    }
    return
}

is that:

thoughts? TIA!

zendern commented 3 years ago

Re: Os passed in to create the purl path. So its by intention but honestly we effectively have it hardcoded to debian right now. That could possibly be wrong. Since debian purl could in theory be debian or ubuntu based on this documentation.

https://ossindex.sonatype.org/ecosystem/debian

And as you can see alpine and rpm dont have that setup at all. https://ossindex.sonatype.org/ecosystem/alpine https://ossindex.sonatype.org/ecosystem/rpm

Probably a question for @ken-duck why that might be different and if we need to accomodate for ubuntu vs debian since we are not today. Or if there is maybe a feature enhancement where we can just not pass it in like we do for alpine/rpm.

cc @deadlysyn

deadlysyn commented 3 years ago

Thanks for the insight @zendern will wait for @ken-duck to provide feedback, but also going to workup an initial PR and link in so there's actual changes/code to review. I'll link that in here, hopefully later today.

Thanks to your pointers, I also found https://github.com/package-url/purl-spec#purl which, if I read correctly, says the OS (or package manager) is the "type" which is required (namespace is optional, but we have stuff like pkg:alpine so I think alpine is "type" from the spec). If that's a correct statement, then we should consistently provide the type field in our output, whether OS or package manager...and from the same doc, using the package manager as the type will be more correct (they give examples of npm, nuget, etc).

Since it's the weekend I'm going to include some of this interpretation in the initial PR, and can fixup as needed after feedback.

deadlysyn commented 3 years ago

So excited to get this submitted, I mistakenly thought Hacktoberfest submissions were already being counted (since they opened up signup page last week). :-D Oh well, at least there are more to do. Let me know if the interface/purl changes are too crazy. TIA for review.