GNU bug report logs - #74034
[PATCH 00/21] Add lint-hidden-cve property for near-leaf packages.

Previous Next

Full log

Message #132 received at 74034 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Nicolas Graves <ngraves <at> ngraves.fr>
Cc: 74034 <at> debbugs.gnu.org
Subject: Re: [bug#74034] [PATCH v2 01/16] guix: cve: Add cpe-vendor and
 lint-hidden-cpe-vendors properties.
Date: Wed, 06 Nov 2024 22:43:51 +0100

Hi,

Nicolas Graves <ngraves <at> ngraves.fr> skribis:

> * guix/cve.scm: Exploit cpe vendors information.
> (cpe->package-name): Rename to cpe->package and use
> cpe_vendor:cpe_name in place or cpe_name.
> (filter-vendors): Add helper function.
> (vulnerabilities->lookup-proc): Extract cpe_name for table
> hashes. Add vendor and hidden-vendor arguments. Adapt condition to
> pass vulnerabilities to result in the fold.
>
> * guix/lint.scm (package-vulnerabilities): Use additional arguments
> from vulnerabilities->lookup-proc.
>
> * tests/cve.scm: Adapt tests.

Nice!

Please mention the names of tests being change in the commit log (see
‘git log’ for examples).

> -(define (cpe->package-name cpe)
> +(define (cpe->package cpe)

Or ‘cpe->package-identifier’?

It’s unpleasant that said identifier is an unparsed “vendor:package”
string.  I wonder if we should instead leave ‘%cpe-package-rx’ unchanged
and return three values: package, version, vendor.

The downside is that it would lead to more changes down the road because
we’d have to carry the vendor bit along.

Thoughts?

> +(define (filter-vendors vuln vendor hidden-vendors)
> +

Always add a docstring for top-level procedures.

> +  (define (vendor-matches? vendor+name)
> +    (if vendor
> +        (string-prefix? (string-append vendor ":") vendor+name)
> +        (if hidden-vendors
> +            (not (any (lambda (v)
> +                        (string-prefix? (string-append v ":") vendor+name))
> +                      hidden-vendors))
> +            #t)))

(if x y #t) => (or (not x) y)
s/v/vendor/

> +  (lambda* (package #:key (version #f) (vendor #f) (hidden-vendors #f))

I’d keep ‘version’ option rather than keyword, for compatibility and
because it’s more commonly useful than the others.

‘hidden-vendors’ should always be a list, the empty list by default
(rule of thumb: avoid “#f | list” polymorphism if a list is as
expressive.)

> +    (vhash-fold*
> +     (lambda (pair result)
> +       (match pair
> +         ((vuln sexp)
> +          (if (and (or (not (or vendor hidden-vendors))
> +                       (and (or vendor hidden-vendors)
> +                            (filter-vendors vuln vendor hidden-vendors)))
> +                   (or (not version)
> +                       (and version (version-matches? version sexp))))

(or (not x) (and x y)) => (or (not x) y)

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.