Skip to content

Conversation

@bobmcwhirter
Copy link
Contributor

Provide enough escape-hatches around filterin/sorting to do my evil deeds. Do some evil deeds in SQL to allow sorting/filtering by synthetic average_score on advisories. Do even more evil, by writing entirely too many SQL functions. Lay in appropriate DOWN migration for cvss3 scoring functions.

Provide enough escape-hatches around filterin/sorting to do my evil deeds.
Do some evil deeds in SQL to allow sorting/filtering by synthetic `average_score` on advisories.
Do even more evil, by writing entirely too many SQL functions.
Lay in appropriate DOWN migration for cvss3 scoring functions.
@bobmcwhirter bobmcwhirter requested review from ctron and jcrossley3 June 11, 2024 18:18
@bobmcwhirter
Copy link
Contributor Author

@carlosthe19916 this should allow sorting by average_score (not severity, since that's just a string derived from the score).

Relates to #383

Copy link
Contributor

@jcrossley3 jcrossley3 left a comment

Choose a reason for hiding this comment

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

I feel a little dirty approving this. But I don't have any better ideas.

Comment on lines +53 to +56
// To be able to ORDER or WHERE using a synthetic column, we must first
// SELECT col, extra_col FROM (SELECT col, random as extra_col FROM...)
// which involves mucking about inside the Select<E> to re-target from
// the original underlying table it expects the entity to live in.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little terrifying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently it's normal! We so similar for SELECT COUNT(*) FROM ( <original select> ) on the limiter stuff.

@bobmcwhirter bobmcwhirter added this pull request to the merge queue Jun 11, 2024
Merged via the queue into guacsec:main with commit 26a977b Jun 11, 2024
@bobmcwhirter bobmcwhirter deleted the severity-sql branch June 11, 2024 18:43
@ctron
Copy link
Contributor

ctron commented Jun 12, 2024

So we are doing all this work just to avoid having an intermediate search model?

@mrizzi
Copy link
Contributor

mrizzi commented Jun 12, 2024

Just a comment after having seen the PLSQL code.
Considering the efforts we're putting into having rustsec/CVSS library to fully cover CVSS 3.1 specs, i.e. rustsec/rustsec#1198, would be possible to calculate the score just once when the vectorString is ingested and then store the value in an column? too basic/not enough approach?

@bobmcwhirter
Copy link
Contributor Author

Marco: yes we should probably just store the computer score.

Will still need the extra magic to sort and filter by the AVG() but it would reduce the work of the DB by some.

I'll enhance.

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.

4 participants