-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(python): add pypa support #96
Conversation
pypa/pypa.go
Outdated
} | ||
type option func(*options) | ||
|
||
type Pypa struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type Pypa struct { | |
type PyPA struct { |
According to their website, PyPA is correct.
https://www.pypa.io/en/latest/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pypa/pypa.go
Outdated
type options struct { | ||
url string | ||
dir string | ||
retry int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retry
seems to be unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pypa/pypa.go
Outdated
bar := pb.StartNew(len(yamlFiles)) | ||
|
||
for _, file := range yamlFiles { | ||
data, err := ioutil.ReadFile(file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ioutil is deprecated
data, err := ioutil.ReadFile(file) | |
data, err := os.ReadFile(file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pypa/pypa_test.go
Outdated
} | ||
|
||
dir := t.TempDir() | ||
c := NewPypa(WithURL(ts.URL+"/"+tt.inputArchive), WithDir(filepath.Join(dir)), WithRetry(0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to pass afero.NewMemMapFs()
and need to use the filesystem below instead of os.ReadDir
and os.ReadFile
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on our chat we decided not to do it, because of go-getter
incompatibility
pypa/pypa_test.go
Outdated
@@ -0,0 +1,78 @@ | |||
package pypa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
package pypa | |
package pypa_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pypa/pypa_test.go
Outdated
} else { | ||
require.NoError(t, err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} else { | |
require.NoError(t, err) | |
} | |
} | |
require.NoError(t, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
main.go
Outdated
case "pypa": | ||
p := pypa.NewPypa() | ||
if err := p.Update(); err != nil { | ||
return xerrors.Errorf("error in Pypa update: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return xerrors.Errorf("error in Pypa update: %w", err) | |
return xerrors.Errorf("error in PyPA update: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
main.go
Outdated
if err := p.Update(); err != nil { | ||
return xerrors.Errorf("error in Pypa update: %w", err) | ||
} | ||
commitMsg = "Pypa Security Advisories" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commitMsg = "Pypa Security Advisories" | |
commitMsg = "PyPA Security Advisories" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pypa/pypa.go
Outdated
return xerrors.Errorf("failed to download %s: %w", pypa.opts.url, err) | ||
} | ||
|
||
vulnDir := filepath.Join(dir, "advisory-db-main/vulns") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vulnDir := filepath.Join(dir, "advisory-db-main/vulns") | |
vulnDir := filepath.Join(dir, "advisory-db-main", "vulns") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
pypa/pypa.go
Outdated
return xerrors.Errorf("unable to parse yaml %s: %w", file, err) | ||
} | ||
|
||
if err := utils.WriteJSON(pypa.AppFs, pypa.opts.dir, fmt.Sprintf("%s.json", osv.Id), osv); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep the directory structure as much as possible. It means we will have package dirs.
https://github.com/pypa/advisory-db/tree/main/vulns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
const ( | ||
pypaDir = "pypa" | ||
securityTrackerURL = "https://github.com/pypa/advisory-db/archive/refs/heads/main.zip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AndreyLevchenko Could you check if we can use the above URL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@knqyf263 I think I can use the url above, but I need to understand benefits better. For now it looks pretty same to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The benefit is we can have a single implementation for some languages (Python, Go and Rust).
I think we should change this PR and #83 |
@patryk4815 It sounds like a nice idea. Go data source in OSV used to have an issue (golang/vulndb#1) and we needed to parse the repository ourselves. But after having a quick look, I found they might change API. Let me look into OSV carefully. If we can implement |
@knqyf263 Maybe we need wait for "purl" support - google/osv.dev#64 |
@knqyf263, the OSV schema specification should at least be stable now per https://ossf.github.io/osv-schema/#status---2021-09-08. Let me know if there are any issues you're experiencing from the OSV API side and I can try to take a look. I've put quite a significant effort into triage for the PyPA Advisory DB, so definitely keen to help where I can to see it used. |
@westonsteimel Thanks! It looks like PyPA Advisory DB works well for us. We have a problem with Go. |
In favor of #113 |
No description provided.