feat: Share util functions (slsa-framework#598)

* Share util functions * update * update * update * comments * update
ramonpetgrave64 · Jul 22, 2022 · 7083535 · 7083535
1 parent 55827f2
commit 7083535
Show file tree

Hide file tree

Showing 17 changed files with 405 additions and 423 deletions.
diff --git a/.github/actions/secure-download-artifact/action.yml b/.github/actions/secure-download-artifact/action.yml
@@ -17,6 +17,19 @@ inputs:
 runs:
   using: "composite"
   steps:
+    - name: Validate the artifact path
+      shell: bash
+      env:
+        UNTRUSTED_PATH: "${{ inputs.path }}"
+      run: |
+      
+        set -euo pipefail
+
+        if [ -e "$UNTRUSTED_PATH" ]; then
+            echo "Path $UNTRUSTED_PATH already exists"
+            exit 5
+        fi
+
     - name: Download the artifact
       uses: actions/download-artifact@fb598a63ae348fa914e94cd0ff38f362e927b741 # tag=v3.0.0
       with:

diff --git a/go.sum b/go.sum
diff --git a/internal/builders/generic/attest.go b/internal/builders/generic/attest.go
@@ -20,9 +20,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
-	"io"
 	"os"
-	"path/filepath"
 	"regexp"
 	"strings"
 
@@ -64,16 +62,6 @@ type errNoName struct {
 	errors.WrappableError
 }
 
-// errInvalidPath indicates an invalid path.
-type errInvalidPath struct {
-	errors.WrappableError
-}
-
-// errInternal indicates an internal error.
-type errInternal struct {
-	errors.WrappableError
-}
-
 // errDuplicateSubject indicates a duplicate subject name.
 type errDuplicateSubject struct {
 	errors.WrappableError
@@ -135,46 +123,6 @@ func parseSubjects(b64str string) ([]intoto.Subject, error) {
 	return parsed, nil
 }
 
-func pathIsUnderCurrentDirectory(path string) error {
-	wd, err := os.Getwd()
-	if err != nil {
-		return errors.Errorf(&errInternal{}, "os.Getwd(): %w", err)
-	}
-	p, err := filepath.Abs(path)
-	if err != nil {
-		return errors.Errorf(&errInternal{}, "filepath.Abs(): %w", err)
-	}
-
-	if !strings.HasPrefix(p, wd+"/") &&
-		wd != p {
-		return errors.Errorf(&errInvalidPath{}, "invalid path: %q", path)
-	}
-
-	return nil
-}
-
-func getFile(path string) (io.Writer, error) {
-	if path == "-" {
-		return os.Stdout, nil
-	}
-
-	if err := pathIsUnderCurrentDirectory(path); err != nil {
-		return nil, err
-	}
-
-	return os.OpenFile(filepath.Clean(path), os.O_WRONLY|os.O_CREATE, 0o600)
-}
-
-func verifyAttestationPath(path string) error {
-	if !strings.HasSuffix(path, "intoto.jsonl") {
-		return errors.Errorf(&errInvalidPath{}, "invalid suffix: %q. Must be .intoto.jsonl", path)
-	}
-	if err := pathIsUnderCurrentDirectory(path); err != nil {
-		return err
-	}
-	return nil
-}
-
 type provenanceOnlyBuild struct {
 	*slsa.GithubActionsBuild
 }
@@ -202,7 +150,7 @@ run in the context of a Github Actions workflow.`,
 			check(err)
 
 			// Verify the extension path and extension.
-			err = verifyAttestationPath(attPath)
+			err = utils.VerifyAttestationPath(attPath)
 			check(err)
 
 			var parsedSubjects []intoto.Subject
@@ -236,7 +184,7 @@ run in the context of a Github Actions workflow.`,
 			p, err := g.Generate(ctx)
 			check(err)
 
-			// Note: we verify the path within getFile().
+			// Note: the path is validated within CreateNewFileUnderCurrentDirectory().
 			if attPath != "" {
 				var attBytes []byte
 				if utils.IsPresubmitTests() {
@@ -257,7 +205,7 @@ run in the context of a Github Actions workflow.`,
 					attBytes = att.Bytes()
 				}
 
-				f, err := getFile(attPath)
+				f, err := utils.CreateNewFileUnderCurrentDirectory(attPath, os.O_WRONLY)
 				check(err)
 
 				_, err = f.Write(attBytes)
@@ -268,7 +216,7 @@ run in the context of a Github Actions workflow.`,
 				pb, err := json.Marshal(p.Predicate)
 				check(err)
 
-				pf, err := getFile(predicatePath)
+				pf, err := utils.CreateNewFileUnderCurrentDirectory(predicatePath, os.O_WRONLY)
 				check(err)
 
 				_, err = pf.Write(pb)

diff --git a/internal/builders/generic/attest_test.go b/internal/builders/generic/attest_test.go
@@ -11,120 +11,6 @@ import (
 	"github.com/slsa-framework/slsa-github-generator/internal/errors"
 )
 
-func Test_pathIsUnderCurrentDirectory(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name     string
-		path     string
-		expected error
-	}{
-		{
-			name:     "valid same path",
-			path:     "./",
-			expected: nil,
-		},
-		{
-			name:     "valid path no slash",
-			path:     "./some/valid/path",
-			expected: nil,
-		},
-		{
-			name:     "valid path with slash",
-			path:     "./some/valid/path/",
-			expected: nil,
-		},
-		{
-			name:     "valid path with no dot",
-			path:     "some/valid/path/",
-			expected: nil,
-		},
-		{
-			name:     "some valid path",
-			path:     "../generic/some/valid/path",
-			expected: nil,
-		},
-		{
-			name:     "parent invalid path",
-			path:     "../invalid/path",
-			expected: &errInvalidPath{},
-		},
-		{
-			name:     "some invalid fullpath",
-			path:     "/some/invalid/fullpath",
-			expected: &errInvalidPath{},
-		},
-	}
-	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()
-
-			err := pathIsUnderCurrentDirectory(tt.path)
-			if (err == nil && tt.expected != nil) ||
-				(err != nil && tt.expected == nil) {
-				t.Fatalf("unexpected error: %v", cmp.Diff(err, tt.expected, cmpopts.EquateErrors()))
-			}
-
-			if err != nil && !errors.As(err, &tt.expected) {
-				t.Fatalf("unexpected error: %v", cmp.Diff(err, tt.expected, cmpopts.EquateErrors()))
-			}
-		})
-	}
-}
-
-func Test_verifyAttestationPath(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name     string
-		path     string
-		expected error
-	}{
-		{
-			name:     "valid file",
-			path:     "./path/to/valid.intoto.jsonl",
-			expected: nil,
-		},
-		{
-			name:     "invalid path",
-			path:     "../some/invalid/valid.intoto.jsonl",
-			expected: &errInvalidPath{},
-		},
-		{
-			name:     "invalid extension",
-			path:     "some/file.ntoto.jsonl",
-			expected: &errInvalidPath{},
-		},
-		{
-			name:     "invalid not exntension",
-			path:     "some/file.intoto.jsonl.",
-			expected: &errInvalidPath{},
-		},
-		{
-			name:     "invalid folder exntension",
-			path:     "file.intoto.jsonl/file",
-			expected: &errInvalidPath{},
-		},
-	}
-	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()
-
-			err := verifyAttestationPath(tt.path)
-			if (err == nil && tt.expected != nil) ||
-				(err != nil && tt.expected == nil) {
-				t.Fatalf("unexpected error: %v", cmp.Diff(err, tt.expected, cmpopts.EquateErrors()))
-			}
-
-			if err != nil && !errors.As(err, &tt.expected) {
-				t.Fatalf("unexpected error: %v", cmp.Diff(err, tt.expected, cmpopts.EquateErrors()))
-			}
-		})
-	}
-}
-
 // TestParseSubjects tests the parseSubjects function.
 func TestParseSubjects(t *testing.T) {
 	testCases := []struct {

diff --git a/internal/builders/go/main.go b/internal/builders/go/main.go
@@ -20,7 +20,6 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -31,6 +30,7 @@ import (
 	_ "github.com/sigstore/cosign/pkg/providers/github"
 
 	"github.com/slsa-framework/slsa-github-generator/internal/builders/go/pkg"
+	"github.com/slsa-framework/slsa-github-generator/internal/utils"
 )
 
 func usage(p string) {
@@ -84,7 +84,11 @@ func runProvenanceGeneration(subject, digest, commands, envs, workingDir, rekor
 	}
 
 	filename := fmt.Sprintf("%s.intoto.jsonl", subject)
-	err = ioutil.WriteFile(filename, attBytes, 0o600)
+	f, err := utils.CreateNewFileUnderCurrentDirectory(filename, os.O_WRONLY)
+	if err != nil {
+		return err
+	}
+	_, err = f.Write(attBytes)
 	if err != nil {
 		return err
 	}

diff --git a/internal/builders/go/main_test.go b/internal/builders/go/main_test.go
@@ -11,17 +11,18 @@ import (
 	"regexp"
 	"testing"
 
-	"github.com/slsa-framework/slsa-github-generator/internal/builders/go/pkg"
-
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+
+	"github.com/slsa-framework/slsa-github-generator/internal/builders/go/pkg"
+	"github.com/slsa-framework/slsa-github-generator/internal/utils"
 )
 
 func errCmp(e1, e2 error) bool {
 	return errors.Is(e1, e2) || errors.Is(e2, e1)
 }
 
-func Test_runVerify(t *testing.T) {
+func Test_runBuild(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
 		subject    string
@@ -237,11 +238,6 @@ func Test_runVerify(t *testing.T) {
 			config: "./pkg/testdata/releaser-invalid-envs.yml",
 			err:    pkg.ErrorInvalidEnvironmentVariable,
 		},
-		{
-			name:   "invalid main",
-			config: "./pkg/testdata/releaser-invalid-main.yml",
-			err:    pkg.ErrorInvalidDirectory,
-		},
 		{
 			name:   "invalid path",
 			config: "../pkg/testdata/releaser-invalid-main.yml",
@@ -391,12 +387,12 @@ func extract(lines string) ([]string, []string, string, string, error) {
 		return []string{}, []string{}, "", "", err
 	}
 
-	cmd, err := pkg.UnmarshallList(scmd)
+	cmd, err := utils.UnmarshalList(scmd)
 	if err != nil {
 		return []string{}, []string{}, "", "", err
 	}
 
-	env, err := pkg.UnmarshallList(senv)
+	env, err := utils.UnmarshalList(senv)
 	if err != nil {
 		return []string{}, []string{}, "", "", err
 	}

diff --git a/internal/builders/go/pkg/build.go b/internal/builders/go/pkg/build.go
@@ -22,6 +22,8 @@ import (
 	"regexp"
 	"strings"
 	"syscall"
+
+	"github.com/slsa-framework/slsa-github-generator/internal/utils"
 )
 
 var (
@@ -117,7 +119,7 @@ func (b *GoBuild) Run(dry bool) error {
 
 		// Share the resolved name of the binary.
 		fmt.Printf("::set-output name=go-binary-name::%s\n", filename)
-		command, err := marshallToString(com)
+		command, err := utils.MarshalToString(com)
 		if err != nil {
 			return err
 		}
@@ -129,7 +131,7 @@ func (b *GoBuild) Run(dry bool) error {
 			return err
 		}
 
-		menv, err := marshallToString(env)
+		menv, err := utils.MarshalToString(env)
 		if err != nil {
 			return err
 		}