Skip to content

refactor(go-website): connect vulnerability page to database - #5838

Open
michaelkedar wants to merge 5 commits into
masterfrom
🕸️🚫🐍-💽

Hidden character warning

The head ref may contain hidden characters: "\ud83d\udd78\ufe0f\ud83d\udeab\ud83d\udc0d-\ud83d\udcbd"
Open

refactor(go-website): connect vulnerability page to database#5838
michaelkedar wants to merge 5 commits into
masterfrom
🕸️🚫🐍-💽

Conversation

@michaelkedar

Copy link
Copy Markdown
Member

Populate the individual vulnerability pages from the GCS/datastore.

  • Plumbed database Stores to server for reading vuln data from GCS.
  • Re-implementent upstream/downstream computation in go.
  • Since human links are using jinja template strings, I had to import a library to render these in go. I want to migrate these to go-native templates, but have to wait until the python website is no longer used to change the fields on the SourceRepository.

Stack created with GitHub Stacks CLIGive Feedback 💬

@another-rex another-rex left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with some nits.

mockDownstream := ComputedHierarchy{}
var mu sync.Mutex
knownIDs := make(map[string]bool)
g, ctx := errgroup.WithContext(r.Context())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set a limit here, we have some very big alias/upstream/related groups

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

arbitrarily chose a limit of 20 here

var vulnIDRegex = regexp.MustCompile(`^[a-zA-Z0-9:_.-]+$`)

// handleVulnerabilityDetails handles rendering the vulnerability details page or raw JSON for /vulnerability/{vuln_id}.
func (s *Server) handleVulnerabilityDetails(w http.ResponseWriter, r *http.Request) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No vulnIDRegex check here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's probably not super necessary to have it here - this specifically for the osv.dev/vulnerability/[ID] page, and it might be hypothetically possible we end up with an ID that doesn't match the regex in the database (maybe some old withdrawn vuln or something) and we probably don't want it to be impossible to navigate to.

return nil, fmt.Errorf("failed to get upstream group: %w", err)
return nil, err
}
upstream := make([]string, len(upstreamGroup.UpstreamIDs))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think we can use slices.Clone here. I think this pattern is also used elsewhere so can replace those too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

replaced most instances of that pattern (even if it's unrelated to this PR 🤷)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants