Skip to content
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

Time travel to the date of advisory publish time when importing #467

Merged
merged 6 commits into from
Jul 3, 2021

Conversation

sbs2001
Copy link
Collaborator

@sbs2001 sbs2001 commented May 30, 2021

This PR gets us one step closer to #140 (comment)

High level working:

  1. While parsing any advisory it obtains the publish date of it. Almost every importer does this atm.
  2. Since each advisory contains version ranges, we need to resolve it. Normally we would fetch all versions of the package from respective API and resolve it. This PR instead changes improves *VersionAPI classes to instead accept a cutoff date, to partition the versions accordingly. Here's a simple snippet
In [3]: import asyncio 
   ...: import pytz 
   ...: from dateutil.parser import parse 
   ...:  
   ...: from vulnerabilities.package_managers import NpmVersionAPI, PypiVersionAPI 
   ...:  
   ...: api = PypiVersionAPI() 
   ...: asyncio.run(api.load_api(["django"])) 
   ...: cutoff_date = parse("2018").replace(tzinfo=pytz.UTC) 
   ...: api.get("django", cutoff_date) 
   ...:  
   ...:                                                                                                                        
Out[3]: 
{'new': { #These are after cutoff_date 2018
.........
  '1.11.14',
  '3.2.4',
  '3.2a1',
  '3.2b1',
  '3.2rc1'
},
 'valid': { # This are before 2018
  '1.2.6'
.......
  '2.0b1',
  '2.0rc1',
  '2.1a1'}}

Things to do

  1. Don't use dictionary with new and valid keys, use a class. @pombredanne please help in naming these.
  2. Ubuntu and Debian VersionAPI need a fix, the current source doesn't give ALL versions.

@sbs2001 sbs2001 changed the title [WIP] Time travel to the date of advisory publish time when importing Time travel to the date of advisory publish time when importing Jun 13, 2021
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Thanks. See my comments inline for your consideration!

vulnerabilities/importers/apache_tomcat.py Show resolved Hide resolved
vulnerabilities/importers/apache_tomcat.py Outdated Show resolved Hide resolved
vulnerabilities/importers/apache_tomcat.py Show resolved Hide resolved
vulnerabilities/importers/github.py Outdated Show resolved Hide resolved
vulnerabilities/package_managers.py Outdated Show resolved Hide resolved
vulnerabilities/package_managers.py Show resolved Hide resolved
@@ -347,22 +429,51 @@ class GitHubTagsAPI(VersionAPI):
package_type = "github"

async def load_api(self, repo_set):
async with client_session() as session:
session = client_session()
async with session as session:
Copy link
Member

Choose a reason for hiding this comment

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

This code looks hard to read... we should have intermediate variable to avoid the nested complexity, possibly using generator expressions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tracking this in #492 .

Could you elaborate an approach there ?

vulnerabilities/package_managers.py Outdated Show resolved Hide resolved
vulnerabilities/package_managers.py Outdated Show resolved Hide resolved
for owner_repo in repo_set
if owner_repo.lower() not in self.cache
]
)

async def fetch(self, owner_repo: str, session) -> None:
async def fetch(self, owner_repo: str, endpoint=None) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

What about using a purl instead owner_repo

Signed-off-by: Shivam Sandbhor <[email protected]>
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