[bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit

all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed

* [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit
@ 2022-01-28 17:31 Ludovic Courtès
  2022-01-28 17:43 ` [bug#53608] [PATCH 1/2] git: Add 'commit-descendant?' Ludovic Courtès
  2022-02-08 23:02 ` [bug#53608] [PATCH 0/2] Rejecting commits unrelated to " Ludovic Courtès
  0 siblings, 2 replies; 6+ messages in thread
From: Ludovic Courtès @ 2022-01-28 17:31 UTC (permalink / raw)
  To: 53608; +Cc: Ludovic Courtès, Maxime Devos, Attila Lendvai

Hello!

This patch series fixes a bug in the checkout authentication code:
it would be possible to authenticate a commit unrelated to the
introductory commit, provided that target commit passes the
authorization invariant (see the commit log for details).

Users of Guix and of third-party channels are safe: this bug does
not have any impact on checkout authentication in those cases.

What concrete cases are affected?  Suppose someone forks Guix and
publishes a new channel introduction for their fork.  The expectation
is that any branch started before the introductory channel, for
instance in the original Guix repo, would fail to be authenticated.
However, because of this bug, such a branch would be considered
authentic in the fork because all its commits pass the authorization
invariant (IOW, they are authentic in the original repository).

Thoughts?

Ludo'.

Ludovic Courtès (2):
  git: Add 'commit-descendant?'.
  git-authenticate: Ensure the target is a descendant of the
    introductory commit.

 doc/guix.texi                  |  4 ++-
 guix/git-authenticate.scm      | 17 ++++++++--
 guix/git.scm                   | 24 +++++++++++++-
 tests/channels.scm             | 60 +++++++++++++++++++++++++++++++++-
 tests/git-authenticate.scm     | 44 +++++++++++++++++++++++++
 tests/git.scm                  | 52 ++++++++++++++++++++++++++++-
 tests/guix-git-authenticate.sh | 17 ++++++++--
 7 files changed, 210 insertions(+), 8 deletions(-)

base-commit: 5052f76afd02e27d6484acf74c86bfa1b6f9cd0e
-- 
2.34.0

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

* [bug#53608] [PATCH 1/2] git: Add 'commit-descendant?'.
  2022-01-28 17:31 [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit Ludovic Courtès
@ 2022-01-28 17:43 ` Ludovic Courtès
  2022-01-28 17:43   ` [bug#53608] [PATCH 2/2] git-authenticate: Ensure the target is a descendant of the introductory commit Ludovic Courtès
  2022-02-08 23:02 ` [bug#53608] [PATCH 0/2] Rejecting commits unrelated to " Ludovic Courtès
  1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2022-01-28 17:43 UTC (permalink / raw)
  To: 53608; +Cc: Ludovic Courtès

* guix/git.scm (commit-descendant?): New procedure.
* tests/git.scm ("commit-descendant?"): New test.
---
 guix/git.scm  | 24 +++++++++++++++++++++++-
 tests/git.scm | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/guix/git.scm b/guix/git.scm
index 43e85a5026..53e7219c8c 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
-;;; Copyright © 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018-2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Kyle Meyer <kyle@kyleam.com>
 ;;; Copyright © 2021 Marius Bakke <marius@gnu.org>
 ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
@@ -46,6 +46,7 @@ (define-module (guix git)
   #:use-module (ice-9 ftw)
   #: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)
   #:export (%repository-cache-directory
@@ -60,6 +61,7 @@ (define-module (guix git)
             latest-repository-commit
             commit-difference
             commit-relation
+            commit-descendant?
 
             remote-refs
 
@@ -623,6 +625,26 @@ (define (commit-relation old new)
               (if (set-contains? oldest new)
                   'descendant
                   'unrelated))))))
+
+(define (commit-descendant? new old)
+  "Return true if NEW is the descendant of one of OLD, a list of commits.
+
+When the expected result is likely #t, this is faster than using
+'commit-relation' since fewer commits need to be traversed."
+  (let ((old (list->setq old)))
+    (let loop ((commits (list new))
+               (visited (setq)))
+      (match commits
+        (()
+         #f)
+        (_
+         ;; Perform a breadth-first search as this is likely going to
+         ;; terminate more quickly than a depth-first search.
+         (let ((commits (remove (cut set-contains? visited <>) commits)))
+           (or (any (cut set-contains? old <>) commits)
+               (loop (append-map commit-parents commits)
+                     (fold set-insert visited commits)))))))))
+
 \f
 ;;
 ;;; Remote operations.
diff --git a/tests/git.scm b/tests/git.scm
index d0646bbc85..ca59d2a33e 100644
--- a/tests/git.scm
+++ b/tests/git.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019, 2020, 2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz
 ;;;
 ;;; This file is part of GNU Guix.
@@ -162,6 +162,56 @@ (define-module (test-git)
               (commit-relation master1 merge)
               (commit-relation merge master1))))))
 
+(unless (which (git-command)) (test-skip 1))
+(test-equal "commit-descendant?"
+  '((master3 master3 => #t)
+    (master1 master3 => #f)
+    (master3 master1 => #t)
+    (master2 branch1 => #f)
+    (master2 branch1 master1 => #t)
+    (branch1 master2 => #f)
+    (branch1 merge   => #f)
+    (merge branch1   => #t)
+    (master1 merge   => #f)
+    (merge master1   => #t))
+  (with-temporary-git-repository directory
+      '((add "a.txt" "A")
+        (commit "first commit")
+        (branch "hack")
+        (checkout "hack")
+        (add "1.txt" "1")
+        (commit "branch commit")
+        (checkout "master")
+        (add "b.txt" "B")
+        (commit "second commit")
+        (add "c.txt" "C")
+        (commit "third commit")
+        (merge "hack" "merge"))
+    (with-repository directory repository
+      (let ((master1 (find-commit repository "first"))
+            (master2 (find-commit repository "second"))
+            (master3 (find-commit repository "third"))
+            (branch1 (find-commit repository "branch"))
+            (merge   (find-commit repository "merge")))
+        (letrec-syntax ((verify
+                         (syntax-rules ()
+                           ((_) '())
+                           ((_ (new old ...) rest ...)
+                            (cons `(new old ... =>
+                                        ,(commit-descendant? new
+                                                             (list old ...)))
+                                  (verify rest ...))))))
+          (verify (master3 master3)
+                  (master1 master3)
+                  (master3 master1)
+                  (master2 branch1)
+                  (master2 branch1 master1)
+                  (branch1 master2)
+                  (branch1 merge)
+                  (merge branch1)
+                  (master1 merge)
+                  (merge master1)))))))
+
 (unless (which (git-command)) (test-skip 1))
 (test-equal "remote-refs"
   '("refs/heads/develop" "refs/heads/master"
-- 
2.34.0





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

* [bug#53608] [PATCH 2/2] git-authenticate: Ensure the target is a descendant of the introductory commit.
  2022-01-28 17:43 ` [bug#53608] [PATCH 1/2] git: Add 'commit-descendant?' Ludovic Courtès
@ 2022-01-28 17:43   ` Ludovic Courtès
  0 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2022-01-28 17:43 UTC (permalink / raw)
  To: 53608; +Cc: Ludovic Courtès

Fixes a bug whereby authentication of a commit *not* descending from the
introductory commit could succeed, provided the commit verifies the
authorization invariant.

In the example below, A is a common ancestor of the introductory commit
I and of commit X.  Authentication of X would succeed, even though it is
not a descendant of I, as long as X is authorized according to the
'.guix-authorizations' in A:

   X   	 I
    \   /
      A

This is because, 'authenticate-repository' would not check whether X
descends from I, and the call (commit-difference X I) would return X.

In practice that only affects forks because it means that ancestors of
the introductory commit already contain a '.guix-authorizations' file.

* guix/git-authenticate.scm (authenticate-repository): Add call to
'commit-descendant?'.
* tests/channels.scm ("authenticate-channel, not a descendant of introductory commit"):
New test.
* tests/git-authenticate.scm ("authenticate-repository, target not a descendant of intro"):
New test.
* tests/guix-git-authenticate.sh: Expect earlier test to fail since
9549f0283a78fe36f2d4ff2a04ef8ad6b0c02604 is not a descendant of
$intro_commit.  Add new test targeting an ancestor of the introductory
commit, and another test targeting the v1.2.0 commit.
* doc/guix.texi (Specifying Channel Authorizations): Add a sentence.
---
 doc/guix.texi                  |  4 ++-
 guix/git-authenticate.scm      | 17 ++++++++--
 tests/channels.scm             | 60 +++++++++++++++++++++++++++++++++-
 tests/git-authenticate.scm     | 44 +++++++++++++++++++++++++
 tests/guix-git-authenticate.sh | 17 ++++++++--
 5 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 62e994ceb1..61f2d7a771 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -5448,7 +5448,9 @@ commit of a channel that should be authenticated.  The first time a
 channel is fetched with @command{guix pull} or @command{guix
 time-machine}, the command looks up the introductory commit and verifies
 that it is signed by the specified OpenPGP key.  From then on, it
-authenticates commits according to the rule above.
+authenticates commits according to the rule above.  Authentication fails
+if the target commit is neither a descendant nor an ancestor of the
+introductory commit.
 
 Additionally, your channel must provide all the OpenPGP keys that were
 ever mentioned in @file{.guix-authorizations}, stored as @file{.key}
diff --git a/guix/git-authenticate.scm b/guix/git-authenticate.scm
index ab3fcd8b2f..419cb85afc 100644
--- a/guix/git-authenticate.scm
+++ b/guix/git-authenticate.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019, 2020, 2021, 2022 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,7 +22,9 @@ (define-module (guix git-authenticate)
   #:use-module (guix base16)
   #:autoload   (guix base64) (base64-encode)
   #:use-module ((guix git)
-                #:select (commit-difference false-if-git-not-found))
+                #:select (commit-difference
+                          commit-descendant?
+                          false-if-git-not-found))
   #:use-module (guix i18n)
   #:use-module ((guix diagnostics) #:select (formatted-message))
   #:use-module (guix openpgp)
@@ -426,6 +428,17 @@ (define commits
           (verify-introductory-commit repository keyring
                                       start-commit signer))
 
+        ;; Make sure END-COMMIT is a descendant of START-COMMIT or of one of
+        ;; AUTHENTICATED-COMMITS, which are known to be descendants of
+        ;; START-COMMIT.
+        (unless (commit-descendant? end-commit
+                                    (cons start-commit
+                                          authenticated-commits))
+          (raise (formatted-message
+                  (G_ "commit ~a is not a descendant of introductory commit ~a")
+                  (oid->string (commit-id end-commit))
+                  (oid->string (commit-id start-commit)))))
+
         (let ((stats (call-with-progress-reporter reporter
                        (lambda (report)
                          (authenticate-commits repository commits
diff --git a/tests/channels.scm b/tests/channels.scm
index d45c450241..0fe870dbaf 100644
--- a/tests/channels.scm
+++ b/tests/channels.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
-;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019, 2020, 2022 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -525,6 +525,64 @@ (define (find-commit* message)
                                   #:keyring-reference-prefix "")
             'failed))))))
 
+(unless (gpg+git-available?) (test-skip 1))
+(test-equal "authenticate-channel, not a descendant of introductory commit"
+  #t
+  (with-fresh-gnupg-setup (list %ed25519-public-key-file
+                                %ed25519-secret-key-file
+                                %ed25519-2-public-key-file
+                                %ed25519-2-secret-key-file)
+    (with-temporary-git-repository directory
+        `((add ".guix-channel"
+               ,(object->string
+                 '(channel (version 0)
+                           (keyring-reference "master"))))
+          (add ".guix-authorizations"
+               ,(object->string
+                 `(authorizations (version 0)
+                                  ((,(key-fingerprint
+                                      %ed25519-public-key-file)
+                                    (name "Charlie"))))))
+          (add "signer.key" ,(call-with-input-file %ed25519-public-key-file
+                               get-string-all))
+          (commit "first commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file)))
+          (branch "alternate-branch")
+          (checkout "alternate-branch")
+          (add "something.txt" ,(random-text))
+          (commit "intro commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file)))
+          (checkout "master")
+          (add "random" ,(random-text))
+          (commit "second commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file))))
+      (with-repository directory repository
+        (let* ((commit1 (find-commit repository "first"))
+               (commit2 (find-commit repository "second"))
+               (commit0 (commit-lookup
+                         repository
+                         (reference-target
+                          (branch-lookup repository "alternate-branch"))))
+               (intro   (make-channel-introduction
+                         (commit-id-string commit0)
+                         (openpgp-public-key-fingerprint
+                          (read-openpgp-packet
+                           %ed25519-public-key-file))))
+               (channel (channel (name 'example)
+                                 (url (string-append "file://" directory))
+                                 (introduction intro))))
+          (guard (c ((formatted-message? c)
+                     (and (string-contains (formatted-message-string c)
+                                           "not a descendant")
+                          (equal? (formatted-message-arguments c)
+                                  (list
+                                   (oid->string (commit-id commit2))
+                                   (oid->string (commit-id commit0)))))))
+            (authenticate-channel channel directory
+                                  (commit-id-string commit2)
+                                  #:keyring-reference-prefix "")
+            'failed))))))
+
 (unless (gpg+git-available?) (test-skip 1))
 (test-equal "authenticate-channel, .guix-authorizations"
   #t
diff --git a/tests/git-authenticate.scm b/tests/git-authenticate.scm
index 6ec55fb2e5..c063920c12 100644
--- a/tests/git-authenticate.scm
+++ b/tests/git-authenticate.scm
@@ -431,4 +431,48 @@ (define (correct? c commit)
                                       #:keyring-reference "master"
                                       #:cache-key (random-text)))))))))
 
+(unless (gpg+git-available?) (test-skip 1))
+(test-equal "authenticate-repository, target not a descendant of intro"
+  'target-commit-not-a-descendant-of-intro
+  (with-fresh-gnupg-setup (list %ed25519-public-key-file
+                                %ed25519-secret-key-file)
+    (let ((fingerprint (key-fingerprint %ed25519-public-key-file)))
+      (with-temporary-git-repository directory
+          `((add "signer.key" ,(call-with-input-file %ed25519-public-key-file
+                                 get-string-all))
+            (add ".guix-authorizations"
+                 ,(object->string
+                   `(authorizations (version 0)
+                                    ((,(key-fingerprint
+                                        %ed25519-public-key-file)
+                                      (name "Charlie"))))))
+            (commit "zeroth commit" (signer ,fingerprint))
+            (branch "pre-intro-branch")
+            (checkout "pre-intro-branch")
+            (add "b.txt" "B")
+            (commit "alternate commit" (signer ,fingerprint))
+            (checkout "master")
+            (add "a.txt" "A")
+            (commit "first commit" (signer ,fingerprint))
+            (add "c.txt" "C")
+            (commit "second commit" (signer ,fingerprint)))
+        (with-repository directory repository
+          (let ((commit1 (find-commit repository "first"))
+                (commit-alt
+                 (commit-lookup repository
+                                (reference-target
+                                 (branch-lookup repository
+                                                "pre-intro-branch")))))
+            (guard (c ((formatted-message? c)
+                       (and (equal? (formatted-message-arguments c)
+                                    (list (oid->string (commit-id commit-alt))
+                                          (oid->string (commit-id commit1))))
+                            'target-commit-not-a-descendant-of-intro)))
+              (authenticate-repository repository
+                                       (commit-id commit1)
+                                       (openpgp-fingerprint fingerprint)
+                                       #:end (commit-id commit-alt)
+                                       #:keyring-reference "master"
+                                       #:cache-key (random-text)))))))))
+
 (test-end "git-authenticate")
diff --git a/tests/guix-git-authenticate.sh b/tests/guix-git-authenticate.sh
index 8ebbea398b..2b90d8a4af 100644
--- a/tests/guix-git-authenticate.sh
+++ b/tests/guix-git-authenticate.sh
@@ -1,5 +1,5 @@
 # GNU Guix --- Functional package management for GNU
-# Copyright © 2020 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2020, 2022 Ludovic Courtès <ludo@gnu.org>
 #
 # This file is part of GNU Guix.
 #
@@ -34,10 +34,18 @@ intro_signer="BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA"
 
 cache_key="test-$$"
 
-guix git authenticate "$intro_commit" "$intro_signer"	\
+# This must fail because the end commit is not a descendant of $intro_commit.
+! guix git authenticate "$intro_commit" "$intro_signer"	\
      --cache-key="$cache_key" --stats			\
      --end=9549f0283a78fe36f2d4ff2a04ef8ad6b0c02604
 
+# The v1.2.0 commit is a descendant of $intro_commit and it satisfies the
+# authorization invariant.
+v1_2_0_commit="a099685659b4bfa6b3218f84953cbb7ff9e88063"
+guix git authenticate "$intro_commit" "$intro_signer"	\
+     --cache-key="$cache_key" --stats			\
+     --end="$v1_2_0_commit"
+
 rm "$XDG_CACHE_HOME/guix/authentication/$cache_key"
 
 # Commit and signer of the 'v1.0.0' tag.
@@ -45,6 +53,11 @@ v1_0_0_commit="6298c3ffd9654d3231a6f25390b056483e8f407c"
 v1_0_0_signer="3CE4 6455 8A84 FDC6 9DB4  0CFB 090B 1199 3D9A EBB5" # civodul
 v1_0_1_commit="d68de958b60426798ed62797ff7c96c327a672ac"
 
+# This should succeed because v1.0.0 is an ancestor of $intro_commit.
+guix git authenticate "$intro_commit" "$intro_signer"	\
+     --cache-key="$cache_key" --stats			\
+     --end="$v1_0_0_commit"
+
 # This should fail because these commits lack '.guix-authorizations'.
 ! guix git authenticate "$v1_0_0_commit" "$v1_0_0_signer" \
        --cache-key="$cache_key" --end="$v1_0_1_commit"
-- 
2.34.0





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

* [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit
  2022-01-28 17:31 [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit Ludovic Courtès
  2022-01-28 17:43 ` [bug#53608] [PATCH 1/2] git: Add 'commit-descendant?' Ludovic Courtès
@ 2022-02-08 23:02 ` Ludovic Courtès
  2022-02-10 22:29   ` Maxime Devos
  1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2022-02-08 23:02 UTC (permalink / raw)
  To: 53608; +Cc: Attila Lendvai, Maxime Devos

Howdy Maxime & Attila,

Did you have a chance to look into this series?

  https://issues.guix.gnu.org/53608

It’s relatively simple but I’d rather have other eyeballs looking at it.

TIA.  :-)

Ludo’.

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

> Hello!
>
> This patch series fixes a bug in the checkout authentication code:
> it would be possible to authenticate a commit unrelated to the
> introductory commit, provided that target commit passes the
> authorization invariant (see the commit log for details).
>
> Users of Guix and of third-party channels are safe: this bug does
> not have any impact on checkout authentication in those cases.
>
> What concrete cases are affected?  Suppose someone forks Guix and
> publishes a new channel introduction for their fork.  The expectation
> is that any branch started before the introductory channel, for
> instance in the original Guix repo, would fail to be authenticated.
> However, because of this bug, such a branch would be considered
> authentic in the fork because all its commits pass the authorization
> invariant (IOW, they are authentic in the original repository).
>
> Thoughts?
>
> Ludo'.
>
> Ludovic Courtès (2):
>   git: Add 'commit-descendant?'.
>   git-authenticate: Ensure the target is a descendant of the
>     introductory commit.
>
>  doc/guix.texi                  |  4 ++-
>  guix/git-authenticate.scm      | 17 ++++++++--
>  guix/git.scm                   | 24 +++++++++++++-
>  tests/channels.scm             | 60 +++++++++++++++++++++++++++++++++-
>  tests/git-authenticate.scm     | 44 +++++++++++++++++++++++++
>  tests/git.scm                  | 52 ++++++++++++++++++++++++++++-
>  tests/guix-git-authenticate.sh | 17 ++++++++--
>  7 files changed, 210 insertions(+), 8 deletions(-)
>
>
> base-commit: 5052f76afd02e27d6484acf74c86bfa1b6f9cd0e




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

* [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit
  2022-02-08 23:02 ` [bug#53608] [PATCH 0/2] Rejecting commits unrelated to " Ludovic Courtès
@ 2022-02-10 22:29   ` Maxime Devos
  2022-02-14 10:33     ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Maxime Devos @ 2022-02-10 22:29 UTC (permalink / raw)
  To: Ludovic Courtès, 53608; +Cc: Attila Lendvai

[-- Attachment #1: Type: text/plain, Size: 406 bytes --]

Ludovic Courtès schreef op wo 09-02-2022 om 00:02 [+0100]:
> Howdy Maxime & Attila,
> 
> Did you have a chance to look into this series?
> 
>   https://issues.guix.gnu.org/53608
> 
> It’s relatively simple but I’d rather have other eyeballs looking at it.
> 
> TIA.  :-)

The concept seems reasonable to me but I cannot tell if the
implementation is good or bad.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

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

* [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit
  2022-02-10 22:29   ` Maxime Devos
@ 2022-02-14 10:33     ` Ludovic Courtès
  0 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2022-02-14 10:33 UTC (permalink / raw)
  To: Maxime Devos; +Cc: Attila Lendvai, 53608

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op wo 09-02-2022 om 00:02 [+0100]:
>> Howdy Maxime & Attila,
>> 
>> Did you have a chance to look into this series?
>> 
>>   https://issues.guix.gnu.org/53608
>> 
>> It’s relatively simple but I’d rather have other eyeballs looking at it.
>> 
>> TIA.  :-)
>
> The concept seems reasonable to me but I cannot tell if the
> implementation is good or bad.

OK.

I went ahead and pushed these two commits:

  ca87601dd9 git-authenticate: Ensure the target is a descendant of the introductory commit.
  87d49346f3 git: Add 'commit-descendant?'.

The actual change is this extra condition:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 844 bytes --]

@@ -426,6 +428,17 @@ (define commits
           (verify-introductory-commit repository keyring
                                       start-commit signer))
 
+        ;; Make sure END-COMMIT is a descendant of START-COMMIT or of one of
+        ;; AUTHENTICATED-COMMITS, which are known to be descendants of
+        ;; START-COMMIT.
+        (unless (commit-descendant? end-commit
+                                    (cons start-commit
+                                          authenticated-commits))
+          (raise (formatted-message
+                  (G_ "commit ~a is not a descendant of introductory commit ~a")
+                  (oid->string (commit-id end-commit))
+                  (oid->string (commit-id start-commit)))))
+
         (let ((stats (call-with-progress-reporter reporter
                        (lambda (report)

[-- Attachment #3: Type: text/plain, Size: 61 bytes --]


I encourage everyone to take a look.

Thanks,
Ludo’.

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

end of thread, other threads:[~2022-02-14 10:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 17:31 [bug#53608] [PATCH 0/2] Rejecting commits unrelated to the introductory commit Ludovic Courtès
2022-01-28 17:43 ` [bug#53608] [PATCH 1/2] git: Add 'commit-descendant?' Ludovic Courtès
2022-01-28 17:43   ` [bug#53608] [PATCH 2/2] git-authenticate: Ensure the target is a descendant of the introductory commit Ludovic Courtès
2022-02-08 23:02 ` [bug#53608] [PATCH 0/2] Rejecting commits unrelated to " Ludovic Courtès
2022-02-10 22:29   ` Maxime Devos
2022-02-14 10:33     ` Ludovic Courtès

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.