Fully implement credential commits

---
type: change
message: |-
  Fully implement credential commits

  The actual commit objects and related refactoring had already been done, this
  commit takes the next step of implementing the access control changes, tests for
  verification, and refactoring of the dehub command to support multiple commit
  message types (as well as a small fix to dcmd).
change_hash: AJyuAR0koVoe+uPBisa5qXsbW8YhlgOKNhnvy9uv7hQ8
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl5tVzoACgkQlcRvpqQRSKyznw//b9lWd4V4G81cFwGAxZtJ3JiFpspYdtTAUUcLi9nogGsDmqkkSQxLdmBCT99QtaenKsxpad+9sXhkZpgWF/AyCX9pN6TTlMKuRcDXeoMUjeKjRpRhCHN0Lt8Sz80NDPYIa81r9cH0o1987GirgGmDEkYNDAFPDdGNDcCad/LLnG+ONwOl9WEM1q5O4etUPurTywlBiELDjHxeLzqgxCo8fMaMejW6mxnMDV6DIHiX6INWZAAG66HPVetmq6EVl9bnFgZmgKNzqKzFVZJRdGQNbhR/WzlOh0HPyJGwCveZPM5Zjd/dpfQUYEGGprVKc0G0YVNU2Hcz6O7hqafGGxWpCFW6zKrNmBRaW2u2zjVJD4ukmWn9gFuKJKhs0kyawRTbHNIX+gonYv9lDFO3cZ5qcsJbSAYSHrCav121z0GsQDoFJMJDQnP0syEEbAaxdQe7Bd7bmOM3SpCOLJLF1+X7Srrq5//u6fiFDxQ82Ylo3hG/r7/QT/vSipUCglx4POq33+z8VEHGhVfl4dgSU6OgIV/S7evKC7EiS/jh/xywU44RHpxFhwS3hthHxZqgRIHTm65DqGYWWZds2Hkr29TTRajuf0t4MxqY2MrLAhNJUc6OmrVN+lWMmm/z1FEhfrOvZ8v7mOSqTKwkvbsZzk5mpeo2RrLdNnnWvTCy87FpA48=
  account: mediocregopher

ROADMAP.md

@@ -6,12 +6,18 @@ set, only a sequence of milestones and the requirements to hit them.
 
 ## Milestone: Be able to add other developers to the project
 
-* Thread commits (comments, signatures)
+* Thread commits (comments)
 * Coalesce command
 * Fast-forward perms on branches
 * Authorship in commit messages
 * README with a "Getting Started" section
 
+## Milestone: refactor accessctl?
+
+The current accessctl is cumbersome and difficult to describe. It might be
+better if it was constructed as an actual ACL, rather than whatever it is
+currently.
+
 ## Milestone: Versions
 
 * Tag commits
@@ -23,6 +29,8 @@ set, only a sequence of milestones and the requirements to hit them.
 * Maybe coalesce the `accessctl`, `fs`, and `sigcred` packages back into the
   root "dehub" package.
 
+* Polish all error messages
+
 * Polish commands
     - New flag system, some kind of interactivity support (e.g. user doesn't
       specify required argument, give them a prompt on the CLI to input it

SPEC.md

@@ -72,18 +72,30 @@ access_controls:
           any_account: true # indicates any account defined in accounts is valid
           count: 1
 
-  # If a branch is not matched by any access control object then the following
+
-  # default object is implied:
+    # credential_access_control is an object describing under what condition
-  #
+    # credential commits can be added to the branch.
-  # branch_pattern: **
+    credential_access_control:
-  # change_access_controls:
+      # never conditions indicate that a commit will never be allowed. In this
-  #   - file_path_pattern: **
+      # case, credential commits are not allowed in the main branch under any
-  #     condition:
+      # circumstances.
-  #       type: signature
+      condition:
-  #       any_account: true
+        type: never
-  #       count: 1
+```
+
+Unless the `config.yml` file in place declares otherwise, the following
+condition is applied to all commits on all branches:
+
+```yaml
+condition:
+  type: signature
+  any_account: true
+  count: 1
 ```
 
+The exception is commits for the `main` branch, for which only change commits
+are allowed by default (under that condition).
+
 # Change Hash
 
 When a change commit (see Commits section) is being signed by a signifier there
@@ -194,7 +206,7 @@ credentials:
 ## Credential Commits
 
 Commits of type `credential` contain one or more credentials for some hash
-(presumably a change hash, but in the future there may be other types).  The
+(presumably a change hash, but in the future there may be other types). The
 commit message head is not spec'd, but should be a human-readable description of
 "who is crediting what, and how".
 

accessctl/access_control.go

@@ -8,42 +8,56 @@ import (
 )
 
 var (
+	// DefaultSignatureCondition represents the Condition which is applied for
+	// default access controls. It requires a single signature credential from
+	// any account defined in the Config.
+	DefaultSignatureCondition = Condition{
+		Signature: &ConditionSignature{
+			AnyAccount: true,
+			Count:      "1",
+		},
+	}
+
 	// DefaultChangeAccessControl represents the ChangeAccessControl which is
 	// applied when a changed file's path does not match any defined patterns
 	// within a BranchAccessControl.
 	DefaultChangeAccessControl = ChangeAccessControl{
 		FilePathPattern: "**",
-		Condition: Condition{
+		Condition:       DefaultSignatureCondition,
-			Signature: &ConditionSignature{
+	}
-				AnyAccount: true,
+
-				Count:      "1",
+	// DefaultCredentialAccessControl represents the CredentialAccessControl
-			},
+	// which is applied when a BranchAccessControl does not have a defined
-		},
+	// CredentialAccessControl.
+	DefaultCredentialAccessControl = CredentialAccessControl{
+		Condition: DefaultSignatureCondition,
 	}
 
 	// DefaultBranchAccessControls represents the BranchAccessControls which are
 	// applied when the name of a branch being interacted with does not match
 	// any defined patterns within the Config.
 	DefaultBranchAccessControls = []BranchAccessControl{
-		// These are currently the same, but they will differ once things like
-		// comments start being implemented.
 		{
-			BranchPattern:        "main",
+			BranchPattern: "main",
-			ChangeAccessControls: []ChangeAccessControl{DefaultChangeAccessControl},
+			CredentialAccessControl: &CredentialAccessControl{
+				Condition: Condition{Never: new(ConditionNever)},
+			},
 		},
 		{
-			BranchPattern:        "**",
+			BranchPattern: "**",
-			ChangeAccessControls: []ChangeAccessControl{DefaultChangeAccessControl},
 		},
 	}
 )
 
+// any account can do anything, except main branch can only get change commits
+
 // BranchAccessControl represents an access control object defined for the
 // purpose of controlling who is able to perform what interactions with a
 // branch.
 type BranchAccessControl struct {
-	BranchPattern        string                `yaml:"branch_pattern"`
+	BranchPattern           string                   `yaml:"branch_pattern"`
-	ChangeAccessControls []ChangeAccessControl `yaml:"change_access_controls"`
+	ChangeAccessControls    []ChangeAccessControl    `yaml:"change_access_controls,omitempty"`
+	CredentialAccessControl *CredentialAccessControl `yaml:"credential_access_control,omitempty"`
 }
 
 // ChangeAccessControl represents an access control object being defined in the
@@ -53,6 +67,13 @@ type ChangeAccessControl struct {
 	Condition       Condition `yaml:"condition"`
 }
 
+// CredentialAccessControl represents an access control object being defined in
+// the Config for the purpose of controlling who is able to create credential
+// commits.
+type CredentialAccessControl struct {
+	Condition Condition `yaml:"condition"`
+}
+
 // MatchInteractions is used as an input to Match to describe all
 // interactions which are being attempted on a particular Branch.
 type MatchInteractions struct {
@@ -63,6 +84,10 @@ type MatchInteractions struct {
 	// FilePathsChanged is the set of file paths (relative to the repo root)
 	// which have been modified in some way.
 	FilePathsChanged []string
+
+	// CredentialAdded indicates a credential commit is being added to the
+	// Branch.
+	CredentialAdded bool
 }
 
 // MatchedChangeAccessControl contains information about a ChangeAccessControl
@@ -75,6 +100,12 @@ type MatchedChangeAccessControl struct {
 	FilePaths []string
 }
 
+// MatchedCredentialAccessControl contains information about a
+// CredentialAccessControl which was matched in Match.
+type MatchedCredentialAccessControl struct {
+	CredentialAccessControl CredentialAccessControl
+}
+
 // MatchResult is the result returned from the Match method.
 type MatchResult struct {
 	// BranchPattern indicates the BranchPattern field of the
@@ -84,6 +115,11 @@ type MatchResult struct {
 	// ChangeAccessControls indicates which ChangeAccessControl objects matched
 	// the files being changed.
 	ChangeAccessControls []MatchedChangeAccessControl
+
+	// CredentialAccessControls indicates which CredentialAccessControl object
+	// matched for a credential commit. Will be nil if CredentialAdded was
+	// false.
+	CredentialAccessControl *MatchedCredentialAccessControl
 }
 
 // Match takes in a set of access controls and a set of interactions taking
@@ -102,7 +138,7 @@ func Match(accessControls []BranchAccessControl, interactions MatchInteractions)
 		for i := range accessControls {
 			ok, err = doublestar.Match(accessControls[i].BranchPattern, interactions.Branch)
 			if err != nil {
-				return res, fmt.Errorf("error matching branch %q to pattern %q: %w",
+				return res, fmt.Errorf("matching branch %q to branch_pattern %q: %w",
 					accessControls[i].BranchPattern, interactions.Branch, err)
 			} else if ok {
 				branchAC = accessControls[i]
@@ -124,7 +160,7 @@ func Match(accessControls []BranchAccessControl, interactions MatchInteractions)
 			var err error
 			for _, ac := range changeACs {
 				if ok, err = doublestar.PathMatch(ac.FilePathPattern, path); err != nil {
-					return res, fmt.Errorf("error matching path %q to patterrn %q: %w",
+					return res, fmt.Errorf("matching path %q to file_path_pattern %q: %w",
 						path, ac.FilePathPattern, err)
 				} else if ok {
 					acToPaths[ac] = append(acToPaths[ac], path)
@@ -150,5 +186,17 @@ func Match(accessControls []BranchAccessControl, interactions MatchInteractions)
 		})
 	}
 
+	// Handle CredentialAccessControl, if applicable
+	if interactions.CredentialAdded {
+		credAC := branchAC.CredentialAccessControl
+		if credAC == nil {
+			credAC = &DefaultCredentialAccessControl
+		}
+
+		res.CredentialAccessControl = &MatchedCredentialAccessControl{
+			CredentialAccessControl: *credAC,
+		}
+	}
+
 	return res, nil
 }

accessctl/access_control_test.go

@@ -161,6 +161,51 @@ func TestMatch(t *testing.T) {
 				},
 			},
 		},
+		{
+			descr: "no defined CredentialAccessControl",
+			branchACs: []BranchAccessControl{{
+				BranchPattern: "main",
+			}},
+			interactions: MatchInteractions{
+				Branch:          "main",
+				CredentialAdded: true,
+			},
+			result: MatchResult{
+				BranchPattern: "main",
+				CredentialAccessControl: &MatchedCredentialAccessControl{
+					CredentialAccessControl: DefaultCredentialAccessControl,
+				},
+			},
+		},
+		{
+			descr: "defined CredentialAccessControl",
+			branchACs: []BranchAccessControl{{
+				BranchPattern: "main",
+				CredentialAccessControl: &CredentialAccessControl{
+					Condition: Condition{
+						Signature: &ConditionSignature{
+							AccountIDs: []string{"foo", "bar", "baz"},
+						},
+					},
+				},
+			}},
+			interactions: MatchInteractions{
+				Branch:          "main",
+				CredentialAdded: true,
+			},
+			result: MatchResult{
+				BranchPattern: "main",
+				CredentialAccessControl: &MatchedCredentialAccessControl{
+					CredentialAccessControl: CredentialAccessControl{
+						Condition: Condition{
+							Signature: &ConditionSignature{
+								AccountIDs: []string{"foo", "bar", "baz"},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {

accessctl/condition.go

@@ -24,6 +24,7 @@ type ConditionInterface interface {
 // Condition represents an access control condition being defined in the Config.
 // Only one of its fields may be filled in at a time.
 type Condition struct {
+	Never     *ConditionNever     `type:"never"`
 	Signature *ConditionSignature `type:"signature"`
 }
 
@@ -47,10 +48,19 @@ func (c Condition) Interface() (ConditionInterface, error) {
 	return el.(ConditionInterface), nil
 }
 
+// ConditionNever is a Condition which is never satisfied.
+type ConditionNever struct{}
+
+// Satisfied always returns an error, because ConditionNever cannot be
+// satisfied.
+func (condNever ConditionNever) Satisfied([]sigcred.Credential) error {
+	return errors.New("condition of type 'never' cannot be satisfied")
+}
+
 // ConditionSignature represents the configuration of an access control
 // condition which requires one or more signatures to be present on a commit.
 //
-// Either AccountIDs or AccountIDsByMeta must be filled.
+// Either AccountIDs or AnyAccount must be filled in.
 type ConditionSignature struct {
 	AccountIDs []string `yaml:"account_ids,omitempty"`
 	AnyAccount bool     `yaml:"any_account,omitempty"`

cmd/dehub/cmd_commit.go

@@ -7,41 +7,20 @@ import (
 	"fmt"
 
 	"dehub/cmd/dehub/dcmd"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
 )
 
 func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 	flag := cmd.FlagSet()
-	msg := flag.String("msg", "", "Commit message")
 	accountID := flag.String("account-id", "", "Account to sign commit as")
-	cmd.Run(func() (context.Context, error) {
-		repo := ctxRepo(ctx)
-
-		// Don't bother checking any of the parameters, especially commit
-		// message, if there's no staged changes,
-		hasStaged, err := repo.HasStagedChanges()
-		if err != nil {
-			return nil, fmt.Errorf("error determining if any changes have been staged: %w", err)
-		} else if !hasStaged {
-			return nil, errors.New("no changes have been staged for commit")
-		}
-
-		if *accountID == "" {
-			return nil, errors.New("-account-id is required")
-		}
 
-		if *msg == "" {
+	repo := ctxRepo(ctx)
-			var err error
+	accreditAndCommit := func(commit dehub.Commit) error {
-			if *msg, err = tmpFileMsg(); err != nil {
-				return nil, fmt.Errorf("error collecting commit message from user: %w", err)
-
-			} else if *msg == "" {
-				return nil, errors.New("empty commit message, not doing anything")
-			}
-		}
 
 		cfg, err := repo.LoadConfig()
 		if err != nil {
-			return nil, err
+			return err
 		}
 
 		var account dehub.Account
@@ -53,33 +32,115 @@ func cmdCommit(ctx context.Context, cmd *dcmd.Cmd) {
 			}
 		}
 		if !ok {
-			return nil, fmt.Errorf("account ID %q not found in config", *accountID)
+			return fmt.Errorf("account ID %q not found in config", *accountID)
 		} else if l := len(account.Signifiers); l == 0 || l > 1 {
-			return nil, fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l)
+			return fmt.Errorf("account %q has %d signifiers, only one is supported right now", *accountID, l)
 		}
 
 		sig := account.Signifiers[0]
 		sigInt, err := sig.Interface(*accountID)
 		if err != nil {
-			return nil, fmt.Errorf("could not cast %+v to SignifierInterface: %w", sig, err)
+			return fmt.Errorf("casting %#v to SignifierInterface: %w", sig, err)
+
+		} else if commit, err = repo.AccreditCommit(commit, sigInt); err != nil {
+			return fmt.Errorf("accrediting commit: %w", err)
 		}
 
-		commit, err := repo.NewCommitChange(*msg)
+		hash, err := repo.Commit(commit, *accountID)
 		if err != nil {
-			return nil, fmt.Errorf("could not construct change commit: %w", err)
+			return fmt.Errorf("committing to git: %w", err)
 		}
 
-		commit, err = repo.AccreditCommit(commit, sigInt)
+		fmt.Printf("committed to HEAD as %s\n", hash)
-		if err != nil {
+		return nil
-			return nil, fmt.Errorf("could not accredit commit: %w", err)
+	}
+
+	var hasStaged bool
+	body := func() (context.Context, error) {
+		if *accountID == "" {
+			return nil, errors.New("-account-id is required")
 		}
 
-		hash, err := repo.Commit(commit, *accountID)
+		var err error
-		if err != nil {
+		if hasStaged, err = repo.HasStagedChanges(); err != nil {
-			return nil, fmt.Errorf("could not commit change commit: %w", err)
+			return nil, fmt.Errorf("determining if any changes have been staged: %w", err)
 		}
+		return ctx, nil
+	}
+
+	cmd.SubCmd("change", "Commit file changes",
+		func(ctx context.Context, cmd *dcmd.Cmd) {
+			flag := cmd.FlagSet()
+			msg := flag.String("msg", "", "Commit message")
+			cmd.Run(func() (context.Context, error) {
+				if !hasStaged {
+					return nil, errors.New("no changes have been staged for commit")
+				}
+
+				if *msg == "" {
+					var err error
+					if *msg, err = tmpFileMsg(); err != nil {
+						return nil, fmt.Errorf("error collecting commit message from user: %w", err)
+
+					} else if *msg == "" {
+						return nil, errors.New("empty commit message, not doing anything")
+					}
+				}
+
+				commit, err := repo.NewCommitChange(*msg)
+				if err != nil {
+					return nil, fmt.Errorf("could not construct change commit: %w", err)
+
+				} else if err := accreditAndCommit(commit); err != nil {
+					return nil, err
+				}
+				return nil, nil
+			})
+		},
+	)
+
+	cmd.SubCmd("credential", "Commit credential of a different commit",
+		func(ctx context.Context, cmd *dcmd.Cmd) {
+			flag := cmd.FlagSet()
+			rev := flag.String("rev", "", "Revision of commit to accredit")
+			cmd.Run(func() (context.Context, error) {
+				if *rev == "" {
+					return nil, errors.New("-rev is required")
+				} else if hasStaged {
+					return nil, errors.New("credential commit cannot have any files changed")
+				}
+
+				// TODO maybe nice to have a helper to do all of this
+				h, err := repo.GitRepo.ResolveRevision(plumbing.Revision(*rev))
+				if err != nil {
+					return nil, fmt.Errorf("resolving revision: %w", err)
+				}
+
+				commitObj, err := repo.GitRepo.CommitObject(*h)
+				if err != nil {
+					return nil, fmt.Errorf("getting commit object %q: %w", h, err)
+				}
+
+				var commit dehub.Commit
+				if err := commit.UnmarshalText([]byte(commitObj.Message)); err != nil {
+					return nil, fmt.Errorf("unmarshaling commit message: %w", err)
+				}
+
+				commitInt, err := commit.Interface()
+				if err != nil {
+					return nil, fmt.Errorf("casting %#v to CommitInterface: %w", commit, err)
+				}
+
+				credCommit, err := repo.NewCommitCredential(commitInt.GetHash())
+				if err != nil {
+					return nil, fmt.Errorf("constructing credential commit: %w", err)
+				} else if err := accreditAndCommit(credCommit); err != nil {
+					return nil, err
+				}
+				return nil, nil
+			})
+		},
+	)
 
-		fmt.Printf("changes committed to HEAD as %s\n", hash)
+	cmd.Run(body)
-		return nil, nil
-	})
 }

cmd/dehub/dcmd/dcmd.go

@@ -149,7 +149,7 @@ func (cmd *Cmd) Run(body func() (context.Context, error)) {
 	}
 
 	// now find that sub-command
-	subCmdName, args := strings.ToLower(args[0]), args[1:]
+	subCmdName := strings.ToLower(subArgs[0])
 	var subCmd subCmd
 	var subCmdOk bool
 	for _, subCmd = range cmd.subCmds {

commit.go

@@ -8,6 +8,7 @@ import (
 	"dehub/typeobj"
 	"encoding"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -122,7 +123,7 @@ func (r *Repo) AccreditCommit(commit Commit, sigInt sigcred.SignifierInterface)
 
 	cred, err := sigInt.Sign(headFS, commitInt.GetHash())
 	if err != nil {
-		return commit, fmt.Errorf("could not accreddit change commit: %w", err)
+		return commit, fmt.Errorf("could not accredit change commit: %w", err)
 	}
 	commit.Credentials = append(commit.Credentials, cred)
 	return commit, nil
@@ -172,13 +173,46 @@ func (r *Repo) HasStagedChanges() (bool, error) {
 	return any, nil
 }
 
+type verificationCtx struct {
+	commit                 *object.Commit
+	commitTree, parentTree *object.Tree
+	isRootCommit           bool
+}
+
+// non-gophers gonna hate on this method, but I say it's fine
+func (r *Repo) verificationCtx(h plumbing.Hash) (vctx verificationCtx, err error) {
+	if vctx.commit, err = r.GitRepo.CommitObject(h); err != nil {
+		return vctx, fmt.Errorf("retrieving commit object: %w", err)
+
+	} else if vctx.commitTree, err = r.GitRepo.TreeObject(vctx.commit.TreeHash); err != nil {
+		return vctx, fmt.Errorf("retrieving commit tree object %q: %w",
+			vctx.commit.TreeHash, err)
+
+	} else if vctx.isRootCommit = vctx.commit.NumParents() == 0; vctx.isRootCommit {
+		vctx.parentTree = new(object.Tree)
+
+	} else if parent, err := vctx.commit.Parent(0); err != nil {
+		return vctx, fmt.Errorf("retrieving commit parent: %w", err)
+
+	} else if vctx.parentTree, err = r.GitRepo.TreeObject(parent.TreeHash); err != nil {
+		return vctx, fmt.Errorf("retrieving commit parent tree object %q: %w",
+			parent.Hash, err)
+	}
+
+	return vctx, nil
+}
+
 func (r *Repo) assertAccessControls(
-	accessCtls []accessctl.BranchAccessControl, creds []sigcred.Credential,
+	accessCtls []accessctl.BranchAccessControl,
-	branch plumbing.ReferenceName, from, to *object.Tree,
+	commit Commit, vctx verificationCtx, branch plumbing.ReferenceName,
-) error {
+) (err error) {
-	filesChanged, err := calcDiff(from, to)
+	filesChanged, err := calcDiff(vctx.parentTree, vctx.commitTree)
 	if err != nil {
-		return err
+		return fmt.Errorf("calculating diff from tree %q to tree %q: %w",
+			vctx.parentTree.Hash, vctx.commitTree.Hash, err)
+
+	} else if len(filesChanged) > 0 && commit.Change == nil {
+		return errors.New("files changes but commit is not a change commit")
 	}
 
 	pathsChanged := make([]string, len(filesChanged))
@@ -189,20 +223,35 @@ func (r *Repo) assertAccessControls(
 	matchRes, err := accessctl.Match(accessCtls, accessctl.MatchInteractions{
 		Branch:           branch.Short(),
 		FilePathsChanged: pathsChanged,
+		CredentialAdded:  commit.Credential != nil,
 	})
 	if err != nil {
-		return fmt.Errorf("could not determine applicable access controls: %w", err)
+		return fmt.Errorf("determining applicable access controls: %w", err)
 	}
 
-	for _, matchedAC := range matchRes.ChangeAccessControls {
+	defer func() {
-		ac := matchedAC.ChangeAccessControl
-		condInt, err := ac.Condition.Interface()
 		if err != nil {
-			return fmt.Errorf("could not cast Condition of file path pattern %q to interface: %w",
+			err = fmt.Errorf("asserting access controls for branch_pattern %q: %w",
-				ac.FilePathPattern, err)
+				matchRes.BranchPattern, err)
-		} else if err := condInt.Satisfied(creds); err != nil {
+		}
-			return fmt.Errorf("access control of file path pattern %q not satisfied: %w\nFiles matched:\n%s",
+	}()
-				ac.FilePathPattern, err, strings.Join(matchedAC.FilePaths, "\n"))
+
+	for _, matchedChangeAC := range matchRes.ChangeAccessControls {
+		ac := matchedChangeAC.ChangeAccessControl
+		if condInt, err := ac.Condition.Interface(); err != nil {
+			return fmt.Errorf("casting ChangeAccessControl.Condition %#v to interface: %w", ac.Condition, err)
+		} else if err := condInt.Satisfied(commit.Credentials); err != nil {
+			return fmt.Errorf("satisfying change_access_control with file_path_pattern %q: %w\nfiles matched:\n%s",
+				ac.FilePathPattern, err, strings.Join(matchedChangeAC.FilePaths, "\n"))
+		}
+	}
+
+	if matchRes.CredentialAccessControl != nil {
+		cond := matchRes.CredentialAccessControl.CredentialAccessControl.Condition
+		if condInt, err := cond.Interface(); err != nil {
+			return fmt.Errorf("casting CredentialAccessControl.Condition %#v to interface: %w", cond, err)
+		} else if err := condInt.Satisfied(commit.Credentials); err != nil {
+			return fmt.Errorf("satisfying credential_access_control: %w", err)
 		}
 	}
 
@@ -212,31 +261,20 @@ func (r *Repo) assertAccessControls(
 // VerifyCommit verifies that the commit at the given hash, which is presumably
 // on the given branch, is gucci.
 func (r *Repo) VerifyCommit(branch plumbing.ReferenceName, h plumbing.Hash) error {
-	commitObj, err := r.GitRepo.CommitObject(h)
+	vctx, err := r.verificationCtx(h)
 	if err != nil {
-		return fmt.Errorf("could not retrieve commit object: %w", err)
+		return err
-	}
-
-	commitTree, err := r.GitRepo.TreeObject(commitObj.TreeHash)
-	if err != nil {
-		return fmt.Errorf("could not retrieve tree object: %w", err)
 	}
 
-	sigTree := commitTree // only for root commit
+	var sigFS fs.FS
-	parentTree := &object.Tree{}
+	if vctx.isRootCommit {
-	if commitObj.NumParents() > 0 {
+		sigFS = fs.FromTree(vctx.commitTree)
-		parent, err := commitObj.Parent(0)
+	} else {
-		if err != nil {
+		sigFS = fs.FromTree(vctx.parentTree)
-			return fmt.Errorf("could not retrieve parent of commit: %w", err)
-		} else if parentTree, err = r.GitRepo.TreeObject(parent.TreeHash); err != nil {
-			return fmt.Errorf("could not retrieve tree object of parent %q: %w", parent.Hash, err)
-		}
-		sigTree = parentTree
 	}
-	sigFS := fs.FromTree(sigTree)
 
 	var commit Commit
-	if err := commit.UnmarshalText([]byte(commitObj.Message)); err != nil {
+	if err := commit.UnmarshalText([]byte(vctx.commit.Message)); err != nil {
 		return err
 	}
 
@@ -245,10 +283,7 @@ func (r *Repo) VerifyCommit(branch plumbing.ReferenceName, h plumbing.Hash) erro
 		return fmt.Errorf("error loading config: %w", err)
 	}
 
-	err = r.assertAccessControls(
+	err = r.assertAccessControls(cfg.AccessControls, commit, vctx, branch)
-		cfg.AccessControls, commit.Credentials,
-		branch, parentTree, commitTree,
-	)
 	if err != nil {
 		return fmt.Errorf("failed to satisfy all access controls: %w", err)
 	}
@@ -259,7 +294,7 @@ func (r *Repo) VerifyCommit(branch plumbing.ReferenceName, h plumbing.Hash) erro
 	}
 
 	changeHash := commitInt.GetHash()
-	expectedChangeHash, err := commitInt.Hash(parentTree, commitTree)
+	expectedChangeHash, err := commitInt.Hash(vctx.parentTree, vctx.commitTree)
 	if err != nil {
 		return fmt.Errorf("error calculating expected change hash: %w", err)
 	} else if !bytes.Equal(changeHash, expectedChangeHash) {

commit_change_test.go

@@ -77,10 +77,6 @@ func TestChangeCommitVerify(t *testing.T) {
 				account := h.cfg.Accounts[0]
 
 				commit, hash := h.changeCommit(step.msg, account.ID, h.sig)
-				if err := h.repo.VerifyCommit(MainRefName, hash); err != nil {
-					t.Fatalf("could not verify hash %v: %v", hash, err)
-				}
-
 				commitObj, err := h.repo.GitRepo.CommitObject(hash)
 				if err != nil {
 					t.Fatalf("failed to retrieve commit %v: %v", hash, err)

commit_credential_test.go

@@ -0,0 +1,79 @@
+package dehub
+
+import (
+	"dehub/accessctl"
+	"dehub/sigcred"
+	"testing"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+)
+
+func TestCredentialCommitVerify(t *testing.T) {
+	h := newHarness(t)
+
+	// create a new account and modify the config so that that account is only
+	// allowed to add verifications to a single branch
+	tootSig, tootPubKeyBody := sigcred.SignifierPGPTmp("toot", h.rand)
+	h.cfg.Accounts = append(h.cfg.Accounts, Account{
+		ID: "toot",
+		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
+			Body: string(tootPubKeyBody),
+		}}},
+	})
+
+	tootBranch := plumbing.NewBranchReferenceName("toot_branch")
+	tootBranchCond := accessctl.Condition{
+		Signature: &accessctl.ConditionSignature{
+			AccountIDs: []string{"root", "toot"},
+			Count:      "1",
+		},
+	}
+	allBranchCond := accessctl.Condition{
+		Signature: &accessctl.ConditionSignature{
+			AccountIDs: []string{"root"},
+			Count:      "1",
+		},
+	}
+	h.cfg.AccessControls = []accessctl.BranchAccessControl{
+		{
+			BranchPattern: tootBranch.Short(),
+			ChangeAccessControls: []accessctl.ChangeAccessControl{
+				{
+					FilePathPattern: "**",
+					Condition:       tootBranchCond,
+				},
+			},
+			CredentialAccessControl: &accessctl.CredentialAccessControl{
+				Condition: tootBranchCond,
+			},
+		},
+		{
+			BranchPattern: "**",
+			ChangeAccessControls: []accessctl.ChangeAccessControl{
+				{
+					FilePathPattern: "**",
+					Condition:       allBranchCond,
+				},
+			},
+			CredentialAccessControl: &accessctl.CredentialAccessControl{
+				Condition: allBranchCond,
+			},
+		},
+	}
+	h.stageCfg()
+	rootCommit, _ := h.changeCommit("initial commit", h.cfg.Accounts[0].ID, h.sig)
+
+	// toot user wants to create a credential commit for the root commit, for
+	// whatever reason.
+	rootChangeHash := rootCommit.Change.ChangeHash
+	credCommit, err := h.repo.NewCommitCredential(rootChangeHash)
+	if err != nil {
+		t.Fatalf("creating credential commit for hash %x: %v", rootChangeHash, err)
+
+	}
+	h.tryCommit(false, credCommit, "toot", tootSig)
+
+	// toot tries again in their own branch, and should be allowed.
+	h.checkout(tootBranch)
+	h.tryCommit(true, credCommit, "toot", tootSig)
+}

commit_test.go

@@ -4,9 +4,7 @@ import (
 	"dehub/sigcred"
 	"testing"
 
-	"gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
-	yaml "gopkg.in/yaml.v2"
 )
 
 func TestConfigChange(t *testing.T) {
@@ -31,20 +29,15 @@ func TestConfigChange(t *testing.T) {
 	h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.AccountIDs = []string{"root", "toot"}
 	h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.Count = "1"
 
-	cfgBody, err := yaml.Marshal(h.cfg)
+	h.stageCfg()
+	badCommit, err := h.repo.NewCommitChange("add toot user")
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("creating CommitChange: %v", err)
 	}
-	h.stage(map[string]string{ConfigPath: string(cfgBody)})
+	h.tryCommit(false, badCommit, h.cfg.Accounts[1].ID, newSig)
-	_, badHash := h.changeCommit("add toot user", h.cfg.Accounts[1].ID, newSig)
-
-	if err := h.repo.VerifyCommit(MainRefName, badHash); err == nil {
-		t.Fatal("toot user shouldn't be able to add itself to config")
-	}
-	h.reset(hash, git.HardReset)
 
 	// now add with the root user, this should work.
-	h.stage(map[string]string{ConfigPath: string(cfgBody)})
+	h.stageCfg()
 	_, hash = h.changeCommit("add toot user", h.cfg.Accounts[0].ID, h.sig)
 	hashes = append(hashes, hash)
 

repo_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"dehub/accessctl"
 	"dehub/sigcred"
+	"errors"
 	"io"
 	"math/rand"
 	"path/filepath"
@@ -107,24 +108,43 @@ func (h *harness) stage(tree map[string]string) {
 	}
 }
 
-func (h *harness) changeCommit(msg, accountID string, sig sigcred.SignifierInterface) (Commit, plumbing.Hash) {
+func (h *harness) stageCfg() {
-	commit, err := h.repo.NewCommitChange(msg)
+	cfgBody, err := yaml.Marshal(h.cfg)
 	if err != nil {
-		h.t.Fatalf("failed to create CommitChange: %v", err)
+		h.t.Fatal(err)
 	}
+	h.stage(map[string]string{ConfigPath: string(cfgBody)})
+}
 
-	if sig != nil {
+func (h *harness) checkout(branch plumbing.ReferenceName) {
-		if commit, err = h.repo.AccreditCommit(commit, sig); err != nil {
+	w, err := h.repo.GitRepo.Worktree()
-			h.t.Fatalf("failed to accredit commit: %v", err)
+	if err != nil {
-		}
+		h.t.Fatal(err)
 	}
 
-	hash, err := h.repo.Commit(commit, accountID)
+	head, _, err := h.repo.head()
 	if err != nil {
-		h.t.Fatalf("failed to commit ChangeCommit: %v", err)
+		h.t.Fatal(err)
 	}
 
-	return commit, hash
+	_, err = h.repo.GitRepo.Branch(branch.Short())
+	if errors.Is(err, git.ErrBranchNotFound) {
+		err = w.Checkout(&git.CheckoutOptions{
+			Hash:   head.Hash,
+			Branch: branch,
+			Create: true,
+		})
+	} else if err != nil {
+		h.t.Fatalf("checking if branch already exists: %v", branch)
+	} else {
+		err = w.Checkout(&git.CheckoutOptions{
+			Branch: branch,
+		})
+	}
+
+	if err != nil {
+		h.t.Fatalf("checking out branch: %v", err)
+	}
 }
 
 func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) {
@@ -142,6 +162,66 @@ func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) {
 	}
 }
 
+func (h *harness) tryCommit(
+	shouldSucceed bool,
+	commit Commit,
+	accountID string, accountSig sigcred.SignifierInterface,
+) (
+	Commit, plumbing.Hash,
+) {
+	if accountSig != nil {
+		var err error
+		if commit, err = h.repo.AccreditCommit(commit, accountSig); err != nil {
+			h.t.Fatalf("accrediting commit: %v", err)
+		}
+	}
+
+	hash, err := h.repo.Commit(commit, accountID)
+	if err != nil {
+		h.t.Fatalf("failed to commit ChangeCommit: %v", err)
+	}
+
+	branch, err := h.repo.CheckedOutBranch()
+	if err != nil {
+		h.t.Fatalf("determining checked out branch: %v", err)
+	}
+
+	err = h.repo.VerifyCommit(branch, hash)
+	if shouldSucceed && err != nil {
+		h.t.Fatalf("verifying commit %q: %v", hash, err)
+	} else if shouldSucceed {
+		return commit, hash
+	} else if !shouldSucceed && err == nil {
+		h.t.Fatalf("verifying commit %q should have failed", hash)
+	}
+
+	// commit verifying didn't succeed, reset it back. first get parent commit
+	// to reset to
+	commitObj, err := h.repo.GitRepo.CommitObject(hash)
+	if err != nil {
+		h.t.Fatalf("getting commit object of unverifiable hash %q: %v", hash, err)
+	} else if commitObj.NumParents() == 0 {
+		h.t.Fatalf("unverifiable commit %q has no parents, but it should", hash)
+	}
+
+	h.reset(commitObj.ParentHashes[0], git.HardReset)
+	return commit, hash
+}
+
+func (h *harness) changeCommit(
+	msg string,
+	accountID string,
+	sig sigcred.SignifierInterface,
+) (
+	Commit, plumbing.Hash,
+) {
+	commit, err := h.repo.NewCommitChange(msg)
+	if err != nil {
+		h.t.Fatalf("creating ChangeCommit: %v", err)
+	}
+	return h.tryCommit(true, commit, accountID, sig)
+}
+
 func TestHasStagedChanges(t *testing.T) {
 	harness := newHarness(t)
 	assertHasStaged := func(expHasStaged bool) {
@@ -191,22 +271,18 @@ access_controls:
         - root
       count: 0
 `})
-	_, hash0 := harness.changeCommit("first commit, this is going great", "root", harness.sig)
 
-	// even though that access_controls doesn't actually require any signatures,
+	// this commit should be created and verify fine
-	// it should be used because it's not well formed.
+	harness.changeCommit("first commit, this is going great", "root", harness.sig)
-	harness.stage(map[string]string{"foo": "no rules!"})
-	_, hash1 := harness.changeCommit("ain't no laws", "toot", nil)
 
-	// verifying the first should work, but not the second.
+	// this commit should not be verifiable, because toot isn't in accounts and
-	if err := harness.repo.VerifyCommit(MainRefName, hash0); err != nil {
+	// the default access controls should be being used
-		t.Fatalf("first commit %q should be verifiable, but got: %v", hash0, err)
+	harness.stage(map[string]string{"foo": "no rules!"})
-	} else if err := harness.repo.VerifyCommit(MainRefName, hash1); err == nil {
+	badCommit, err := harness.repo.NewCommitChange("ain't no laws")
-		t.Fatalf("second commit %q should not have been verified", hash1)
+	if err != nil {
+		t.Fatalf("creating CommitChange: %v", err)
 	}
-
+	harness.tryCommit(false, badCommit, "toot", nil)
-	// reset back to hash0
-	harness.reset(hash0, git.HardReset)
 
 	// make a commit fixing the config. everything should still be fine.
 	harness.stage(map[string]string{ConfigPath: `
@@ -217,10 +293,7 @@ accounts:
       - type: pgp_public_key_file
         path: ".dehub/root.asc"
 `})
-	_, hash2 := harness.changeCommit("Fix the config!", "root", harness.sig)
+	harness.changeCommit("Fix the config!", "root", harness.sig)
-	if err := harness.repo.VerifyCommit(MainRefName, hash2); err != nil {
-		t.Fatalf("config fix commit %q should be verifiable, but got: %v", hash2, err)
-	}
 }
 
 // TestThisRepoStillVerifies opens this actual repository and ensures that all