| Message ID | 20220605000425.20480-1-me@tobias.gr |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#55892] pull: Fail if cache directory ownership is suspect. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Tobias Geerinckx-Rice via Guix-patches via schreef op zo 05-06-2022 om 02:04 [+0200]: > Hi Guix, > > Another one in the ‘low-level support noise paper-cut’ series. > The XXX comment would not land upstream, I think. > > I didn't test this on a foreign distribution. My understanding is > that distributions where sudo already defaults to ‘-i’ won't throw > the warning nor suffer from the problem. > > Kind regards, > > T G-R > Concept looks sounds to me! Nitpick: + (let ((our:user (passwd:name (getpwuid our:uid))) + (dir:user (passwd:name (getpwuid dir:uid)))) what if the current user does not have an entry in /etc/passwd or equivalent? (E.g. if the user accidentally removed an entry in /etc/passwd on a foreign system and then runs "guix pull" & "guix shell THE_EDITOR" to get their favourite editor to edit /etc/passwd back?) Maybe in that case, it should be reported as NNNN (NNNN = user number)? Or would that be simply considered unsupported? Greetings, Maxime.
Maxime, Thanks for the swift review! Maxime Devos 写道： > Maybe in that case, it should be reported as NNNN (NNNN = user > number)? > Or would that be simply considered unsupported? Er… I'd say it's veering confidently into unsupported territory, yes. But falling back to user IDs costs next to nothing so I made the change. Thanks for the suggestion. Odd feeling that the error message might be more robust than some other part of the code now :-) Pushed as 7c52cad0464175370c44bd4695e4c01a62b8268f. Kind regards, T G-R
diff --git a/guix/scripts/pull.scm b/guix/scripts/pull.scm index f01764637b..1eaf8f087b 100644 --- a/guix/scripts/pull.scm +++ b/guix/scripts/pull.scm @@ -49,6 +49,7 @@ (define-module (guix scripts pull) #:autoload (gnu packages bootstrap) (%bootstrap-guile) #:autoload (gnu packages certs) (le-certs) #:use-module (srfi srfi-1) + #:use-module (srfi srfi-11) #:use-module (srfi srfi-26) #:use-module (srfi srfi-34) #:use-module (srfi srfi-35) @@ -810,6 +811,31 @@ (define (no-arguments arg _) ((assoc-ref opts 'generation) (process-generation-change opts profile)) (else + ;; Bail out early when users accidentally run, e.g., ’sudo guix pull’. + ;; If CACHE-DIRECTORY doesn't yet exist, test where it would end up. + (let-values (((st dir) (let loop ((dir (cache-directory))) + (let ((st (stat dir #f))) + (if st + (values (stat dir #f) dir) + (loop (dirname dir))))))) + (let ((dir:uid (stat:uid st)) + (our:uid (getuid))) + (unless (= dir:uid our:uid) + (let ((our:user (passwd:name (getpwuid our:uid))) + (dir:user (passwd:name (getpwuid dir:uid)))) + (raise + (condition + (&message + (message + (format #f (G_ "directory ‘~a’ is not owned by user ~a") + dir dir:user))) + (&fix-hint + (hint + ;; XXX We could check (getenv "SUDO_USER") to display this + ;; only under sudo, but that would imply handling doas… &c. + (format #f (G_ "You should run this command as ~a; use ‘sudo -i’ or equivalent if you really want to pull as ~a.") + dir:user our:user))))))))) + (with-store store (with-status-verbosity (assoc-ref opts 'verbosity) (parameterize ((%current-system (assoc-ref opts 'system))