fix: filesystemStore fails to prepend target file hashes on Windows (#…

…274) * fix: filesystemStore fails to prepend target file hashes on Windows The filesystemStore LocalStore implementation conflates paths and filepaths, which cause issues on systems where they take different formats (Windows). Signed-off-by: Torin Carey <[email protected]> * Fix tests for windows Signed-off-by: Torin Carey <[email protected]> * Use path instead of filepath for delegated role matching Signed-off-by: Torin Carey <[email protected]> * Add windows-latest to workflows Signed-off-by: Torin Carey <[email protected]> * Add gitattributes For the satisfaction of golangci-lint, go files should have LF line endings. Since the integrity of the test data is important, the metadata files should be marked as binary to prevent git from mangling them. Signed-off-by: Torin Carey <[email protected]>
theupdateframework · Jul 26, 2022 · 37601e1 · 37601e1
1 parent 4febe4c
commit 37601e1
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 52 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -0,0 +1,5 @@
+# go enforces lf line endings
+*.go eol=lf
+
+# testdata should not be mangled by git
+*.json binary
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -15,7 +15,7 @@ jobs:
   run:
     strategy:
       matrix:
-        os: [ubuntu-latest, macos-latest]
+        os: [ubuntu-latest, macos-latest, windows-latest]
         go-version: ${{ fromJSON(needs.get-go-versions.outputs.matrix) }}
     runs-on: ${{ matrix.os }}
     needs: get-go-versions
@@ -54,7 +54,7 @@ jobs:
     strategy:
       matrix:
         go-version: ${{ fromJSON(needs.get-go-versions.outputs.matrix) }}
-        os: [ubuntu-latest, macos-latest]
+        os: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.os }}
     needs: get-go-versions
     steps:

diff --git a/client/testdata/go-tuf/generator/generator.go b/client/testdata/go-tuf/generator/generator.go
@@ -3,10 +3,11 @@ package generator
 import (
 	"encoding/json"
 	"fmt"
+	"io"
+	"io/fs"
 	"io/ioutil"
 	"log"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"time"
 
@@ -28,9 +29,41 @@ func assertNoError(err error) {
 	}
 }
 
-func copyRepo(src string, dst string) {
-	cmd := exec.Command("cp", "-r", src, dst)
-	assertNoError(cmd.Run())
+// copyRepo recursively copies all regular files and directories under src
+// to dst.  In the case where any destination file/directory exists
+// (including dst itself), an error is returned.
+func copyRepo(src string, dst string) error {
+	copyToDest := func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		rel, err := filepath.Rel(src, path)
+		if err != nil {
+			return err
+		}
+		target := filepath.Join(dst, rel)
+		mode := info.Mode()
+		if mode.IsDir() {
+			return os.Mkdir(target, mode.Perm())
+		} else if mode.IsRegular() {
+			sfile, err := os.Open(path)
+			if err != nil {
+				return err
+			}
+			defer sfile.Close()
+			dfile, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
+			if err != nil {
+				return err
+			}
+			defer dfile.Close()
+			if _, err := io.Copy(dfile, sfile); err != nil {
+				return err
+			}
+			return nil
+		}
+		return fmt.Errorf("unknown mode %v", mode)
+	}
+	return filepath.Walk(src, copyToDest)
 }
 
 func newRepo(dir string) *tuf.Repo {
@@ -103,7 +136,7 @@ func generateRepos(dir string, roleKeys map[string][][]*data.PrivateKey, consist
 		// Setup the repo.
 		stepName := fmt.Sprintf("%d", i)
 		d := filepath.Join(dir, stepName)
-		copyRepo(oldDir, d)
+		assertNoError(copyRepo(oldDir, d))
 		repo := newRepo(d)
 		addKeys(repo, keys)
 
@@ -125,7 +158,7 @@ func generateRepos(dir string, roleKeys map[string][][]*data.PrivateKey, consist
 	// Add another target file to make sure the workflow worked.
 	stepName := fmt.Sprintf("%d", i)
 	d := filepath.Join(dir, stepName)
-	copyRepo(oldDir, d)
+	assertNoError(copyRepo(oldDir, d))
 	repo := newRepo(d)
 	addKeys(repo, keys)
 	addTargets(repo, d, map[string][]byte{stepName: []byte(stepName)})

diff --git a/data/types.go b/data/types.go
@@ -7,7 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"path/filepath"
+	"path"
 	"strings"
 	"sync"
 	"time"
@@ -241,7 +241,7 @@ func (d *DelegatedRole) MatchesPath(file string) (bool, error) {
 	}
 
 	for _, pattern := range d.Paths {
-		if matched, _ := filepath.Match(pattern, file); matched {
+		if matched, _ := path.Match(pattern, file); matched {
 			return true, nil
 		}
 	}

diff --git a/local_store.go b/local_store.go
@@ -311,44 +311,44 @@ func (f *fileSystemStore) createDirs() error {
 
 func (f *fileSystemStore) WalkStagedTargets(paths []string, targetsFn TargetsWalkFunc) error {
 	if len(paths) == 0 {
-		walkFunc := func(path string, info os.FileInfo, err error) error {
+		walkFunc := func(fpath string, info os.FileInfo, err error) error {
 			if err != nil {
 				return err
 			}
 			if info.IsDir() || !info.Mode().IsRegular() {
 				return nil
 			}
-			rel, err := filepath.Rel(filepath.Join(f.stagedDir(), "targets"), path)
+			rel, err := filepath.Rel(filepath.Join(f.stagedDir(), "targets"), fpath)
 			if err != nil {
 				return err
 			}
-			file, err := os.Open(path)
+			file, err := os.Open(fpath)
 			if err != nil {
 				return err
 			}
 			defer file.Close()
-			return targetsFn(rel, file)
+			return targetsFn(filepath.ToSlash(rel), file)
 		}
 		return filepath.Walk(filepath.Join(f.stagedDir(), "targets"), walkFunc)
 	}
 
 	// check all the files exist before processing any files
 	for _, path := range paths {
-		realPath := filepath.Join(f.stagedDir(), "targets", path)
-		if _, err := os.Stat(realPath); err != nil {
+		realFilepath := filepath.Join(f.stagedDir(), "targets", path)
+		if _, err := os.Stat(realFilepath); err != nil {
 			if os.IsNotExist(err) {
-				return ErrFileNotFound{realPath}
+				return ErrFileNotFound{realFilepath}
 			}
 			return err
 		}
 	}
 
 	for _, path := range paths {
-		realPath := filepath.Join(f.stagedDir(), "targets", path)
-		file, err := os.Open(realPath)
+		realFilepath := filepath.Join(f.stagedDir(), "targets", path)
+		file, err := os.Open(realFilepath)
 		if err != nil {
 			if os.IsNotExist(err) {
-				return ErrFileNotFound{realPath}
+				return ErrFileNotFound{realFilepath}
 			}
 			return err
 		}
@@ -373,23 +373,24 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
 	isTarget := func(path string) bool {
 		return strings.HasPrefix(path, "targets/")
 	}
-	copyToRepo := func(path string, info os.FileInfo, err error) error {
+	copyToRepo := func(fpath string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
 		if info.IsDir() || !info.Mode().IsRegular() {
 			return nil
 		}
-		rel, err := filepath.Rel(f.stagedDir(), path)
+		rel, err := filepath.Rel(f.stagedDir(), fpath)
 		if err != nil {
 			return err
 		}
+		relpath := filepath.ToSlash(rel)
 
 		var paths []string
-		if isTarget(rel) {
-			paths = computeTargetPaths(consistentSnapshot, rel, hashes)
+		if isTarget(relpath) {
+			paths = computeTargetPaths(consistentSnapshot, relpath, hashes)
 		} else {
-			paths = computeMetadataPaths(consistentSnapshot, rel, versions)
+			paths = computeMetadataPaths(consistentSnapshot, relpath, versions)
 		}
 		var files []io.Writer
 		for _, path := range paths {
@@ -400,7 +401,7 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
 			defer file.Close()
 			files = append(files, file)
 		}
-		staged, err := os.Open(path)
+		staged, err := os.Open(fpath)
 		if err != nil {
 			return err
 		}
@@ -411,43 +412,44 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
 		return nil
 	}
 	// Checks if target file should be deleted
-	needsRemoval := func(path string) bool {
+	needsRemoval := func(fpath string) bool {
 		if consistentSnapshot {
 			// strip out the hash
-			name := strings.SplitN(filepath.Base(path), ".", 2)
+			name := strings.SplitN(filepath.Base(fpath), ".", 2)
 			if len(name) != 2 || name[1] == "" {
 				return false
 			}
-			path = filepath.Join(filepath.Dir(path), name[1])
+			fpath = filepath.Join(filepath.Dir(fpath), name[1])
 		}
-		_, ok := hashes[path]
+		_, ok := hashes[filepath.ToSlash(fpath)]
 		return !ok
 	}
 	// Checks if folder is empty
-	folderNeedsRemoval := func(path string) bool {
-		f, err := os.Open(path)
+	folderNeedsRemoval := func(fpath string) bool {
+		f, err := os.Open(fpath)
 		if err != nil {
 			return false
 		}
 		defer f.Close()
 		_, err = f.Readdirnames(1)
 		return err == io.EOF
 	}
-	removeFile := func(path string, info os.FileInfo, err error) error {
+	removeFile := func(fpath string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
-		rel, err := filepath.Rel(f.repoDir(), path)
+		rel, err := filepath.Rel(f.repoDir(), fpath)
 		if err != nil {
 			return err
 		}
-		if !info.IsDir() && isTarget(rel) && needsRemoval(rel) {
+		relpath := filepath.ToSlash(rel)
+		if !info.IsDir() && isTarget(relpath) && needsRemoval(rel) {
 			// Delete the target file
-			if err := os.Remove(path); err != nil {
+			if err := os.Remove(fpath); err != nil {
 				return err
 			}
 			// Delete the target folder too if it's empty
-			targetFolder := filepath.Dir(path)
+			targetFolder := filepath.Dir(fpath)
 			if folderNeedsRemoval(targetFolder) {
 				if err := os.Remove(targetFolder); err != nil {
 					return err

diff --git a/repo_test.go b/repo_test.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -1070,20 +1071,20 @@ func (rs *RepoSuite) TestConsistentSnapshot(c *C) {
 	c.Assert(err, IsNil)
 
 	// root.json, targets.json and snapshot.json should exist at both versioned and unversioned paths
-	for _, path := range []string{"root.json", "targets.json", "snapshot.json"} {
-		repoPath := filepath.Join("repository", path)
-		if path != "root.json" {
-			c.Assert(len(hashes[path]) > 0, Equals, true)
+	for _, meta := range []string{"root.json", "targets.json", "snapshot.json"} {
+		repoPath := path.Join("repository", meta)
+		if meta != "root.json" {
+			c.Assert(len(hashes[meta]) > 0, Equals, true)
 		}
-		tmp.assertHashedFilesNotExist(repoPath, hashes[path])
-		tmp.assertVersionedFileExist(repoPath, versions[path])
+		tmp.assertHashedFilesNotExist(repoPath, hashes[meta])
+		tmp.assertVersionedFileExist(repoPath, versions[meta])
 		tmp.assertExists(repoPath)
 	}
 
 	// target files should exist at hashed but not unhashed paths
-	for _, path := range []string{"targets/foo.txt", "targets/dir/bar.txt"} {
-		repoPath := filepath.Join("repository", path)
-		tmp.assertHashedFilesExist(repoPath, hashes[path])
+	for _, target := range []string{"targets/foo.txt", "targets/dir/bar.txt"} {
+		repoPath := path.Join("repository", target)
+		tmp.assertHashedFilesExist(repoPath, hashes[target])
 		tmp.assertNotExist(repoPath)
 	}
 
@@ -1110,13 +1111,13 @@ func (rs *RepoSuite) TestConsistentSnapshot(c *C) {
 	c.Assert(err, IsNil)
 
 	// root.json, targets.json and snapshot.json should exist at both versioned and unversioned paths
-	for _, path := range []string{"root.json", "targets.json", "snapshot.json"} {
-		repoPath := filepath.Join("repository", path)
-		if path != "root.json" {
-			c.Assert(len(hashes[path]) > 0, Equals, true)
+	for _, meta := range []string{"root.json", "targets.json", "snapshot.json"} {
+		repoPath := path.Join("repository", meta)
+		if meta != "root.json" {
+			c.Assert(len(hashes[meta]) > 0, Equals, true)
 		}
-		tmp.assertHashedFilesNotExist(repoPath, hashes[path])
-		tmp.assertVersionedFileExist(repoPath, versions[path])
+		tmp.assertHashedFilesNotExist(repoPath, hashes[meta])
+		tmp.assertVersionedFileExist(repoPath, versions[meta])
 		tmp.assertExists(repoPath)
 	}