Skip to content

feat(vulnfeeds): rewrite debian first version finder and DSA/DLA/DTSA converters in Go#4961

Merged
jess-lowe merged 26 commits intogoogle:masterfrom
jess-lowe:feat/rewrite-deb-in-go
Mar 25, 2026
Merged

feat(vulnfeeds): rewrite debian first version finder and DSA/DLA/DTSA converters in Go#4961
jess-lowe merged 26 commits intogoogle:masterfrom
jess-lowe:feat/rewrite-deb-in-go

Conversation

@jess-lowe
Copy link
Copy Markdown
Contributor

I have checked whether semantically the new versions are producing the same output as the last, and they are.

Also moved these out of the vulnfeeds/tools directory and into go/cmd/first-version-finder and vulnfeeds/cmd/converters/dsa-dla-dtsa respectively

@jess-lowe jess-lowe requested a review from michaelkedar March 4, 2026 02:25
@jess-lowe
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request rewrites the Debian feed tools from Python to Go, aiming to improve performance and maintainability. A critical security concern has been identified: a potential command injection vulnerability in the run_convert_debian.sh script due to unquoted environment variables. Furthermore, the review highlighted critical issues related to incorrect paths in the Dockerfiles and script arguments, which are likely to cause build and script failures. To enhance robustness, suggestions include adding timeouts to HTTP requests and validating command-line flags. Addressing these points will ensure a solid and reliable Go implementation.

Comment thread go/cmd/first_package_finder/Dockerfile Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/Dockerfile Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/run_convert_debian.sh Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/run_convert_debian.sh Outdated
Comment thread go/cmd/first_package_finder/main.go Outdated
Comment thread go/cmd/first_package_finder/main.go Outdated
Comment thread go/cmd/first_package_finder/main.go Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/main.go Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/main.go
Copy link
Copy Markdown
Member

@michaelkedar michaelkedar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not reviewed the go code yet, but a couple of comments for the infra stuff

Comment thread deployment/clouddeploy/gke-workers/environments/oss-vdb-test/debian-convert.yaml Outdated
Comment thread deployment/clouddeploy/gke-workers/environments/oss-vdb/debian-convert.yaml Outdated
Comment thread go/cmd/first_package_finder/Dockerfile
Comment thread go/cmd/first_package_finder/Dockerfile
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/Dockerfile
@michaelkedar
Copy link
Copy Markdown
Member

Need to update build-and-stage.yaml to build the two new images (and add the new one here)

You'll also need to change the base kubernetes configs to remove the command
and make them use a different named image

@jess-lowe jess-lowe force-pushed the feat/rewrite-deb-in-go branch from b6fcbf7 to 181140f Compare March 5, 2026 03:46
Copy link
Copy Markdown
Member

@michaelkedar michaelkedar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems alright, but this is quite hard to follow with the comments stripped

Comment thread go/cmd/first_package_finder/main.go
Comment thread go/cmd/first_package_finder/main.go Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/main.go Outdated
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/main.go
Comment thread vulnfeeds/cmd/converters/dsa-dla-dtsa/main.go Outdated
@jess-lowe jess-lowe requested a review from michaelkedar March 10, 2026 02:36
michaelkedar
michaelkedar previously approved these changes Mar 10, 2026
Copy link
Copy Markdown
Member

@michaelkedar michaelkedar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems good to me

return generateVulnerabilities(advisories)
}

func cloneRepo(url, dest string) error {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not worth it right now, but maybe this should be using the gitter

@jess-lowe jess-lowe added the await release This PR should be fixed in the next release (Tuesday) label Mar 15, 2026
@jess-lowe jess-lowe merged commit 6fadc4f into google:master Mar 25, 2026
22 checks passed
@jess-lowe jess-lowe deleted the feat/rewrite-deb-in-go branch March 25, 2026 02:23
jess-lowe added a commit that referenced this pull request Mar 26, 2026
Cron job wasn't running as it should since #4961 was merged
tymzd pushed a commit to tymzd/osv.dev that referenced this pull request Apr 13, 2026
… converters in Go (google#4961)

I have checked whether semantically the new versions are producing the
same output as the last, and they are.

Also moved these out of the vulnfeeds/tools directory and into
go/cmd/first-version-finder and vulnfeeds/cmd/converters/dsa-dla-dtsa
respectively
tymzd pushed a commit to tymzd/osv.dev that referenced this pull request Apr 13, 2026
Cron job wasn't running as it should since google#4961 was merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

await release This PR should be fixed in the next release (Tuesday)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants