Refactor access controls to support multiple branches

message: |-
  Refactor access controls to support multiple branches

  This was a big lift. It implements a backwards incompatible change to overhaul
  access control patterns to also encompass which branch is being interacted with,
  not only which files. The `accessctl` package was significantly rewritten to
  support this, as well as some of the code modifying it. The INTRODUCTION and
  SPEC were also appropriately updated.

  The change to the SPEC is _technically_ backwards incompatible, but it won't
  effect anything. The `access_control` previously being used will just be
  ignored, and the changes to `accessctl` include the definition of fallback
  access controls which will automatically be applied if nothing else matches, so
  when verifying the older history of this repo those will be used.
change_hash: AIfNYLmOLGpuyTiVodT3hDe9lF4E+5DbOTgSdkbjJONb
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl5aw0sACgkQlcRvpqQRSKy7kw//UMyS/waV/tE1vntZrMbmEtFmiXPcMVNal76cjhdiF3He50qXoWG6m0qWz+arD1tbjoZml6pvU+Xt45y/Uc54DZizzz0E9azoFW0/uvZiLApftFRArZbT9GhbDs2afalyoTJx/xvQu+a2FD/zfljEWE8Zix+bwHCLojiYHHVA65HFLEt8RsH33jFyzWvS9a2yYqZXL0qrU9tdV68hazdIm1LCp+lyVV74TjwxPAZDOmNAE9l4EjIk1pgr2Qo4u2KwJqCGdVCvka8TiFFYiP7C6319ZhDMyj4m9yZsd1xGtBd9zABVBDgmzCEjt0LI3Tv35lPd2tpFBkjQy0WGcMAhwSHWSP7lxukQMCEB7og/SwtKaExiBJhf1HRO6H9MlhNSW4X1xwUgP+739ixKKUY/RcyXgZ4pkzt6sewAMVbUOcmzXdUvuyDJQ0nhDFowgicpSh1m8tTkN1aLUx18NfnGZRgkgBeE6EpT5/+NBfFwvpiQkXZ3bcMiNhNTU/UnWMyqjKlog+8Ca/7CqgswYppMaw4iPaC54H8P6JTH+XnqDlLKSkvh7PiJJa5nFDG07fqc8lYKm1KGv6virAhEsz/AYKLoNGIsqXt+mYUDLvQpjlRsiN52euxyn5e41LcrH0RidIGMVeaS+7re1pWbkCkMMMtYlnCbC5L6mfrBu6doN8o=
  account: mediocregopher

.dehub/config.yml

@@ -4,11 +4,3 @@ accounts:
     signifiers:
       - type: pgp_public_key_file
         path: ".dehub/mediocregopher.asc"
-
-access_controls:
-  - pattern: "**"
-    condition:
-      type: signature
-      account_ids:
-        - mediocregopher
-      count: 100%

INTRODUCTION.md

@@ -39,9 +39,9 @@ platforms into the git history itself, including dehub's own configuration.
 
 By doing this, the server-side git component can be reduced to a mere
 pre-receive hook (if anything at all). This opens the door for much more
-lightweight and flexible hosting of git projects, and even more radical
+lightweight and flexible hosting of git projects, as well as even more radical
-solutions like hosting git projects on completely decentralized platforms like
+solutions; dehub can enable hosting git projects on completely decentralized
-IPFS.
+platforms like IPFS.
 
 ### Example
 
@@ -53,16 +53,17 @@ equivalent of the `master` branch). MyProject's repo would contain a
 ```
 # ...
 access_controls:
-
+  - branch_pattern: trunk
-  # matches all files, but could be used for more fine-grained control
+    change_access_controls:
-  - pattern: "**"
+    # matches all files, but could be used for more fine-grained control
-    condition:
+    - file_path_pattern: "**"
-      type: signature
+      condition:
-      account_ids:
+        type: signature
-        - alice
+        account_ids:
-        - bob
+          - alice
-        - carol
+          - bob
-      count: 2
+          - carol
+        count: 2
 ```
 
 A commit in the `trunk` branch would have a message with the following form:
@@ -102,8 +103,7 @@ needed to verify commits in `trunk` when they are pushed or pulled.
 
 ## dehub Thread Branches
 
-The `trunk` branch is the project's source-of-truth; all commits in it must have
+The `trunk` branch is the project's source-of-truth. Other branches, called
-dehub encoded message bodies with acceptable credentials. Other branches, called
 threads, are used to coordinate new changes, and then coalesce those changes
 into a commit suitable for `trunk`.
 

SPEC.md

@@ -28,41 +28,60 @@ accounts:
           - type: "keybase"
             user: "some_keybase_user_id"
 
-# access_controls defines under what conditions different files in the repo may
+# access_controls define who may do what in the repo. The value is a list of
-# be modified. For each file modified in a commit, all access control patterns
+# access control objects, each applying to one or more potential branch names.
-# are applied sequentially until one matches, and the associated access control
-# conditions are checked. A commit is only allowed if the conditions of all
-# modified files are met.
 access_controls:
 
-    # pattern is a glob pattern describing what files this access control
+  # branch_pattern is a glob pattern describing what branch names this access
-    # applies to. Single star matches all characters except path separators,
+  # control applies to. The first matching branch_pattern for a branch name
-    # double star matches everything.
+  # defines which access controls are applied.
-    - pattern: ".dehub/**"
+  - branch_pattern: trunk
 
-      # signature conditions indicate that a commit must be signed by one or
+    # change_access_controls is an array of possible access controls applied for
-      # more accounts to be allowed.
+    # files being changed in the branch
-      condition:
+    change_access_controls:
-        type: signature
 
-        # account_ids lists all accounts whose signature will count towards
+      # file_path_pattern is a glob pattern describing what files this access control
-        # meeting the condition
+      # applies to. Single star matches all characters except path separators,
-        account_ids:
+      # double star matches everything. The first matching file_path_pattern for a
-            - some_user_id
+      # file path (relative to the repo root) defines which access controls are
+      # applied.
+      - file_path_pattern: ".dehub/**"
 
-        # count describes how many signatures are required. It can be either a
+        # signature conditions indicate that a commit must be signed by one or
-        # contrete integer (e.g. 2, meaning any 2 accounts listed by
+        # more accounts in order to be allowed.
-        # account_ids) or a percent.
+        condition:
-        count: 100%
+          type: signature
 
-    # This catch-all pattern for the rest of the repo requires that changes to
+          # account_ids lists all accounts whose signature will count towards
-    # any files not under `.dehub/` are signed by at least one of the
+          # meeting the condition
-    # defined accounts.
+          account_ids:
-    - pattern: "**"
+              - some_user_id
-      condition:
+
-        type: signature
+          # count describes how many signatures are required. It can be either a
-        any_account: true # indicates any account defined in accounts is valid
+          # contrete integer (e.g. 2, meaning any 2 accounts listed by
-        count: 1
+          # account_ids) or a percent.
+          count: 100% # all accounts in account_ids must sign
+
+      # This catch-all pattern for the rest of the repo requires that changes to
+      # any files not under `.dehub/` are signed by at least one of the
+      # defined accounts.
+      - file_path_pattern: "**"
+        condition:
+          type: signature
+          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:
+  #
+  # branch_pattern: **
+  # change_access_controls:
+  #   - file_path_pattern: **
+  #     condition:
+  #       type: signature
+  #       any_account: true
+  #       count: 1
 ```
 
 # Change Hash

accessctl/access_control.go

@@ -2,52 +2,153 @@ package accessctl
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/bmatcuk/doublestar"
 )
 
-// AccessControl represents an access control object being defined in the
+var (
-// Config.
+	// DefaultChangeAccessControl represents the ChangeAccessControl which is
-type AccessControl struct {
+	// applied when a changed file's path does not match any defined patterns
-	Pattern   string    `yaml:"pattern"`
+	// within a BranchAccessControl.
-	Condition Condition `yaml:"condition"`
+	DefaultChangeAccessControl = ChangeAccessControl{
+		FilePathPattern: "**",
+		Condition: Condition{
+			Signature: &ConditionSignature{
+				AnyAccount: true,
+				Count:      "1",
+			},
+		},
+	}
+
+	// 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:        "trunk",
+			ChangeAccessControls: []ChangeAccessControl{DefaultChangeAccessControl},
+		},
+		{
+			BranchPattern:        "**",
+			ChangeAccessControls: []ChangeAccessControl{DefaultChangeAccessControl},
+		},
+	}
+)
+
+// 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"`
+	ChangeAccessControls []ChangeAccessControl `yaml:"change_access_controls"`
 }
 
-// ErrNoApplicableAccessControls is returned from ApplicableAccessControls when
+// ChangeAccessControl represents an access control object being defined in the
-// a changed path has no applicable AccessControls which match it.
+// Config for the purpose of controlling who is able to change which files.
-type ErrNoApplicableAccessControls struct {
+type ChangeAccessControl struct {
-	Path string
+	FilePathPattern string    `yaml:"file_path_pattern"`
+	Condition       Condition `yaml:"condition"`
 }
 
-func (err ErrNoApplicableAccessControls) Error() string {
+// MatchInteractions is used as an input to Match to describe all
-	return fmt.Sprintf("no AccessControls which apply to changed file %q", err.Path)
+// interactions which are being attempted on a particular Branch.
+type MatchInteractions struct {
+	// Branch is the name of the branch the interactions are being attempted on.
+	// It is required.
+	Branch string
+
+	// FilePathsChanged is the set of file paths (relative to the repo root)
+	// which have been modified in some way.
+	FilePathsChanged []string
 }
 
-// ApplicableAccessControls returns a subset of the given AccessControls which
+// MatchedChangeAccessControl contains information about a ChangeAccessControl
-// are applicable to the given file paths (ie those whose Conditions must be met
+// which was matched in Match
-// in order for the changes to go through.
+type MatchedChangeAccessControl struct {
-func ApplicableAccessControls(accessControls []AccessControl, filesChanged []string) ([]AccessControl, error) {
+	ChangeAccessControl ChangeAccessControl
-	applicableSet := map[AccessControl]struct{}{}
+
-	for _, path := range filesChanged {
+	// FilePaths contains all FilePaths to which this access control was found
-		var any bool
+	// to be applicable.
-		for _, ac := range accessControls {
+	FilePaths []string
-			if ok, err := doublestar.PathMatch(ac.Pattern, path); err != nil {
+}
-				return nil, fmt.Errorf("error matching path %q to patterrn %q: %w",
+
-					path, ac.Pattern, err)
+// MatchResult is the result returned from the Match method.
+type MatchResult struct {
+	// BranchPattern indicates the BranchPattern field of the
+	// BranchAccessControl object which matched the inputs.
+	BranchPattern string
+
+	// ChangeAccessControls indicates which ChangeAccessControl objects matched
+	// the files being changed.
+	ChangeAccessControls []MatchedChangeAccessControl
+}
+
+// Match takes in a set of access controls and a set of interactions taking
+// place, and returns a MatchResult describing the access controls which should
+// be applied to the interactions.
+func Match(accessControls []BranchAccessControl, interactions MatchInteractions) (MatchResult, error) {
+	var res MatchResult
+
+	accessControls = append(accessControls, DefaultBranchAccessControls...)
+
+	// find the applicable BranchAccessControl
+	var branchAC BranchAccessControl
+	{
+		var ok bool
+		var err error
+		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",
+					accessControls[i].BranchPattern, interactions.Branch, err)
 			} else if ok {
-				applicableSet[ac] = struct{}{}
+				branchAC = accessControls[i]
-				any = true
 				break
 			}
 		}
-		if !any {
+		if !ok {
-			return nil, ErrNoApplicableAccessControls{Path: path}
+			panic(fmt.Sprintf("no patterns matched branch %q, which shouldn't be possible", interactions.Branch))
 		}
+		res.BranchPattern = branchAC.BranchPattern
 	}
 
-	applicable := make([]AccessControl, 0, len(applicableSet))
+	// determine ChangeAccessControl for each path in FilesChanged
-	for ac := range applicableSet {
+	{
-		applicable = append(applicable, ac)
+		changeACs := append(branchAC.ChangeAccessControls, DefaultChangeAccessControl)
+		acToPaths := map[ChangeAccessControl][]string{}
+		for _, path := range interactions.FilePathsChanged {
+			var ok bool
+			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",
+						path, ac.FilePathPattern, err)
+				} else if ok {
+					acToPaths[ac] = append(acToPaths[ac], path)
+					break
+				}
+			}
+			if !ok {
+				panic(fmt.Sprintf("no patterns matched change path %q, which shouldn't be possible", path))
+			}
+		}
+		for ac, paths := range acToPaths {
+			res.ChangeAccessControls = append(res.ChangeAccessControls, MatchedChangeAccessControl{
+				ChangeAccessControl: ac,
+				FilePaths:           paths,
+			})
+		}
+
+		// sort result for determinancy
+		sort.Slice(res.ChangeAccessControls, func(i, j int) bool {
+			pi := res.ChangeAccessControls[i].ChangeAccessControl.FilePathPattern
+			pj := res.ChangeAccessControls[j].ChangeAccessControl.FilePathPattern
+			return pi < pj
+		})
 	}
-	return applicable, nil
+
+	return res, nil
 }

accessctl/access_control_test.go

@@ -1,117 +1,177 @@
 package accessctl
 
 import (
-	"errors"
 	"reflect"
-	"sort"
 	"testing"
+
+	"github.com/davecgh/go-spew/spew"
 )
 
-func TestApplicableAccessControls(t *testing.T) {
+func normalizeResult(res MatchResult) MatchResult {
+	if len(res.ChangeAccessControls) == 0 {
+		res.ChangeAccessControls = nil
+	}
+	return res
+}
+
+func TestMatch(t *testing.T) {
+	secondCond := Condition{
+		Signature: &ConditionSignature{
+			AnyAccount: true,
+			Count:      "2",
+		},
+	}
+
 	tests := []struct {
-		descr                  string
+		descr string
-		patterns, filesChanged []string
+
-		exp                    []string
+		branchACs    []BranchAccessControl
-		expErrPath             string
+		interactions MatchInteractions
+		result       MatchResult
 	}{
 		{
-			descr: "empty input empty output",
+			descr: "empty input empty result",
+			result: MatchResult{
+				BranchPattern: "**",
+			},
 		},
 		{
-			descr:        "empty patterns",
+			descr: "empty access controls",
-			filesChanged: []string{"foo", "bar"},
+			interactions: MatchInteractions{
-			expErrPath:   "foo",
+				Branch:           "trunk",
+				FilePathsChanged: []string{"foo", "bar"},
+			},
+			result: MatchResult{
+				BranchPattern: "trunk",
+				ChangeAccessControls: []MatchedChangeAccessControl{
+					{
+						ChangeAccessControl: DefaultChangeAccessControl,
+						FilePaths:           []string{"foo", "bar"},
+					},
+				},
+			},
 		},
 		{
-			descr:    "empty filesChanged",
+			descr:        "empty filesPathsChanged",
-			patterns: []string{"patternA", "patternB"},
+			branchACs:    DefaultBranchAccessControls,
+			interactions: MatchInteractions{Branch: "trunk"},
+			result:       MatchResult{BranchPattern: "trunk"},
 		},
 		{
-			descr:        "no applicable files",
+			descr: "no matching branch patterns",
-			filesChanged: []string{"foo"},
+			branchACs: []BranchAccessControl{{
-			patterns:     []string{"bar"},
+				BranchPattern: "dunk",
-			expErrPath:   "foo",
+				ChangeAccessControls: []ChangeAccessControl{{
+					FilePathPattern: "**",
+					Condition:       secondCond,
+				}},
+			}},
+			interactions: MatchInteractions{
+				Branch:           "crunk",
+				FilePathsChanged: []string{"foo"},
+			},
+			result: MatchResult{
+				BranchPattern: "**",
+				ChangeAccessControls: []MatchedChangeAccessControl{{
+					ChangeAccessControl: DefaultChangeAccessControl,
+					FilePaths:           []string{"foo"},
+				}},
+			},
 		},
 		{
-			descr:        "all applicable files",
+			descr: "no matching files",
-			filesChanged: []string{"foo", "bar"},
+			branchACs: []BranchAccessControl{{
-			patterns:     []string{"**"},
+				BranchPattern: "trunk",
-			exp:          []string{"**"},
+				ChangeAccessControls: []ChangeAccessControl{{
+					FilePathPattern: "boo",
+					Condition:       secondCond,
+				}},
+			}},
+			interactions: MatchInteractions{
+				Branch:           "trunk",
+				FilePathsChanged: []string{"foo"},
+			},
+			result: MatchResult{
+				BranchPattern: "trunk",
+				ChangeAccessControls: []MatchedChangeAccessControl{{
+					ChangeAccessControl: DefaultChangeAccessControl,
+					FilePaths:           []string{"foo"},
+				}},
+			},
 		},
 		{
-			descr:        "pattern precedent",
+			descr: "branch pattern precedent",
-			filesChanged: []string{"foo"},
+			branchACs: []BranchAccessControl{
-			patterns:     []string{"foo", "**"},
+				{
-			exp:          []string{"foo"},
+					BranchPattern: "trunk",
+					ChangeAccessControls: []ChangeAccessControl{{
+						FilePathPattern: "foo",
+						Condition:       secondCond,
+					}},
+				},
+				{
+					BranchPattern: "**",
+					ChangeAccessControls: []ChangeAccessControl{
+						DefaultChangeAccessControl,
+					},
+				},
+			},
+			interactions: MatchInteractions{
+				Branch:           "trunk",
+				FilePathsChanged: []string{"foo"},
+			},
+			result: MatchResult{
+				BranchPattern: "trunk",
+				ChangeAccessControls: []MatchedChangeAccessControl{{
+					ChangeAccessControl: ChangeAccessControl{
+						FilePathPattern: "foo",
+						Condition:       secondCond,
+					},
+					FilePaths: []string{"foo"},
+				}},
+			},
 		},
 		{
-			descr:        "pattern precedent inv",
+			descr: "multiple files matching FilePathPatterns",
-			filesChanged: []string{"foo"},
+			branchACs: []BranchAccessControl{{
-			patterns:     []string{"**", "foo"},
+				BranchPattern: "trunk",
-			exp:          []string{"**"},
+				ChangeAccessControls: []ChangeAccessControl{{
-		},
+					FilePathPattern: "foo*",
-		{
+					Condition:       secondCond,
-			descr:        "individual matches",
+				}},
-			filesChanged: []string{"foo", "bar/baz"},
+			}},
-			patterns:     []string{"foo", "bar/baz"},
+			interactions: MatchInteractions{
-			exp:          []string{"foo", "bar/baz"},
+				Branch:           "trunk",
-		},
+				FilePathsChanged: []string{"foo_a", "bar", "foo_b"},
-		{
+			},
-			descr:        "star match dir",
+			result: MatchResult{
-			filesChanged: []string{"foo", "bar/baz"},
+				BranchPattern: "trunk",
-			patterns:     []string{"foo", "bar/*"},
+				ChangeAccessControls: []MatchedChangeAccessControl{
-			exp:          []string{"foo", "bar/*"},
+					{
-		},
+						ChangeAccessControl: DefaultChangeAccessControl,
-		{
+						FilePaths:           []string{"bar"},
-			descr:        "star not match dir",
+					},
-			filesChanged: []string{"foo", "bar/baz/biz"},
+					{
-			patterns:     []string{"foo", "bar/*"},
+						ChangeAccessControl: ChangeAccessControl{
-			expErrPath:   "bar/baz/biz",
+							FilePathPattern: "foo*",
-		},
+							Condition:       secondCond,
-		{
+						},
-			descr:        "doublestar match dir",
+						FilePaths: []string{"foo_a", "foo_b"},
-			filesChanged: []string{"foo", "bar/bar", "bar/baz/biz"},
+					},
-			patterns:     []string{"foo", "bar/**"},
+				},
-			exp:          []string{"foo", "bar/**"},
+			},
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.descr, func(t *testing.T) {
-			accessControls := make([]AccessControl, len(test.patterns))
+			res, err := Match(test.branchACs, test.interactions)
-			for i := range test.patterns {
+			if err != nil {
-				accessControls[i] = AccessControl{Pattern: test.patterns[i]}
+				t.Fatalf("error matching: %v", err)
 			}
-
+			res, expRes := normalizeResult(res), normalizeResult(test.result)
-			out, err := ApplicableAccessControls(accessControls, test.filesChanged)
+			if !reflect.DeepEqual(res, expRes) {
-			if err != nil && test.expErrPath == "" {
+				t.Fatalf("expected:%s\ngot:    %s", spew.Sdump(expRes), spew.Sdump(res))
-				t.Fatalf("unexpected error: %v", err)
-			} else if test.expErrPath != "" {
-				if noAppErr := (ErrNoApplicableAccessControls{}); !errors.As(err, &noAppErr) {
-					t.Fatalf("expected ErrNoApplicableAccessControls for path %q, but got %v", test.expErrPath, err)
-				} else if test.expErrPath != noAppErr.Path {
-					t.Fatalf("expected ErrNoApplicableAccessControls for path %q, but got one for path %q", test.expErrPath, noAppErr.Path)
-				}
-				return
-			}
-
-			outPatterns := make([]string, len(out))
-			for i := range out {
-				outPatterns[i] = out[i].Pattern
-			}
-
-			clean := func(s []string) []string {
-				if len(s) == 0 {
-					return nil
-				}
-				sort.Strings(s)
-				return s
-			}
-
-			outPatterns = clean(outPatterns)
-			test.exp = clean(test.exp)
-			if !reflect.DeepEqual(outPatterns, test.exp) {
-				t.Fatalf("expected: %+v\ngot: %+v", test.exp, outPatterns)
 			}
 		})
 	}

cmd/dehub/main.go

@@ -174,6 +174,7 @@ var subCmds = []subCmd{
 		descr: "verifies one or more commits as having the proper credentials",
 		body: func(sctx subCmdCtx) error {
 			rev := sctx.flag.String("rev", "HEAD", "Revision of commit to verify")
+			branch := sctx.flag.String("branch", "", "Branch that the revision is on. If not given then the currently checked out branch is assumed")
 			sctx.flagParse()
 
 			h, err := sctx.repo().GitRepo.ResolveRevision(plumbing.Revision(*rev))
@@ -181,7 +182,16 @@ var subCmds = []subCmd{
 				return fmt.Errorf("could not resolve revision %q: %w", *rev, err)
 			}
 
-			if err := sctx.repo().VerifyChangeCommit(*h); err != nil {
+			var branchName plumbing.ReferenceName
+			if *branch == "" {
+				if branchName, err = sctx.repo().CheckedOutBranch(); err != nil {
+					return fmt.Errorf("could not determined currently checked out branch: %w", err)
+				}
+			} else {
+				branchName = plumbing.NewBranchReferenceName(*branch)
+			}
+
+			if err := sctx.repo().VerifyChangeCommit(branchName, *h); err != nil {
 				return fmt.Errorf("could not verify commit at %q (%s): %w", *rev, *h, err)
 			}
 
@@ -209,15 +219,14 @@ var subCmds = []subCmd{
 				} else if err != nil {
 					return fmt.Errorf("error reading next line from stdin: %w", err)
 				}
+				fmt.Printf("Processing line %q\n", strings.TrimSpace(line))
 
 				lineParts := strings.Fields(line)
 				if len(lineParts) < 3 {
 					return fmt.Errorf("malformed pre-receive hook stdin line %q", line)
 				}
 
-				if plumbing.ReferenceName(lineParts[2]) != dehub.Trunk {
+				branchName := plumbing.ReferenceName(lineParts[2])
-					return fmt.Errorf("only commits to the trunk branch are allowed at the moment (tried to push to %q)", lineParts[2])
-				}
 
 				// the zeroRevision gets sent on the very first push
 				const zeroRevision plumbing.Revision = "0000000000000000000000000000000000000000"
@@ -269,8 +278,8 @@ var subCmds = []subCmd{
 				for i := len(hashesToCheck) - 1; i >= 0; i-- {
 					hash := hashesToCheck[i]
 					fmt.Printf("Verifying change commit %q\n", hash)
-					if err := sctx.repo().VerifyChangeCommit(hash); err != nil {
+					if err := sctx.repo().VerifyChangeCommit(branchName, hash); err != nil {
-						return fmt.Errorf("could not verify change commit %q", hash)
+						return fmt.Errorf("could not verify change commit %q: %w", hash, err)
 					}
 				}
 				fmt.Println("All pushed commits have been verified, well done.")

commit.go

@@ -116,8 +116,8 @@ func (r *Repo) HasStagedChanges() (bool, error) {
 	return any, nil
 }
 
-// NewChangeCommit constructs a ChangeCommit using the given SignifierInterface
+// NewChangeCommit constructs a ChangeCommit. If sig is given then it is used to
-// to create a Credential for it.
+// create a Credential for the ChangeCommit.
 func (r *Repo) NewChangeCommit(msg, accountID string, sig sigcred.SignifierInterface) (ChangeCommit, error) {
 	_, headTree, err := r.head()
 	if errors.Is(err, plumbing.ErrReferenceNotFound) {
@@ -131,49 +131,38 @@ func (r *Repo) NewChangeCommit(msg, accountID string, sig sigcred.SignifierInter
 		return ChangeCommit{}, err
 	}
 
-	// this is necessarily different than headTree for the case of there being
-	// no HEAD (ie it's the first commit). In that case we want headTree to be
-	// empty (because it's being used to generate the change hash), but we want
-	// the signifier to use the raw fs (because that's where the signifier's
-	// data might be).
-	sigFS, err := r.headOrRawFS()
-	if err != nil {
-		return ChangeCommit{}, err
-	}
-
-	cfg, err := r.loadConfig(sigFS)
-	if err != nil {
-		return ChangeCommit{}, fmt.Errorf("could not load config: %w", err)
-	}
-
 	changeHash := genChangeHash(nil, msg, headTree, stagedTree)
-	cred, err := sig.Sign(sigFS, changeHash)
-	if err != nil {
-		return ChangeCommit{}, fmt.Errorf("failed to sign commit hash: %w", err)
-	}
-	cred.AccountID = accountID
 
-	// This isn't strictly necessary, but we want to save people the effort of
+	var creds []sigcred.Credential
-	// creating an invalid commit, pushing it, having it be rejected, then
+	if sig != nil {
-	// having to reset on the commit.
+		// this is necessarily different than headTree for the case of there
-	err = r.assertAccessControls(
+		// being no HEAD (ie it's the first commit). In that case we want
-		cfg.AccessControls, []sigcred.Credential{cred},
+		// headTree to be empty (because it's being used to generate the change
-		headTree, stagedTree,
+		// hash), but we want the signifier to use the raw fs (because that's
-	)
+		// where the signifier's data might be).
-	if err != nil {
+		sigFS, err := r.headOrRawFS()
-		return ChangeCommit{}, fmt.Errorf("commit would not satisfy access controls: %w", err)
+		if err != nil {
+			return ChangeCommit{}, err
+		}
+
+		cred, err := sig.Sign(sigFS, changeHash)
+		if err != nil {
+			return ChangeCommit{}, fmt.Errorf("failed to sign commit hash: %w", err)
+		}
+		cred.AccountID = accountID
+		creds = append(creds, cred)
 	}
 
 	return ChangeCommit{
 		Message:     msg,
 		ChangeHash:  changeHash,
-		Credentials: []sigcred.Credential{cred},
+		Credentials: creds,
 	}, nil
 }
 
 func (r *Repo) assertAccessControls(
-	accessCtls []accessctl.AccessControl, creds []sigcred.Credential,
+	accessCtls []accessctl.BranchAccessControl, creds []sigcred.Credential,
-	from, to *object.Tree,
+	branch plumbing.ReferenceName, from, to *object.Tree,
 ) error {
 	filesChanged, err := calcDiff(from, to)
 	if err != nil {
@@ -185,18 +174,23 @@ func (r *Repo) assertAccessControls(
 		pathsChanged[i] = filesChanged[i].path
 	}
 
-	accessCtls, err = accessctl.ApplicableAccessControls(accessCtls, pathsChanged)
+	matchRes, err := accessctl.Match(accessCtls, accessctl.MatchInteractions{
+		Branch:           branch.Short(),
+		FilePathsChanged: pathsChanged,
+	})
 	if err != nil {
 		return fmt.Errorf("could not determine applicable access controls: %w", err)
 	}
 
-	for _, accessCtl := range accessCtls {
+	for _, matchedAC := range matchRes.ChangeAccessControls {
-		condInt, err := accessCtl.Condition.Interface()
+		ac := matchedAC.ChangeAccessControl
+		condInt, err := ac.Condition.Interface()
 		if err != nil {
-			return fmt.Errorf("could not cast Condition to interface: %w", err)
+			return fmt.Errorf("could not cast Condition of file path pattern %q to interface: %w",
+				ac.FilePathPattern, err)
 		} else if err := condInt.Satisfied(creds); err != nil {
-			return fmt.Errorf("access control for pattern %q not satisfied: %w",
+			return fmt.Errorf("access control of file path pattern %q not satisfied: %w\nFiles matched:\n%s",
-				accessCtl.Pattern, err)
+				ac.FilePathPattern, err, strings.Join(matchedAC.FilePaths, "\n"))
 		}
 	}
 
@@ -204,8 +198,8 @@ func (r *Repo) assertAccessControls(
 }
 
 // VerifyChangeCommit verifies that the change commit at the given hash, which
-// is presumably on the repo trunk, is gucci.
+// is presumably on the given branch, is gucci.
-func (r *Repo) VerifyChangeCommit(h plumbing.Hash) error {
+func (r *Repo) VerifyChangeCommit(branch plumbing.ReferenceName, h plumbing.Hash) error {
 	commit, err := r.GitRepo.CommitObject(h)
 	if err != nil {
 		return fmt.Errorf("could not retrieve commit object: %w", err)
@@ -241,7 +235,7 @@ func (r *Repo) VerifyChangeCommit(h plumbing.Hash) error {
 
 	err = r.assertAccessControls(
 		cfg.AccessControls, changeCommit.Credentials,
-		parentTree, commitTree,
+		branch, parentTree, commitTree,
 	)
 	if err != nil {
 		return fmt.Errorf("failed to satisfy all access controls: %w", err)

commit_test.go

@@ -1,14 +1,13 @@
 package dehub
 
 import (
-	"dehub/accessctl"
 	"dehub/sigcred"
-	"errors"
 	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/davecgh/go-spew/spew"
+	"gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	yaml "gopkg.in/yaml.v2"
 )
@@ -82,7 +81,7 @@ func TestChangeCommitVerify(t *testing.T) {
 				account := h.cfg.Accounts[0]
 
 				changeCommit, hash := h.changeCommit(step.msg, account.ID, h.sig)
-				if err := h.repo.VerifyChangeCommit(hash); err != nil {
+				if err := h.repo.VerifyChangeCommit(TrunkRefName, hash); err != nil {
 					t.Fatalf("could not verify hash %v: %v", hash, err)
 				}
 
@@ -119,8 +118,8 @@ func TestConfigChange(t *testing.T) {
 	_, hash := h.changeCommit("commit configuration", h.cfg.Accounts[0].ID, h.sig)
 	hashes = append(hashes, hash)
 
-	// create a new account and add it to the configuration. It should not be
+	// create a new account and add it to the configuration. That commit should
-	// able to actually make that commit though.
+	// not be verifiable, though
 	newSig, newPubKeyBody := sigcred.SignifierPGPTmp(h.rand)
 	h.cfg.Accounts = append(h.cfg.Accounts, Account{
 		ID: "toot",
@@ -128,21 +127,23 @@ func TestConfigChange(t *testing.T) {
 			Body: string(newPubKeyBody),
 		}}},
 	})
-	h.cfg.AccessControls[0].Condition.Signature.AccountIDs = []string{"root", "toot"}
+	h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.AccountIDs = []string{"root", "toot"}
-	h.cfg.AccessControls[0].Condition.Signature.Count = "1"
+	h.cfg.AccessControls[0].ChangeAccessControls[0].Condition.Signature.Count = "1"
 
 	cfgBody, err := yaml.Marshal(h.cfg)
 	if err != nil {
 		t.Fatal(err)
 	}
 	h.stage(map[string]string{ConfigPath: string(cfgBody)})
+	_, badHash := h.changeCommit("add toot user", h.cfg.Accounts[1].ID, newSig)
 
-	_, err = h.repo.NewChangeCommit("add toot user", h.cfg.Accounts[1].ID, newSig)
+	if err := h.repo.VerifyChangeCommit(TrunkRefName, badHash); err == nil {
-	if aclErr := (accessctl.ErrConditionSignatureUnsatisfied{}); !errors.As(err, &aclErr) {
+		t.Fatal("toot user shouldn't be able to add itself to config")
-		t.Fatalf("NewChangeCommit should have returned an ErrConditionSignatureUnsatisfied, but returned %v", err)
 	}
+	h.reset(hash, git.HardReset)
 
 	// now add with the root user, this should work.
+	h.stage(map[string]string{ConfigPath: string(cfgBody)})
 	_, hash = h.changeCommit("add toot user", h.cfg.Accounts[0].ID, h.sig)
 	hashes = append(hashes, hash)
 
@@ -152,7 +153,7 @@ func TestConfigChange(t *testing.T) {
 	hashes = append(hashes, hash)
 
 	for i, hash := range hashes {
-		if err := h.repo.VerifyChangeCommit(hash); err != nil {
+		if err := h.repo.VerifyChangeCommit(TrunkRefName, hash); err != nil {
 			t.Fatalf("commit %d (%v) should have been verified but wasn't: %v", i, hash, err)
 		}
 	}

config.go

@@ -20,8 +20,8 @@ type Account struct {
 // Config represents the structure of the main dehub configuration file, and is
 // used to marshal/unmarshal the yaml file.
 type Config struct {
-	Accounts       []Account                 `yaml:"accounts"`
+	Accounts       []Account                       `yaml:"accounts"`
-	AccessControls []accessctl.AccessControl `yaml:"access_controls"`
+	AccessControls []accessctl.BranchAccessControl `yaml:"access_controls"`
 }
 
 func (r *Repo) loadConfig(fs fs.FS) (Config, error) {

repo.go

@@ -12,6 +12,7 @@ import (
 	"gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/src-d/go-git.v4/plumbing/object"
+	"gopkg.in/src-d/go-git.v4/storage"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
 )
 
@@ -25,8 +26,11 @@ var (
 	// ConfigPath defines the expected path to the Repo's configuration file.
 	ConfigPath = filepath.Join(DehubDir, "config.yml")
 
-	// Trunk defines the reference name of the trunk branch.
+	// Trunk defines the name of the trunk branch.
-	Trunk = plumbing.ReferenceName("refs/heads/trunk")
+	Trunk = "trunk"
+
+	// TrunkRefName defines the reference name of the trunk branch.
+	TrunkRefName = plumbing.NewBranchReferenceName(Trunk)
 )
 
 type repoOpts struct {
@@ -48,6 +52,7 @@ func OpenBare(bare bool) OpenOption {
 // Repo is an object which allows accessing and modifying the dehub repo.
 type Repo struct {
 	GitRepo *git.Repository
+	Storer  storage.Storer
 }
 
 // OpenRepo opens the dehub repo in the given directory and returns the object
@@ -69,6 +74,7 @@ func OpenRepo(path string, options ...OpenOption) (*Repo, error) {
 	if r.GitRepo, err = git.PlainOpenWithOptions(path, openOpts); err != nil {
 		return nil, fmt.Errorf("could not open git repo: %w", err)
 	}
+
 	return &r, nil
 }
 
@@ -79,7 +85,19 @@ func InitMemRepo() *Repo {
 		panic(err)
 	}
 
-	return &Repo{GitRepo: r}
+	repo := &Repo{GitRepo: r}
+	if err := repo.init(); err != nil {
+		panic(err)
+	}
+	return repo
+}
+
+func (r *Repo) init() error {
+	h := plumbing.NewSymbolicReference(plumbing.HEAD, TrunkRefName)
+	if err := r.GitRepo.Storer.SetReference(h); err != nil {
+		return fmt.Errorf("could not set HEAD to %q: %w", TrunkRefName, err)
+	}
+	return nil
 }
 
 func (r *Repo) billyFilesystem() (billy.Filesystem, error) {
@@ -90,6 +108,35 @@ func (r *Repo) billyFilesystem() (billy.Filesystem, error) {
 	return w.Filesystem, nil
 }
 
+// CheckedOutBranch returns the name of the currently checked out branch.
+func (r *Repo) CheckedOutBranch() (plumbing.ReferenceName, error) {
+	// Head() can't be used for this, because it doesn't handle the case of a
+	// newly initialized repo very well.
+	ogRef, err := r.GitRepo.Storer.Reference(plumbing.HEAD)
+	if err != nil {
+		return "", fmt.Errorf("couldn't de-reference HEAD (is it a bare repo?): %w", err)
+	}
+
+	ref := ogRef
+	for {
+		if ref.Type() != plumbing.SymbolicReference {
+			break
+		}
+
+		target := ref.Target()
+		if target.IsBranch() {
+			return target, nil
+		}
+
+		ref, err = r.GitRepo.Storer.Reference(target)
+		if err != nil {
+			break
+		}
+	}
+
+	return "", fmt.Errorf("could not de-reference HEAD to a branch: %w", err)
+}
+
 func (r *Repo) head() (*object.Commit, *object.Tree, error) {
 	head, err := r.GitRepo.Head()
 	if err != nil {

repo_test.go

@@ -10,6 +10,7 @@ import (
 	"runtime/debug"
 	"testing"
 
+	"gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	yaml "gopkg.in/yaml.v2"
 )
@@ -34,13 +35,18 @@ func newHarness(t *testing.T) *harness {
 				Path: pubKeyPath,
 			}}},
 		}},
-		AccessControls: []accessctl.AccessControl{
+		AccessControls: []accessctl.BranchAccessControl{
 			{
-				Pattern: "**",
+				BranchPattern: "**",
-				Condition: accessctl.Condition{
+				ChangeAccessControls: []accessctl.ChangeAccessControl{
-					Signature: &accessctl.ConditionSignature{
+					{
-						AccountIDs: []string{"root"},
+						FilePathPattern: "**",
-						Count:      "100%",
+						Condition: accessctl.Condition{
+							Signature: &accessctl.ConditionSignature{
+								AccountIDs: []string{"root"},
+								Count:      "100%",
+							},
+						},
 					},
 				},
 			},
@@ -115,6 +121,21 @@ func (h *harness) changeCommit(msg, accountID string, sig sigcred.SignifierInter
 	return tc, hash
 }
 
+func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) {
+	w, err := h.repo.GitRepo.Worktree()
+	if err != nil {
+		h.t.Fatal(err)
+	}
+
+	err = w.Reset(&git.ResetOptions{
+		Commit: to,
+		Mode:   mode,
+	})
+	if err != nil {
+		h.t.Fatal(err)
+	}
+}
+
 func TestHasStagedChanges(t *testing.T) {
 	harness := newHarness(t)
 	assertHasStaged := func(expHasStaged bool) {
@@ -141,3 +162,57 @@ func TestHasStagedChanges(t *testing.T) {
 	harness.changeCommit("second commit", "root", harness.sig)
 	assertHasStaged(false)
 }
+
+// TestOldConfig tests that having an older, now malformed, Config doesn't mess
+// with the current parsing, as long as the default access controls still work.
+func TestOldConfig(t *testing.T) {
+	harness := newHarness(t)
+
+	// overwrite the currently staged config file with an older form
+	harness.stage(map[string]string{ConfigPath: `
+---
+accounts:
+  - id: root
+    signifiers:
+      - type: pgp_public_key_file
+        path: ".dehub/root.asc"
+
+access_controls:
+  - pattern: "**"
+    condition:
+      type: signature
+      account_ids:
+        - 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,
+	// it should be used because it's not well formed.
+	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.
+	if err := harness.repo.VerifyChangeCommit(TrunkRefName, hash0); err != nil {
+		t.Fatalf("first commit %q should be verifiable, but got: %v", hash0, err)
+	} else if err := harness.repo.VerifyChangeCommit(TrunkRefName, hash1); err == nil {
+		t.Fatalf("second commit %q should not have been verified", hash1)
+	}
+
+	// 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: `
+---
+accounts:
+  - id: root
+    signifiers:
+      - type: pgp_public_key_file
+        path: ".dehub/root.asc"
+`})
+	_, hash2 := harness.changeCommit("Fix the config!", "root", harness.sig)
+	if err := harness.repo.VerifyChangeCommit(TrunkRefName, hash2); err != nil {
+		t.Fatalf("config fix commit %q should be verifiable, but got: %v", hash2, err)
+	}
+}