[PATCH] gnu: Validate substitute URLs for guix service.

  • Open
  • quality assurance status badge
Details
2 participants
  • Antero Mejr
  • Richard Sent
Owner
unassigned
Submitted by
Antero Mejr
Severity
normal
A
A
Antero Mejr wrote on 24 May 00:33 +0200
(address . bug-guix@gnu.org)
87cypcrwe1.fsf@antr.me
* gnu/services/base.scm (guix-service-type): Validate `substitute-urls' field.

Change-Id: I11ed74304ab02ae550db5479be9f02601857f294
---
If you forget to write "https://" when specifying a substitute URL in
guix-configuration, the system will end up in a bad state where any
`guix pull` or `guix reconfigure` operation immediately fail. It's
difficult to fix, so validate the field to avoid the problem.

gnu/services/base.scm | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (41 lines)
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 85160bd3ab..da26b86c83 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -99,6 +99,7 @@ (define-module (gnu services base)
#:use-module (srfi srfi-35)
#:use-module (ice-9 match)
#:use-module (ice-9 format)
+ #:use-module (web uri)
#:re-export (user-processes-service-type ;backwards compatibility
%default-substitute-urls)
#:export (fstab-service-type
@@ -2075,6 +2076,11 @@ (define (guix-extension-merge a b)
(chroot-directories (append (guix-extension-chroot-directories a)
(guix-extension-chroot-directories b)))))
+(define (validate-substitute-url url-str)
+ (if (string->uri url-str)
+ url-str
+ (error "Not a valid substitute URL: " url-str)))
+
(define guix-service-type
(service-type
(name 'guix)
@@ -2093,8 +2099,10 @@ (define guix-service-type
(inherit config)
(authorized-keys (append (guix-extension-authorized-keys extension)
(guix-configuration-authorized-keys config)))
- (substitute-urls (append (guix-extension-substitute-urls extension)
- (guix-configuration-substitute-urls config)))
+ (substitute-urls
+ (map validate-substitute-url
+ (append (guix-extension-substitute-urls extension)
+ (guix-configuration-substitute-urls config))))
(build-machines
(and (or (guix-configuration-build-machines config)
(pair? (guix-extension-build-machines extension)))

base-commit: 9901416233867233192b63fde7f616751127b189
--
2.41.0
R
R
Richard Sent wrote on 24 May 16:19 +0200
(name . Antero Mejr)(address . mail@antr.me)(address . 71153@debbugs.gnu.org)
87cypbuwaz.fsf@freakingpenguin.com
Antero Mejr <mail@antr.me> writes:

Toggle quote (11 lines)
> +(define (validate-substitute-url url-str)
> + (if (string->uri url-str)
> + url-str
> + (error "Not a valid substitute URL: " url-str)))
> +

> + (substitute-urls
> + (map validate-substitute-url
> + (append (guix-extension-substitute-urls extension)
> + (guix-configuration-substitute-urls config))))

Should we instead create a validate-substitute-urls and use that as a
sanitizer for the guix-extension and guix-configuration records? This
would catch errors during record creation instead of service creation,
as well as still perform validation if anything else does or will use
those records in the future.

--
Take it easy,
Richard Sent
Making my computer weirder one commit at a time.
A
A
Antero Mejr wrote on 24 May 20:26 +0200
(name . Richard Sent)(address . richard@freakingpenguin.com)(address . 71153@debbugs.gnu.org)
87a5kfjcb3.fsf@antr.me
Richard Sent <richard@freakingpenguin.com> writes:

Toggle quote (6 lines)
> Should we instead create a validate-substitute-urls and use that as a
> sanitizer for the guix-extension and guix-configuration records? This
> would catch errors during record creation instead of service creation,
> as well as still perform validation if anything else does or will use
> those records in the future.

The problem mentioned in the initial patch only occurs when the invalid
URLs are used in the Guix service type. Having them in the record isn't
a problem. I don't there there is a major difference in where the
validation occurs though.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 71153@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71153
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch