Separate branch controls reading from commit checking (#213)

* Rename GetBranchControls to GetBranchControlsAtCommit

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

* Add GetBranchControls() func

This commit follows up to add a GetBranchControls() function that
retrieves the controls enabled for a branch (regardless of a commit
time).

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

* Use new GetBranchControls() to check status

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

---------

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

Author: puerco

sourcetool/cmd/checklevel.go

@@ -53,7 +53,7 @@ func doCheckLevel(cla *CheckLevelArgs) error {
 	ghconnection.Options.AllowMergeCommits = cla.allowMergeCommits
 
 	ctx := context.Background()
-	controlStatus, err := ghconnection.GetBranchControls(ctx, cla.commit, ghconnection.GetFullRef())
+	controlStatus, err := ghconnection.GetBranchControlsAtCommit(ctx, cla.commit, ghconnection.GetFullRef())
 	if err != nil {
 		return err
 	}

sourcetool/pkg/attest/provenance.go

@@ -148,7 +148,7 @@ func addPredToStatement(provPred any, predicateType, commit string) (*spb.Statem
 
 // Create provenance for the current commit without any context from the previous provenance (if any).
 func (pa ProvenanceAttestor) createCurrentProvenance(ctx context.Context, commit, prevCommit, ref string) (*spb.Statement, error) {
-	controlStatus, err := pa.gh_connection.GetBranchControls(ctx, commit, ref)
+	controlStatus, err := pa.gh_connection.GetBranchControlsAtCommit(ctx, commit, ref)
 	if err != nil {
 		return nil, err
 	}

sourcetool/pkg/audit/audit.go

@@ -76,7 +76,7 @@ func (a *Auditor) AuditCommit(ctx context.Context, commit string) (ar *AuditComm
 		// If there's no provenance, let's check the controls to see how they're looking.
 		// It could be that provenance generation failed, but the controls were still
 		// in place.
-		controlStatus, err = a.ghc.GetBranchControls(ctx, commit, a.ghc.GetFullRef())
+		controlStatus, err = a.ghc.GetBranchControlsAtCommit(ctx, commit, a.ghc.GetFullRef())
 		if err != nil {
 			// Let's still return ar so they can continue if they want.
 			return ar, fmt.Errorf("could not get controls for %s on %s: %w", commit, a.ghc.GetFullRef(), err)

sourcetool/pkg/ghcontrol/checklevel.go

@@ -232,49 +232,41 @@ func (ghc *GitHubConnection) getOldestActiveRule(ctx context.Context, rules []*g
 	return oldestActive, nil
 }
 
-// Determines the controls that are in place for a branch at a specific commit using GitHub's APIs
-// This is necessarily only as good as GitHub's controls and existing APIs.
-func (ghc *GitHubConnection) GetBranchControls(ctx context.Context, commit, ref string) (*GhControlStatus, error) {
-	// We want to know when this commit was pushed to ensure the rules were active _then_.
-	activity, err := ghc.commitActivity(ctx, commit, ref)
-	if err != nil {
-		return nil, err
-	}
-
-	controlStatus := GhControlStatus{
-		CommitPushTime: activity.Timestamp,
-		ActivityType:   activity.ActivityType,
-		ActorLogin:     activity.Actor.Login,
-		Controls:       slsa.Controls{},
-	}
-
+// GetBranchControls returns a list of the controls enabled at present for a branch.
+// This function does not take into account a commit date, it just returns those controls
+// that are active when called.
+func (ghc *GitHubConnection) GetBranchControls(ctx context.Context, ref string) (*slsa.Controls, error) {
 	branch := GetBranchFromRef(ref)
 	if branch == "" {
 		return nil, fmt.Errorf("ref %s is not a branch", ref)
 	}
+
+	controls := &slsa.Controls{}
+
 	// Do the branch specific stuff.
 	branchRules, _, err := ghc.Client().Repositories.GetRulesForBranch(ctx, ghc.Owner(), ghc.Repo(), branch)
 	if err != nil {
 		return nil, err
 	}
+
 	// Compute the controls enforced.
 	continuityControl, err := ghc.computeContinuityControl(ctx, branchRules)
 	if err != nil {
 		return nil, fmt.Errorf("could not populate ContinuityControl: %w", err)
 	}
-	controlStatus.AddControl(continuityControl)
+	controls.AddControl(continuityControl)
 
 	reviewControl, err := ghc.computeReviewControl(ctx, branchRules.PullRequest)
 	if err != nil {
 		return nil, fmt.Errorf("could not populate ReviewControl: %w", err)
 	}
-	controlStatus.AddControl(reviewControl)
+	controls.AddControl(reviewControl)
 
 	requiredCheckControls, err := ghc.computeRequiredChecks(ctx, branchRules.RequiredStatusChecks)
 	if err != nil {
 		return nil, fmt.Errorf("could not populate RequiredChecks: %w", err)
 	}
-	controlStatus.AddControl(requiredCheckControls...)
+	controls.AddControl(requiredCheckControls...)
 
 	// Check the tag rules.
 	allRulesets, _, err := ghc.Client().Repositories.GetAllRulesets(ctx, ghc.Owner(), ghc.Repo(), true)
@@ -285,7 +277,38 @@ func (ghc *GitHubConnection) GetBranchControls(ctx context.Context, commit, ref
 	if err != nil {
 		return nil, fmt.Errorf("could not populate TagHygieneControl: %w", err)
 	}
-	controlStatus.AddControl(TagHygieneControl)
+	controls.AddControl(TagHygieneControl)
+
+	return controls, nil
+}
+
+// GetBranchControlsAtCommit determines the controls that are in place for a branch
+// at a specific commit using GitHub's APIs. This is necessarily only as good as
+// GitHub's controls and existing APIs.
+func (ghc *GitHubConnection) GetBranchControlsAtCommit(ctx context.Context, commit, ref string) (*GhControlStatus, error) {
+	// We want to know when this commit was pushed to ensure the rules were active _then_.
+	activity, err := ghc.commitActivity(ctx, commit, ref)
+	if err != nil {
+		return nil, err
+	}
+
+	controlStatus := GhControlStatus{
+		CommitPushTime: activity.Timestamp,
+		ActivityType:   activity.ActivityType,
+		ActorLogin:     activity.Actor.Login,
+		Controls:       slsa.Controls{},
+	}
+
+	activeControls, err := ghc.GetBranchControls(ctx, ref)
+	if err != nil {
+		return nil, fmt.Errorf("reading active controls: %w", err)
+	}
+
+	// Add the controls to the control status object. This will
+	// discard any that were not active when the commit merged.
+	for _, c := range *activeControls {
+		controlStatus.AddControl(&c)
+	}
 
 	return &controlStatus, nil
 }

sourcetool/pkg/ghcontrol/checklevel_test.go

@@ -258,7 +258,7 @@ func TestBuiltinBranchControls(t *testing.T) {
 					github.RulesetEnforcementActive, priorTime, tt.rulesetRules),
 				activityForBranch("abc123", "refs/heads/branch_name"), &tt.branchRules)
 
-			controlStatus, err := ghc.GetBranchControls(t.Context(), "abc123", "refs/heads/branch_name")
+			controlStatus, err := ghc.GetBranchControlsAtCommit(t.Context(), "abc123", "refs/heads/branch_name")
 			if err != nil {
 				t.Fatalf("Error getting branch controls: %v", err)
 			}
@@ -302,7 +302,7 @@ func TestBuiltinBranchControlsEnabledLater(t *testing.T) {
 					github.RulesetEnforcementActive, laterTime, tt.rulesetRules),
 				activityForBranch("abc123", "refs/heads/branch_name"), &tt.branchRules)
 
-			controlStatus, err := ghc.GetBranchControls(t.Context(), "abc123", "refs/heads/branch_name")
+			controlStatus, err := ghc.GetBranchControlsAtCommit(t.Context(), "abc123", "refs/heads/branch_name")
 			if err != nil {
 				t.Fatalf("Error getting branch controls: %v", err)
 			}
@@ -342,7 +342,7 @@ func TestGetBranchControlsRequiredChecks(t *testing.T) {
 					github.RulesetEnforcementActive, priorTime, rulesForRequiredChecks()),
 				activityForBranch("abc123", "refs/heads/branch_name"), &tt.checks)
 
-			controlStatus, err := ghc.GetBranchControls(t.Context(), "abc123", "refs/heads/branch_name")
+			controlStatus, err := ghc.GetBranchControlsAtCommit(t.Context(), "abc123", "refs/heads/branch_name")
 			if err != nil {
 				t.Fatalf("Error getting branch controls: %v", err)
 			}

sourcetool/pkg/sourcetool/implementation.go

@@ -29,18 +29,12 @@ func (impl *defaultToolImplementation) GetActiveControls(opts *Options) (slsa.Co
 		return nil, err
 	}
 
-	if err := opts.EnsureCommit(); err != nil {
-		return nil, err
-	}
-
 	// Get the active controls
-	activeControls, err := ghc.GetBranchControls(ctx, opts.Commit, ghcontrol.BranchToFullRef(opts.Branch))
+	activeControls, err := ghc.GetBranchControls(ctx, ghcontrol.BranchToFullRef(opts.Branch))
 	if err != nil {
 		return nil, fmt.Errorf("checking status: %w", err)
 	}
 
-	ret := activeControls.Controls
-
 	// We need to manually check for PROVENANCE_AVAILABLE which is not
 	// handled by ghcontrol
 	attestor := attest.NewProvenanceAttestor(
@@ -53,10 +47,10 @@ func (impl *defaultToolImplementation) GetActiveControls(opts *Options) (slsa.Co
 		return nil, fmt.Errorf("attempting to read provenance from commit: %w", err)
 	}
 	if attestation != nil {
-		ret.AddControl(&slsa.Control{
+		activeControls.AddControl(&slsa.Control{
 			Name: slsa.ProvenanceAvailable,
 		})
 	}
 
-	return ret, nil
+	return *activeControls, nil
 }