patchman 4 by furlongm · Pull Request #654 · furlongm/patchman

furlongm · 2025-02-11T04:42:14Z

switch to django4
2024/2025 distro support
additional errata sources, new errata module
support for cves and cwes
switch from obsolete django-tagging to django-taggit
improved operating system support
preliminary gentoo support

code-review-doctor

Some food for thought. View full project report here.

code-review-doctor · 2025-02-11T04:42:42Z

security/models.py

+class CWE(models.Model):
+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)


Suggested change

name = models.CharField(max_length=255, blank=True, null=True)

name = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Explained here.

code-review-doctor · 2025-02-11T04:42:42Z

security/models.py

+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)
+    description = models.CharField(max_length=65535, blank=True, null=True)


Suggested change

description = models.CharField(max_length=65535, blank=True, null=True)

description = models.TextField(blank=True, default='')

TextField might be better used here, instead of CharField with a huge max_length. More.

Again, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-11T04:42:42Z

security/models.py

+
+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)


Suggested change

score = models.DecimalField(max_digits=3, decimal_places=1, null=True)

score = models.DecimalField(max_digits=3, decimal_places=1, null=True, blank=True)

Expect unwanted behavior if null and blank are different values: null controls if the database allows no value for score and blank controls if the application allows no value for score. Consider setting null and blank to the same value for score. More.

code-review-doctor · 2025-02-11T04:42:42Z

security/models.py

+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)


Suggested change

severity = models.CharField(max_length=255, blank=True, null=True)

severity = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-11T04:42:42Z

security/models.py

+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)
+    version = models.DecimalField(max_digits=2, decimal_places=1)
+    vector_string = models.CharField(max_length=255, blank=True, null=True)


Suggested change

vector_string = models.CharField(max_length=255, blank=True, null=True)

vector_string = models.CharField(max_length=255, blank=True, default='')

As above, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-11T04:42:43Z

operatingsystems/models.py

    arch = models.ForeignKey(MachineArchitecture, blank=True, null=True, on_delete=models.CASCADE)
-    osgroup = models.ForeignKey(OSGroup, blank=True, null=True,
-                                on_delete=models.SET_NULL)
+    osrelease = models.ForeignKey(OSRelease, blank=True, null=True, on_delete=models.SET_NULL)


Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. More.

code-review-doctor · 2025-02-11T04:42:43Z

packages/models.py

-                                   choices=PACKAGE_TYPES,
-                                   blank=True,
-                                   null=True)
+    packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True)


null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More details.

code-review-doctor · 2025-02-11T04:42:43Z

packages/models.py

-                                   blank=True,
-                                   null=True)
+    packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True)
+    category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL)


Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. More.

code-review-doctor · 2025-02-11T04:42:43Z

packages/models.py

    release = models.CharField(max_length=255, blank=True, null=True)
    arch = models.CharField(max_length=255)
    packagetype = models.CharField(max_length=1, blank=True, null=True)
+    category = models.CharField(max_length=255, blank=True, null=True)


Suggested change

category = models.CharField(max_length=255, blank=True, null=True)

category = models.CharField(max_length=255, blank=True, default='')

Similarly, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-11T04:42:43Z

packages/urls.py

-    path('', views.package_list, name='package_list'),
-    path('<str:packagename>/', views.package_detail, name='package_detail'),
+    path('', views.package_name_list, name='package_name_list'),
+    path('name/', views.package_name_list, name='package_name_list'),


URL names must be unique but multiple urls.py entires are called package_name_list. If reverse("package_name_list") or {% url package_name_list %} is ran then only one of those urls will be returned. The user will probably be sent to the wrong view. More info.

errata/utils.py

+        er_type = 'Mailing List'
+    if er_type == 'bugzilla' or 'bug' in url.hostname or 'bugs' in url.path:
+        er_type = 'Bug Tracker'
+    if ('ubuntu.com' in url.hostname and 'usn/' in url.path) or url.hostname == 'usn.ubuntu.com':


To fix the problem, we need to ensure that the URL's hostname is properly parsed and checked to confirm that it belongs to the intended domain. Instead of using a substring check, we should use a more robust method to verify the hostname. Specifically, we can use the urlparse function to parse the URL and then check if the hostname ends with the intended domain, ensuring that it handles arbitrary subdomain sequences correctly.

We will modify the code in errata/utils.py to use the urlparse function and check if the hostname ends with .ubuntu.com instead of using the substring check 'ubuntu.com' in url.hostname.

errata/utils.py

+        url = url._replace(netloc=netloc, path=path)
+    if url.hostname == 'ubuntu.com' and url.path.startswith('/security/notices/USN'):
+        er_type = 'USN'
+    if 'launchpad.net' in url.hostname:


To fix the problem, we need to ensure that the hostname is exactly launchpad.net or a valid subdomain of it. This can be achieved by parsing the URL and performing a proper check on the hostname.

Use the urlparse function to parse the URL and extract the hostname.

Check if the hostname is exactly launchpad.net or ends with .launchpad.net to allow valid subdomains.

Update the relevant lines in the fixup_erratum_reference function to use this secure check.

code-review-doctor

Worth considering. View full project report here.

code-review-doctor · 2025-02-13T08:07:39Z

reports/models.py

            if verbose:
-                text = 'Processing report '
-                text += f'{self.id!s} - {self.host!s}'
+                text = 'Processing report {self.id} - {self.host}'


Suggested change

text = 'Processing report {self.id} - {self.host}'

text = f'Processing report {self.id} - {self.host}'

If this was meant to be f-string then f prefix is missing. More details.

code-review-doctor · 2025-02-13T08:07:39Z

reports/models.py

                if verbose:
-                    text = 'Finding updates for report '
-                    text += f'{self.id!s} - {self.host!s}'
+                    text = 'Finding updates for report {self.id} - {self.host}'


Suggested change

text = 'Finding updates for report {self.id} - {self.host}'

text = f'Finding updates for report {self.id} - {self.host}'

Likewise, Missing f prefix?

code-review-doctor · 2025-02-13T08:07:39Z

reports/models.py

            else:
-                text = 'Error: OS, kernel or arch not sent '
-                text += f'with report {self.id!s}'
+                text = 'Error: OS, kernel or arch not sent with report {self.id}'


Suggested change

text = 'Error: OS, kernel or arch not sent with report {self.id}'

text = f'Error: OS, kernel or arch not sent with report {self.id}'

Again, Missing f prefix?

code-review-doctor · 2025-02-13T08:07:39Z

repos/models.py

            else:
-                text = 'Error: unknown repo type for repo '
-                text += f'{self.id!s}: {self.repotype!s}'
+                text = 'Error: unknown repo type for repo {self.id}: {self.repotype}'


Suggested change

text = 'Error: unknown repo type for repo {self.id}: {self.repotype}'

text = f'Error: unknown repo type for repo {self.id}: {self.repotype}'

If this was meant to be f-string then f prefix is missing. Read more.

code-review-doctor · 2025-02-13T08:07:39Z

operatingsystems/models.py

+class OSRelease(models.Model):

-    name = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, unique=True, blank=False, null=False)


Suggested change

name = models.CharField(max_length=255, unique=True, blank=False, null=False)

name = models.CharField(max_length=255, unique=True)

False is the default value Django uses for blank, so blank=False can be removed. More details.

Again, redundant default arguments can be removed.

code-review-doctor · 2025-02-13T08:07:39Z

operatingsystems/models.py

+    name = models.CharField(max_length=255, unique=True, blank=False, null=False)
    repos = models.ManyToManyField(Repository, blank=True)
+    codename = models.CharField(max_length=255, blank=True)
+    cpe_name = models.CharField(max_length=255, null=True, blank=True, unique=True)


null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More.

code-review-doctor · 2025-02-13T08:07:40Z

operatingsystems/views.py

        page = paginator.page(paginator.num_pages)

-    empty_oses = list(OS.objects.filter(host__isnull=True))
+    nohost_osvariants = OSVariant.objects.filter(host__isnull=True).count() >= 1


Suggested change

nohost_osvariants = OSVariant.objects.filter(host__isnull=True).count() >= 1

nohost_osvariants = OSVariant.objects.filter(host__isnull=True).exists()

Comparing OSVariant.objects.filter(host__isnull=True).count() is less efficient than checking OSVariant.objects.filter(host__isnull=True).exists() More details.

code-review-doctor · 2025-02-13T08:07:40Z

operatingsystems/views.py

-                for os in oses:
-                    os.delete()
-                text = f'{len(oses)!s} OS\'s have been deleted'
+            if not osvariants:


Again, consider osvariants.exists()

code-review-doctor · 2025-02-13T08:07:40Z

operatingsystems/views.py

+                return redirect(reverse('operatingsystems:osvariant_list'))
+            for osvariant in osvariants:
+                osvariant.delete()
+            text = f'{len(osvariants)} OS Variants have been deleted'


len(queryset) reads all the records from the database and checks the length at application level. It would probably be more efficient to do this at database level via queryset.count(). More details.

code-review-doctor

Some food for thought. View full project report here.

code-review-doctor · 2025-02-26T05:29:13Z

reports/models.py

            if verbose:
-                text = 'Processing report '
-                text += f'{self.id!s} - {self.host!s}'
+                text = 'Processing report {self.id} - {self.host}'


Suggested change

text = 'Processing report {self.id} - {self.host}'

text = f'Processing report {self.id} - {self.host}'

If this was meant to be f-string then f prefix is missing. Explained here.

code-review-doctor · 2025-02-26T05:29:14Z

reports/models.py

                if verbose:
-                    text = 'Finding updates for report '
-                    text += f'{self.id!s} - {self.host!s}'
+                    text = 'Finding updates for report {self.id} - {self.host}'


Suggested change

text = 'Finding updates for report {self.id} - {self.host}'

text = f'Finding updates for report {self.id} - {self.host}'

Again, Missing f prefix?

code-review-doctor · 2025-02-26T05:29:14Z

reports/models.py

            else:
-                text = 'Error: OS, kernel or arch not sent '
-                text += f'with report {self.id!s}'
+                text = 'Error: OS, kernel or arch not sent with report {self.id}'


Suggested change

text = 'Error: OS, kernel or arch not sent with report {self.id}'

text = f'Error: OS, kernel or arch not sent with report {self.id}'

Same as above: Missing f prefix?

code-review-doctor · 2025-02-26T05:29:14Z

repos/models.py

            else:
-                text = 'Error: unknown repo type for repo '
-                text += f'{self.id!s}: {self.repotype!s}'
+                text = 'Error: unknown repo type for repo {self.id}: {self.repotype}'


Suggested change

text = 'Error: unknown repo type for repo {self.id}: {self.repotype}'

text = f'Error: unknown repo type for repo {self.id}: {self.repotype}'

If this was meant to be f-string then f prefix is missing. More info.

errata/admin.py

code-review-doctor · 2025-02-26T05:29:15Z

packages/urls.py

-    path('', views.package_list, name='package_list'),
-    path('<str:packagename>/', views.package_detail, name='package_detail'),
+    path('', views.package_name_list, name='package_name_list'),
+    path('name/', views.package_name_list, name='package_name_list'),


URL names must be unique but multiple urls.py entires are called package_name_list. If reverse("package_name_list") or {% url package_name_list %} is ran then only one of those urls will be returned. The user will probably be sent to the wrong view. More details.

code-review-doctor · 2025-02-26T05:29:15Z

security/models.py

+class CWE(models.Model):
+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)


Suggested change

name = models.CharField(max_length=255, blank=True, null=True)

name = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Explained here.

code-review-doctor · 2025-02-26T05:29:15Z

security/models.py

+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)
+    description = models.CharField(max_length=65535, blank=True, null=True)


Suggested change

description = models.CharField(max_length=65535, blank=True, null=True)

description = models.TextField(blank=True, default='')

TextField might be better used here, instead of CharField with a huge max_length. More info.

Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-26T05:29:15Z

security/models.py

+
+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)


Suggested change

score = models.DecimalField(max_digits=3, decimal_places=1, null=True)

score = models.DecimalField(max_digits=3, decimal_places=1, null=True, blank=True)

Expect unwanted behavior if null and blank are different values: null controls if the database allows no value for score and blank controls if the application allows no value for score. Consider setting null and blank to the same value for score. Explained here.

code-review-doctor · 2025-02-26T05:29:15Z

security/models.py

+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)


Suggested change

severity = models.CharField(max_length=255, blank=True, null=True)

severity = models.CharField(max_length=255, blank=True, default='')

Again, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor

Looks good. Worth considering though. View full project report here.

code-review-doctor · 2025-02-26T05:29:17Z

security/models.py

+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)
+    version = models.DecimalField(max_digits=2, decimal_places=1)
+    vector_string = models.CharField(max_length=255, blank=True, null=True)


Suggested change

vector_string = models.CharField(max_length=255, blank=True, null=True)

vector_string = models.CharField(max_length=255, blank=True, default='')

Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-26T05:29:17Z

security/models.py

+class CVE(models.Model):
+
+    cve_id = models.CharField(max_length=255, unique=True)
+    title = models.CharField(max_length=255, blank=True, null=True)


Suggested change

title = models.CharField(max_length=255, blank=True, null=True)

title = models.CharField(max_length=255, blank=True, default='')

Similarly, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-26T05:29:17Z

security/models.py

+
+    cve_id = models.CharField(max_length=255, unique=True)
+    title = models.CharField(max_length=255, blank=True, null=True)
+    description = models.CharField(max_length=65535)


Suggested change

description = models.CharField(max_length=65535)

description = models.TextField()

Similarly, consider using a TextField.

code-review-doctor

Some food for thought. View full project report here.

code-review-doctor · 2025-02-27T20:27:29Z

security/models.py

+class CWE(models.Model):
+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)


Suggested change

name = models.CharField(max_length=255, blank=True, null=True)

name = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More details.

code-review-doctor · 2025-02-27T20:27:29Z

security/models.py

+
+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)


Suggested change

score = models.DecimalField(max_digits=3, decimal_places=1, null=True)

score = models.DecimalField(max_digits=3, decimal_places=1, null=True, blank=True)

Expect unwanted behavior if null and blank are different values: null controls if the database allows no value for score and blank controls if the application allows no value for score. Consider setting null and blank to the same value for score. More details.

code-review-doctor · 2025-02-27T20:27:29Z

security/models.py

+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)


Suggested change

severity = models.CharField(max_length=255, blank=True, null=True)

severity = models.CharField(max_length=255, blank=True, default='')

As above, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-27T20:27:29Z

security/models.py

+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)
+    version = models.DecimalField(max_digits=2, decimal_places=1)
+    vector_string = models.CharField(max_length=255, blank=True, null=True)


Suggested change

vector_string = models.CharField(max_length=255, blank=True, null=True)

vector_string = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-27T20:27:29Z

security/models.py

+class CVE(models.Model):
+
+    cve_id = models.CharField(max_length=255, unique=True)
+    title = models.CharField(max_length=255, blank=True, null=True)


Suggested change

title = models.CharField(max_length=255, blank=True, null=True)

title = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-27T20:27:30Z

operatingsystems/views.py

-                for os in oses:
-                    os.delete()
-                text = f'{len(oses)!s} OS\'s have been deleted'
+            if not osvariants:


Comparing osvariants.count() is less efficient than checking osvariants.exists() More details.

code-review-doctor · 2025-02-27T20:27:30Z

packages/models.py

-                                   choices=PACKAGE_TYPES,
-                                   blank=True,
-                                   null=True)
+    packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True)


null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Read more.

code-review-doctor · 2025-02-27T20:27:30Z

packages/models.py

-                                   blank=True,
-                                   null=True)
+    packagetype = models.CharField(max_length=1, choices=PACKAGE_TYPES, blank=True, null=True)
+    category = models.ForeignKey(PackageCategory, blank=True, null=True, on_delete=models.SET_NULL)


Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up. Explained here.

code-review-doctor · 2025-02-27T20:27:30Z

packages/models.py

    release = models.CharField(max_length=255, blank=True, null=True)
    arch = models.CharField(max_length=255)
    packagetype = models.CharField(max_length=1, blank=True, null=True)
+    category = models.CharField(max_length=255, blank=True, null=True)


Suggested change

category = models.CharField(max_length=255, blank=True, null=True)

category = models.CharField(max_length=255, blank=True, default='')

As above, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-02-27T20:27:30Z

packages/urls.py

-    path('', views.package_list, name='package_list'),
-    path('<str:packagename>/', views.package_detail, name='package_detail'),
+    path('', views.package_name_list, name='package_name_list'),
+    path('name/', views.package_name_list, name='package_name_list'),


URL names must be unique but multiple urls.py entires are called package_name_list. If reverse("package_name_list") or {% url package_name_list %} is ran then only one of those urls will be returned. The user will probably be sent to the wrong view. Read more.

code-review-doctor

Some food for thought. View full project report here.

code-review-doctor · 2025-03-01T20:20:27Z

repos/models.py

    url = models.CharField(max_length=255, unique=True)
    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

packages_checksum = models.CharField(max_length=255, blank=True, null=True)

packages_checksum = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Explained here.

code-review-doctor · 2025-03-01T20:20:27Z

repos/models.py

    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

modules_checksum = models.CharField(max_length=255, blank=True, null=True)

modules_checksum = models.CharField(max_length=255, blank=True, default='')

Again, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:20:27Z

repos/models.py

-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)
+    errata_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

errata_checksum = models.CharField(max_length=255, blank=True, null=True)

errata_checksum = models.CharField(max_length=255, blank=True, default='')

Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor

Worth considering. View full project report here.

code-review-doctor · 2025-03-01T20:38:32Z

security/models.py

+class CWE(models.Model):
+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)


Suggested change

name = models.CharField(max_length=255, blank=True, null=True)

name = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More.

code-review-doctor · 2025-03-01T20:38:32Z

security/models.py

+
+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)


Suggested change

score = models.DecimalField(max_digits=3, decimal_places=1, null=True)

score = models.DecimalField(max_digits=3, decimal_places=1, null=True, blank=True)

Expect unwanted behavior if null and blank are different values: null controls if the database allows no value for score and blank controls if the application allows no value for score. Consider setting null and blank to the same value for score. Explained here.

code-review-doctor · 2025-03-01T20:38:32Z

security/models.py

+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)


Suggested change

severity = models.CharField(max_length=255, blank=True, null=True)

severity = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:38:32Z

security/models.py

+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)
+    version = models.DecimalField(max_digits=2, decimal_places=1)
+    vector_string = models.CharField(max_length=255, blank=True, null=True)


Suggested change

vector_string = models.CharField(max_length=255, blank=True, null=True)

vector_string = models.CharField(max_length=255, blank=True, default='')

Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:38:32Z

security/models.py

+class CVE(models.Model):
+
+    cve_id = models.CharField(max_length=255, unique=True)
+    title = models.CharField(max_length=255, blank=True, null=True)


Suggested change

title = models.CharField(max_length=255, blank=True, null=True)

title = models.CharField(max_length=255, blank=True, default='')

Similarly, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:38:33Z

packages/models.py

    release = models.CharField(max_length=255, blank=True, null=True)
    arch = models.CharField(max_length=255)
    packagetype = models.CharField(max_length=1, blank=True, null=True)
+    category = models.CharField(max_length=255, blank=True, null=True)


Suggested change

category = models.CharField(max_length=255, blank=True, null=True)

category = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:38:33Z

packages/urls.py

-    path('', views.package_list, name='package_list'),
-    path('<str:packagename>/', views.package_detail, name='package_detail'),
+    path('', views.package_name_list, name='package_name_list'),
+    path('name/', views.package_name_list, name='package_name_list'),


URL names must be unique but multiple urls.py entires are called package_name_list. If reverse("package_name_list") or {% url package_name_list %} is ran then only one of those urls will be returned. The user will probably be sent to the wrong view. More.

code-review-doctor · 2025-03-01T20:38:33Z

repos/models.py

    url = models.CharField(max_length=255, unique=True)
    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

packages_checksum = models.CharField(max_length=255, blank=True, null=True)

packages_checksum = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More info.

code-review-doctor · 2025-03-01T20:38:33Z

repos/models.py

    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

modules_checksum = models.CharField(max_length=255, blank=True, null=True)

modules_checksum = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:38:33Z

repos/models.py

-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)
+    errata_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

errata_checksum = models.CharField(max_length=255, blank=True, null=True)

errata_checksum = models.CharField(max_length=255, blank=True, default='')

Again, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor

Some food for thought. View full project report here.

code-review-doctor · 2025-03-01T20:44:15Z

security/models.py

+class CWE(models.Model):
+
+    cwe_id = models.CharField(max_length=255, unique=True)
+    name = models.CharField(max_length=255, blank=True, null=True)


Suggested change

name = models.CharField(max_length=255, blank=True, null=True)

name = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Read more.

code-review-doctor · 2025-03-01T20:44:15Z

security/models.py

+
+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)


Suggested change

score = models.DecimalField(max_digits=3, decimal_places=1, null=True)

score = models.DecimalField(max_digits=3, decimal_places=1, null=True, blank=True)

Expect unwanted behavior if null and blank are different values: null controls if the database allows no value for score and blank controls if the application allows no value for score. Consider setting null and blank to the same value for score. More details.

code-review-doctor · 2025-03-01T20:44:15Z

security/models.py

+class CVSS(models.Model):
+
+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)


Suggested change

severity = models.CharField(max_length=255, blank=True, null=True)

severity = models.CharField(max_length=255, blank=True, default='')

As above, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:44:15Z

security/models.py

+    score = models.DecimalField(max_digits=3, decimal_places=1, null=True)
+    severity = models.CharField(max_length=255, blank=True, null=True)
+    version = models.DecimalField(max_digits=2, decimal_places=1)
+    vector_string = models.CharField(max_length=255, blank=True, null=True)


Suggested change

vector_string = models.CharField(max_length=255, blank=True, null=True)

vector_string = models.CharField(max_length=255, blank=True, default='')

Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:44:15Z

security/models.py

+class CVE(models.Model):
+
+    cve_id = models.CharField(max_length=255, unique=True)
+    title = models.CharField(max_length=255, blank=True, null=True)


Suggested change

title = models.CharField(max_length=255, blank=True, null=True)

title = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:44:16Z

packages/models.py

    release = models.CharField(max_length=255, blank=True, null=True)
    arch = models.CharField(max_length=255)
    packagetype = models.CharField(max_length=1, blank=True, null=True)
+    category = models.CharField(max_length=255, blank=True, null=True)


Suggested change

category = models.CharField(max_length=255, blank=True, null=True)

category = models.CharField(max_length=255, blank=True, default='')

Similarly, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:44:16Z

packages/urls.py

-    path('', views.package_list, name='package_list'),
-    path('<str:packagename>/', views.package_detail, name='package_detail'),
+    path('', views.package_name_list, name='package_name_list'),
+    path('name/', views.package_name_list, name='package_name_list'),


URL names must be unique but multiple urls.py entires are called package_name_list. If reverse("package_name_list") or {% url package_name_list %} is ran then only one of those urls will be returned. The user will probably be sent to the wrong view. More.

code-review-doctor · 2025-03-01T20:44:17Z

repos/models.py

    url = models.CharField(max_length=255, unique=True)
    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

packages_checksum = models.CharField(max_length=255, blank=True, null=True)

packages_checksum = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". Explained here.

code-review-doctor · 2025-03-01T20:44:17Z

repos/models.py

    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

modules_checksum = models.CharField(max_length=255, blank=True, null=True)

modules_checksum = models.CharField(max_length=255, blank=True, default='')

Again, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:44:17Z

repos/models.py

-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)
+    errata_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

errata_checksum = models.CharField(max_length=255, blank=True, null=True)

errata_checksum = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

skip-checks: true

code-review-doctor

Looks good. Worth considering though. View full project report here.

code-review-doctor · 2025-03-01T20:45:21Z

repos/models.py

    url = models.CharField(max_length=255, unique=True)
    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

packages_checksum = models.CharField(max_length=255, blank=True, null=True)

packages_checksum = models.CharField(max_length=255, blank=True, default='')

null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="". More.

code-review-doctor · 2025-03-01T20:45:21Z

repos/models.py

    last_access_ok = models.BooleanField(default=False)
-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

modules_checksum = models.CharField(max_length=255, blank=True, null=True)

modules_checksum = models.CharField(max_length=255, blank=True, default='')

Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor · 2025-03-01T20:45:21Z

repos/models.py

-    file_checksum = models.CharField(max_length=255, blank=True, null=True)
+    packages_checksum = models.CharField(max_length=255, blank=True, null=True)
+    modules_checksum = models.CharField(max_length=255, blank=True, null=True)
+    errata_checksum = models.CharField(max_length=255, blank=True, null=True)


Suggested change

errata_checksum = models.CharField(max_length=255, blank=True, null=True)

errata_checksum = models.CharField(max_length=255, blank=True, default='')

Likewise, consider replacing null=True with default="" (and blank=True to pass validation checks).

code-review-doctor

Some food for thought. View full project report here.

code-review-doctor · 2025-03-12T01:19:36Z

security/utils.py

+        if refs:
+            ref = refs.first()
+            if ref.url != reference.get('url') and update_ref_type:
+                ref.ref_type = ref_type
+                ref.save()
+        else:
+            ref, created = Reference.objects.get_or_create(
+                ref_type=reference.get('ref_type'),
+                url=reference.get('url'),
+            )


Suggested change

if refs:

ref = refs.first()

if ref.url != reference.get('url') and update_ref_type:

ref.ref_type = ref_type

ref.save()

else:

ref, created = Reference.objects.get_or_create(

ref_type=reference.get('ref_type'),

url=reference.get('url'),

)

if refs.exists():

ref = refs.first()

if ref.url != reference.get('url') and update_ref_type:

ref.ref_type = ref_type

ref.save()

else:

(ref, created) = Reference.objects.get_or_create(

ref_type=reference.get('ref_type'), url=reference.get('url')

)

Checking refs truthiness is less efficient than checking refs.exists() or refs is not None. Checking queryset truthiness evaluates the queryset, therefore reading the records from the database. More info.

code-review-doctor bot suggested changes Feb 11, 2025

View reviewed changes

github-advanced-security bot found potential problems Feb 11, 2025

View reviewed changes

furlongm force-pushed the develop branch 5 times, most recently from 5b82c72 to 96cd496 Compare February 13, 2025 03:21

code-review-doctor bot suggested changes Feb 13, 2025

View reviewed changes

furlongm force-pushed the develop branch 2 times, most recently from fccc3bf to 1d056a5 Compare February 26, 2025 05:28

code-review-doctor bot suggested changes Feb 26, 2025

View reviewed changes

furlongm force-pushed the develop branch 7 times, most recently from ccb2b38 to 02f90f7 Compare February 27, 2025 20:27

code-review-doctor bot suggested changes Feb 27, 2025

View reviewed changes

code-review-doctor bot suggested changes Mar 1, 2025

View reviewed changes

furlongm force-pushed the develop branch from 3e28484 to 2866a92 Compare March 1, 2025 20:38

code-review-doctor bot suggested changes Mar 1, 2025

View reviewed changes

furlongm force-pushed the develop branch from 2866a92 to 9e969d3 Compare March 1, 2025 20:43

code-review-doctor bot suggested changes Mar 1, 2025

View reviewed changes

furlongm added 2 commits March 1, 2025 20:45

auto-commit to update version

8cf653c

skip-checks: true

auto-commit to update debian changelog

c08e72a

skip-checks: true

furlongm force-pushed the develop branch from 9e969d3 to 7ceeac4 Compare March 1, 2025 20:45

code-review-doctor bot suggested changes Mar 1, 2025

View reviewed changes

support for timezone-aware datetimes

51f971a

furlongm added 20 commits March 9, 2025 13:11

use reverse-date ordering for cves and errata

fdb97f9

better cve mismatch message

9378543

handle irregularly-named CVEs

6538fc7

get debian DSCs from package file maps

6d66c40

improve get_matching_packages

7489754

use package-specific display options

14485b1

add concurrent processing for yum repo errata

14129b1

additional options for yum repo errata handling

ef36646

remove errata view ordering

f77806e

use arch_id for packages

423ca73

add distro-specific references to errata

89b3f1a

don't use acronym, keep flake8 happy

084840e

update gentoo repo handling

3eb2d94

add errata affected and fixed packages

6964bed

tweak yum reference handling

1e7790a

add osv.dev reference to errata detail page

f6d7567

make packagestring distro-specific handling match package

93dbdcf

add host errata migration

487a3ca

bump to latest 4.2 django

e09037c

updates for cvss and cve handling

c1001df

code-review-doctor bot suggested changes Mar 12, 2025

View reviewed changes

allow search to keep existing filters

c3fe0e9

furlongm merged commit bb0fc18 into master Mar 20, 2025
2 of 3 checks passed

furlongm deleted the develop branch March 20, 2025 20:42

This was referenced Mar 20, 2025

gentoo client support #652

Closed

switch from django-tagging to django-taggit #630

Closed

support other errata sources #633

Closed

Bump django from 3.2.25 to 4.2.17 #623

Closed

furlongm restored the develop branch March 21, 2025 21:38

furlongm deleted the develop branch March 21, 2025 21:45

@@ -67,3 +67,3 @@
                     er_type = 'Bug Tracker'
-                if ('ubuntu.com' in url.hostname and 'usn/' in url.path) or url.hostname == 'usn.ubuntu.com':
+                if (url.hostname and url.hostname.endswith('.ubuntu.com') and 'usn/' in url.path) or url.hostname == 'usn.ubuntu.com':
                     netloc = url.netloc.replace('usn.', '').replace('www.', '')

	name = models.CharField(max_length=255, blank=True, null=True)
	name = models.CharField(max_length=255, blank=True, default='')

	description = models.CharField(max_length=65535, blank=True, null=True)
	description = models.TextField(blank=True, default='')


		class CVSS(models.Model):

		score = models.DecimalField(max_digits=3, decimal_places=1, null=True)

	severity = models.CharField(max_length=255, blank=True, null=True)
	severity = models.CharField(max_length=255, blank=True, default='')

	vector_string = models.CharField(max_length=255, blank=True, null=True)
	vector_string = models.CharField(max_length=255, blank=True, default='')

	category = models.CharField(max_length=255, blank=True, null=True)
	category = models.CharField(max_length=255, blank=True, default='')

	text = 'Processing report {self.id} - {self.host}'
	text = f'Processing report {self.id} - {self.host}'

	text = 'Finding updates for report {self.id} - {self.host}'
	text = f'Finding updates for report {self.id} - {self.host}'

	text = 'Error: OS, kernel or arch not sent with report {self.id}'
	text = f'Error: OS, kernel or arch not sent with report {self.id}'

	text = 'Error: unknown repo type for repo {self.id}: {self.repotype}'
	text = f'Error: unknown repo type for repo {self.id}: {self.repotype}'

	nohost_osvariants = OSVariant.objects.filter(host__isnull=True).count() >= 1
	nohost_osvariants = OSVariant.objects.filter(host__isnull=True).exists()

	title = models.CharField(max_length=255, blank=True, null=True)
	title = models.CharField(max_length=255, blank=True, default='')

	description = models.CharField(max_length=65535)
	description = models.TextField()

	packages_checksum = models.CharField(max_length=255, blank=True, null=True)
	packages_checksum = models.CharField(max_length=255, blank=True, default='')

	modules_checksum = models.CharField(max_length=255, blank=True, null=True)
	modules_checksum = models.CharField(max_length=255, blank=True, default='')

	errata_checksum = models.CharField(max_length=255, blank=True, null=True)
	errata_checksum = models.CharField(max_length=255, blank=True, default='')

Uh oh!

Conversation

furlongm commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

code-review-doctor bot left a comment

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

Check failure

Copilot Autofix

Check failure

Copilot Autofix

code-review-doctor bot left a comment

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot left a comment

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

code-review-doctor bot Feb 26, 2025

Choose a reason for hiding this comment

furlongm commented Feb 11, 2025 •

edited

Loading