diff --git a/cmd/format/format.go b/cmd/format/format.go index 3da196df..e216e0de 100644 --- a/cmd/format/format.go +++ b/cmd/format/format.go @@ -88,8 +88,8 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) // ensure db is closed after we're finished defer func() { - if err := db.Close(); err != nil { - log.Errorf("failed to close cache: %v", err) + if closeErr := db.Close(); closeErr != nil { + log.Errorf("failed to close cache: %v", closeErr) } }() } @@ -126,30 +126,8 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) return errors.New("exactly one path should be specified when using the --stdin flag") } - // 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: %w", 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 walkType != walk.Stdin { - if _, err = os.Stat(absolutePath); err != nil { - return fmt.Errorf("path %s not found", path) - } - } + if err = resolvePaths(paths, walkType, cfg.TreeRoot); err != nil { + return err } // create a composite formatter which will handle applying the correct formatters to each file we traverse @@ -237,3 +215,40 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) return nil } + +// resolvePaths checks all paths are contained within the tree root and exist +// also "normalize" paths so they're relative to `cfg.TreeRoot` +// Symlinks are allowed in `paths` and we resolve them here, since +// the readers will ignore symlinks. +func resolvePaths(paths []string, walkType walk.Type, treeRoot string) error { + for i, path := range paths { + log.Errorf("Resolving path '%s': %v", path, walkType) + + absolutePath, err := filepath.Abs(path) + if err != nil { + return fmt.Errorf("error computing absolute path of %s: %w", path, err) + } + + if walkType != walk.Stdin { + realPath, err := filepath.EvalSymlinks(absolutePath) + if err != nil { + return fmt.Errorf("path %s not found: %w", absolutePath, err) + } + + absolutePath = realPath + } + + relativePath, err := filepath.Rel(treeRoot, absolutePath) + if err != nil { + return fmt.Errorf("error computing relative path from %s to %s: %w", treeRoot, absolutePath, err) + } + + if strings.HasPrefix(relativePath, "..") { + return fmt.Errorf("path %s not inside the tree root %s", path, treeRoot) + } + + paths[i] = relativePath + } + + return nil +} diff --git a/cmd/root_test.go b/cmd/root_test.go index ef99c89b..9dfb2c20 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1411,9 +1411,9 @@ func TestGit(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 78, - stats.Matched: 78, - stats.Formatted: 48, // the echo formatter should only be applied to the new files + stats.Traversed: 79, + stats.Matched: 79, + stats.Formatted: 49, // the echo formatter should only be applied to the new files stats.Changed: 0, }), ) @@ -1463,7 +1463,7 @@ func TestGit(t *testing.T) { withArgs("-C", tempDir, "haskell", "foo"), withConfig(configPath, cfg), withError(func(as *require.Assertions, err error) { - as.ErrorContains(err, "path foo not found") + as.ErrorContains(err, "foo not found") }), ) @@ -1592,7 +1592,7 @@ func TestPathsArg(t *testing.T) { treefmt(t, withArgs("rust/src/main.rs", "haskell/Nested/Bar.hs"), withError(func(as *require.Assertions, err error) { - as.ErrorContains(err, "path haskell/Nested/Bar.hs not found") + as.ErrorContains(err, "Bar.hs not found") }), ) @@ -1610,7 +1610,7 @@ func TestPathsArg(t *testing.T) { // specify a relative path outside the tree root relativeExternalPath := "../outside_tree.go" - as.FileExists(relativeExternalPath, "exernal file must exist") + as.FileExists(relativeExternalPath, "external file must exist") treefmt(t, withArgs(relativeExternalPath), @@ -1842,7 +1842,7 @@ func TestRunInSubdir(t *testing.T) { treefmt(t, withArgs("-c", "go/main.go", "haskell/Nested/Foo.hs"), withError(func(as *require.Assertions, err error) { - as.ErrorContains(err, "path go/main.go not found") + as.ErrorContains(err, "go/main.go not found") }), ) diff --git a/test/examples/yaml/test-rel-symlink.yaml b/test/examples/yaml/test-rel-symlink.yaml new file mode 120000 index 00000000..c40da6f9 --- /dev/null +++ b/test/examples/yaml/test-rel-symlink.yaml @@ -0,0 +1 @@ +../yaml/test.yaml \ No newline at end of file diff --git a/walk/git.go b/walk/git.go index cb5c4c86..84cbbe7d 100644 --- a/walk/git.go +++ b/walk/git.go @@ -91,16 +91,22 @@ LOOP: g.log.Debugf("processing file: %s", path) - info, err := os.Stat(path) - if os.IsNotExist(err) { + info, err := os.Lstat(path) + + switch { + case os.IsNotExist(err): // the underlying file might have been removed g.log.Warnf( "Path %s is in the worktree but appears to have been removed from the filesystem", path, ) continue - } else if err != nil { + case err != nil: return n, fmt.Errorf("failed to stat %s: %w", path, err) + case info.Mode()&os.ModeSymlink == os.ModeSymlink: + // we skip reporting symlinks stored in Git, they should + // point to local files which we would list anyway. + continue } files[n] = &File{ @@ -108,6 +114,7 @@ LOOP: RelPath: filepath.Join(g.path, entry), Info: info, } + n++ } else { // nothing more to read diff --git a/walk/walk.go b/walk/walk.go index 48507ae3..eeecb811 100644 --- a/walk/walk.go +++ b/walk/walk.go @@ -234,6 +234,9 @@ func NewReader( return reader, err } +// NewCompositeReader returns a composite reader for the `root` and all `paths`. It +// never follows symlinks. +// //nolint:ireturn func NewCompositeReader( walkType Type, @@ -268,16 +271,21 @@ func NewCompositeReader( // create a clean absolute path path := filepath.Clean(filepath.Join(root, relPath)) - // check the path exists + // check the path exists (don't follow symlinks) info, err = os.Lstat(path) if err != nil { return nil, fmt.Errorf("failed to stat %s: %w", path, err) } - if info.IsDir() { + switch { + case info.Mode()&os.ModeSymlink == os.ModeSymlink: + // for symlinks -> we ignore them since it does not make sense to follow them + // as normal files in the `root` will be picked up nevertheless. + continue + case info.IsDir(): // for directories, we honour the walk type as we traverse them readers[idx], err = NewReader(walkType, root, relPath, db, statz) - } else { + default: // for files, we enforce a simple filesystem read readers[idx], err = NewReader(Filesystem, root, relPath, db, statz) }