GNU bug report logs - #77383
[PATCH 0/2] Run speakersafetyd as unprivileged user

Previous Next

Full log

View this message in rfc822 format

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Roman Scherer <roman <at> burningswell.com>
Cc: 77383 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: [bug#77383] [PATCH 2/2] gnu: speakersafetyd: Run as unprivileged user.
Date: Thu, 03 Apr 2025 19:43:23 +0900

Hi,

Roman Scherer <roman <at> burningswell.com> writes:

> * gnu/services/sound.scm (speakersafetyd): Run as unprivileged user.

Sounds good, perhaps also mention it adds a log file (is this related to
this change?).

[...]

> +(define speakersafetyd-accounts
> +  (match-record-lambda <speakersafetyd-configuration>
> +      (blackbox-directory configuration-directory group log-file maximum-gain-reduction speakersafetyd user)

Please break this and next long lines into something that fits < 80
characters.  You can use the Emacs indentation hack to do so and leave a
space after the opening parens to ensure it gets indented as data and
not a procedure:

         ( blackbox-directory configuration-directory ...
           speakersafetyd user)
         
> +    (list (user-group
> +           (name group)
> +           (system? #t))
> +          (user-account
> +           (name user)
> +           (group group)
> +           (system? #t)
> +           (home-directory "/var/empty")
> +           (shell (file-append shadow "/sbin/nologin"))
> +           (supplementary-groups '("audio"))))))
> +
> +(define speakersafetyd-activation
> +  (match-record-lambda <speakersafetyd-configuration>
> +      (blackbox-directory configuration-directory group log-file maximum-gain-reduction speakersafetyd user)

Line width > 80 columns.

> +    (with-imported-modules (source-module-closure
> +                            '((gnu build activation)
> +                              (guix build utils)))

Looks like you only use (gnu build activation), not (guix build utils)
in the below snippet.

> +      #~(begin
> +          (use-modules (gnu build activation))
> +          (let ((user (getpwnam #$user)))
> +            (mkdir-p/perms "/run/speakersafetyd" user #o755)
> +            (mkdir-p/perms "/var/lib/speakersafetyd" user #o755)
> +            ;; Blackbox files contain audio recordings and might be sensitive information
> +            (mkdir-p/perms #$blackbox-directory user #o700))))))
>  
>  (define speakersafetyd-shepherd-service
>    (match-record-lambda <speakersafetyd-configuration>
> -      (blackbox-directory configuration-directory maximum-gain-reduction speakersafetyd)
> +      (blackbox-directory configuration-directory group log-file maximum-gain-reduction speakersafetyd user)

Line width > 80 columns.

>      (shepherd-service
>       (documentation "Run the speaker safety daemon")
>       (provision '(speakersafetyd))
> @@ -306,7 +345,11 @@ (define speakersafetyd-shepherd-service
>                 (list #$(file-append speakersafetyd "/bin/speakersafetyd")
>                       "--config-path" #$configuration-directory
>                       "--blackbox-path" #$blackbox-directory
> -                     "--max-reduction" (number->string #$maximum-gain-reduction))))
> +                     "--max-reduction" (number->string #$maximum-gain-reduction))
> +               #:group #$group
> +               #:log-file #$log-file
> +               #:supplementary-groups '("audio")
> +               #:user #$user))
>       (stop #~(make-kill-destructor)))))
>  
>  (define speakersafetyd-service-type
> @@ -324,7 +367,13 @@ (define speakersafetyd-service-type
>             (compose list speakersafetyd-configuration-speakersafetyd))
>            (service-extension
>             profile-service-type
> -           (compose list speakersafetyd-configuration-speakersafetyd))))
> +           (compose list speakersafetyd-configuration-speakersafetyd))
> +          (service-extension
> +           account-service-type
> +           speakersafetyd-accounts)
> +          (service-extension
> +           activation-service-type
> +           speakersafetyd-activation)))

nitpick but I like to put at least one argument on the same line unless
respecting the 80 columns max width is challenging, as in:

--8<---------------cut here---------------start------------->8---
(service-extension account-service-type
                   speakersafetyd-accounts)
--8<---------------cut here---------------end--------------->8---

etc.

Other than these tiny details, it LGTM.  Could you please send a v2?

-- 
Thanks,
Maxim

Previous Next

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