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

From: Ian Eure <[email-masked]>
Date: Sat, 29 Mar 2025 10:09:24 -0700

Hi Rutherther,

Rutherther <[email-masked]> 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