diff --git a/Dockerfile.dehub-remote b/Dockerfile.dehub-remote index 770ffb8..63d8635 100644 --- a/Dockerfile.dehub-remote +++ b/Dockerfile.dehub-remote @@ -36,9 +36,6 @@ COPY --from=0 /usr/bin/git-http-server /usr/bin/git-http-server # Create config files for container startup and nginx COPY cmd/dehub-remote/nginx.conf /etc/nginx/nginx.conf -# Create pre-receive -COPY cmd/dehub-remote/pre-receive /pre-receive - # Create start.sh COPY cmd/dehub-remote/start.sh /start.sh RUN chmod +x /start.sh diff --git a/ROADMAP.md b/ROADMAP.md index 2a5c8c5..c3a6bbd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -9,7 +9,6 @@ set, only a sequence of milestones and the requirements to hit them. Must be able to feel good about showing the project publicly, as well as be able to accept help from people asking to help. -* Fast-forward perms on branches (so they can be deleted) * Figure out commit range syntax, use that everywhere. * Create a branch which is just a public "welcome thread", which can be part of the tutorials. diff --git a/accessctl/access_control.go b/accessctl/access_control.go index e59cd05..b94acdc 100644 --- a/accessctl/access_control.go +++ b/accessctl/access_control.go @@ -28,6 +28,11 @@ var DefaultAccessControlsStr = ` any_account: true count: 1 + - action: deny + filters: + - type: commit_attributes + non_fast_forward: true + - action: allow filters: - type: branch @@ -67,6 +72,11 @@ type CommitRequest struct { // FilesChanged is the set of file paths (relative to the repo root) which // have been modified in some way. FilesChanged []string + + // NonFastForward should be set to true if the branch HEAD and this commit + // are not directly related (i.e. neither is a direct ancestor of the + // other). + NonFastForward bool } // Action describes what action an AccessControl should perform diff --git a/accessctl/filter.go b/accessctl/filter.go index aade201..3109125 100644 --- a/accessctl/filter.go +++ b/accessctl/filter.go @@ -1,9 +1,10 @@ package accessctl import ( - "dehub.dev/src/dehub.git/typeobj" "errors" "fmt" + + "dehub.dev/src/dehub.git/typeobj" ) // ErrFilterNoMatch is returned from a FilterInterface's Match method when the @@ -28,11 +29,12 @@ type FilterInterface interface { // Filter represents an access control filter being defined in the Config. Only // one of its fields may be filled at a time. type Filter struct { - Signature *FilterSignature `type:"signature"` - Branch *FilterBranch `type:"branch"` - FilesChanged *FilterFilesChanged `type:"files_changed"` - CommitType *FilterCommitType `type:"commit_type"` - Not *FilterNot `type:"not"` + Signature *FilterSignature `type:"signature"` + Branch *FilterBranch `type:"branch"` + FilesChanged *FilterFilesChanged `type:"files_changed"` + CommitType *FilterCommitType `type:"commit_type"` + CommitAttributes *FilterCommitAttributes `type:"commit_attributes"` + Not *FilterNot `type:"not"` } // MarshalYAML implements the yaml.Marshaler interface. @@ -100,3 +102,20 @@ func (f FilterCommitType) MatchCommit(req CommitRequest) error { return errors.New(`one of the following fields must be set: "commit_type", "commit_types"`) } } + +// FilterCommitAttributes filters by one more attributes a commit can have. If +// more than one field is filled in then all relevant attributes must be present +// on the commit for this filter to match. +type FilterCommitAttributes struct { + NonFastForward bool `yaml:"non_fast_forward"` +} + +var _ FilterInterface = FilterCommitAttributes{} + +// MatchCommit implements the method for FilterInterface. +func (f FilterCommitAttributes) MatchCommit(req CommitRequest) error { + if f.NonFastForward && !req.NonFastForward { + return ErrFilterNoMatch{Err: errors.New("commit is a fast-forward")} + } + return nil +} diff --git a/accessctl/filter_test.go b/accessctl/filter_test.go index a0a916b..3cad9ff 100644 --- a/accessctl/filter_test.go +++ b/accessctl/filter_test.go @@ -86,3 +86,52 @@ func TestFilterCommitType(t *testing.T) { }, }) } + +func TestFilterCommitAttributes(t *testing.T) { + mkReq := func(nonFF bool) CommitRequest { + return CommitRequest{NonFastForward: nonFF} + } + + runCommitMatchTests(t, []filterCommitMatchTest{ + { + descr: "ff with empty filter", + filter: FilterCommitAttributes{}, + req: mkReq(false), + match: true, + }, + { + descr: "non-ff with empty filter", + filter: FilterCommitAttributes{}, + req: mkReq(true), + match: true, + }, + { + descr: "ff with non-ff filter", + filter: FilterCommitAttributes{NonFastForward: true}, + req: mkReq(false), + match: false, + }, + { + descr: "non-ff with non-ff filter", + filter: FilterCommitAttributes{NonFastForward: true}, + req: mkReq(true), + match: true, + }, + { + descr: "ff with inverted non-ff filter", + filter: FilterNot{Filter: Filter{ + CommitAttributes: &FilterCommitAttributes{NonFastForward: true}, + }}, + req: mkReq(false), + match: true, + }, + { + descr: "non-ff with inverted non-ff filter", + filter: FilterNot{Filter: Filter{ + CommitAttributes: &FilterCommitAttributes{NonFastForward: true}, + }}, + req: mkReq(true), + match: false, + }, + }) +} diff --git a/cmd/dehub/cmd_hook.go b/cmd/dehub/cmd_hook.go index 2354eb1..1751ad6 100644 --- a/cmd/dehub/cmd_hook.go +++ b/cmd/dehub/cmd_hook.go @@ -45,23 +45,16 @@ func cmdHook(ctx context.Context, cmd *dcmd.Cmd) { return nil, fmt.Errorf("malformed pre-receive hook stdin line %q", line) } - startRev := plumbing.Revision(lineParts[0]) - endRev := plumbing.Revision(lineParts[1]) + endHash := plumbing.NewHash(lineParts[1]) branchName := plumbing.ReferenceName(lineParts[2]) if !branchName.IsBranch() { return nil, fmt.Errorf("reference %q is not a branch, can't push to it", branchName) + } else if endHash == plumbing.ZeroHash { + return nil, errors.New("deleting remote branches is not currently supported") } - gitCommits, err := repo.GetGitRevisionRange(startRev, endRev) - if err != nil { - return nil, fmt.Errorf("getting commits from %q to %q: %w", - startRev, endRev, err) - - } else if err := repo.VerifyCommits(branchName, gitCommits); err != nil { - return nil, fmt.Errorf("verifying commits from %q to %q: %w", - startRev, endRev, err) - } + return nil, repo.VerifyCanSetBranchHEADTo(branchName, endHash) } fmt.Println("All pushed commits have been verified, well done.") diff --git a/commit.go b/commit.go index 158bcac..00594fc 100644 --- a/commit.go +++ b/commit.go @@ -294,50 +294,102 @@ func (r *Repo) HasStagedChanges() (bool, error) { // VerifyCommits verifies that the given commits, which are presumably on the // given branch, are gucci. -func (r *Repo) VerifyCommits(branch plumbing.ReferenceName, gitCommits []GitCommit) error { +func (r *Repo) VerifyCommits(branchName plumbing.ReferenceName, gitCommits []GitCommit) error { + // this isn't strictly necessary for this method, but it helps discover bugs + // in other parts of the code. + if len(gitCommits) == 0 { + return errors.New("cannot call VerifyCommits with empty commit slice") + } // First determine the root of the main branch. All commits need to be an - // ancestor of it. - var root plumbing.Hash + // ancestor of it. If the main branch has not been created yet then there + // might not be a root commit yet. + var rootCommit *object.Commit mainGitCommit, err := r.GetGitRevision(plumbing.Revision(MainRefName)) - if err != nil { - return fmt.Errorf("retrieving commit at HEAD of main: %w", err) - } + if errors.Is(err, plumbing.ErrReferenceNotFound) { - rootCommit := mainGitCommit.GitCommit - for { - if rootCommit.NumParents() == 0 { - break - } else if rootCommit.NumParents() > 1 { - return fmt.Errorf("commit %q in main branch has more than one parent", root) - } else if rootCommit, err = rootCommit.Parent(0); err != nil { - return fmt.Errorf("retrieving parent commit of %q: %w", root, err) + // main branch hasn't been created yet. The commits can only be verified + // if they are for the main branch and they include the root commit. + if branchName != MainRefName { + return fmt.Errorf("cannot verify commits in branch %q when no main branch exists", branchName) + } + + for _, gitCommit := range gitCommits { + if gitCommit.GitCommit.NumParents() == 0 { + rootCommit = gitCommit.GitCommit + break + } + } + if rootCommit == nil { + return errors.New("root commit of main branch cannot be determined") + } + + } else if err != nil { + return fmt.Errorf("retrieving commit at HEAD of %q: %w", MainRefName.Short(), err) + + } else { + rootCommit = mainGitCommit.GitCommit + for { + if rootCommit.NumParents() == 0 { + break + } else if rootCommit.NumParents() > 1 { + return fmt.Errorf("commit %q in main branch has more than one parent", rootCommit.Hash) + } else if rootCommit, err = rootCommit.Parent(0); err != nil { + return fmt.Errorf("retrieving parent commit of %q: %w", rootCommit.Hash, err) + } } } + // We also need the HEAD of the given branch, if it exists. + branchGitCommit, err := r.GetGitRevision(plumbing.Revision(branchName)) + if err != nil && !errors.Is(err, plumbing.ErrReferenceNotFound) { + return fmt.Errorf("retrieving commit at HEAD of %q: %w", branchName.Short(), err) + } + for i, gitCommit := range gitCommits { // It's not a requirement that the given GitCommits are in ancestral - // order, but usually they are, so we can help verifyCommit not have to - // calculate the parentTree if the previous commit is the parent of this - // one, and not have to determine that each commit is an ancestor of - // main manually. + // order, but usually they are; if the previous commit is the parent of + // this one we can skip a bunch of work. var parentTree *object.Tree + var isNonFF bool if i > 0 && gitCommits[i-1].GitCommit.Hash == gitCommit.GitCommit.ParentHashes[0] { parentTree = gitCommits[i-1].GitTree } else if gitCommit.GitCommit.Hash == rootCommit.Hash { - // looking at the root commit itself, assume it's ok + // looking at the root commit, assume it's ok - } else if isAncestor, err := rootCommit.IsAncestor(gitCommit.GitCommit); err != nil { - return fmt.Errorf("determining if %q is an ancestor of %q (root of main): %w", - gitCommit.GitCommit.Hash, rootCommit.Hash, err) + } else { + var err error + isAncestor := func(older, younger *object.Commit) bool { + var isAncestor bool + if err != nil { + return false + } else if isAncestor, err = older.IsAncestor(younger); err != nil { + err = fmt.Errorf("determining if %q is an ancestor of %q: %w", + younger.Hash, older.Hash, err) + return false + + } + return isAncestor + } - } else if !isAncestor { - return fmt.Errorf("%q is not an ancestor of %q (root of main)", - gitCommit.GitCommit.Hash, rootCommit.Hash) + ancestorOfRoot := isAncestor(rootCommit, gitCommit.GitCommit) + if branchGitCommit.GitCommit != nil { + // if the branch doesn't actually exist then this couldn't + // possibly be a nonFF + isNonFF = !isAncestor(branchGitCommit.GitCommit, gitCommit.GitCommit) + } + + if err != nil { + return err + } else if !ancestorOfRoot { + return fmt.Errorf("commit %q must be direct descendant of root commit of %q (%q)", + gitCommit.GitCommit.Hash, MainRefName.Short(), rootCommit.Hash, + ) + } } - if err := r.verifyCommit(branch, gitCommit, parentTree); err != nil { + if err := r.verifyCommit(branchName, gitCommit, parentTree, isNonFF); err != nil { return fmt.Errorf("verifying commit %q: %w", gitCommit.GitCommit.Hash, err) } @@ -367,7 +419,12 @@ func (r *Repo) parentTree(commitObj *object.Commit) (*object.Tree, error) { } // if parentTree is nil then it will be inferred. -func (r *Repo) verifyCommit(branch plumbing.ReferenceName, gitCommit GitCommit, parentTree *object.Tree) error { +func (r *Repo) verifyCommit( + branchName plumbing.ReferenceName, + gitCommit GitCommit, + parentTree *object.Tree, + isNonFF bool, +) error { parentTree, err := r.parentTree(gitCommit.GitCommit) if err != nil { return fmt.Errorf("retrieving parent tree of commit: %w", err) @@ -407,10 +464,11 @@ func (r *Repo) verifyCommit(branch plumbing.ReferenceName, gitCommit GitCommit, } err = accessctl.AssertCanCommit(cfg.AccessControls, accessctl.CommitRequest{ - Type: commitType, - Branch: branch.Short(), - Credentials: gitCommit.Commit.Common.Credentials, - FilesChanged: pathsChanged, + Type: commitType, + Branch: branchName.Short(), + Credentials: gitCommit.Commit.Common.Credentials, + FilesChanged: pathsChanged, + NonFastForward: isNonFF, }) if err != nil { return fmt.Errorf("asserting access controls: %w", err) @@ -494,3 +552,59 @@ func (r *Repo) changeRangeInfo(commits []GitCommit) (changeRangeInfo, error) { info.changeHash = genChangeHash(nil, info.msg, changedFiles) return info, nil } + +// VerifyCanSetBranchHEADTo is used to verify that a branch's HEAD can be set to +// the given hash. It verifies any new commits which are being added, and +// handles verifying non-fast-forward commits as well. +// +// If the given hash matches the current HEAD of the branch then this performs +// no further checks and returns nil. +func (r *Repo) VerifyCanSetBranchHEADTo(branchName plumbing.ReferenceName, hash plumbing.Hash) error { + oldCommitRef, err := r.GitRepo.Reference(branchName, true) + if errors.Is(err, plumbing.ErrReferenceNotFound) { + // if the branch is being created then just pull all of its commits and + // verify them. + // TODO optimize this so that it tries to use the merge-base with main, + // so we're not re-verifying a ton of commits unecessarily + commits, err := r.GetGitCommitRange(plumbing.ZeroHash, hash) + if err != nil { + return fmt.Errorf("retrieving %q and all its ancestors: %w", hash, err) + } + return r.VerifyCommits(branchName, commits) + + } else if err != nil { + return fmt.Errorf("resolving branch reference to a hash: %w", err) + + } else if oldCommitRef.Hash() == hash { + // if the HEAD is already at the given hash then it must be fine. + return nil + } + + oldCommitObj, err := r.GitRepo.CommitObject(oldCommitRef.Hash()) + if err != nil { + return fmt.Errorf("retrieving commit object %q: %w", oldCommitRef.Hash(), err) + } + + newCommitObj, err := r.GitRepo.CommitObject(hash) + if err != nil { + return fmt.Errorf("retrieving commit object %q: %w", hash, err) + } + + mbCommits, err := oldCommitObj.MergeBase(newCommitObj) + if err != nil { + return fmt.Errorf("determining merge-base between %q and %q: %w", + oldCommitObj.Hash, newCommitObj.Hash, err) + } else if len(mbCommits) == 0 { + return fmt.Errorf("%q and %q have no ancestors in common", + oldCommitObj.Hash, newCommitObj.Hash) + } else if len(mbCommits) == 2 { + return fmt.Errorf("%q and %q have more than one ancestor in common", + oldCommitObj.Hash, newCommitObj.Hash) + } + + commits, err := r.GetGitCommitRange(mbCommits[0].Hash, hash) + if err != nil { + return fmt.Errorf("retrieving commits %q to %q: %w", mbCommits[0].Hash, hash, err) + } + return r.VerifyCommits(branchName, commits) +} diff --git a/commit_change_test.go b/commit_change_test.go index 94fbc1f..c0c6f40 100644 --- a/commit_change_test.go +++ b/commit_change_test.go @@ -78,7 +78,7 @@ func TestChangeCommitVerify(t *testing.T) { for _, step := range test.steps { h.stage(step.tree) - gitCommit := h.assertCommitChange(true, step.msg, rootSig) + gitCommit := h.assertCommitChange(verifyShouldSucceed, step.msg, rootSig) if step.msgHead == "" { step.msgHead = strings.TrimSpace(step.msg) + "\n\n" } @@ -105,7 +105,7 @@ func TestCombineCommitChanges(t *testing.T) { // commit initial config, so the root user can modify it in the next commit rootSig := h.stageNewAccount("root", false) - h.assertCommitChange(true, "initial commit", rootSig) + h.assertCommitChange(verifyShouldSucceed, "initial commit", rootSig) // add a toot user and modify the access controls such that both accounts // are required for the main branch @@ -131,21 +131,21 @@ func TestCombineCommitChanges(t *testing.T) { any_account: true count: 1 `) - tootCommit := h.assertCommitChange(true, "add toot", rootSig) + tootCommit := h.assertCommitChange(verifyShouldSucceed, "add toot", rootSig) // make a single change commit in another branch using root. Then add a // credential using toot, and combine them onto main. otherBranch := plumbing.NewBranchReferenceName("other") h.checkout(otherBranch) h.stage(map[string]string{"foo": "bar"}) - fooCommit := h.assertCommitChange(true, "add foo file", rootSig) + fooCommit := h.assertCommitChange(verifyShouldSucceed, "add foo file", rootSig) // now adding a credential commit from toot should work credCommitObj, err := h.repo.NewCommitCredential(fooCommit.Interface.StoredHash()) if err != nil { t.Fatal(err) } - credCommit := h.tryCommit(true, credCommitObj, tootSig) + credCommit := h.tryCommit(verifyShouldSucceed, credCommitObj, tootSig) allCommits, err := h.repo.GetGitCommitRange( tootCommit.GitCommit.Hash, diff --git a/commit_credential_test.go b/commit_credential_test.go index ac80c75..2cb0396 100644 --- a/commit_credential_test.go +++ b/commit_credential_test.go @@ -32,7 +32,7 @@ func TestCredentialCommitVerify(t *testing.T) { account_ids: - root `) - rootGitCommit := h.assertCommitChange(true, "initial commit", rootSig) + rootGitCommit := h.assertCommitChange(verifyShouldSucceed, "initial commit", rootSig) // toot user wants to create a credential commit for the root commit, for // whatever reason. @@ -42,9 +42,9 @@ func TestCredentialCommitVerify(t *testing.T) { t.Fatalf("creating credential commit for hash %x: %v", rootChangeHash, err) } - h.tryCommit(false, credCommit, tootSig) + h.tryCommit(verifyShouldFail, credCommit, tootSig) // toot tries again in their own branch, and should be allowed. h.checkout(tootBranch) - h.tryCommit(true, credCommit, tootSig) + h.tryCommit(verifyShouldSucceed, credCommit, tootSig) } diff --git a/commit_test.go b/commit_test.go index 77dfc8f..d9f3c9f 100644 --- a/commit_test.go +++ b/commit_test.go @@ -1,8 +1,13 @@ package dehub import ( + "errors" + "regexp" "testing" + "dehub.dev/src/dehub.git/accessctl" + "dehub.dev/src/dehub.git/sigcred" + "gopkg.in/src-d/go-git.v4" "gopkg.in/src-d/go-git.v4/plumbing" ) @@ -14,23 +19,23 @@ func TestConfigChange(t *testing.T) { // commit the initial staged changes, which merely include the config and // public key - gitCommit := h.assertCommitChange(true, "commit configuration", rootSig) + gitCommit := h.assertCommitChange(verifyShouldSucceed, "commit configuration", rootSig) gitCommits = append(gitCommits, gitCommit) // create a new account and add it to the configuration. That commit should // not be verifiable, though tootSig := h.stageNewAccount("toot", false) h.stageCfg() - h.assertCommitChange(false, "add toot user", tootSig) + h.assertCommitChange(verifyShouldFail, "add toot user", tootSig) // now add with the root user, this should work. h.stageCfg() - gitCommit = h.assertCommitChange(true, "add toot user", rootSig) + gitCommit = h.assertCommitChange(verifyShouldSucceed, "add toot user", rootSig) gitCommits = append(gitCommits, gitCommit) // _now_ the toot user should be able to do things. h.stage(map[string]string{"foo/bar": "what a cool file"}) - gitCommit = h.assertCommitChange(true, "add a cool file", tootSig) + gitCommit = h.assertCommitChange(verifyShouldSucceed, "add a cool file", tootSig) gitCommits = append(gitCommits, gitCommit) if err := h.repo.VerifyCommits(MainRefName, gitCommits); err != nil { @@ -47,14 +52,13 @@ func TestMainAncestryRequirement(t *testing.T) { // stage and try to add to the "other" branch, it shouldn't work though h.stageCfg() - h.assertCommitChange(false, "starting new branch at other", rootSig) + h.assertCommitChange(verifyShouldFail, "starting new branch at other", rootSig) }) t.Run("new branch, single commit", func(t *testing.T) { h := newHarness(t) rootSig := h.stageNewAccount("root", false) - h.stageCfg() - h.assertCommitChange(true, "add cfg", rootSig) + h.assertCommitChange(verifyShouldSucceed, "add cfg", rootSig) // set HEAD to this other branch which doesn't really exist ref := plumbing.NewSymbolicReference(plumbing.HEAD, otherBranch) @@ -63,7 +67,7 @@ func TestMainAncestryRequirement(t *testing.T) { } h.stageCfg() - h.assertCommitChange(false, "starting new branch at other", rootSig) + h.assertCommitChange(verifyShouldFail, "starting new branch at other", rootSig) }) } @@ -77,5 +81,354 @@ func TestAnonymousCommits(t *testing.T) { - type: signature any: true `) - h.assertCommitChange(true, "this will work", anonSig) + h.assertCommitChange(verifyShouldSucceed, "this will work", anonSig) +} + +func TestNonFastForwardCommits(t *testing.T) { + h := newHarness(t) + rootSig := h.stageNewAccount("root", false) + initCommit := h.assertCommitChange(verifyShouldSucceed, "init", rootSig) + + // add another commit + h.stage(map[string]string{"foo": "foo"}) + fooCommit := h.assertCommitChange(verifyShouldSucceed, "foo", rootSig) + + commitOn := func(hash plumbing.Hash, msg string) GitCommit { + ref := plumbing.NewHashReference(plumbing.HEAD, hash) + if err := h.repo.GitRepo.Storer.SetReference(ref); err != nil { + h.t.Fatal(err) + } else if commitChange, err := h.repo.NewCommitChange("bar"); err != nil { + h.t.Fatal(err) + } else if commitChange, err = h.repo.AccreditCommit(commitChange, rootSig); err != nil { + h.t.Fatal(err) + } else if gitCommit, err := h.repo.Commit(commitChange); err != nil { + h.t.Fatal(err) + } else { + return gitCommit + } + panic("can't get here") + } + + // checkout initCommit directly, make a new commit on top of it, and try to + // verify that (this is too fancy for the harness, must be done manually). + h.stage(map[string]string{"bar": "bar"}) + barCommit := commitOn(initCommit.GitCommit.Hash, "bar") + err := h.repo.VerifyCommits(MainRefName, []GitCommit{barCommit}) + if !errors.As(err, new(accessctl.ErrCommitRequestDenied)) { + h.t.Fatalf("expected ErrCommitRequestDenied, got: %v", err) + } + + // check main back out (fooCommit should be checked out), and modify the + // config to allow nonFF commits, and add another bogus commit on top. + h.checkout(MainRefName) + h.stageAccessControls(` + - action: allow + filters: + - type: commit_attributes + non_fast_forward: true`) + h.stageCfg() + allowNonFFCommit := h.assertCommitChange(verifyShouldSucceed, "allow non-ff", rootSig) + + h.stage(map[string]string{"foo": "foo foo"}) + h.assertCommitChange(verifyShouldSucceed, "foo foo", rootSig) + + // checking out allowNonFFCommit directly and performing a nonFF commit + // should work now. + h.stage(map[string]string{"baz": "baz"}) + bazCommit := commitOn(allowNonFFCommit.GitCommit.Hash, "baz") + if err = h.repo.VerifyCommits(MainRefName, []GitCommit{bazCommit}); err != nil { + h.t.Fatal(err) + } + + // verifying the full history should also work + gitCommits := []GitCommit{initCommit, fooCommit, allowNonFFCommit, bazCommit} + if err = h.repo.VerifyCommits(MainRefName, gitCommits); err != nil { + h.t.Fatal(err) + } +} + +func TestCanSetBranchHEADTo(t *testing.T) { + type toTest struct { + // branchName and hash are the arguments passed into + // VerifyCanSetBranchHEADTo. + branchName plumbing.ReferenceName + hash plumbing.Hash + + // if set then the branch will have its HEAD reset to this hash prior to + // calling VerifyCanSetBranchHEADTo. + resetTo plumbing.Hash + } + + type test struct { + descr string + init func(h *harness, rootSig sigcred.SignifierInterface) toTest + + // If true then the verify call is expected to fail. The string is a + // regex which should match the unwrapped error returned. + expErr string + } + + tests := []test{ + { + descr: "creation of main", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + // checkout other and build on top of that, so that when + // VerifyCanSetBranchHEADTo is called main won't exist. + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + return toTest{ + branchName: MainRefName, + hash: initCommit.GitCommit.Hash, + } + }, + }, + { + descr: "main ff", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + h.stage(map[string]string{"foo": "foo"}) + nextCommit := h.assertCommitChange(verifySkip, "next", rootSig) + return toTest{ + branchName: MainRefName, + hash: nextCommit.GitCommit.Hash, + resetTo: initCommit.GitCommit.Hash, + } + }, + }, + { + descr: "new branch, no main", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + // checkout other and build on top of that, so that when + // VerifyCanSetBranchHEADTo is called main won't exist. + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + return toTest{ + branchName: plumbing.NewBranchReferenceName("other2"), + hash: initCommit.GitCommit.Hash, + } + }, + expErr: `^cannot verify commits in branch "refs/heads/other2" when no main branch exists$`, + }, + { + // this case isn't generally possible, unless someone manually + // creates a branch in an empty repo on the remote + descr: "existing branch, no main", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + // checkout other and build on top of that, so that when + // VerifyCanSetBranchHEADTo is called main won't exist. + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + h.stage(map[string]string{"foo": "foo"}) + fooCommit := h.assertCommitChange(verifySkip, "foo", rootSig) + + return toTest{ + branchName: other, + hash: fooCommit.GitCommit.Hash, + resetTo: initCommit.GitCommit.Hash, + } + }, + expErr: `^cannot verify commits in branch "refs/heads/other" when no main branch exists$`, + }, + { + descr: "new branch, not ancestor of main", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + h.assertCommitChange(verifySkip, "init", rootSig) + + // create new branch with no HEAD, and commit on that. + other := plumbing.NewBranchReferenceName("other") + ref := plumbing.NewSymbolicReference(plumbing.HEAD, other) + if err := h.repo.GitRepo.Storer.SetReference(ref); err != nil { + t.Fatal(err) + } + + h.stageCfg() + h.stage(map[string]string{"foo": "foo"}) + badInitCommit := h.assertCommitChange(verifySkip, "a different init", rootSig) + return toTest{ + branchName: plumbing.NewBranchReferenceName("other2"), + hash: badInitCommit.GitCommit.Hash, + } + }, + expErr: `^commit "[0-9a-f]+" must be direct descendant of root commit of "main" \("[0-9a-f]+"\)$`, + }, + { + // this case isn't generally possible, unless someone manually + // creates a branch in an empty repo on the remote + descr: "existing branch, not ancestor of main", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + h.assertCommitChange(verifySkip, "init", rootSig) + + // create new branch with no HEAD, and commit on that. + other := plumbing.NewBranchReferenceName("other") + ref := plumbing.NewSymbolicReference(plumbing.HEAD, other) + if err := h.repo.GitRepo.Storer.SetReference(ref); err != nil { + t.Fatal(err) + } + + h.stageCfg() + h.stage(map[string]string{"foo": "foo"}) + badInitCommit := h.assertCommitChange(verifySkip, "a different init", rootSig) + + h.stage(map[string]string{"bar": "bar"}) + barCommit := h.assertCommitChange(verifySkip, "bar", rootSig) + + return toTest{ + branchName: other, + hash: barCommit.GitCommit.Hash, + resetTo: badInitCommit.GitCommit.Hash, + } + }, + expErr: `^commit "[0-9a-f]+" must be direct descendant of root commit of "main" \("[0-9a-f]+"\)$`, + }, + { + descr: "new branch off of main", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + other := plumbing.NewBranchReferenceName("other") + + h.checkout(other) + h.stage(map[string]string{"foo": "foo"}) + fooCommit := h.assertCommitChange(verifySkip, "foo", rootSig) + + return toTest{ + branchName: other, + hash: fooCommit.GitCommit.Hash, + resetTo: initCommit.GitCommit.Hash, + } + }, + }, + { + descr: "new branch off of older main commit", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + + h.stage(map[string]string{"foo": "foo"}) + h.assertCommitChange(verifySkip, "foo", rootSig) + + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + h.reset(initCommit.GitCommit.Hash, git.HardReset) + h.stage(map[string]string{"bar": "bar"}) + barCommit := h.assertCommitChange(verifySkip, "bar", rootSig) + + return toTest{ + branchName: other, + hash: barCommit.GitCommit.Hash, + resetTo: initCommit.GitCommit.Hash, + } + }, + }, + { + descr: "branch ff", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + h.assertCommitChange(verifySkip, "init", rootSig) + + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + + var commits []GitCommit + for _, str := range []string{"foo", "bar", "baz", "biz", "buz"} { + h.stage(map[string]string{str: str}) + commit := h.assertCommitChange(verifySkip, str, rootSig) + commits = append(commits, commit) + } + + return toTest{ + branchName: other, + hash: commits[len(commits)-1].GitCommit.Hash, + resetTo: commits[0].GitCommit.Hash, + } + }, + }, + { + descr: "main nonff", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + initCommit := h.assertCommitChange(verifySkip, "init", rootSig) + h.stage(map[string]string{"foo": "foo"}) + h.assertCommitChange(verifySkip, "foo", rootSig) + + // start another branch back at init and make a new commit on it + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + h.reset(initCommit.GitCommit.Hash, git.HardReset) + h.stage(map[string]string{"bar": "bar"}) + barCommit := h.assertCommitChange(verifySkip, "bar", rootSig) + + return toTest{ + branchName: MainRefName, + hash: barCommit.GitCommit.Hash, + } + }, + expErr: `^commit matched and denied by this access control:`, + }, + { + descr: "branch nonff", + init: func(h *harness, rootSig sigcred.SignifierInterface) toTest { + h.assertCommitChange(verifySkip, "init", rootSig) + + other := plumbing.NewBranchReferenceName("other") + h.checkout(other) + h.stage(map[string]string{"foo": "foo"}) + fooCommit := h.assertCommitChange(verifySkip, "foo", rootSig) + h.stage(map[string]string{"bar": "bar"}) + h.assertCommitChange(verifySkip, "bar", rootSig) + + other2 := plumbing.NewBranchReferenceName("other2") + h.checkout(other2) + h.reset(fooCommit.GitCommit.Hash, git.HardReset) + h.stage(map[string]string{"baz": "baz"}) + bazCommit := h.assertCommitChange(verifySkip, "baz", rootSig) + + return toTest{ + branchName: other, + hash: bazCommit.GitCommit.Hash, + } + }, + }, + } + + for _, test := range tests { + t.Run(test.descr, func(t *testing.T) { + h := newHarness(t) + rootSig := h.stageNewAccount("root", false) + toTest := test.init(h, rootSig) + + if toTest.resetTo != plumbing.ZeroHash { + ref := plumbing.NewHashReference(toTest.branchName, toTest.resetTo) + if err := h.repo.GitRepo.Storer.SetReference(ref); err != nil { + t.Fatal(err) + } + } + + err := h.repo.VerifyCanSetBranchHEADTo(toTest.branchName, toTest.hash) + if test.expErr == "" { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return + } else if err == nil { + t.Fatal("expected verification to fail") + } + + ogErr := err + for { + if unwrappedErr := errors.Unwrap(err); unwrappedErr != nil { + err = unwrappedErr + } else { + break + } + } + + errRegex := regexp.MustCompile(test.expErr) + if !errRegex.MatchString(err.Error()) { + t.Fatalf("\nexpected error of form %q\nbut got: %v", test.expErr, ogErr) + } + }) + } } diff --git a/repo.go b/repo.go index 7ae6ccb..edda58c 100644 --- a/repo.go +++ b/repo.go @@ -396,6 +396,10 @@ func (r *Repo) GetGitCommitRange(start, end plumbing.Hash) ([]GitCommit, error) var commits []GitCommit var found bool for { + if found = start != plumbing.ZeroHash && curr.GitCommit.Hash == start; found { + break + } + commits = append(commits, curr) numParents := curr.GitCommit.NumParents() if numParents == 0 { @@ -409,9 +413,6 @@ func (r *Repo) GetGitCommitRange(start, end plumbing.Hash) ([]GitCommit, error) parent, err := r.GetGitCommit(parentHash) if err != nil { return nil, fmt.Errorf("retrieving commit %q: %w", parentHash, err) - } else if start != plumbing.ZeroHash && parentHash == start { - found = true - break } curr = parent } diff --git a/repo_test.go b/repo_test.go index c6806d8..5fb6272 100644 --- a/repo_test.go +++ b/repo_test.go @@ -97,7 +97,6 @@ func (h *harness) stageAccessControls(aclYAML string) { } func (h *harness) checkout(branch plumbing.ReferenceName) { - w, err := h.repo.GitRepo.Worktree() if err != nil { h.t.Fatal(err) @@ -105,8 +104,8 @@ func (h *harness) checkout(branch plumbing.ReferenceName) { head, err := h.repo.GetGitHead() if errors.Is(err, ErrHeadIsZero) { - // if HEAD is resolvable to any hash than the Checkout method doesn't - // work, just set HEAD manually. + // if HEAD is not resolvable to any hash than the Checkout method + // doesn't work, just set HEAD manually. ref := plumbing.NewSymbolicReference(plumbing.HEAD, branch) if err := h.repo.GitRepo.Storer.SetReference(ref); err != nil { h.t.Fatal(err) @@ -151,8 +150,16 @@ func (h *harness) reset(to plumbing.Hash, mode git.ResetMode) { } } +type verifyExpectation int + +const ( + verifyShouldSucceed verifyExpectation = 1 + verifyShouldFail verifyExpectation = 0 + verifySkip verifyExpectation = -1 +) + func (h *harness) tryCommit( - shouldSucceed bool, + verifyExp verifyExpectation, commit Commit, accountSig sigcred.SignifierInterface, ) GitCommit { @@ -166,6 +173,8 @@ func (h *harness) tryCommit( gitCommit, err := h.repo.Commit(commit) if err != nil { h.t.Fatalf("failed to commit ChangeCommit: %v", err) + } else if verifyExp == verifySkip { + return gitCommit } branch, err := h.repo.ReferenceToBranchName(plumbing.HEAD) @@ -173,6 +182,8 @@ func (h *harness) tryCommit( h.t.Fatalf("determining checked out branch: %v", err) } + shouldSucceed := verifyExp > 0 + err = h.repo.VerifyCommits(branch, []GitCommit{gitCommit}) if shouldSucceed && err != nil { h.t.Fatalf("verifying commit %q: %v", gitCommit.GitCommit.Hash, err) @@ -192,7 +203,7 @@ func (h *harness) tryCommit( } func (h *harness) assertCommitChange( - shouldSucceed bool, + verifyExp verifyExpectation, msg string, sig sigcred.SignifierInterface, ) GitCommit { @@ -200,7 +211,7 @@ func (h *harness) assertCommitChange( if err != nil { h.t.Fatalf("creating ChangeCommit: %v", err) } - return h.tryCommit(shouldSucceed, commit, sig) + return h.tryCommit(verifyExp, commit, sig) } func TestHasStagedChanges(t *testing.T) { @@ -220,12 +231,12 @@ func TestHasStagedChanges(t *testing.T) { h.stage(map[string]string{"foo": "bar"}) assertHasStaged(true) - h.assertCommitChange(true, "first commit", rootSig) + h.assertCommitChange(verifyShouldSucceed, "first commit", rootSig) assertHasStaged(false) h.stage(map[string]string{"foo": ""}) // delete foo assertHasStaged(true) - h.assertCommitChange(true, "second commit", rootSig) + h.assertCommitChange(verifyShouldSucceed, "second commit", rootSig) assertHasStaged(false) } @@ -263,7 +274,7 @@ func TestShortHashResolving(t *testing.T) { // but that's hard... h := newHarness(t) rootSig := h.stageNewAccount("root", false) - hash := h.assertCommitChange(true, "first commit", rootSig).GitCommit.Hash + hash := h.assertCommitChange(verifyShouldSucceed, "first commit", rootSig).GitCommit.Hash hashStr := hash.String() t.Log(hashStr)