unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Disabling authentication checks for tests in local Guix checkouts
@ 2024-06-10  6:20 Ada Stevenson
  2024-06-17 13:05 ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Ada Stevenson @ 2024-06-10  6:20 UTC (permalink / raw)
  To: Guix Devel; +Cc: Ludovic Courtès

Hi Guix,

I'm currently trying to help test the changes to GRUB submitted in issue 
#71348[1]. Unfortunately, `make check`, whilst building the local Guix 
channel, authenticates every commit. That means commits not signed by 
people in `guix-authorizations` will stop the channel from building and 
prevents running any of the tests.

I have tried to fix the issue by writing a patch:
-----
diff --git a/etc/system-tests.scm b/etc/system-tests.scm
index 221a63bb7f..6655850396 100644
--- a/etc/system-tests.scm
+++ b/etc/system-tests.scm
@@ -49,7 +49,7 @@ (define (tests-for-current-guix source commit)
    ;; of tests to run in the usual way:
    ;;
    ;;   make check-system TESTS=installed-os
-  (let ((guix (channel-source->package source #:commit commit)))
+  (let ((guix (channel-source->package source #:commit commit 
#:authenticate? #f)))
      (map (lambda (test)
             (system-test
              (inherit test)
diff --git a/gnu/packages/package-management.scm 
b/gnu/packages/package-management.scm
index 54521ab3c4..46d66604c7 100644
--- a/gnu/packages/package-management.scm
+++ b/gnu/packages/package-management.scm
@@ -563,7 +563,7 @@ (define-public guix
  the Nix package manager.")
        (license license:gpl3+))))

-(define* (channel-source->package source #:key commit)
+(define* (channel-source->package source #:key commit (authenticate? #t))
    "Return a package for the given channel SOURCE, a lowerable object."
    (package
      (inherit guix)
@@ -571,7 +571,8 @@ (define* (channel-source->package source #:key commit)
                              (if commit (string-take commit 7) "")))
      (build-system channel-build-system)
      (arguments `(#:source ,source
-                 #:commit ,commit))
+                 #:commit ,commit
+                 #:authenticate? ,authenticate?))
      (inputs '())
      (native-inputs '())
      (propagated-inputs '())))
-----

Initially this seems like it works, as it starts to index all of the 
commits in the local checkout, which didn't occur beforehand, instead 
immediately giving a `Git error: cannot locate remote-tracking branch 
'origin/keyring'` (I've run `git fetch -a`, and my Savannah remote is 
called `origin`).

I'm not sure where this second building of the channel is occurring; the 
channel-build-system is being passed the `authenticate? #f` flag, so I'm 
at a loss.

I'd really appreciate any input or help!

Warmly,
Ada

[1] https://issues.guix.gnu.org/issue/71348


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Disabling authentication checks for tests in local Guix checkouts
  2024-06-10  6:20 Disabling authentication checks for tests in local Guix checkouts Ada Stevenson
@ 2024-06-17 13:05 ` Ludovic Courtès
  2024-06-17 14:02   ` Suhail Singh
  2024-06-18  8:25   ` Ada Stevenson
  0 siblings, 2 replies; 6+ messages in thread
From: Ludovic Courtès @ 2024-06-17 13:05 UTC (permalink / raw)
  To: Ada Stevenson; +Cc: Guix Devel

Hi Ada,

Ada Stevenson <adanskana@gmail.com> skribis:

> I'm currently trying to help test the changes to GRUB submitted in
> issue #71348[1]. Unfortunately, `make check`, whilst building the
> local Guix channel, authenticates every commit. That means commits not
> signed by people in `guix-authorizations` will stop the channel from
> building and prevents running any of the tests.

Bummer.  :-(

> I have tried to fix the issue by writing a patch:

This patch looks like the right thing to do.

I’m not sure how to integrate it though: in the general case, we
probably want to keep authentication enabled by default, but how to
allow users to easily disable it when using a personal checkout?

> Initially this seems like it works, as it starts to index all of the
> commits in the local checkout, which didn't occur beforehand, instead
> immediately giving a `Git error: cannot locate remote-tracking branch
> 'origin/keyring'` (I've run `git fetch -a`, and my Savannah remote is
> called `origin`).

Guix caches clones under ~/.cache/guix/checkout.  It may be that there’s
a clone of your local repo there, but that the clone itself lack a copy
of the ‘keyring’ branch?  (You can run ‘git fetch -a’ in there.)

Thanks for testing!

Ludo’.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Disabling authentication checks for tests in local Guix checkouts
  2024-06-17 13:05 ` Ludovic Courtès
@ 2024-06-17 14:02   ` Suhail Singh
  2024-06-17 21:52     ` Ludovic Courtès
  2024-06-18  8:25   ` Ada Stevenson
  1 sibling, 1 reply; 6+ messages in thread
From: Suhail Singh @ 2024-06-17 14:02 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Ada Stevenson, Guix Devel

Ludovic Courtès <ludo@gnu.org> writes:

> I’m not sure how to integrate it though: in the general case, we
> probably want to keep authentication enabled by default, but how to
> allow users to easily disable it when using a personal checkout?

Could you please elaborate on what the challenge is?

Is the challenge in inferring when they are using a personal checkout?
Would it be a challenge to have the user provide an option instead?
Said option could either be passed in explicitly every-time, or a
command provided to allow the user to set/unset the option (which would
get stored, say, in .git/config under guix "authentication") and the
code updated to read the preference from there.

-- 
Suhail


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Disabling authentication checks for tests in local Guix checkouts
  2024-06-17 14:02   ` Suhail Singh
@ 2024-06-17 21:52     ` Ludovic Courtès
  2024-06-18  2:36       ` Suhail Singh
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2024-06-17 21:52 UTC (permalink / raw)
  To: Suhail Singh; +Cc: Ada Stevenson, Guix Devel

Suhail Singh <suhailsingh247@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> I’m not sure how to integrate it though: in the general case, we
>> probably want to keep authentication enabled by default, but how to
>> allow users to easily disable it when using a personal checkout?
>
> Could you please elaborate on what the challenge is?
>
> Is the challenge in inferring when they are using a personal checkout?
> Would it be a challenge to have the user provide an option instead?
> Said option could either be passed in explicitly every-time, or a
> command provided to allow the user to set/unset the option (which would
> get stored, say, in .git/config under guix "authentication") and the
> code updated to read the preference from there.

The challenge is in determining that Guix is running from a local
checkout.  Now that I think about it, it’s not that hard: ./pre-inst-env
sets ‘GUIX_UNINSTALLED’.  So we could do:

  #:authenticate? (not (getenv "GUIX_UNINSTALLED"))

Problem is that an attacker could lead a user to disable authentication
by getting them to set this seemingly unrelated environment variable.

The ‘.git/config’ option you propose is not available because that all
happens with the Guix-managed cached checkout under
~/.cache/guix/checkouts.

Maybe a specific environment variable would do?

Ludo’.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Disabling authentication checks for tests in local Guix checkouts
  2024-06-17 21:52     ` Ludovic Courtès
@ 2024-06-18  2:36       ` Suhail Singh
  0 siblings, 0 replies; 6+ messages in thread
From: Suhail Singh @ 2024-06-18  2:36 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Suhail Singh, Ada Stevenson, Guix Devel

Ludovic Courtès <ludo@gnu.org> writes:

> The challenge is in determining that Guix is running from a local
> checkout.  Now that I think about it, it’s not that hard: ./pre-inst-env
> sets ‘GUIX_UNINSTALLED’.  So we could do:
>
>   #:authenticate? (not (getenv "GUIX_UNINSTALLED"))
>
> Problem is that an attacker could lead a user to disable authentication
> by getting them to set this seemingly unrelated environment variable.
>
> The ‘.git/config’ option you propose is not available because that all
> happens with the Guix-managed cached checkout under
> ~/.cache/guix/checkouts.

Thank you for the detailed explanation.

> Maybe a specific environment variable would do?

Perhaps.  What is the threat model of the attacker?

-- 
Suhail


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Disabling authentication checks for tests in local Guix checkouts
  2024-06-17 13:05 ` Ludovic Courtès
  2024-06-17 14:02   ` Suhail Singh
@ 2024-06-18  8:25   ` Ada Stevenson
  1 sibling, 0 replies; 6+ messages in thread
From: Ada Stevenson @ 2024-06-18  8:25 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Guix Devel

Hi Ludo'

On 17/06/2024 1:05 pm, Ludovic Courtès wrote:
> Hi Ada,
> 
> Ada Stevenson <adanskana@gmail.com> skribis:
> 
>> I'm currently trying to help test the changes to GRUB submitted in
>> issue #71348[1]. Unfortunately, `make check`, whilst building the
>> local Guix channel, authenticates every commit. That means commits not
>> signed by people in `guix-authorizations` will stop the channel from
>> building and prevents running any of the tests.
> 
> Bummer.  :-(
> 
>> I have tried to fix the issue by writing a patch:
> 
> This patch looks like the right thing to do.
> 
> I’m not sure how to integrate it though: in the general case, we
> probably want to keep authentication enabled by default, but how to
> allow users to easily disable it when using a personal checkout?
> 
>> Initially this seems like it works, as it starts to index all of the
>> commits in the local checkout, which didn't occur beforehand, instead
>> immediately giving a `Git error: cannot locate remote-tracking branch
>> 'origin/keyring'` (I've run `git fetch -a`, and my Savannah remote is
>> called `origin`).
> 
> Guix caches clones under ~/.cache/guix/checkout.  It may be that there’s
> a clone of your local repo there, but that the clone itself lack a copy
> of the ‘keyring’ branch?  (You can run ‘git fetch -a’ in there.)

I tried invoking the tests from within a `./pre-inst-env guix repl` and 
managed to get both of the tests to run using my patch! They both passed 
too. I think you're right, there must have been some sort of caching error.

Anyway, I'm new to contributing using Debbugs. Is there a way to tag an 
issue as 'reviewed' or something like that? Or do I just send an email 
saying I tested the patch and everything looks good? Let me know :-)

> 
> Thanks for testing!

You're welcome! Thanks for writing the patch.

> 
> Ludo’.

Warmly,
Ada



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-06-18  8:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-10  6:20 Disabling authentication checks for tests in local Guix checkouts Ada Stevenson
2024-06-17 13:05 ` Ludovic Courtès
2024-06-17 14:02   ` Suhail Singh
2024-06-17 21:52     ` Ludovic Courtès
2024-06-18  2:36       ` Suhail Singh
2024-06-18  8:25   ` Ada Stevenson

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).