From 8518e1a537b869b1ca19f8952aff9b6da4bd2354 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 21:45:03 +0000 Subject: [PATCH 1/8] Allow byob builders ref at main for e2e tests Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 29 ++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 8e92439ea..292648b69 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -294,17 +294,42 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { if err != nil { return err } + + sourceURI, err := prov.SourceURI() + if err != nil { + return err + } + + uri, _, err := utils.ParseGitURIAndRef(sourceURI) + if err != nil { + return err + } + parts := strings.Split(id, "@") if len(parts) != 2 { return fmt.Errorf("%w: %s", serrors.ErrorInvalidBuilderID, id) } + ref := parts[1] // Exception for JReleaser builders. // See https://github.com/slsa-framework/slsa-github-generator/issues/2035#issuecomment-1579963802. if strings.HasPrefix(parts[0], JReleaserRepository) { - return utils.IsValidJreleaserBuilderTag(parts[1]) + return utils.IsValidJreleaserBuilderTag(ref) } - return utils.IsValidBuilderTag(parts[1], false) + + // To enable e2e tests for BYBO-built builders + // referenced at main. + e2eRepo := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) + uri = utils.NormalizeGitURI(uri) + if (uri == e2eRepo) && + options.TestingEnabled() { + // Allow verification on the main branch to support e2e tests. + if ref == "refs/heads/main" { + return nil + } + } + + return utils.IsValidBuilderTag(ref, false) } // builderID returns the trusted builder ID from the provenance. From 3f4a897056c29436d1bcaea373ded47b54ac0f28 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 21:49:02 +0000 Subject: [PATCH 2/8] typo Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 292648b69..e90b3d121 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -317,12 +317,10 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { return utils.IsValidJreleaserBuilderTag(ref) } - // To enable e2e tests for BYBO-built builders - // referenced at main. + // Exeption to enable e2e tests for BYOB builders referenced at main. e2eRepo := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) uri = utils.NormalizeGitURI(uri) - if (uri == e2eRepo) && - options.TestingEnabled() { + if uri == e2eRepo && options.TestingEnabled() { // Allow verification on the main branch to support e2e tests. if ref == "refs/heads/main" { return nil From be6af71489a0e23f433283c67bf268cebe467ae3 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 21:50:31 +0000 Subject: [PATCH 3/8] comments Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index e90b3d121..821bc50f7 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -318,9 +318,9 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { } // Exeption to enable e2e tests for BYOB builders referenced at main. - e2eRepo := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) - uri = utils.NormalizeGitURI(uri) - if uri == e2eRepo && options.TestingEnabled() { + nrmalizedE2eRepo := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) + normalizedUri := utils.NormalizeGitURI(uri) + if normalizedUri == nrmalizedE2eRepo && options.TestingEnabled() { // Allow verification on the main branch to support e2e tests. if ref == "refs/heads/main" { return nil From 9e6d871a6d26ad1367e3619e3f5066b0c763bb48 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 21:50:59 +0000 Subject: [PATCH 4/8] typo Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 821bc50f7..412f550fd 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -318,9 +318,9 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { } // Exeption to enable e2e tests for BYOB builders referenced at main. - nrmalizedE2eRepo := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) + normalizedE2eRepoUri := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) normalizedUri := utils.NormalizeGitURI(uri) - if normalizedUri == nrmalizedE2eRepo && options.TestingEnabled() { + if normalizedUri == normalizedE2eRepoUri && options.TestingEnabled() { // Allow verification on the main branch to support e2e tests. if ref == "refs/heads/main" { return nil From cdd74b1f0834e1c3d324dff04e5540b2e7add486 Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 21:52:59 +0000 Subject: [PATCH 5/8] rename ref Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 412f550fd..2d4be3397 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -309,12 +309,12 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { if len(parts) != 2 { return fmt.Errorf("%w: %s", serrors.ErrorInvalidBuilderID, id) } - ref := parts[1] + builderRef := parts[1] // Exception for JReleaser builders. // See https://github.com/slsa-framework/slsa-github-generator/issues/2035#issuecomment-1579963802. if strings.HasPrefix(parts[0], JReleaserRepository) { - return utils.IsValidJreleaserBuilderTag(ref) + return utils.IsValidJreleaserBuilderTag(builderRef) } // Exeption to enable e2e tests for BYOB builders referenced at main. @@ -322,12 +322,12 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { normalizedUri := utils.NormalizeGitURI(uri) if normalizedUri == normalizedE2eRepoUri && options.TestingEnabled() { // Allow verification on the main branch to support e2e tests. - if ref == "refs/heads/main" { + if builderRef == "refs/heads/main" { return nil } } - return utils.IsValidBuilderTag(ref, false) + return utils.IsValidBuilderTag(builderRef, false) } // builderID returns the trusted builder ID from the provenance. From 4934eba8c29d70f24ae7172c9b16f8b730b78b7b Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 21:55:29 +0000 Subject: [PATCH 6/8] typo Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index 2d4be3397..d2bffe23b 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -317,7 +317,7 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { return utils.IsValidJreleaserBuilderTag(builderRef) } - // Exeption to enable e2e tests for BYOB builders referenced at main. + // Exception to enable e2e tests for BYOB builders referenced at main. normalizedE2eRepoUri := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) normalizedUri := utils.NormalizeGitURI(uri) if normalizedUri == normalizedE2eRepoUri && options.TestingEnabled() { From f45620da07e0c81df03f20cc9265d4dbec98338c Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Tue, 15 Aug 2023 22:28:03 +0000 Subject: [PATCH 7/8] typo Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index d2bffe23b..b2d977996 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -318,11 +318,12 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { } // Exception to enable e2e tests for BYOB builders referenced at main. - normalizedE2eRepoUri := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) - normalizedUri := utils.NormalizeGitURI(uri) - if normalizedUri == normalizedE2eRepoUri && options.TestingEnabled() { + normalizedE2eRepoURI := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) + normalizedURI := utils.NormalizeGitURI(uri) + if normalizedURI == normalizedE2eRepoURI && options.TestingEnabled() { // Allow verification on the main branch to support e2e tests. if builderRef == "refs/heads/main" { + fmt.Println("here") return nil } } From fa761db210f9da5352e394a6934f2080eecd5e5a Mon Sep 17 00:00:00 2001 From: laurentsimon Date: Wed, 16 Aug 2023 00:41:01 +0000 Subject: [PATCH 8/8] unit tests Signed-off-by: laurentsimon --- verifiers/internal/gha/provenance.go | 20 +++++------ verifiers/internal/gha/provenance_test.go | 43 +++++++++++++++++++---- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index b2d977996..f9e592e5c 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -295,16 +295,6 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { return err } - sourceURI, err := prov.SourceURI() - if err != nil { - return err - } - - uri, _, err := utils.ParseGitURIAndRef(sourceURI) - if err != nil { - return err - } - parts := strings.Split(id, "@") if len(parts) != 2 { return fmt.Errorf("%w: %s", serrors.ErrorInvalidBuilderID, id) @@ -317,13 +307,21 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error { return utils.IsValidJreleaserBuilderTag(builderRef) } + sourceURI, err := prov.SourceURI() + if err != nil { + return err + } + + uri, _, err := utils.ParseGitURIAndRef(sourceURI) + if err != nil { + return err + } // Exception to enable e2e tests for BYOB builders referenced at main. normalizedE2eRepoURI := utils.NormalizeGitURI(httpsGithubCom + e2eTestRepository) normalizedURI := utils.NormalizeGitURI(uri) if normalizedURI == normalizedE2eRepoURI && options.TestingEnabled() { // Allow verification on the main branch to support e2e tests. if builderRef == "refs/heads/main" { - fmt.Println("here") return nil } } diff --git a/verifiers/internal/gha/provenance_test.go b/verifiers/internal/gha/provenance_test.go index 7fbb31a18..eb0c4e7c2 100644 --- a/verifiers/internal/gha/provenance_test.go +++ b/verifiers/internal/gha/provenance_test.go @@ -403,45 +403,76 @@ func Test_verifySourceURI(t *testing.T) { } func Test_isValidDelegatorBuilderID(t *testing.T) { - t.Parallel() tests := []struct { - name string - builderID string - err error + name string + builderID string + sourceURI string + testingEnabled bool + err error }{ { name: "no @", builderID: "some/builderID", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, err: serrors.ErrorInvalidBuilderID, }, { name: "invalid ref", builderID: "some/builderID@v1.2.3", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, err: serrors.ErrorInvalidRef, }, { name: "invalid ref not tag", builderID: "some/builderID@refs/head/v1.2.3", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, err: serrors.ErrorInvalidRef, }, { name: "invalid ref not full semver", builderID: "some/builderID@refs/heads/v1.2", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, err: serrors.ErrorInvalidRef, }, { name: "valid builder", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, builderID: "some/builderID@refs/tags/v1.2.3", }, + { + name: "invalid builder ref not e2e repo with testing enabled", + sourceURI: "git+" + httpsGithubCom + "some/repo", + builderID: "some/builderID@refs/heads/main", + testingEnabled: true, + err: serrors.ErrorInvalidRef, + }, + { + name: "invalid builder ref e2e repo with testing enabled", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, + builderID: "some/builderID@refs/heads/main", + testingEnabled: true, + }, + { + name: "invalid builder ref e2e repo", + sourceURI: "git+" + httpsGithubCom + e2eTestRepository, + builderID: "some/builderID@refs/heads/main", + err: serrors.ErrorInvalidRef, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { - t.Parallel() - prov := &testProvenance{ builderID: tt.builderID, + sourceURI: tt.sourceURI, + } + + if tt.testingEnabled { + t.Setenv("SLSA_VERIFIER_TESTING", "1") + } else { + // Ensure that the variable is not set. + t.Setenv("SLSA_VERIFIER_TESTING", "") } err := isValidDelegatorBuilderID(prov)