Fix the presentation URL - #616
Closed
ziadhany wants to merge 46 commits into
Closed
Conversation
useless after a530627 Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Improver look strikingly similar to importers with some enhancements. (Eg: It doesn't use an importer_yielder alternative, see issue 501). Overview: Improvers maintain a contract (like Advisory) with improve_runner which is named Inference. Inference class embeds an advisory and a confidence score for that advisory. It is the job of an improver to fetch data to improve from the database (probably using some helper functions) then use whatever means necessary to improve that data sample and return with Inferences. Do note, that Inferences which have already been "imported" by importers would be totally discarded as redundant. Also, in case of two inferences on same data point, the one with highest confidence will be taken into the database. Food for thought: Pssst... Probably Inference class is useless and Advisory class can itself have that confidence score, but then the importers would have to mention that whatever they import have 100% confidence which might be susceptible to typo errors making some importers not mention their confidence thus zeroing on confidence. Anyway, importer and improvers should be different and separated. If not, then we could totally discard the idea of improvers and embed everything in an importer with a confidence score. Well, then, where goes the idea of modularity and keeping things simple ? Also, data coming from an "import"er should always be absolutely correct. This will also ensure that if downstream doesn't want any "improved" data then they don't get our guesses. The whole point of separating importers and improvers is that running improvers could be totally optional and based on downstream taste. Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
This is work in progress, there are a few bugs and a few fixmes as well. Everything will be replaced before the final commit Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Importing Advisory data is now 2 step process.
1) Import Advisory
2) Create relations (default improver)
importers are now required to return AdvisoryData instead of Advisory.
The data contained inside AdvisoryData is converted into a model named
Advisory and then saved into the database. This will be useful later
for improvers to work on existing Advisories.
All the relationships for the Advisories are generated using a default
improver at improvers/default.py.
NOTE: There are errors in the UI due to models.py
# TODO: Cannot resolve keyword 'resolved_vulnerabilities' into field
# make vulnerabilities and resolved_vulnerabilities use the `fix` flag of PackageRelatedVulnerability
Knows defects (in current PR):
-[ ] UI break (see above)
-[ ] might crash in multiple imports / improves
-[ ] No improver than default improver is implemented yet
-[ ] normalized function of ``AdvisoryData`` has no body
-[ ] nginx importer still has remains of set_api etc
-[ ] Inference -> AdvisoryData encapsulation
-[ ] Duplicated data in database
-[ ] ???
Knows defects (to be solved in different PR):
-[ ] inconsistent naming - will be resolved in a different PR
-[ ] unordered imports
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com>
Signed-off-by: Ziad <ziadhany2016@gmail.com>
Earlier Inference used to nest Advisory class, this doesn't make sense logically. Now, the Inference class is of it's own and contains some attributes that could be present in an advisory but are not required. Technical reason behind this decoupling is that the PackageURL in AffectedPackage is NOT supposed to contain version information. This information must be present inside an Inference (that's the whole point). Improvers are required to read AdvisoryData, modify the relevant fields, add inference specific fields (confidence, source) and expand the VersionSpecifier contained inside AffectedPackage package as they deem fit into PackageURLs. Known defects (in current PR): -[ ] UI break (see above) -[ ] might crash in multiple imports / improves -[ ] No improver than default improver is implemented yet -[ ] normalized function of ``AdvisoryData`` has no body -[ ] nginx importer still has remains of set_api etc -[x] Inference -> AdvisoryData encapsulation -[ ] Duplicated data in database -[ ] ??? Knows defects (to be solved in different PR): -[ ] inconsistent naming - will be resolved in a different PR -[ ] unordered imports Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Recent model changes break the UI as now the PackageRelatedVulnerability contains a ``fix`` flag to mark the relationship as a fix. This is leveraged to eliminate multiple columns like patched_package or vulnerable_package. Known defects (in current PR): -[x] UI break -[ ] might crash in multiple imports / improves -[ ] No improver than default improver is implemented yet -[ ] normalized function of ``AdvisoryData`` has no body -[ ] nginx importer still has remains of set_api etc -[x] Inference -> AdvisoryData encapsulation -[ ] Duplicated data in database -[ ] ??? Knows defects (to be solved in different PR): -[ ] inconsistent naming - will be resolved in a different PR -[ ] unordered imports Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
The refactors are based on #525 (review) - class Inference: order should be the logic order from most important to least important fields - class Improver: docs - data_source.py: Use to_dict() than json() - models: Advisory: docs - improvers/nginx.py: Removed, relevant improvers will now reside inside importer module. In this case, importers/nginx.py - black formatting Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
AffectedPackges now contains all affected versions and one fix version. This is inspired from the design documented at https://docs.google.com/document/d/1sylBGNooKtf220RHQn1I8pZRmqXZQADDQ_TOABrKTpA Under "Format Overview", along the lines of: "affects": { "ranges": [ { "type": string, "repo": string, "introduced": string, "fixed": string } ], "versions": [ string ] }, Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
This is still not perfect because univers is not stable yet. The uncertainty about the structure of version_specifier needs to be resolved. As of now, there are many redundant AffectedPackage objects which would be gone after aboutcode-org/univers#8 is fixed. Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
There must be an affected or fixed purls if there's no vulnerability id. Otherwise, when vulnerability id is present, any (except summery for now) could be present. It does not make sense to neither have vulnerability id nor affected or fixed purls, such relation would be meaningless. Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Updated way: - Each Improver has a method to process a single Advisory model instance such as Improver.get_inferences(self, advisory): -> Inference - The framework then iterates on an Improver-provided QuerySet such as Improver.interesting_advisories that has the Advisories it is interested in. - In the framework, there is an atomic transaction that updates both the Advisory (e.g. date and later a log of improvements with select for update) and whatever is updated or create from the Inference This avoids failing the entire improver when only a single inference is erroneous. Also, the atomic transaction for every advisory and its inferences makes sure that improved_on date of advisory is consistent. Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Git rid of extraneous methods with empty body and commented code. Now as AdvisoryData has all the parameters optional, make sure there's a __post_init__ to assert the constraints. Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
aboutcode-org/univers#10 - parse_version() should accept None and empty strings Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
This is based on PR review done at #525 (review) Majorly removes commented code, adds some doctest Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
This adds univer from the current branch that support "vers". See aboutcode-org/univers#12 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
All of this comes from the review done at #525 (review) Co-authored-by: Philippe Ombredanne <pombredanne@gmail.com> Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
This commit adopts the new univers, this also requires to reset all the migrations as migration 0003 was using a univers method which is no longer available. There are also a few refactors based on #525 (review) Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
because why not ? Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
The following suggestions were acception plus some code refactor https://github.com/nexB/vulnerablecode/blob/main/vulnerabilities/migrations/0003_populate_patched_package.py Is this hand written migration ? Why ? I'm resetting migrations, this is breaks on changes in univers - > move to init migration, provide data dump https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/import_runner.py#L73 - [x] name should be inside the importer class https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/import_runner.py#L108 - [ ] Advisory. get or create in the loop itself https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/import_runner.py#L95 - [x] Make it a list (or iterable is better), not set class AdvisoryData: """ This data class expresses the contract between data sources and the import runner. """ vulnerability_id: Optional[str] = None summary: str = None affected_packages: List[AffectedPackage] = dataclasses.field(default_factory=list) references: List[Reference] = dataclasses.field(default_factory=list) date_published: Optional[datetime.datetime] = None - [x] Use this to create an Advisory model and store List objects as json https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/data_source.py#L99 - [x] affected_version_range - [x] https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/data_source.py#L120 VersionRange.version_class to get the Version subclass https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L71 - [ ] docify this - [x] advisory_data should return an iterable as a contract https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L77 - [x] yield better https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L158 - [ ] use getattr https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L162 - [x] only for nginx advisory https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L188 - [x] return a dict and use ** on 77 https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L99 - [x] _,_,fixed_versions = https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L86 - [x] how does a paragraph look https://github.com/Hritik14/vulnerablecode/blob/ee0dba45f1d5b6680e121d91ce59b050325a5e67/vulnerabilities/importers/nginx.py#L116 - [x] remove branch qualifier Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Rewrite following improvers: DefaultImprover Plus adjust the improver framework Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
VersionRange is always a flat list of constraints Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
An alias is a unique vulnerability identifier in some database, such as the NVD, PYSEC, CVE or similar. These databases guarantee that these identifiers are unique within their namespace. An alias may also be used as a Reference. But in contrast with some Reference may not be an identifier for a single vulnerability, for instance, security advisories such as Debian security advisory reference various vulnerabilities. Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
from_dict is a factory, should be a classmethod Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
The entire view is messed up. Will probably need a rewrite Co-authored-by: Philippe Ombredanne <pombredanne@gmail.com> Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Fixes are in accordance with the final review for this PR done at : #525 (review) Signed-off-by: Hritik Vijay <hritikxx8@gmail.com> Co-authored-by: Philippe Ombredanne <pombredanne@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Fixes: #550 Signed-off-by: keshav-space <keshav6240@gmail.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com> Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Ziad <ziadhany2016@gmail.com>
Signed-off-by: Ziad <ziadhany2016@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.