Skip to content

Commit

Permalink
🐛 Support POST workflow verification for inter-repo reusable workflows (
Browse files Browse the repository at this point in the history
#295)

* Allow scenario where workflow path is in a separate repo.

Signed-off-by: Spencer Schrock <[email protected]>

* Fix workflow verification for resuable workflows that are in different repositories than the repo they analyze.

Signed-off-by: Spencer Schrock <[email protected]>

* Add e2e tests for verifying reusable workflows.

Signed-off-by: Spencer Schrock <[email protected]>

* Remove sentinel error

Signed-off-by: Spencer Schrock <[email protected]>

* Add splitFullPath tests.

Signed-off-by: Spencer Schrock <[email protected]>

* Expose token to test step so e2e tests don't fail rate limit.

Signed-off-by: Spencer Schrock <[email protected]>

Signed-off-by: Spencer Schrock <[email protected]>
  • Loading branch information
spencerschrock authored Dec 27, 2022
1 parent 02020a3 commit bda2e3d
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 23 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,7 @@ jobs:
go env -w GOFLAGS=-mod=mod
make scorecard-webapp
- name: Tests
env:
GITHUB_AUTH_TOKEN: ${{ github.token }} # needed for the e2e tests
run: |
cd app/server && go test
43 changes: 33 additions & 10 deletions app/server/post_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type certInfo struct {
repoBranchRef string
repoSHA string
workflowPath string
workflowRef string
}

type tlogEntry struct {
Expand Down Expand Up @@ -142,13 +143,13 @@ func processRequest(host, org, repo string, scorecardResult *models.VerifiedScor
if err != nil {
return fmt.Errorf("error extracting cert info: %w", err)
}
if info.repoFullName != fmt.Sprintf("%s/%s", org, repo) ||
if info.repoFullName != fullName(org, repo) ||
(info.repoBranchRef != scorecardResult.Branch &&
info.repoBranchRef != fmt.Sprintf("refs/heads/%s", scorecardResult.Branch)) {
return fmt.Errorf("%w", errMismatchedCertAndRequest)
}

if err := getAndVerifyWorkflowContent(ctx, org, repo, scorecardResult, info); err != nil {
if err := getAndVerifyWorkflowContent(ctx, scorecardResult, info); err != nil {
return fmt.Errorf("error verifying workflow: %w", err)
}

Expand All @@ -167,12 +168,31 @@ func processRequest(host, org, repo string, scorecardResult *models.VerifiedScor
return nil
}

func fullName(org, repo string) string {
return fmt.Sprintf("%s/%s", org, repo)
}

// splitFullPath extracts the org, repo, and path from a full path of the form org/repo/rest/of/path.
func splitFullPath(path string) (org, repo, subPath string, ok bool) {
parts := strings.SplitN(path, "/", 3)
if len(parts) < 3 {
return "", "", "", false
}
return parts[0], parts[1], parts[2], true
}

// getAndVerifyWorkflowContent retrieves the workflow content from the repository and verifies it.
// It verifies the branch is a default branch and gets the scorecard workflow from the repository
// from the specific commit and verifies it to ensure that it hasn't been tampered with.
func getAndVerifyWorkflowContent(ctx context.Context,
org, repo string, scorecardResult *models.VerifiedScorecardResult, info certInfo,
scorecardResult *models.VerifiedScorecardResult, info certInfo,
) error {
org, repo, path, ok := splitFullPath(info.workflowPath)
if !ok {
return fmt.Errorf("cert workflow path is malformed")
}
workflowRepoFullName := fullName(org, repo)

// Get the corresponding GitHub repository.
httpClient := http.DefaultClient
if scorecardResult.AccessToken != "" {
Expand All @@ -193,9 +213,14 @@ func getAndVerifyWorkflowContent(ctx context.Context,
return fmt.Errorf("branch of cert isn't the repo's default branch")
}

// Get workflow file from cert commit SHA.
// Use the cert commit SHA if the workflow file is in the repo being analyzed.
// Otherwise fall back to the workflowRef, which may be a commit SHA, or it may be more vague e.g. refs/heads/main
opts := &github.RepositoryContentGetOptions{Ref: info.repoSHA}
contents, _, _, err := client.Repositories.GetContents(ctx, org, repo, info.workflowPath, opts)
if workflowRepoFullName != info.repoFullName {
opts.Ref = info.workflowRef
}

contents, _, _, err := client.Repositories.GetContents(ctx, org, repo, path, opts)
if err != nil {
return fmt.Errorf("error downloading workflow contents from repo: %v", err)
}
Expand Down Expand Up @@ -486,12 +511,10 @@ func extractCertInfo(cert *x509.Certificate) (certInfo, error) {
return ret, errCertWorkflowPathEmpty
}

// Remove repo path from workflow filepath.
ind := strings.Index(workflowRef, ret.repoFullName) + len(ret.repoFullName) + 1
ret.workflowPath = workflowRef[ind:]

// url.URL.Path may have leading slashes
ret.workflowPath = strings.TrimLeft(workflowRef, "/")
// Remove repo ref tag from workflow filepath.
ret.workflowPath = ret.workflowPath[:strings.Index(ret.workflowPath, "@")]
ret.workflowPath, ret.workflowRef, _ = strings.Cut(ret.workflowPath, "@")
return ret, nil
}

Expand Down
57 changes: 49 additions & 8 deletions app/server/post_results_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,77 @@ package server

import (
"context"
"io/ioutil"
"io"
"os"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/ossf/scorecard-webapp/app/generated/models"
)

var _ = Describe("E2E Test: extractAndVerifyCertForPayload", func() {
Context("E2E Test: Validate functionality", func() {
AssertValidPayload := func(filename string) {
It("Should successfully extract cert for payload", func() {
testFile, err := os.Open("testdata/results/valid-payload.json")
testFile, err := os.Open(filename)
Expect(err).Should(BeNil())

payload, err := ioutil.ReadAll(testFile)
payload, err := io.ReadAll(testFile)
Expect(err).Should(BeNil())

_, errCertExtract := extractAndVerifyCertForPayload(context.Background(), payload)
Expect(errCertExtract).Should(BeNil())
})
}
Context("E2E Test: Validate functionality", func() {
AssertValidPayload("testdata/results/valid-payload.json")
})
Context("E2E Test: Validate functionality", func() {
It("Should successfully extract cert for payload", func() {
testFile, err := os.Open("testdata/results/valid-payload-2.json")
AssertValidPayload("testdata/results/valid-payload-2.json")
})
})

// readGitHubTokens is used to authenticate the github client in the getAndVerifyWorkflowContent tests
// The CI/CD will have GITHUB_TOKEN available.
func readGitHubTokens() (string, bool) {
githubAuthTokens := []string{"GITHUB_AUTH_TOKEN", "GITHUB_TOKEN", "GH_TOKEN", "GH_AUTH_TOKEN"}
for _, name := range githubAuthTokens {
if token, exists := os.LookupEnv(name); exists && token != "" {
return token, exists
}
}
return "", false
}

var _ = Describe("E2E Test: getAndVerifyWorkflowContent", func() {
AssertValidWorkflowContent := func(filename string) {
It("Should successfully extract cert and verify workflow for payload", func() {
testFile, err := os.Open(filename)
Expect(err).Should(BeNil())

payload, err := ioutil.ReadAll(testFile)
payload, err := io.ReadAll(testFile)
Expect(err).Should(BeNil())

_, errCertExtract := extractAndVerifyCertForPayload(context.Background(), payload)
ctx := context.Background()
cert, errCertExtract := extractAndVerifyCertForPayload(ctx, payload)
Expect(errCertExtract).Should(BeNil())

info, errCertExtractInfo := extractCertInfo(cert)
Expect(errCertExtractInfo).Should(BeNil())

token, _ := readGitHubTokens()
scorecardResult := &models.VerifiedScorecardResult{
AccessToken: token,
Branch: "main",
Result: string(payload),
}
Expect(getAndVerifyWorkflowContent(ctx, scorecardResult, info)).Should(BeNil())
})
}
Context("E2E Test: Validate functionality intra-repo", func() {
AssertValidWorkflowContent("testdata/results/reusable-workflow-intra-repo-results.json")
})
Context("E2E Test: Validate functionality inter-repo", func() {
AssertValidWorkflowContent("testdata/results/reusable-workflow-inter-repo-results.json")
})
})
50 changes: 48 additions & 2 deletions app/server/post_results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,15 @@ func Test_extractCertInfo(t *testing.T) {
{
Scheme: "https",
Host: "test.com",
Path: "https://test.com/foo/bar/workflow@c8416b0b2bf627c349ca92fc8e3de51a64b005cf",
Path: "/foo/bar/workflow@c8416b0b2bf627c349ca92fc8e3de51a64b005cf",
},
},
},
},
want: certInfo{
repoFullName: "https://test.com/",
workflowPath: "oo/bar/workflow",
workflowPath: "foo/bar/workflow",
workflowRef: "c8416b0b2bf627c349ca92fc8e3de51a64b005cf",
repoBranchRef: "https://test.com/",
repoSHA: "https://test.com/",
},
Expand Down Expand Up @@ -209,3 +210,48 @@ func FuzzExtractCertInfo(f *testing.F) {
extractCertInfo(cert)
})
}

func Test_splitFullPath(t *testing.T) {
t.Parallel()
type results struct {
org, repo, subPath string
ok bool
}
tests := []struct {
name string
path string
want results
}{
{
name: "valid path",
path: "org/repo/rest/of/path@ref",
want: results{
org: "org",
repo: "repo",
subPath: "rest/of/path@ref",
ok: true,
},
},
{
name: "malformed path",
path: "malformed/path",
want: results{
org: "",
repo: "",
subPath: "",
ok: false,
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
o, r, p, ok := splitFullPath(tt.path)
assert.Equal(t, tt.want.ok, ok)
assert.Equal(t, tt.want.org, o)
assert.Equal(t, tt.want.repo, r)
assert.Equal(t, tt.want.subPath, p)
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"date":"2022-12-20","repo":{"name":"github.com/ossf-tests/scorecard-webapp-reusable-workflow-caller-e2e","commit":"d495bbd26ec7761a6dc287097d9e8a51ca48df41"},"scorecard":{"version":"v4.8.0","commit":"c40859202d739b31fd060ac5b30d17326cd74275"},"score":4.1,"checks":[{"details":null,"score":10,"reason":"no binaries found in the repo","name":"Binary-Artifacts","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#binary-artifacts","short":"Determines if the project has generated executable (binary) artifacts in the source repository."}},{"details":["Warn: branch protection not enabled for branch 'main'"],"score":0,"reason":"branch protection not enabled on development/release branches","name":"Branch-Protection","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#branch-protection","short":"Determines if the default and release branches are protected with GitHub's branch protection settings."}},{"details":null,"score":-1,"reason":"no pull request found","name":"CI-Tests","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#ci-tests","short":"Determines if the project runs tests before pull requests are merged."}},{"details":null,"score":0,"reason":"no badge detected","name":"CII-Best-Practices","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#cii-best-practices","short":"Determines if the project has an OpenSSF (formerly CII) Best Practices Badge."}},{"details":null,"score":0,"reason":"0 out of last 1 changesets reviewed before merge -- score normalized to 0","name":"Code-Review","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#code-review","short":"Determines if the project requires code review before pull requests (aka merge requests) are merged."}},{"details":["Info: contributors work for "],"score":0,"reason":"0 different organizations found -- score normalized to 0","name":"Contributors","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#contributors","short":"Determines if the project has a set of contributors from multiple organizations (e.g., companies)."}},{"details":null,"score":10,"reason":"no dangerous workflow patterns detected","name":"Dangerous-Workflow","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#dangerous-workflow","short":"Determines if the project's GitHub Action workflows avoid dangerous patterns."}},{"details":["Warn: Config file not detected in source location for dependabot, renovatebot, Sonatype Lift, or\n\t\t\tPyUp (Python). We recommend setting this configuration in code so it can be easily verified by others."],"score":0,"reason":"no update tool detected","name":"Dependency-Update-Tool","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#dependency-update-tool","short":"Determines if the project uses a dependency update tool."}},{"details":null,"score":0,"reason":"project is not fuzzed","name":"Fuzzing","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#fuzzing","short":"Determines if the project uses fuzzing."}},{"details":null,"score":0,"reason":"license file not detected","name":"License","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#license","short":"Determines if the project has defined a license."}},{"details":["Warn: repo was created in the last 90 days (Created at: 2022-12-20T21:17:35Z), please review its contents carefully"],"score":0,"reason":"repo was created 0 days ago, not enough maintenance history","name":"Maintained","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#maintained","short":"Determines if the project is \"actively maintained\"."}},{"details":["Warn: no GitHub publishing workflow detected"],"score":-1,"reason":"no published package detected","name":"Packaging","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#packaging","short":"Determines if the project is published as a package that others can easily download, install, easily update, and uninstall."}},{"details":["Info: GitHub-owned GitHubActions are pinned","Info: Third-party GitHubActions are pinned","Info: Dockerfile dependencies are pinned","Info: no insecure (not pinned by hash) dependency downloads found in Dockerfiles","Info: no insecure (not pinned by hash) dependency downloads found in shell scripts"],"score":10,"reason":"all dependencies are pinned","name":"Pinned-Dependencies","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#pinned-dependencies","short":"Determines if the project has declared and pinned the dependencies of its build process."}},{"details":["Warn: no pull requests merged into dev branch","Warn: CodeQL tool not detected"],"score":0,"reason":"no SAST tool detected","name":"SAST","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#sast","short":"Determines if the project uses static code analysis."}},{"details":null,"score":0,"reason":"security policy file not detected","name":"Security-Policy","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#security-policy","short":"Determines if the project has published a security policy."}},{"details":["Warn: no GitHub releases found"],"score":-1,"reason":"no releases found","name":"Signed-Releases","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#signed-releases","short":"Determines if the project cryptographically signs release artifacts."}},{"details":["Info: topLevel permissions set to 'read-all': .github/workflows/scorecard.yml:8","Info: jobLevel 'checks' permission set to 'read': .github/workflows/scorecard.yml:20","Info: jobLevel 'contents' permission set to 'read': .github/workflows/scorecard.yml:21","Info: jobLevel 'discussions' permission set to 'read': .github/workflows/scorecard.yml:24","Info: jobLevel 'pages' permission set to 'read': .github/workflows/scorecard.yml:26","Info: jobLevel 'actions' permission set to 'read': .github/workflows/scorecard.yml:19","Info: jobLevel 'issues' permission set to 'read': .github/workflows/scorecard.yml:23","Info: jobLevel 'packages' permission set to 'read': .github/workflows/scorecard.yml:25","Info: jobLevel 'pull-requests' permission set to 'read': .github/workflows/scorecard.yml:27","Info: jobLevel 'repository-projects' permission set to 'read': .github/workflows/scorecard.yml:28","Info: jobLevel 'statuses' permission set to 'read': .github/workflows/scorecard.yml:29","Warn: jobLevel 'security-events' permission set to 'write': .github/workflows/scorecard.yml:15: update your workflow using https://app.stepsecurity.io/secureworkflow/ossf-tests/scorecard-webapp-reusable-workflow-caller-e2e/scorecard.yml/main?enable=permissions","Info: jobLevel 'deployments' permission set to 'read': .github/workflows/scorecard.yml:22"],"score":9,"reason":"non read-only tokens detected in GitHub workflows","name":"Token-Permissions","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#token-permissions","short":"Determines if the project's workflows follow the principle of least privilege."}},{"details":null,"score":10,"reason":"no vulnerabilities detected","name":"Vulnerabilities","documentation":{"url":"https://github.com/ossf/scorecard/blob/c40859202d739b31fd060ac5b30d17326cd74275/docs/checks.md#vulnerabilities","short":"Determines if the project has open, known unfixed vulnerabilities."}}],"metadata":null}
Loading

0 comments on commit bda2e3d

Please sign in to comment.