GNU bug report logs - #77201
[PATCH] guix: substitute-key-authorization: Fix case when acl symlink is broken

Previous Next

To reply to this bug, email your comments to 77201 AT debbugs.gnu.org.

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

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

From: Rutherther <rutherther <at> ditigal.xyz>
To: guix-patches <at> gnu.org
Cc: Rutherther <rutherther <at> ditigal.xyz>
Subject: [PATCH] guix: substitute-key-authorization: Fix case when acl symlink
 is broken
Date: Sun, 23 Mar 2025 10:48:28 +0100

One possible solution for an issue when /etc/guix/acl file exists, but points
to a non-existent location. This can for example happen if one is
reinitializing the system, and remove only /gnu/store and /var/guix, keep the
rest okay. This is a major advantage of guix as compared to other distros that
usually need you to reinitialize the whole root partition. But this will leave
the user with acl file pointing to non-existent location. The file-exists?
procedure will return #f for broken symbolic links.

I think that another reason one would get this issue is, if one was booted in
a live iso, chrooted, fixing their system. They would switch generations to
one with different acl file, delete other generations gc rooting the original
acl file and then gc. One could do this approach for example when recovering
from file corruptions in the store, to get rid of the unsubstitutable paths
that can't be repaired with guix gc --verify.

I don't know if there is a better way as I am not that proficient in guile,
but I definitely think this should be fixed and it should be checked if
anything exists in that place, not that the link points to a known location.
Does Guile have a procedure for that that I am missing? If not, shouldn't
we create one in Guix? I can imagine this being a common mistake, where we
want to check if something exists at place 'x', without caring if it's
actually an accessible file. I was looking online and someone made themselves
a function 'file-exists??' that checked basically this what I did here - that
it's either a valid file on the disk, or a broken symlink.

During debugging this issue I also noticed similar issue can occur in special
files and /run/current-system with the .new files that are created with
symlink procedure without checking for their existence. While it's not likely
(especially because the symlink is moved the second it's created)
the user would have /run/current-system.new nor /bin/sh.new etc., I still
think it would be worth fixing to make sure the system can boot even in cases
where something goes horribly wrong.

* gnu/services/base.scm (substitute-key-authorization): Check if acl file is a
(broken) symbolic link

Change-Id: I2f8170606b2f4afeea48f04acfd738b04cafc7cf
---
 gnu/services/base.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 0d2bb31190..e419d043ae 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1845,7 +1845,7 @@ (define (substitute-key-authorization keys guix)
         ;; If the ACL already exists, move it out of the way.  Create a backup
         ;; if it's a regular file: it's likely that the user manually updated
         ;; it with 'guix archive --authorize'.
-        (if (file-exists? acl-file)
+        (if (or (file-exists? acl-file) (symbolic-link? acl-file))
             (if (and (symbolic-link? acl-file)
                      (store-file-name? (readlink acl-file)))
                 (delete-file acl-file)

base-commit: fbfd2b93831978aadbb96f32cafdab997b04c6c6
prerequisite-patch-id: cf473eb15513404ca1d287f5b7eca109c848203c
prerequisite-patch-id: a46e75bdd193acb5e276e0aa31c77197a3254699
prerequisite-patch-id: a2b4aa0a33d89ee3f6c483aeb71a783cb0e63aa9
-- 
2.49.0

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

From: Ian Eure <ian <at> retrospec.tv>
To: Rutherther <rutherther <at> ditigal.xyz>
Cc: 77201 <at> debbugs.gnu.org
Subject: Re: [bug#77201] [PATCH] guix: substitute-key-authorization: Fix
 case when acl symlink is broken
Date: Sat, 29 Mar 2025 10:09:24 -0700

Hi Rutherther,

Rutherther <rutherther <at> ditigal.xyz> writes:

> One possible solution for an issue when /etc/guix/acl file 
> exists, but points
> to a non-existent location. This can for example happen if one 
> is
> reinitializing the system, and remove only /gnu/store and 
> /var/guix, keep the
> rest okay. This is a major advantage of guix as compared to 
> other distros that
> usually need you to reinitialize the whole root partition. But 
> this will leave
> the user with acl file pointing to non-existent location. The 
> file-exists?
> procedure will return #f for broken symbolic links.
>
> I think that another reason one would get this issue is, if one 
> was booted in
> a live iso, chrooted, fixing their system. They would switch 
> generations to
> one with different acl file, delete other generations gc rooting 
> the original
> acl file and then gc. One could do this approach for example 
> when recovering
> from file corruptions in the store, to get rid of the 
> unsubstitutable paths
> that can't be repaired with guix gc --verify.
>
> I don't know if there is a better way as I am not that 
> proficient in guile,
> but I definitely think this should be fixed and it should be 
> checked if
> anything exists in that place, not that the link points to a 
> known location.
> Does Guile have a procedure for that that I am missing? If not, 
> shouldn't
> we create one in Guix? I can imagine this being a common 
> mistake, where we
> want to check if something exists at place 'x', without caring 
> if it's
> actually an accessible file. I was looking online and someone 
> made themselves
> a function 'file-exists??' that checked basically this what I 
> did here - that
> it's either a valid file on the disk, or a broken symlink.
>
> During debugging this issue I also noticed similar issue can 
> occur in special
> files and /run/current-system with the .new files that are 
> created with
> symlink procedure without checking for their existence. While 
> it's not likely
> (especially because the symlink is moved the second it's 
> created)
> the user would have /run/current-system.new nor /bin/sh.new 
> etc., I still
> think it would be worth fixing to make sure the system can boot 
> even in cases
> where something goes horribly wrong.

Thanks for the explanation.

> * gnu/services/base.scm (substitute-key-authorization): Check if 
> acl file is a
> (broken) symbolic link
>
> Change-Id: I2f8170606b2f4afeea48f04acfd738b04cafc7cf
> ---
>  gnu/services/base.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/services/base.scm b/gnu/services/base.scm
> index 0d2bb31190..e419d043ae 100644
> --- a/gnu/services/base.scm
> +++ b/gnu/services/base.scm
> @@ -1845,7 +1845,7 @@ (define (substitute-key-authorization keys 
> guix)
>          ;; If the ACL already exists, move it out of the way. 
>          Create a backup
>          ;; if it's a regular file: it's likely that the user 
>          manually updated
>          ;; it with 'guix archive --authorize'.
> -        (if (file-exists? acl-file)
> +        (if (or (file-exists? acl-file) (symbolic-link? 
> acl-file))

Guile semantics are unhelpful here: `file-exists?' returns #f for 
a broken symlink, but `symbolic-link?' raises an exception if 
given a nonexistent path.  The means that if /etc/guix/acl doesn’t 
exist, an exception will be raised.  There doesn’t appear to be a 
simple way to determine if a file exists which doesn’t resolve 
symlinks, which I think is a Guile bug.

Thinking through the possible situations here:

If /etc/guix/acl is a good symlink pointing into /gnu/store -> 
delete it.

If /etc/guix/acl is a broken symlink pointing anywhere -> delete 
it.

If /etc/guix/acl is a file -> rename it to ".bak"

else /etc/guix/acl must be missing -> mkdir-p /etc/acl.

...then populate /etc/guix/acl.

I think the right move here is to refactor the nested `if's into a 
cond to simplify the logic, and wrap `symbolic-link?' in 
`with-exception-handler' (possibly `let’-binding its result, since 
multiple things need it).

Thanks,

 -- Ian

Previous Next

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