Merge pull request #445 from jfly/relative-and-absolute-path-handling

fix: correct handling of absolute and relative paths

Author: brianmcgee

cmd/format/format.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"runtime/pprof"
+	"strings"
 	"syscall"
 	"time"
 
@@ -173,11 +174,27 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string)
 			return fmt.Errorf("exactly one path should be specified when using the --stdin flag")
 		}
 	} else {
-		// checks all paths are contained within the tree root
-		for _, path := range paths {
-			rootPath := filepath.Join(cfg.TreeRoot, path)
-			if _, err = os.Stat(rootPath); err != nil {
-				return fmt.Errorf("path %s not found within the tree root %s", path, cfg.TreeRoot)
+		// checks all paths are contained within the tree root and exist
+		// also "normalize" paths so they're relative to cfg.TreeRoot
+		for i, path := range paths {
+			absolutePath, err := filepath.Abs(path)
+			if err != nil {
+				return fmt.Errorf("error computing absolute path of %s: %w", path, err)
+			}
+
+			relativePath, err := filepath.Rel(cfg.TreeRoot, absolutePath)
+			if err != nil {
+				return fmt.Errorf("error computing relative path from %s to %s: %s", cfg.TreeRoot, absolutePath, err)
+			}
+
+			if strings.HasPrefix(relativePath, "..") {
+				return fmt.Errorf("path %s not inside the tree root %s", path, cfg.TreeRoot)
+			}
+
+			paths[i] = relativePath
+
+			if _, err = os.Stat(absolutePath); err != nil {
+				return fmt.Errorf("path %s not found", path)
 			}
 		}
 	}
@@ -188,8 +205,6 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string)
 		return fmt.Errorf("failed to create walker: %w", err)
 	}
 
-	//
-
 	files := make([]*walk.File, BatchSize)
 	for {
 		ctx, cancel := context.WithTimeout(ctx, 1*time.Second)

cmd/root_test.go

@@ -620,7 +620,7 @@ func TestGitWorktree(t *testing.T) {
 
 	// try with a bad path
 	_, _, err = treefmt(t, "-C", tempDir, "-c", "haskell", "foo")
-	as.ErrorContains(err, "path foo not found within the tree root")
+	as.ErrorContains(err, "path foo not found")
 
 	// try with a path not in the git index, e.g. it is skipped
 	_, err = os.Create(filepath.Join(tempDir, "foo.txt"))
@@ -647,11 +647,20 @@ func TestPathsArg(t *testing.T) {
 		as.NoError(os.Chdir(cwd))
 	})
 
-	tempDir := test.TempExamples(t)
-	configPath := filepath.Join(tempDir, "/treefmt.toml")
+	// create a project root under a temp dir, in order verify behavior with
+	// files inside of temp dir, but outside of the project root
+	tempDir := t.TempDir()
+	treeRoot := filepath.Join(tempDir, "tree-root")
+	test.TempExamplesInDir(t, treeRoot)
 
-	// change working directory to temp root
-	as.NoError(os.Chdir(tempDir))
+	configPath := filepath.Join(treeRoot, "/treefmt.toml")
+
+	// create a file outside of treeRoot
+	externalFile, err := os.Create(filepath.Join(tempDir, "outside_tree.go"))
+	as.NoError(err)
+
+	// change working directory to project root
+	as.NoError(os.Chdir(treeRoot))
 
 	// basic config
 	cfg := &config.Config{
@@ -675,14 +684,29 @@ func TestPathsArg(t *testing.T) {
 	as.NoError(err)
 	assertStats(t, as, statz, 2, 2, 2, 0)
 
+	// specify an absolute path
+	absoluteInternalPath, err := filepath.Abs("elm/elm.json")
+	as.NoError(err)
+	_, statz, err = treefmt(t, "-c", absoluteInternalPath)
+	as.NoError(err)
+	assertStats(t, as, statz, 1, 1, 1, 0)
+
 	// specify a bad path
 	_, _, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Bar.hs")
-	as.ErrorContains(err, "path haskell/Nested/Bar.hs not found within the tree root")
+	as.Errorf(err, "path haskell/Nested/Bar.hs not found")
 
-	// specify a path outside the tree root
-	externalPath := filepath.Join(cwd, "go.mod")
-	_, _, err = treefmt(t, "-c", externalPath)
-	as.ErrorContains(err, fmt.Sprintf("path %s not found within the tree root", externalPath))
+	// specify an absolute path outside the tree root
+	absoluteExternalPath, err := filepath.Abs(externalFile.Name())
+	as.NoError(err)
+	as.FileExists(absoluteExternalPath, "exernal file must exist")
+	_, _, err = treefmt(t, "-c", absoluteExternalPath)
+	as.Errorf(err, "path %s not found within the tree root", absoluteExternalPath)
+
+	// specify a relative path outside the tree root
+	relativeExternalPath := "../outside_tree.go"
+	as.FileExists(relativeExternalPath, "exernal file must exist")
+	_, _, err = treefmt(t, "-c", relativeExternalPath)
+	as.Errorf(err, "path %s not found within the tree root", relativeExternalPath)
 }
 
 func TestStdin(t *testing.T) {
@@ -852,7 +876,12 @@ func TestRunInSubdir(t *testing.T) {
 	assertStats(t, as, statz, 32, 32, 32, 0)
 
 	// specify some explicit paths, relative to the tree root
-	_, statz, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Foo.hs")
+	// this should not work, as we're in a subdirectory
+	_, _, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Foo.hs")
+	as.ErrorContains(err, "path elm/elm.json not found")
+
+	// specify some explicit paths, relative to the current directory
+	_, statz, err = treefmt(t, "-c", "elm.json", "../haskell/Nested/Foo.hs")
 	as.NoError(err)
 	assertStats(t, as, statz, 2, 2, 2, 0)
 }

test/temp.go

@@ -27,13 +27,16 @@ func WriteConfig(t *testing.T, path string, cfg *config.Config) {
 
 func TempExamples(t *testing.T) string {
 	tempDir := t.TempDir()
-	require.NoError(t, cp.Copy("../test/examples", tempDir), "failed to copy test data to temp dir")
+	TempExamplesInDir(t, tempDir)
+	return tempDir
+}
+
+func TempExamplesInDir(t *testing.T, dir string) {
+	require.NoError(t, cp.Copy("../test/examples", dir), "failed to copy test data to dir")
 
 	// we have second precision mod time tracking, so we wait a second before returning, so we don't trigger false
 	//positives for things like fail on change
 	time.Sleep(time.Second)
-
-	return tempDir
 }
 
 func TempFile(t *testing.T, dir string, pattern string, contents *string) *os.File {