GNU bug report logs - #45570
operating-system definitions allow duplicate passwd and group entries

Previous Next

Package: guix;

Reported by: Jason Conroy <conjaroy <at> gmail.com>

Date: Thu, 31 Dec 2020 18:15:02 UTC

Severity: normal

Done: Leo Prikler <leo.prikler <at> student.tugraz.at>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45570 in the body.
You can then email your comments to 45570 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Thu, 31 Dec 2020 18:15:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Jason Conroy <conjaroy <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-guix <at> gnu.org. (Thu, 31 Dec 2020 18:15:03 GMT) Full text and rfc822 format available.

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

From: Jason Conroy <conjaroy <at> gmail.com>
To: bug-guix <at> gnu.org
Subject: operating-system definitions allow duplicate passwd and group entries
Date: Thu, 31 Dec 2020 13:14:19 -0500
[Message part 1 (text/plain, inline)]
When an operating-system contains multiple users or groups with the same
name, instantiating it with `guix system` does not cause a validation
failure, nor are the duplicate entries filtered from the resulting /etc
files.

This duplication can happen in a few different ways:

- both entries are manually included in the "users" or "groups" fields of
the operating-system
- a manually-specified entry collides with an entry defined by a service
(via an account-service-type extension)
- multiple services define entries that collide with each other

Steps to reproduce: call "guix system container" with the attached
operating-system definition.
[Message part 2 (text/html, inline)]
[duplicate-users-and-groups.scm (application/octet-stream, attachment)]

Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Fri, 01 Jan 2021 11:15:02 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: 45570 <at> debbugs.gnu.org
Cc: conjaroy <at> gmail.com
Subject: [PATCH] system: Assert, that user and group names are unique.
Date: Fri,  1 Jan 2021 12:13:10 +0100
*gnu/system/shadow.scm (assert-unique-account-names)
(assert-unique-group-names): New variables.
(account-activation): Use them here.
---
 gnu/system/shadow.scm | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..61562f225e 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -222,6 +222,32 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
                          (rename-file ".nanorc" ".config/nano/nanorc"))
                        #t))))
 
+(define (assert-unique-account-names users)
+  (let loop ((names '())
+             (users users))
+    (unless (null? users)
+      (let ((name (user-account-name (car users))))
+        (if (member name names)
+            (raise (condition
+                    (&message
+                     (message
+                      (format #f (G_ "account with name '~a' found twice")
+                              name)))))
+            (loop (cons name names) (cdr users)))))))
+
+(define (assert-unique-group-names groups)
+  (let loop ((names '())
+             (groups groups))
+    (unless (null? groups)
+      (let ((name (user-account-name (car groups))))
+        (if (member name names)
+            (raise (condition
+                    (&message
+                     (message
+                      (format #f (G_ "group with name '~a' found twice")
+                              name)))))
+            (loop (cons name names) (cdr groups)))))))
+
 (define (assert-valid-users/groups users groups)
   "Raise an error if USERS refer to groups not listed in GROUPS."
   (let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +318,8 @@ group."
   (define group-specs
     (map user-group->gexp groups))
 
+  (assert-unique-account-names accounts)
+  (assert-unique-group-names groups)
   (assert-valid-users/groups accounts groups)
 
   ;; Add users and user groups.
-- 
2.29.2





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Sat, 02 Jan 2021 01:17:01 GMT) Full text and rfc822 format available.

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: bug#45570: [PATCH] system: Assert, that user and group names
 are unique.
Date: Sat, 2 Jan 2021 02:16:01 +0100
[Message part 1 (text/plain, inline)]
Hi Leo,

I agree that this is a good idea.

Please use (ice-9 match) instead of car and cdr.

Something among these lines would be more transparent:

    (define (find-duplicates list accessor)
      (match list
        (() '())
        ((head . tail)
         (if (member head tail accessor) ; (srfi srfi-1) member
             (cons head (find-duplicates tail accessor))
             (find-duplicates tail accessor)))))

(find-duplicates users
                 (lambda (a b)
                   (string=? (user-account-name a)
                             (user-account-name b)))

(I think one could also use srfi-1 delete-duplicates and then compare the
lengths.  Then the entire thing is a one-liner--the only complication is
to find the duplicates again after doing it (for the error message))
[Message part 2 (application/pgp-signature, inline)]

Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Sat, 02 Jan 2021 05:59:01 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: 45570 <at> debbugs.gnu.org
Cc: dannym <at> scratchpost.org, conjaroy <at> gmail.com
Subject: [PATCH] system: Assert, that user and group names are unique.
Date: Sat,  2 Jan 2021 06:57:29 +0100
*gnu/system/shadow.scm (find-duplicates): New variable.
(assert-unique-account-names, assert-unique-group-names): New variables.
(account-activation): Use them here.
---
 gnu/system/shadow.scm | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..3a5ea4dc70 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -34,6 +34,7 @@
   #:use-module ((gnu packages admin)
                 #:select (shadow))
   #:use-module (gnu packages bash)
+  #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -222,6 +223,38 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
                          (rename-file ".nanorc" ".config/nano/nanorc"))
                        #t))))
 
+(define (find-duplicates list =)
+  (match list
+    ('() '())
+    ((first . rest)
+     (if (member first rest =) ; (srfi srfi-1) member
+         (cons first (find-duplicates rest =))
+         (find-duplicates rest =)))))
+
+(define (assert-unique-account-names users)
+  (for-each
+   (lambda (account)
+     (raise (condition
+             (&message
+              (message
+               (format #f (G_ "account with name '~a' found twice.")
+                       (user-account-name account)))))))
+   (find-duplicates users (lambda (alice bob)
+                            (string=? (user-account-name alice)
+                                      (user-account-name bob))))))
+
+(define (assert-unique-group-names groups)
+  (for-each
+   (lambda (group)
+     (raise (condition
+             (&message
+              (message
+               (format #f (G_ "group with name '~a' found twice.")
+                       (user-group-name group)))))))
+   (find-duplicates groups (lambda (red blue)
+                             (string=? (user-group-name red)
+                                       (user-group-name blue))))))
+
 (define (assert-valid-users/groups users groups)
   "Raise an error if USERS refer to groups not listed in GROUPS."
   (let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +325,8 @@ group."
   (define group-specs
     (map user-group->gexp groups))
 
+  (assert-unique-account-names accounts)
+  (assert-unique-group-names groups)
   (assert-valid-users/groups accounts groups)
 
   ;; Add users and user groups.
-- 
2.29.2





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Wed, 06 Jan 2021 09:57:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: bug#45570: [PATCH] system: Assert, that user and group names
 are unique.
Date: Wed, 06 Jan 2021 10:56:23 +0100
Hi,

Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:

> *gnu/system/shadow.scm (find-duplicates): New variable.
> (assert-unique-account-names, assert-unique-group-names): New variables.
> (account-activation): Use them here.

[...]

> +(define (find-duplicates list =)
> +  (match list
> +    ('() '())

This should be:

  (match list
    (() '())
    …)

I’m surprised '() works as a pattern.

> +    ((first . rest)
> +     (if (member first rest =) ; (srfi srfi-1) member
> +         (cons first (find-duplicates rest =))
> +         (find-duplicates rest =)))))

Note that this is quadratic; it’s fine as long as we don’t have “too
many” users, which may be the case in general.

> +(define (assert-unique-account-names users)
> +  (for-each
> +   (lambda (account)
> +     (raise (condition
> +             (&message
> +              (message
> +               (format #f (G_ "account with name '~a' found twice.")
> +                       (user-account-name account)))))))
> +   (find-duplicates users (lambda (alice bob)
> +                            (string=? (user-account-name alice)
> +                                      (user-account-name bob))))))

‘for-each’ looks awkward since we’ll stop on the first one.  How about
something like:

  (define (assert-unique-account-names users)
    (match (find-duplicates things …)
      (() #t)
      (lst
       (raise (formatted-message (G_ "the following accounts appear more than once:~{ ~a~}~%"
                                 lst))))))

?

Thanks!

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Wed, 06 Jan 2021 12:35:02 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: bug#45570: [PATCH] system: Assert, that user and group names
 are unique.
Date: Wed, 06 Jan 2021 13:34:26 +0100
Hi,

Am Mittwoch, den 06.01.2021, 10:56 +0100 schrieb Ludovic Courtès:
> Hi,
> 
> Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:
> 
> > *gnu/system/shadow.scm (find-duplicates): New variable.
> > (assert-unique-account-names, assert-unique-group-names): New
> > variables.
> > (account-activation): Use them here.
> 
> [...]
> 
> > +(define (find-duplicates list =)
> > +  (match list
> > +    ('() '())
> 
> This should be:
> 
>   (match list
>     (() '())
>     …)
> 
> I’m surprised '() works as a pattern.
I think it's because matching literals works, but you're right.

> > +    ((first . rest)
> > +     (if (member first rest =) ; (srfi srfi-1) member
> > +         (cons first (find-duplicates rest =))
> > +         (find-duplicates rest =)))))
> 
> Note that this is quadratic; it’s fine as long as we don’t have “too
> many” users, which may be the case in general.
It is indeed quadratic, but would there even be an n log n solution?
I've once done an n log n sort+delete-duplicates!, perhaps that'd be a
nicer solution here?

> > +(define (assert-unique-account-names users)
> > +  (for-each
> > +   (lambda (account)
> > +     (raise (condition
> > +             (&message
> > +              (message
> > +               (format #f (G_ "account with name '~a' found
> > twice.")
> > +                       (user-account-name account)))))))
> > +   (find-duplicates users (lambda (alice bob)
> > +                            (string=? (user-account-name alice)
> > +                                      (user-account-name bob))))))
> 
> ‘for-each’ looks awkward since we’ll stop on the first one.  How
> about
> something like:
> 
>   (define (assert-unique-account-names users)
>     (match (find-duplicates things …)
>       (() #t)
>       (lst
>        (raise (formatted-message (G_ "the following accounts appear
> more than once:~{ ~a~}~%"
>                                  lst))))))
> 
> ?
That'd be weird for duplicate duplicates, hence just reporting the
first.  Of course we could always count occurrences by allocating a
local hash table and then do some fancy hash-map->list conversion.  If
we do use hash-tables, perhaps this could even be a linear algorithm?

Regards,
Leo





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Wed, 06 Jan 2021 13:33:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: bug#45570: [PATCH] system: Assert, that user and group names
 are unique.
Date: Wed, 06 Jan 2021 14:32:10 +0100
Hi,

Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:

>> > +    ((first . rest)
>> > +     (if (member first rest =) ; (srfi srfi-1) member
>> > +         (cons first (find-duplicates rest =))
>> > +         (find-duplicates rest =)))))
>> 
>> Note that this is quadratic; it’s fine as long as we don’t have “too
>> many” users, which may be the case in general.
> It is indeed quadratic, but would there even be an n log n solution?
> I've once done an n log n sort+delete-duplicates!, perhaps that'd be a
> nicer solution here?

You could first build a hash table or vhash or set with all the names,
then traverse again the list of names and check whether they’re in that
table.  That’d be linear (assuming the table is well balanced), but the
constant factor would be higher.

>>   (define (assert-unique-account-names users)
>>     (match (find-duplicates things …)
>>       (() #t)
>>       (lst
>>        (raise (formatted-message (G_ "the following accounts appear
>> more than once:~{ ~a~}~%"
>>                                  lst))))))
>> 
>> ?
> That'd be weird for duplicate duplicates, hence just reporting the
> first.

You could do (delete-duplicates lst) in the message above?

Thanks,
Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Wed, 06 Jan 2021 21:01:01 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: bug#45570: [PATCH] system: Assert, that user and group names
 are unique.
Date: Wed, 06 Jan 2021 22:00:03 +0100
Hi,

Am Mittwoch, den 06.01.2021, 14:32 +0100 schrieb Ludovic Courtès:
> Hi,
> 
> Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:
> 
> > > > +    ((first . rest)
> > > > +     (if (member first rest =) ; (srfi srfi-1) member
> > > > +         (cons first (find-duplicates rest =))
> > > > +         (find-duplicates rest =)))))
> > > 
> > > Note that this is quadratic; it’s fine as long as we don’t have
> > > “too
> > > many” users, which may be the case in general.
> > It is indeed quadratic, but would there even be an n log n
> > solution?
> > I've once done an n log n sort+delete-duplicates!, perhaps that'd
> > be a
> > nicer solution here?
> 
> You could first build a hash table or vhash or set with all the
> names,
> then traverse again the list of names and check whether they’re in
> that
> table.  That’d be linear (assuming the table is well balanced), but
> the
> constant factor would be higher.
Yeah, I think the hash table solution would make the most sense here. 
Since VHashes are based on VLists, they're not actually purely
functional, are they?

> > >   (define (assert-unique-account-names users)
> > >     (match (find-duplicates things …)
> > >       (() #t)
> > >       (lst
> > >        (raise (formatted-message (G_ "the following accounts
> > > appear
> > > more than once:~{ ~a~}~%"
> > >                                  lst))))))
> > > 
> > > ?
> > That'd be weird for duplicate duplicates, hence just reporting the
> > first.
> 
> You could do (delete-duplicates lst) in the message above?
Sure, but that'd be O(n^2) on top of O(n^2), which is less than ideal.
I think I'll try working on a hash-based implementation for now.

Regards,
Leo





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Wed, 06 Jan 2021 21:23:01 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: 45570 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, conjaroy <at> gmail.com
Subject: [PATCH v2] system: Assert, that user and group names are unique.
Date: Wed,  6 Jan 2021 22:21:49 +0100
*gnu/system/shadow.scm (find-duplicates): New variable.
(assert-unique-account-names, assert-unique-group-names): New variables.
(account-activation): Use them here.
---
 gnu/system/shadow.scm | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..4dbd578e1e 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -20,6 +20,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu system shadow)
+  #:use-module ((guix diagnostics) #:select (formatted-message))
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (guix store)
@@ -34,6 +35,7 @@
   #:use-module ((gnu packages admin)
                 #:select (shadow))
   #:use-module (gnu packages bash)
+  #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -222,6 +224,40 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
                          (rename-file ".nanorc" ".config/nano/nanorc"))
                        #t))))
 
+(define (find-duplicates list)
+  (let loop ((table (make-hash-table))
+             (list list))
+    (match list
+      (()
+       (hash-fold (lambda (key value seed)
+                    (if (> value 1)
+                        (cons key seed)
+                        seed))
+                  '()
+                  table))
+      ((first . rest)
+       (hash-set! table first
+                  (1+ (hash-ref table first 0)))
+       (loop table rest)))))
+
+(define (assert-unique-account-names users)
+  (match (find-duplicates (map user-account-name users))
+    (() *unspecified*)
+    (duplicates
+     (raise
+      (formatted-message
+       (G_ "the following accounts appear more than once:~{ ~a~}~%")
+       duplicates)))))
+
+(define (assert-unique-group-names groups)
+  (match (find-duplicates (map user-group-name groups))
+    (() *unspecified*)
+    (duplicates
+     (raise
+      (formatted-message
+       (G_ "the following groups appear more than once:~{ ~a~}~%")
+       duplicates)))))
+
 (define (assert-valid-users/groups users groups)
   "Raise an error if USERS refer to groups not listed in GROUPS."
   (let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +328,8 @@ group."
   (define group-specs
     (map user-group->gexp groups))
 
+  (assert-unique-account-names accounts)
+  (assert-unique-group-names groups)
   (assert-valid-users/groups accounts groups)
 
   ;; Add users and user groups.
-- 
2.30.0





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Thu, 07 Jan 2021 08:31:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: bug#45570: [PATCH] system: Assert, that user and group names
 are unique.
Date: Thu, 07 Jan 2021 09:29:59 +0100
Hi,

Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:

> Am Mittwoch, den 06.01.2021, 14:32 +0100 schrieb Ludovic Courtès:
>> Hi,
>> 
>> Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:
>> 
>> > > > +    ((first . rest)
>> > > > +     (if (member first rest =) ; (srfi srfi-1) member
>> > > > +         (cons first (find-duplicates rest =))
>> > > > +         (find-duplicates rest =)))))
>> > > 
>> > > Note that this is quadratic; it’s fine as long as we don’t have
>> > > “too
>> > > many” users, which may be the case in general.
>> > It is indeed quadratic, but would there even be an n log n
>> > solution?
>> > I've once done an n log n sort+delete-duplicates!, perhaps that'd
>> > be a
>> > nicer solution here?
>> 
>> You could first build a hash table or vhash or set with all the
>> names,
>> then traverse again the list of names and check whether they’re in
>> that
>> table.  That’d be linear (assuming the table is well balanced), but
>> the
>> constant factor would be higher.
> Yeah, I think the hash table solution would make the most sense here. 
> Since VHashes are based on VLists, they're not actually purely
> functional, are they?

Their implementation is not “purely functional” but it’s
inconsequential; it’s a persistent data structure, and that’s what
matters (info "(guile) VLists").

>> > >   (define (assert-unique-account-names users)
>> > >     (match (find-duplicates things …)
>> > >       (() #t)
>> > >       (lst
>> > >        (raise (formatted-message (G_ "the following accounts
>> > > appear
>> > > more than once:~{ ~a~}~%"
>> > >                                  lst))))))
>> > > 
>> > > ?
>> > That'd be weird for duplicate duplicates, hence just reporting the
>> > first.
>> 
>> You could do (delete-duplicates lst) in the message above?
> Sure, but that'd be O(n^2) on top of O(n^2), which is less than ideal.

Yes, but it’s a small ‘n’, typically one or two.

Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Thu, 07 Jan 2021 08:36:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: [PATCH v2] system: Assert, that user and group names are unique.
Date: Thu, 07 Jan 2021 09:35:43 +0100
Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:

> *gnu/system/shadow.scm (find-duplicates): New variable.
> (assert-unique-account-names, assert-unique-group-names): New variables.
> (account-activation): Use them here.

Final nitpicks!  :-)

> +(define (find-duplicates list)

Please add a docstring.

> +  (let loop ((table (make-hash-table))
> +             (list list))

You can move ‘table’ out of the ‘loop’ arguments since it’s mutated
anyway.

OK with these changes!

Thanks,
Ludo’.




Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Thu, 07 Jan 2021 11:12:01 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: 45570 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org, conjaroy <at> gmail.com
Subject: [PATCH v3] system: Assert, that user and group names are unique.
Date: Thu,  7 Jan 2021 12:10:20 +0100
*gnu/system/shadow.scm (find-duplicates): New variable.
(assert-unique-account-names, assert-unique-group-names): New variables.
(account-activation): Use them here.
---
 gnu/system/shadow.scm | 44 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..183b2cd387 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -20,6 +20,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu system shadow)
+  #:use-module ((guix diagnostics) #:select (formatted-message))
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (guix store)
@@ -34,6 +35,7 @@
   #:use-module ((gnu packages admin)
                 #:select (shadow))
   #:use-module (gnu packages bash)
+  #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -222,6 +224,46 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
                          (rename-file ".nanorc" ".config/nano/nanorc"))
                        #t))))
 
+(define (find-duplicates list)
+  "Find duplicate entries in @var{list}.
+Two entries are considered duplicates, if they are @code{equal?} to each other.
+This implementation is made asymptotically faster than @code{delete-duplicates}
+through the internal use of hash tables."
+  (let loop ((list list)
+             ;; We actually modify table in-place, but still allocate it here
+             ;; so that we only need one level of indentation.
+             (table (make-hash-table)))
+    (match list
+      (()
+       (hash-fold (lambda (key value seed)
+                    (if (> value 1)
+                        (cons key seed)
+                        seed))
+                  '()
+                  table))
+      ((first . rest)
+       (hash-set! table first
+                  (1+ (hash-ref table first 0)))
+       (loop rest table)))))
+
+(define (assert-unique-account-names users)
+  (match (find-duplicates (map user-account-name users))
+    (() *unspecified*)
+    (duplicates
+     (raise
+      (formatted-message
+       (G_ "the following accounts appear more than once:~{ ~a~}")
+       duplicates)))))
+
+(define (assert-unique-group-names groups)
+  (match (find-duplicates (map user-group-name groups))
+    (() *unspecified*)
+    (duplicates
+     (raise
+      (formatted-message
+       (G_ "the following groups appear more than once:~{ ~a~}")
+       duplicates)))))
+
 (define (assert-valid-users/groups users groups)
   "Raise an error if USERS refer to groups not listed in GROUPS."
   (let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +334,8 @@ group."
   (define group-specs
     (map user-group->gexp groups))
 
+  (assert-unique-account-names accounts)
+  (assert-unique-group-names groups)
   (assert-valid-users/groups accounts groups)
 
   ;; Add users and user groups.
-- 
2.30.0





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Thu, 07 Jan 2021 11:14:02 GMT) Full text and rfc822 format available.

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

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: [PATCH v2] system: Assert, that user and group names are unique.
Date: Thu, 07 Jan 2021 12:13:31 +0100
Am Donnerstag, den 07.01.2021, 09:35 +0100 schrieb Ludovic Courtès:
> Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:
> 
> > *gnu/system/shadow.scm (find-duplicates): New variable.
> > (assert-unique-account-names, assert-unique-group-names): New
> > variables.
> > (account-activation): Use them here.
> 
> Final nitpicks!  :-)
> 
> > +(define (find-duplicates list)
> 
> Please add a docstring.
Done, see v3.

> > +  (let loop ((table (make-hash-table))
> > +             (list list))
> 
> You can move ‘table’ out of the ‘loop’ arguments since it’s mutated
> anyway.
I don't see any benefit from doing so, however.  It'd be an additional
layer of mutation and if we ever wanted to change to vhashes or alists
we'd have to refactor that.  

Regards,
Leo





Information forwarded to bug-guix <at> gnu.org:
bug#45570; Package guix. (Mon, 11 Jan 2021 13:10:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Leo Prikler <leo.prikler <at> student.tugraz.at>
Cc: 45570 <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: [PATCH v3] system: Assert, that user and group names are unique.
Date: Mon, 11 Jan 2021 14:09:49 +0100
Hi,

Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:

> *gnu/system/shadow.scm (find-duplicates): New variable.
> (assert-unique-account-names, assert-unique-group-names): New variables.
> (account-activation): Use them here.

LGTM, thanks! :-)

Ludo’.




Reply sent to Leo Prikler <leo.prikler <at> student.tugraz.at>:
You have taken responsibility. (Mon, 11 Jan 2021 15:07:02 GMT) Full text and rfc822 format available.

Notification sent to Jason Conroy <conjaroy <at> gmail.com>:
bug acknowledged by developer. (Mon, 11 Jan 2021 15:07:02 GMT) Full text and rfc822 format available.

Message #49 received at 45570-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45570-done <at> debbugs.gnu.org, conjaroy <at> gmail.com
Subject: Re: [PATCH v3] system: Assert, that user and group names are unique.
Date: Mon, 11 Jan 2021 16:06:45 +0100
Am Montag, den 11.01.2021, 14:09 +0100 schrieb Ludovic Courtès:
> Hi,
> 
> Leo Prikler <leo.prikler <at> student.tugraz.at> skribis:
> 
> > *gnu/system/shadow.scm (find-duplicates): New variable.
> > (assert-unique-account-names, assert-unique-group-names): New
> > variables.
> > (account-activation): Use them here.
> 
> LGTM, thanks! :-)
> 
> Ludo’.
Aaaand it's pushed.





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 09 Feb 2021 12:24:10 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 49 days ago.

Previous Next


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