diff mbox series

[bug#77396,v2,1/2] least-authority: Preserve systemd LISTEN_* environment variables.

Message ID a9698fbc790a5ab4a1eabd9a65dd552077a29f7b.1743662589.git.maxim.cournoyer@gmail.com
State New
Headers
Series [bug#77396,v2,1/2] least-authority: Preserve systemd LISTEN_* environment variables. |

Commit Message

Maxim Cournoyer April 3, 2025, 6:43 a.m. UTC 
Otherwise, combining make-systemd-constructor with least-authority-wrapper
would not work correctly out of the box.

* guix/least-authority.scm (%precious-variables): Rename to...
(%default-preserved-environment-variables): ... this, and export it.
Add "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES" environment variables.
(least-authority-wrapper): Adjust accordingly.

Change-Id: Idd259b15463920965f530e1917d76bf97def3b7b
---
 guix/least-authority.scm | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)


base-commit: 8c43056aabc2d22da61dc86049b143f7ae1ef516

Comments

Ludovic Courtès April 3, 2025, 9:29 a.m. UTC | #1 
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Otherwise, combining make-systemd-constructor with least-authority-wrapper
> would not work correctly out of the box.
>
> * guix/least-authority.scm (%precious-variables): Rename to...
> (%default-preserved-environment-variables): ... this, and export it.
> Add "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES" environment variables.
> (least-authority-wrapper): Adjust accordingly.
>
> Change-Id: Idd259b15463920965f530e1917d76bf97def3b7b

[...]

> -(define %precious-variables
> +(define %default-preserved-environment-variables
>    ;; Environment variables preserved by the wrapper by default.
> -  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"))
> +  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"
> +    "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES")) ;for make-systemd-constructor

I would not export this variable, but otherwise LGTM!

Thanks,
Ludo’.
Maxim Cournoyer April 3, 2025, 10:47 a.m. UTC | #2 
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Otherwise, combining make-systemd-constructor with least-authority-wrapper
>> would not work correctly out of the box.
>>
>> * guix/least-authority.scm (%precious-variables): Rename to...
>> (%default-preserved-environment-variables): ... this, and export it.
>> Add "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES" environment variables.
>> (least-authority-wrapper): Adjust accordingly.
>>
>> Change-Id: Idd259b15463920965f530e1917d76bf97def3b7b
>
> [...]
>
>> -(define %precious-variables
>> +(define %default-preserved-environment-variables
>>    ;; Environment variables preserved by the wrapper by default.
>> -  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"))
>> +  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"
>> +    "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES")) ;for make-systemd-constructor
>
> I would not export this variable, but otherwise LGTM!

It aims to make extending the list easier.  Otherwise one has to peek
into the code, and copy the existing list to be consed to.

Perhaps you mean that you don't think this should be extensible?  And if
something important is missing we can simply add it like I've done for
the LISTEN_* variables here?
Ludovic Courtès April 8, 2025, 9:49 a.m. UTC | #3 
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>>
>>> Otherwise, combining make-systemd-constructor with least-authority-wrapper
>>> would not work correctly out of the box.
>>>
>>> * guix/least-authority.scm (%precious-variables): Rename to...
>>> (%default-preserved-environment-variables): ... this, and export it.
>>> Add "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES" environment variables.
>>> (least-authority-wrapper): Adjust accordingly.
>>>
>>> Change-Id: Idd259b15463920965f530e1917d76bf97def3b7b
>>
>> [...]
>>
>>> -(define %precious-variables
>>> +(define %default-preserved-environment-variables
>>>    ;; Environment variables preserved by the wrapper by default.
>>> -  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"))
>>> +  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"
>>> +    "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES")) ;for make-systemd-constructor
>>
>> I would not export this variable, but otherwise LGTM!
>
> It aims to make extending the list easier.  Otherwise one has to peek
> into the code, and copy the existing list to be consed to.

Ah yes, that makes sense to me.  Let’s export it then!

Ludo’.
Maxim Cournoyer April 14, 2025, 1:19 a.m. UTC | #4 
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>>> -(define %precious-variables
>>>> +(define %default-preserved-environment-variables
>>>>    ;; Environment variables preserved by the wrapper by default.
>>>> -  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"))
>>>> +  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"
>>>> +    "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES")) ;for make-systemd-constructor
>>>
>>> I would not export this variable, but otherwise LGTM!
>>
>> It aims to make extending the list easier.  Otherwise one has to peek
>> into the code, and copy the existing list to be consed to.
>
> Ah yes, that makes sense to me.  Let’s export it then!

I had already pushed this without exporting it.  Let's revisit the next
time we have a reason to extend the list.
diff mbox series

Patch

diff --git a/guix/least-authority.scm b/guix/least-authority.scm
index 3465fe9a48..cd846aaa61 100644
--- a/guix/least-authority.scm
+++ b/guix/least-authority.scm
@@ -26,7 +26,8 @@  (define-module (guix least-authority)
                                           spec->file-system
                                           file-system->spec
                                           file-system-mapping->bind-mount)
-  #:export (least-authority-wrapper))
+  #:export (least-authority-wrapper
+            %default-preserved-environment-variables))
 
 ;;; Commentary:
 ;;;
@@ -35,9 +36,10 @@  (define-module (guix least-authority)
 ;;;
 ;;; Code:
 
-(define %precious-variables
+(define %default-preserved-environment-variables
   ;; Environment variables preserved by the wrapper by default.
-  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"))
+  '("HOME" "USER" "LOGNAME" "DISPLAY" "XAUTHORITY" "TERM" "TZ" "PAGER"
+    "LISTEN_PID" "LISTEN_FDS" "LISTEN_FDNAMES")) ;for make-systemd-constructor
 
 (define* (least-authority-wrapper program
                                   #:key (name "pola-wrapper")
@@ -49,7 +51,7 @@  (define* (least-authority-wrapper program
                                   (namespaces %namespaces)
                                   (directory "/")
                                   (preserved-environment-variables
-                                   %precious-variables))
+                                   %default-preserved-environment-variables))
   "Return a wrapper of PROGRAM that executes it with the least authority.
 
 PROGRAM is executed in separate namespaces according to NAMESPACES, a list of