Skip to content

Commit 5ecb339

Browse files
authored
Add --allow-merge-commit experimental flag (#179)
* Fix panic in eval when no policy is found Signed-off-by: Adolfo Garcia Veytia (puerco) <[email protected]> * Add GitHUb connector options Signed-off-by: Adolfo Garcia Veytia (puerco) <[email protected]> * Add --allow-merge-commit flag Signed-off-by: Adolfo Garcia Veytia (puerco) <[email protected]> * Wire allowMergeCommit value Signed-off-by: Adolfo Garcia Veytia (puerco) <[email protected]> --------- Signed-off-by: Adolfo Garcia Veytia (puerco) <[email protected]>
1 parent 952e9eb commit 5ecb339

File tree

5 files changed

+49
-23
lines changed

5 files changed

+49
-23
lines changed

sourcetool/cmd/checklevel.go

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package cmd
55

66
import (
77
"context"
8+
"errors"
89
"fmt"
910
"log"
1011
"os"
@@ -18,6 +19,14 @@ import (
1819

1920
type CheckLevelArgs struct {
2021
commit, owner, repo, branch, outputVsa, outputUnsignedVsa, useLocalPolicy string
22+
allowMergeCommits bool
23+
}
24+
25+
func (cla *CheckLevelArgs) Validate() error {
26+
if cla.commit == "" || cla.owner == "" || cla.repo == "" || cla.branch == "" {
27+
return errors.New("must set commit, owner, repo, and branch flags")
28+
}
29+
return nil
2130
}
2231

2332
// checklevelCmd represents the checklevel command
@@ -31,20 +40,23 @@ var (
3140
3241
This is meant to be run within the corresponding GitHub Actions workflow.`,
3342
Run: func(cmd *cobra.Command, args []string) {
34-
doCheckLevel(checkLevelArgs.commit, checkLevelArgs.owner, checkLevelArgs.repo, checkLevelArgs.branch, checkLevelArgs.outputVsa, checkLevelArgs.outputUnsignedVsa)
43+
doCheckLevel(&checkLevelArgs)
3544
},
3645
}
3746
)
3847

39-
func doCheckLevel(commit, owner, repo, branch, outputVsa, outputUnsignedVsa string) {
40-
if commit == "" || owner == "" || repo == "" || branch == "" {
41-
log.Fatal("Must set commit, owner, repo, and branch flags.")
48+
func doCheckLevel(cla *CheckLevelArgs) {
49+
if err := cla.Validate(); err != nil {
50+
log.Fatalf("Error: %v", err)
4251
}
4352

44-
gh_connection := gh_control.NewGhConnection(owner, repo, gh_control.BranchToFullRef(branch)).WithAuthToken(githubToken)
45-
ctx := context.Background()
53+
gh_connection := gh_control.NewGhConnection(
54+
cla.owner, cla.repo, gh_control.BranchToFullRef(cla.branch),
55+
).WithAuthToken(githubToken)
56+
gh_connection.Options.AllowMergeCommits = cla.allowMergeCommits
4657

47-
controlStatus, err := gh_connection.GetBranchControls(ctx, commit, gh_connection.GetFullRef())
58+
ctx := context.Background()
59+
controlStatus, err := gh_connection.GetBranchControls(ctx, cla.commit, gh_connection.GetFullRef())
4860
if err != nil {
4961
log.Fatal(err)
5062
}
@@ -56,24 +68,23 @@ func doCheckLevel(commit, owner, repo, branch, outputVsa, outputUnsignedVsa stri
5668
}
5769
fmt.Print(verifiedLevels)
5870

59-
unsignedVsa, err := attest.CreateUnsignedSourceVsa(gh_connection.GetRepoUri(), gh_connection.GetFullRef(), commit, verifiedLevels, policyPath)
71+
unsignedVsa, err := attest.CreateUnsignedSourceVsa(gh_connection.GetRepoUri(), gh_connection.GetFullRef(), cla.commit, verifiedLevels, policyPath)
6072
if err != nil {
6173
log.Fatal(err)
6274
}
63-
if outputUnsignedVsa != "" {
64-
err = os.WriteFile(outputUnsignedVsa, []byte(unsignedVsa), 0644)
65-
if err != nil {
75+
if cla.outputUnsignedVsa != "" {
76+
if err = os.WriteFile(cla.outputUnsignedVsa, []byte(unsignedVsa), 0644); err != nil {
6677
log.Fatal(err)
6778
}
6879
}
6980

70-
if outputVsa != "" {
81+
if cla.outputVsa != "" {
7182
// This will output in the sigstore bundle format.
7283
signedVsa, err := attest.Sign(unsignedVsa)
7384
if err != nil {
7485
log.Fatal(err)
7586
}
76-
err = os.WriteFile(outputVsa, []byte(signedVsa), 0644)
87+
err = os.WriteFile(cla.outputVsa, []byte(signedVsa), 0644)
7788
if err != nil {
7889
log.Fatal(err)
7990
}
@@ -84,13 +95,12 @@ func init() {
8495
rootCmd.AddCommand(checklevelCmd)
8596

8697
// Here you will define your flags and configuration settings.
87-
8898
checklevelCmd.Flags().StringVar(&checkLevelArgs.commit, "commit", "", "The commit to check.")
8999
checklevelCmd.Flags().StringVar(&checkLevelArgs.owner, "owner", "", "The GitHub repository owner - required.")
90100
checklevelCmd.Flags().StringVar(&checkLevelArgs.repo, "repo", "", "The GitHub repository name - required.")
91101
checklevelCmd.Flags().StringVar(&checkLevelArgs.branch, "branch", "", "The branch within the repository - required.")
92102
checklevelCmd.Flags().StringVar(&checkLevelArgs.outputVsa, "output_vsa", "", "The path to write a signed VSA with the determined level.")
93103
checklevelCmd.Flags().StringVar(&checkLevelArgs.outputUnsignedVsa, "output_unsigned_vsa", "", "The path to write an unsigned vsa with the determined level.")
94104
checklevelCmd.Flags().StringVar(&checkLevelArgs.useLocalPolicy, "use_local_policy", "", "UNSAFE: Use the policy at this local path instead of the official one.")
95-
105+
checklevelCmd.Flags().BoolVar(&checkLevelArgs.allowMergeCommits, "allow-merge-commits", false, "[EXPERIMENTAL] Allow merge commits in branch.")
96106
}

sourcetool/cmd/checklevelprov.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type CheckLevelProvArgs struct {
2929
expectedIssuer string
3030
expectedSan string
3131
useLocalPolicy string
32+
allowMergeCommits bool
3233
}
3334

3435
// checklevelprovCmd represents the checklevelprov command
@@ -47,6 +48,7 @@ var (
4748
func doCheckLevelProv(checkLevelProvArgs CheckLevelProvArgs) {
4849
gh_connection :=
4950
gh_control.NewGhConnection(checkLevelProvArgs.owner, checkLevelProvArgs.repo, gh_control.BranchToFullRef(checkLevelProvArgs.branch)).WithAuthToken(githubToken)
51+
gh_connection.Options.AllowMergeCommits = checkLevelProvArgs.allowMergeCommits
5052
ctx := context.Background()
5153

5254
prevCommit := checkLevelProvArgs.prevCommit
@@ -135,5 +137,5 @@ func init() {
135137
checklevelprovCmd.Flags().StringVar(&checkLevelProvArgs.outputUnsignedBundle, "output_unsigned_bundle", "", "The path to write a bundle of unsigned attestations.")
136138
checklevelprovCmd.Flags().StringVar(&checkLevelProvArgs.outputSignedBundle, "output_signed_bundle", "", "The path to write a bundle of signed attestations.")
137139
checklevelprovCmd.Flags().StringVar(&checkLevelProvArgs.useLocalPolicy, "use_local_policy", "", "UNSAFE: Use the policy at this local path instead of the official one.")
138-
140+
checklevelprovCmd.Flags().BoolVar(&checkLevelProvArgs.allowMergeCommits, "allow-merge-commits", false, "[EXPERIMENTAL] Allow merge commits in branch")
139141
}

sourcetool/pkg/gh_control/connection.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
// Manages a connection to a GitHub repository.
1111
type GitHubConnection struct {
1212
client *github.Client
13+
Options Options
1314
owner, repo, ref string
1415
}
1516

@@ -19,10 +20,12 @@ func NewGhConnection(owner, repo, ref string) *GitHubConnection {
1920

2021
func NewGhConnectionWithClient(owner, repo, ref string, client *github.Client) *GitHubConnection {
2122
return &GitHubConnection{
22-
client: client,
23-
owner: owner,
24-
repo: repo,
25-
ref: ref}
23+
client: client,
24+
owner: owner,
25+
repo: repo,
26+
ref: ref,
27+
Options: defaultOptions,
28+
}
2629
}
2730

2831
func (ghc *GitHubConnection) Client() *github.Client {
@@ -68,7 +71,7 @@ func (ghc *GitHubConnection) GetPriorCommit(ctx context.Context, sha string) (st
6871
return "", fmt.Errorf("there is no commit earlier than %s, that isn't yet supported", sha)
6972
}
7073

71-
if len(commit.Parents) > 1 {
74+
if len(commit.Parents) > 1 && !ghc.Options.AllowMergeCommits {
7275
return "", fmt.Errorf("commit %s has more than one parent (%v), which is not supported", sha, commit.Parents)
7376
}
7477

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package gh_control
2+
3+
var defaultOptions = Options{
4+
AllowMergeCommits: false,
5+
}
6+
7+
type Options struct {
8+
// AllowMergeCommits causes the GitHub connector to reject merge
9+
// commits when set to false.
10+
AllowMergeCommits bool
11+
}

sourcetool/pkg/policy/policy.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ func (pe PolicyEvaluator) EvaluateControl(ctx context.Context, gh_connection *gh
459459
// We want to check to ensure the repo hasn't enabled/disabled the rules since
460460
// setting the 'since' field in their policy.
461461
rp, policyPath, err := pe.getPolicy(ctx, gh_connection)
462-
if err != nil {
462+
if err != nil || rp == nil {
463463
return slsa_types.SourceVerifiedLevels{}, "", err
464464
}
465465

@@ -485,7 +485,7 @@ func (pe PolicyEvaluator) EvaluateControl(ctx context.Context, gh_connection *gh
485485
// Evaluates the provenance against the policy and returns the resulting source level and policy path
486486
func (pe PolicyEvaluator) EvaluateSourceProv(ctx context.Context, gh_connection *gh_control.GitHubConnection, prov *spb.Statement) (slsa_types.SourceVerifiedLevels, string, error) {
487487
rp, policyPath, err := pe.getPolicy(ctx, gh_connection)
488-
if err != nil {
488+
if err != nil || rp == nil {
489489
return slsa_types.SourceVerifiedLevels{}, "", err
490490
}
491491

0 commit comments

Comments
 (0)