GNU bug report logs - #76376
[PATCH] guix: gexp: canonicalize file paths for import

Previous Next

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

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

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

From: Ryan Sundberg <ryan <at> arctype.co>
To: guix-patches <at> gnu.org
Cc: Ryan Sundberg <ryan <at> arctype.co>
Subject: [PATCH] guix: gexp: canonicalize file paths for import
Date: Mon, 17 Feb 2025 13:58:44 -0800

When we intern a file from the store during `imported-modules`, if the
file is a symlink (e.g., from a Guix profile), a dangling symlink can be
created in the module-import builder.

Follow any symlinks before interning the files to the store, so that the
file itself is imported and not the dangling link.

See also: https://issues.guix.gnu.org/73275

* guix/gexp.scm (imported-files/derivation): canonicalize-path

Change-Id: Ic0af90cda7c5c5819526e455cf62300e18408dbd
---
 guix/gexp.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guix/gexp.scm b/guix/gexp.scm
index e44aea6420..85351b0322 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -1576,7 +1576,7 @@ (define* (imported-files/derivation files
   (define file-pair
     (match-lambda
      ((final-path . (? string? file-name))
-      (mlet %store-monad ((file (interned-file file-name
+      (mlet %store-monad ((file (interned-file (canonicalize-path file-name)
                                                (basename final-path))))
         (return (list final-path file))))
      ((final-path . file-like)

base-commit: 91b18baa4274a025d28f06133682a9269217730d
-- 
2.41.0

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

From: Ryan Sundberg <ryan <at> arctype.co>
To: 76376 <at> debbugs.gnu.org
Subject: Additional context for #76376 - gexp / canonicalize-path
Date: Mon, 17 Feb 2025 14:18:05 -0800

Hello team,

This is a patch for a "deep" bug in Guix gexp processing which evokes in 
circumstances when using g-expressions to build things that try to 
create module closures with code that is referenced in the current 
environment via symlink.

It can manifest in difficult to comprehend errors, such as "no code for 
module: (guix utils)` when guix/utils.scm is correctly defined in the 
load path of the program and exists (but it is a symlink, such as by 
using `guix shell` to load another guix environment, e.g. where the 
shell imports a different guix itself).

In my use case, I was using `guix` to build raw os disk images with my 
own set of customized packages and services when this bug blocked me at 
a dead stop.

The root cause of this after much complex debugging, tracing, and 
reading helped me to identify the bug report from Ludo at 
https://issues.guix.gnu.org/73275 and understand the dangling symlink 
issue. What happens here, and what this patch fixes, is that the 
`interned-file` procedure will not follow symlinks, and will intern a 
symlink if it is told to. In most scenarios this is harmless as the 
symlinks intersect to something (e.g. guix/utils.scm) which is already 
in the profile anyways, so the bug is dormant.
However, in other cases, it is possible to create a dangling symlink 
here when `imported-modules` references a file which is a symlink on the 
Guile %load-path, and `interned-file` in this line of gexp.scm can 
intern a dangling symlink.

This patch closes that possibility by canonicalizing the path of the 
interned file before loading it into the module closure path, so that 
`imported-modules` will never import a dangling symlink to a guile file 
used by a module-closure.

--Ryan

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ryan Sundberg <ryan <at> arctype.co>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 76376 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#76376] [PATCH] guix: gexp: canonicalize file paths for import
Date: Fri, 21 Feb 2025 11:56:48 +0100

Hi,

Ryan Sundberg <ryan <at> arctype.co> skribis:

> When we intern a file from the store during `imported-modules`, if the
> file is a symlink (e.g., from a Guix profile), a dangling symlink can be
> created in the module-import builder.
>
> Follow any symlinks before interning the files to the store, so that the
> file itself is imported and not the dangling link.
>
> See also: https://issues.guix.gnu.org/73275
>
> * guix/gexp.scm (imported-files/derivation): canonicalize-path
>
> Change-Id: Ic0af90cda7c5c5819526e455cf62300e18408dbd

[...]

>       ((final-path . (? string? file-name))
> -      (mlet %store-monad ((file (interned-file file-name
> +      (mlet %store-monad ((file (interned-file (canonicalize-path file-name)
>                                                 (basename final-path))))

Instead of calling ‘canonicalize-path’, which leads to many syscalls,
I’d suggest:

   (interned-file file-name (basename final-path)
                  #:recursive? #f)

I believe that would have the desired effect.

Could you also add a test that reproduces the problem?

Thanks,
Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ryan Sundberg <ryan <at> arctype.co>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 76376 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>,
 Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#76376] [PATCH] guix: gexp: canonicalize file paths for import
Date: Tue, 25 Feb 2025 16:08:24 +0100

[Message part 1 (text/plain, inline)]

Hello Ryan,

Ludovic Courtès <ludo <at> gnu.org> skribis:

>>       ((final-path . (? string? file-name))
>> -      (mlet %store-monad ((file (interned-file file-name
>> +      (mlet %store-monad ((file (interned-file (canonicalize-path file-name)
>>                                                 (basename final-path))))
>
> Instead of calling ‘canonicalize-path’, which leads to many syscalls,
> I’d suggest:
>
>    (interned-file file-name (basename final-path)
>                   #:recursive? #f)

It was missing one bit; attached is an version of it that works.

I chose ‘readlink*’ because it’s less expensive that
‘canonicalize-path’: only one extra syscall (readlink) when ‘file-name’
is already a regular file.

For the record, I stumbled upon this bug just today while working on
<https://issues.guix.gnu.org/75810>: the "imported-files does not create
symlinks" in ‘tests/gexp.scm’ would fail when running in an isolated
environment because file “x” would be a symlink to a file outside the
store.

Ludo’.

[Message part 2 (text/x-patch, inline)]

diff --git a/guix/gexp.scm b/guix/gexp.scm
index ad51bc55b78..ddd2e1a0812 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -1584,8 +1584,9 @@ (define* (imported-files/derivation files
   (define file-pair
     (match-lambda
      ((final-path . (? string? file-name))
-      (mlet %store-monad ((file (interned-file file-name
-                                               (basename final-path))))
+      (mlet %store-monad ((file (interned-file (readlink* file-name)
+                                               (basename final-path)
+                                               #:recursive? #f)))
         (return (list final-path file))))
      ((final-path . file-like)
       (mlet %store-monad ((file (lower-object file-like system)))

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ryan Sundberg <ryan <at> arctype.co>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Christopher Baines <guix <at> cbaines.net>,
 76376-done <at> debbugs.gnu.org
Subject: Re: [bug#76376] [PATCH] guix: gexp: canonicalize file paths for import
Date: Mon, 10 Mar 2025 15:45:02 +0100

Hi Ryan,

Ludovic Courtès <ludo <at> gnu.org> skribis:

> I chose ‘readlink*’ because it’s less expensive that
> ‘canonicalize-path’: only one extra syscall (readlink) when ‘file-name’
> is already a regular file.

I went ahead and pushed this change as
24478808756c2787e4cb60d2d0e97f87924d2aa4, but leaving #:recursive? #t
since we still want to be able to import executable files.

Thanks again,
Ludo’.

Previous Next

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