Code review comment for ~alexmurray/ubuntu-cve-tracker:package-db-json-validation

Revision history for this message
Steve Beattie (sbeattie) wrote :

A couple of comments with the utmost cursory review:

- active_edit seems broken after this change:

$ git show --quiet --format=fixes HEAD
Fixes: 7f1a0d5ff9 ("package-db: Print details when assertion check fails")
$ ./scripts/active_edit -p linux -c CVE-2022-9999999 -P 2022-09-20
Traceback (most recent call last):
  File "$HOME/git/ubuntu-cve-tracker/./scripts/active_edit", line 260, in <module>
    create_or_update_cve(cve, pkgs, priority=options.priority, bug_urls=options.bug_urls, ref_urls=options.ref_urls, public_date=options.public_date, desc=options.description, cvss=options.cvss, embargoed=option
s.embargoed)
  File "$HOME/git/ubuntu-cve-tracker/./scripts/active_edit", line 129, in create_or_update_cve
    if p in pkg_db:
TypeError: argument of type 'NoneType' is not iterable

- having all the package information in one really large file is likely to be unpleasant for git, especially as we start editing it on the regular. We already have this problem somewhat with the `ignored/not-for-us.txt` file, `git annotate` on it takes quite a while (the debian tracker CVE data file is a more extreme example of this). It's also not really great experience for editing.

- I am not sure how to interact with the data structure when I need to update it, as I'm not entirely sure what the distinction between an alias and a pkg is, and what I would use for common situations: e.g. adding linux-fancy-processor-5.17 (where there is a common linux source package), gcc-42 (where there is not a common gcc source package), or discovering zlibg is vendored into yet another source package.

- I'd really like accessor functions/methods where possible, so that we don't have to encode the details of the data structure all over the place. They also might make it easier to write some unit tests.

One of the things that I like about an expanded source package db like this is adding upstream references for matching during cve triage. But we need to sort out access

« Back to merge proposal