diff --git a/.github/workflows/bench-regression.yml b/.github/workflows/bench-regression.yml index 69c343a..4ad9028 100644 --- a/.github/workflows/bench-regression.yml +++ b/.github/workflows/bench-regression.yml @@ -52,6 +52,9 @@ jobs: go-version: stable cache-dependency-path: go.sum + - name: Download modules + run: go mod download + - name: Run benchmarks id: bench run: go test -run='^$' -bench="$BENCH_FILTER" -benchmem -count=$BENCH_COUNT $BENCH_PATH > base.txt 2>&1 @@ -83,6 +86,9 @@ jobs: go-version: stable cache-dependency-path: go.sum + - name: Download modules + run: go mod download + - name: Run benchmarks id: bench run: go test -run='^$' -bench="$BENCH_FILTER" -benchmem -count=$BENCH_COUNT $BENCH_PATH > pr.txt 2>&1 diff --git a/go.mod b/go.mod index 4eb4969..5d0c5f6 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,8 @@ module github.com/go-git/go-billy/v6 -// go-git supports the last 3 stable Go versions. -go 1.24.0 +go 1.25.0 require ( - github.com/cyphar/filepath-securejoin v0.6.1 github.com/stretchr/testify v1.11.1 golang.org/x/sys v0.41.0 ) diff --git a/go.sum b/go.sum index 04aa362..d7a8112 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/cyphar/filepath-securejoin v0.6.1 h1:5CeZ1jPXEiYt3+Z6zqprSAgSWiggmpVyciv8syjIpVE= -github.com/cyphar/filepath-securejoin v0.6.1/go.mod h1:A8hd4EnAeyujCJRrICiOWqjS1AX0a9kM5XL+NwKoYSc= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/osfs/os.go b/osfs/os.go index 1d9d702..3cefeb7 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -22,63 +22,19 @@ const ( var Default = &ChrootOS{} // New returns a new OS filesystem. -// By default paths are deduplicated, but still enforced -// under baseDir. For more info refer to WithDeduplicatePath. func New(baseDir string, opts ...Option) billy.Filesystem { - o := &options{ - deduplicatePath: true, - } + o := &options{} for _, opt := range opts { opt(o) } if o.Type == BoundOSFS { - return newBoundOS(baseDir, o.deduplicatePath) + return newBoundOS(baseDir) } return newChrootOS(baseDir) } -// WithBoundOS returns the option of using a Bound filesystem OS. -func WithBoundOS() Option { - return func(o *options) { - o.Type = BoundOSFS - } -} - -// WithChrootOS returns the option of using a Chroot filesystem OS. -func WithChrootOS() Option { - return func(o *options) { - o.Type = ChrootOSFS - } -} - -// WithDeduplicatePath toggles the deduplication of the base dir in the path. -// This occurs when absolute links are being used. -// Assuming base dir /base/dir and an absolute symlink /base/dir/target: -// -// With DeduplicatePath (default): /base/dir/target -// Without DeduplicatePath: /base/dir/base/dir/target -// -// This option is only used by the BoundOS OS type. -func WithDeduplicatePath(enabled bool) Option { - return func(o *options) { - o.deduplicatePath = enabled - } -} - -type options struct { - Type - deduplicatePath bool -} - -type Type int - -const ( - ChrootOSFS Type = iota - BoundOSFS -) - func tempFile(dir, prefix string) (billy.File, error) { f, err := os.CreateTemp(dir, prefix) if err != nil { diff --git a/osfs/os_bound.go b/osfs/os_bound.go index 0e64b66..33fc8e3 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -19,234 +19,371 @@ package osfs import ( + "errors" "fmt" - "io/fs" + gofs "io/fs" "os" "path/filepath" - "strings" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/util" ) -var dotPrefixes = []string{"./", ".\\"} +// ErrPathEscapesParent represents when an action leads to escaping from the +// given dir the filesystem is bound to. +// +// The upstream version of this error used by [os.Root] is not public. +var ErrPathEscapesParent = errors.New("path escapes from parent") + +// FromRoot creates a new instance of BoundOS from an [os.Root]. +// The base dir is set to root.Name(). Unlike [New] with [WithBoundOS], +// the provided root is used directly for all operations rather than +// opening a fresh [os.Root] per operation; the caller is responsible +// for the root's lifecycle. If root is nil, all filesystem operations +// will fail with an error. +func FromRoot(root *os.Root) billy.Filesystem { + if root == nil { + return &BoundOS{rootError: errors.New("root is nil")} + } + return &BoundOS{baseDir: root.Name(), root: root} +} -// BoundOS is a fs implementation based on the OS filesystem which is bound to -// a base dir. -// Prefer this fs implementation over ChrootOS. +// BoundOS is a fs implementation based on the OS filesystem which relies on +// Go's os.Root. Prefer this fs implementation over ChrootOS. +// +// When created via [New] with [WithBoundOS], a new [os.Root] is opened and +// closed for each filesystem operation. When created via [FromRoot], the +// provided [os.Root] is used directly for all operations and the caller is +// responsible for its lifecycle. // // Behaviours of note: // 1. Read and write operations can only be directed to files which descends // from the base dir. // 2. Symlinks don't have their targets modified, and therefore can point // to locations outside the base dir or to non-existent paths. -// 3. Readlink and Lstat ensures that the link file is located within the base -// dir, evaluating any symlinks that file or base dir may contain. +// 3. Operations leading to escapes to outside the [os.Root] location results +// in [ErrPathEscapesParent]. type BoundOS struct { - baseDir string - deduplicatePath bool + baseDir string + root *os.Root // non-nil only for FromRoot; newBoundOS opens per-op + rootError error } -func newBoundOS(d string, deduplicatePath bool) billy.Filesystem { - return &BoundOS{baseDir: d, deduplicatePath: deduplicatePath} +func newBoundOS(d string) billy.Filesystem { + return &BoundOS{baseDir: d} } func (fs *BoundOS) Capabilities() billy.Capability { return billy.DefaultCapabilities & billy.SyncCapability } -func (fs *BoundOS) Create(filename string) (billy.File, error) { - return fs.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, defaultCreateMode) +func (fs *BoundOS) Create(name string) (billy.File, error) { + return fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, defaultCreateMode) } -func (fs *BoundOS) OpenFile(filename string, flag int, perm fs.FileMode) (billy.File, error) { - filename = fs.expandDot(filename) - fn, err := fs.abs(filename) +func (fs *BoundOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.File, error) { + root, cleanup, err := fs.fsRoot() if err != nil { return nil, err } + defer cleanup() - return openFile(fn, flag, perm, fs.createDir) -} + // When not creating, read symlink links so that they can be made + // relative and therefore work. + if flag&os.O_CREATE == 0 { + fi, err := root.Lstat(name) + if err == nil && fi.Mode()&gofs.ModeSymlink != 0 { + fn, err := root.Readlink(name) + if err != nil { + return nil, err + } + name = fn + } + } + + if filepath.IsAbs(name) { + fn, err := filepath.Rel(fs.baseDir, name) + if err != nil { + return nil, err + } + name = fn + } + + if flag&os.O_CREATE != 0 { + if err = fs.createDir(root, name); err != nil { + return nil, translateError(err, name) + } + } -func (fs *BoundOS) ReadDir(path string) ([]fs.DirEntry, error) { - path = fs.expandDot(path) - dir, err := fs.abs(path) + f, err := root.OpenFile(name, flag, perm) if err != nil { return nil, err } - - return os.ReadDir(dir) + return &file{File: f}, nil } -func (fs *BoundOS) Rename(from, to string) error { - if from == "." || from == fs.baseDir { - return billy.ErrBaseDirCannotBeRenamed +func (fs *BoundOS) ReadDir(name string) ([]gofs.DirEntry, error) { + if filepath.IsAbs(name) { + fn, err := filepath.Rel(fs.baseDir, name) + if err != nil { + return nil, err + } + name = fn } - from = fs.expandDot(from) - _, err := fs.Lstat(from) + if name == "" { + name = "." + } + + root, cleanup, err := fs.fsRoot() if err != nil { - return err + return nil, err } + defer cleanup() - f, err := fs.abs(from) + f, err := root.Open(name) if err != nil { - return err + return nil, translateError(err, name) } - to = fs.expandDot(to) - t, err := fs.abs(to) + e, err := f.ReadDir(-1) if err != nil { - return err + return nil, translateError(err, name) + } + return e, nil +} + +func (fs *BoundOS) Rename(from, to string) error { + if from == "." || from == fs.baseDir { + return billy.ErrBaseDirCannotBeRenamed } - // MkdirAll for target name. - if err := fs.createDir(t); err != nil { + root, cleanup, err := fs.fsRoot() + if err != nil { return err } + defer cleanup() + + // Ensure the target directory exists. + err = root.MkdirAll(filepath.Dir(to), defaultDirectoryMode) + if err == nil { + err = root.Rename(from, to) + } - return os.Rename(f, t) + return translateError(err, to) } -func (fs *BoundOS) MkdirAll(path string, perm fs.FileMode) error { - path = fs.expandDot(path) - dir, err := fs.abs(path) +func (fs *BoundOS) MkdirAll(name string, _ gofs.FileMode) error { + root, cleanup, err := fs.fsRoot() if err != nil { return err } - return os.MkdirAll(dir, perm) + defer cleanup() + + // os.Root errors when perm contains bits other than the nine least-significant bits (0o777). + err = root.MkdirAll(name, 0o777) + return translateError(err, name) } -func (fs *BoundOS) Open(filename string) (billy.File, error) { - return fs.OpenFile(filename, os.O_RDONLY, 0) +func (fs *BoundOS) Open(name string) (billy.File, error) { + return fs.OpenFile(name, os.O_RDONLY, 0) } -func (fs *BoundOS) Stat(filename string) (os.FileInfo, error) { - filename = fs.expandDot(filename) - filename, err := fs.abs(filename) +func (fs *BoundOS) Stat(name string) (os.FileInfo, error) { + if filepath.IsAbs(name) { + fn, err := filepath.Rel(fs.baseDir, name) + if err != nil { + return nil, err + } + + _, err = os.Stat(fs.baseDir) + if err != nil && os.IsNotExist(err) { + err = os.MkdirAll(fs.baseDir, defaultDirectoryMode) + if err != nil { + return nil, err + } + } + name = fn + } + + if name == "" { + name = "." + } + + root, cleanup, err := fs.fsRoot() if err != nil { return nil, err } - return os.Stat(filename) + defer cleanup() + + fi, err := root.Stat(name) + if err != nil { + return nil, translateError(err, name) + } + + return fi, nil } -func (fs *BoundOS) Remove(filename string) error { - if filename == "." || filename == fs.baseDir { +func (fs *BoundOS) Remove(name string) error { + if name == "." || name == fs.baseDir { return billy.ErrBaseDirCannotBeRemoved } - fn, err := fs.abs(filename) + if filepath.IsAbs(name) { + fn, err := filepath.Rel(fs.baseDir, name) + if err != nil { + return err + } + name = fn + } + + root, cleanup, err := fs.fsRoot() if err != nil { return err } - return os.Remove(fn) + defer cleanup() + + err = root.Remove(name) + if err == nil { + return nil + } + + return translateError(err, name) } // TempFile creates a temporary file. If dir is empty, the file -// will be created within the OS Temporary dir. If dir is provided -// it must descend from the current base dir. +// will be created within a .tmp dir. +// +// If dir is outside the bound dir, [ErrPathEscapesParent] is returned. func (fs *BoundOS) TempFile(dir, prefix string) (billy.File, error) { - if dir != "" { - var err error - dir, err = fs.abs(dir) - if err != nil { - return nil, err - } - - _, err = os.Stat(dir) - if err != nil && os.IsNotExist(err) { - err = os.MkdirAll(dir, defaultDirectoryMode) - if err != nil { - return nil, err - } - } - } - - return tempFile(dir, prefix) + return util.TempFile(fs, dir, prefix) } func (fs *BoundOS) Join(elem ...string) string { return filepath.Join(elem...) } -func (fs *BoundOS) RemoveAll(path string) error { - if path == "." || path == fs.baseDir { +func (fs *BoundOS) RemoveAll(name string) error { + if name == "." || name == fs.baseDir { return billy.ErrBaseDirCannotBeRemoved } - path = fs.expandDot(path) - dir, err := fs.abs(path) + if filepath.IsAbs(name) { + fn, err := filepath.Rel(fs.baseDir, name) + if err != nil { + return err + } + name = fn + } + + root, cleanup, err := fs.fsRoot() if err != nil { return err } - return os.RemoveAll(dir) + defer cleanup() + + return root.RemoveAll(name) } -func (fs *BoundOS) Symlink(target, link string) error { - link = fs.expandDot(link) - ln, err := fs.abs(link) +func (fs *BoundOS) Symlink(oldname, newname string) error { + if filepath.IsAbs(newname) { + fn, err := filepath.Rel(fs.baseDir, newname) + if err != nil { + return err + } + newname = fn + } + + root, cleanup, err := fs.fsRoot() if err != nil { return err } - // MkdirAll for containing dir. - if err := fs.createDir(ln); err != nil { + defer cleanup() + + err = fs.createDir(root, newname) + if err != nil { return err } - return os.Symlink(target, ln) + + return root.Symlink(oldname, newname) } -func (fs *BoundOS) expandDot(p string) string { - if p == "." { - return fs.baseDir - } - for _, prefix := range dotPrefixes { - if strings.HasPrefix(p, prefix) { - return filepath.Join(fs.baseDir, strings.TrimPrefix(p, prefix)) +func (fs *BoundOS) Lstat(name string) (os.FileInfo, error) { + if filepath.IsAbs(name) { + fn, err := filepath.Rel(fs.baseDir, name) + if err != nil { + return nil, err } + name = fn } - return p -} -func (fs *BoundOS) Lstat(filename string) (os.FileInfo, error) { - filename = fs.expandDot(filename) - filename = filepath.Clean(filename) - if !filepath.IsAbs(filename) { - filename = filepath.Join(fs.baseDir, filename) - } - if ok, err := fs.insideBaseDirEval(filename); !ok { + root, cleanup, err := fs.fsRoot() + if err != nil { return nil, err } - return os.Lstat(filename) -} + defer cleanup() -func (fs *BoundOS) Readlink(link string) (string, error) { - link = fs.expandDot(link) - if !filepath.IsAbs(link) { - link = filepath.Clean(filepath.Join(fs.baseDir, link)) + fi, err := root.Lstat(name) + if err != nil { + return nil, translateError(err, name) } - if ok, err := fs.insideBaseDirEval(link); !ok { + + return fi, nil +} + +func (fs *BoundOS) Readlink(name string) (string, error) { + root, cleanup, err := fs.fsRoot() + if err != nil { return "", err } - return os.Readlink(link) + defer cleanup() + + lnk, err := root.Readlink(name) + if err != nil { + return "", translateError(err, name) + } + + return lnk, nil } -func (fs *BoundOS) Chmod(path string, mode fs.FileMode) error { - abspath, err := fs.abs(path) +func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { + root, cleanup, err := fs.fsRoot() if err != nil { return err } - return os.Chmod(abspath, mode) + defer cleanup() + return root.Chmod(path, mode) } // Chroot returns a new BoundOS filesystem, with the base dir set to the // result of joining the provided path with the underlying base dir. func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { - joined, err := securejoin.SecureJoin(fs.baseDir, path) + root, cleanup, err := fs.fsRoot() if err != nil { return nil, err } - return New(joined, WithBoundOS()), nil + defer cleanup() + + fi, err := root.Lstat(path) + if errors.Is(err, os.ErrNotExist) { + err := root.MkdirAll(path, defaultDirectoryMode) + if err != nil { + return nil, fmt.Errorf("failed to auto create dir: %w", err) + } + } else if err != nil { + return nil, err + } + if fi != nil && !fi.IsDir() { + return nil, fmt.Errorf("cannot chroot: path is not dir") + } + + childRoot, err := root.OpenRoot(path) + if err != nil { + return nil, fmt.Errorf("unable to chroot: %w", err) + } + defer childRoot.Close() + + return New(childRoot.Name(), WithBoundOS()), nil } // Root returns the current base dir of the billy.Filesystem. @@ -256,10 +393,10 @@ func (fs *BoundOS) Root() string { return fs.baseDir } -func (fs *BoundOS) createDir(fullpath string) error { +func (fs *BoundOS) createDir(root *os.Root, fullpath string) error { dir := filepath.Dir(fullpath) if dir != "." { - if err := os.MkdirAll(dir, defaultDirectoryMode); err != nil { + if err := root.MkdirAll(dir, defaultDirectoryMode); err != nil { return err } } @@ -267,49 +404,33 @@ func (fs *BoundOS) createDir(fullpath string) error { return nil } -// abs transforms filename to an absolute path, taking into account the base dir. -// Relative paths won't be allowed to ascend the base dir, so `../file` will become -// `/working-dir/file`. -// -// Note that if filename is a symlink, the returned address will be the target of the -// symlink. -func (fs *BoundOS) abs(filename string) (string, error) { - if filename == fs.baseDir { - filename = string(filepath.Separator) +// fsRoot returns the [os.Root] to use for filesystem operations along with +// a cleanup function that must be called when the root is no longer needed. +// For BoundOS instances created via [FromRoot], the cleanup is a no-op and +// the caller-provided root is returned directly. Otherwise, a new [os.Root] +// is opened for each call and closed by the cleanup function. +func (fs *BoundOS) fsRoot() (*os.Root, func(), error) { + if fs.rootError != nil { + return nil, func() {}, fs.rootError } - - path, err := securejoin.SecureJoin(fs.baseDir, filename) - if err != nil { - return "", err + if fs.root != nil { + return fs.root, func() {}, nil } - - if fs.deduplicatePath { - vol := filepath.VolumeName(fs.baseDir) - dup := filepath.Join(fs.baseDir, fs.baseDir[len(vol):]) - if strings.HasPrefix(path, dup+string(filepath.Separator)) { - return fs.abs(path[len(dup):]) - } + r, err := os.OpenRoot(fs.baseDir) + if err != nil { + return nil, func() {}, err } - return path, nil + return r, func() { r.Close() }, nil } -// insideBaseDirEval checks whether filename is contained within -// a dir that is within the fs.baseDir, by first evaluating any symlinks -// that either filename or fs.baseDir may contain. -func (fs *BoundOS) insideBaseDirEval(filename string) (bool, error) { - if fs.baseDir == "/" || fs.baseDir == "" || fs.baseDir == filename { - return true, nil +func translateError(err error, file string) error { + if err == nil { + return nil } - dir, err := filepath.EvalSymlinks(filepath.Dir(filename)) - if dir == "" || os.IsNotExist(err) { - dir = filepath.Dir(filename) - } - wd, err := filepath.EvalSymlinks(fs.baseDir) - if wd == "" || os.IsNotExist(err) { - wd = fs.baseDir - } - if filename != wd && dir != wd && !strings.HasPrefix(dir, wd+string(filepath.Separator)) { - return false, fmt.Errorf("%q: path outside base dir %q: %w", filename, fs.baseDir, os.ErrNotExist) + + if errors.Unwrap(err).Error() == ErrPathEscapesParent.Error() { + return fmt.Errorf("%w: %q", ErrPathEscapesParent, file) } - return true, nil + + return err } diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index c035fdc..1c9f413 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -19,7 +19,6 @@ package osfs import ( - "fmt" "os" "path/filepath" "runtime" @@ -33,14 +32,72 @@ import ( func TestBoundOSCapabilities(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) - _, ok := fs.(billy.Capable) + fs := newBoundOS(dir) + c, ok := fs.(billy.Capable) assert.True(t, ok) - caps := billy.Capabilities(fs) + caps := c.Capabilities() assert.Equal(t, billy.DefaultCapabilities&billy.SyncCapability, caps) } +func TestFromRoot(t *testing.T) { + validRoot, err := os.OpenRoot(t.TempDir()) + require.NoError(t, err) + t.Cleanup(func() { validRoot.Close() }) + + closedRoot, err := os.OpenRoot(t.TempDir()) + require.NoError(t, err) + require.NoError(t, closedRoot.Close()) + + tests := []struct { + name string + root *os.Root + wantRoot string + wantErr string + }{ + { + name: "valid root", + root: validRoot, + wantRoot: validRoot.Name(), + }, + { + name: "nil root", + root: nil, + wantErr: "root is nil", + }, + { + name: "closed root", + root: closedRoot, + wantRoot: closedRoot.Name(), + wantErr: "file already closed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + + fs := FromRoot(tt.root) + require.NotNil(t, fs) + assert.IsType(&BoundOS{}, fs) + assert.Equal(tt.wantRoot, fs.Root()) + + if tt.wantErr != "" { + _, err := fs.Stat(".") + require.ErrorContains(t, err, tt.wantErr) + return + } + + f, err := fs.Create("test-file") + require.NoError(t, err) + require.NoError(t, f.Close()) + + _, err = fs.Stat("test-file") + require.NoError(t, err) + }) + } +} + func TestOpen(t *testing.T) { tests := []struct { name string @@ -54,7 +111,7 @@ func TestOpen(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", }, @@ -63,7 +120,7 @@ func TestOpen(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "./test-file", }, @@ -72,9 +129,10 @@ func TestOpen(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "rel-above-cwd"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "../../rel-above-cwd", + wantErr: ErrPathEscapesParent.Error(), }, { name: "file: rel path to below cwd", @@ -83,7 +141,7 @@ func TestOpen(t *testing.T) { require.NoError(t, err) err = os.WriteFile(filepath.Join(dir, "sub/rel-below-cwd"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "sub/rel-below-cwd", }, @@ -92,7 +150,7 @@ func TestOpen(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "abs-test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "abs-test-file", makeAbs: true, @@ -100,51 +158,77 @@ func TestOpen(t *testing.T) { { name: "file: abs outside cwd", before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "/some/path/outside/cwd", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent.Error(), }, { - name: "symlink: same dir", + name: "symlink: same dir abs", before: func(dir string) billy.Filesystem { target := filepath.Join(dir, "target-file") err := os.WriteFile(target, []byte("anything"), 0o600) require.NoError(t, err) err = os.Symlink(target, filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) + }, + filename: "symlink", + }, + { + name: "symlink: same dir rel", + before: func(dir string) billy.Filesystem { + target := filepath.Join(dir, "target-file") + err := os.WriteFile(target, []byte("anything"), 0o600) + require.NoError(t, err) + err = os.Symlink("target-file", filepath.Join(dir, "symlink")) + require.NoError(t, err) + return newBoundOS(dir) }, filename: "symlink", }, + { + name: "symlink: symlink to symlink", + before: func(dir string) billy.Filesystem { + target := filepath.Join(dir, "target-file") + err := os.WriteFile(target, []byte("anything"), 0o600) + require.NoError(t, err) + err = os.Symlink("target-file", filepath.Join(dir, "symlink")) + require.NoError(t, err) + err = os.Symlink("symlink", filepath.Join(dir, "symlink2")) + require.NoError(t, err) + return newBoundOS(dir) + }, + filename: "symlink2", + }, { name: "symlink: rel outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("../../../../../../outside/cwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: true, - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent.Error(), }, { name: "symlink: abs outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: true, - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent.Error(), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) if tt.before != nil { fs = tt.before(dir) @@ -179,6 +263,7 @@ func Test_Symlink(t *testing.T) { name string link string target string + makeAbs bool before func(dir string) billy.Filesystem wantStatErr string }{ @@ -187,6 +272,12 @@ func Test_Symlink(t *testing.T) { link: "symlink", target: filepath.FromSlash("/etc/passwd"), }, + { + name: "abs link to abs valid target", + link: "symlink", + target: filepath.FromSlash("/etc/passwd"), + makeAbs: true, + }, { name: "dot link to abs valid target", link: "./symlink", @@ -210,7 +301,7 @@ func Test_Symlink(t *testing.T) { { name: "auto create dir", link: "new-dir/symlink", - target: filepath.FromSlash("../../../some/random/path"), + target: filepath.FromSlash("/etc/passwd"), }, { name: "keep dir filemode if exists", @@ -218,7 +309,7 @@ func Test_Symlink(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.Mkdir(filepath.Join(dir, "new-dir"), 0o701) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, target: filepath.FromSlash("../../../some/random/path"), }, @@ -227,7 +318,7 @@ func Test_Symlink(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) if tt.before != nil { fs = tt.before(dir) @@ -242,10 +333,16 @@ func Test_Symlink(t *testing.T) { diBefore, _ := os.Lstat(filepath.Dir(link)) - err = fs.Symlink(tt.target, tt.link) + lnk := tt.link + if tt.makeAbs { + lnk = link + } + + err = fs.Symlink(tt.target, lnk) require.NoError(t, err) fi, err := os.Lstat(link) + if tt.wantStatErr != "" { require.ErrorContains(t, err, tt.wantStatErr) } else { @@ -270,37 +367,40 @@ func Test_Symlink(t *testing.T) { func TestTempFile(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) + // No dir provided means bound dir + `/.tmp`. f, err := fs.TempFile("", "prefix") require.NoError(t, err) assert.NotNil(f) - assert.Contains(f.Name(), os.TempDir()) + prefix := filepath.Join(".tmp", "prefix") + assert.True(strings.HasPrefix(f.Name(), filepath.Join(dir, prefix)), f.Name(), prefix) require.NoError(t, f.Close()) f, err = fs.TempFile("/above/cwd", "prefix") - require.NoError(t, err) - assert.NotNil(f) - assert.Contains(f.Name(), filepath.Join(dir, "/above/cwd", "prefix")) - require.NoError(t, f.Close()) + require.ErrorIs(t, err, ErrPathEscapesParent) + assert.Nil(f) dir = os.TempDir() + f, err = fs.TempFile("../../../above/cwd", "prefix") + require.ErrorIs(t, err, ErrPathEscapesParent) + assert.Nil(f) + + dir = filepath.Join(dir, "/tmp") // For windows, volume name must be removed. if v := filepath.VolumeName(dir); v != "" { dir = strings.TrimPrefix(dir, v) } f, err = fs.TempFile(dir, "prefix") - require.NoError(t, err) - assert.NotNil(f) - assert.Contains(f.Name(), filepath.Join(dir, "prefix")) - require.NoError(t, f.Close()) + require.ErrorIs(t, err, ErrPathEscapesParent) + assert.Nil(f) } func TestChroot(t *testing.T) { assert := assert.New(t) tmp := t.TempDir() - fs := newBoundOS(tmp, true) + fs := newBoundOS(tmp) f, err := fs.Chroot("test") require.NoError(t, err) @@ -312,7 +412,7 @@ func TestChroot(t *testing.T) { func TestRoot(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) root := fs.Root() assert.Equal(dir, root) @@ -326,14 +426,14 @@ func TestReadLink(t *testing.T) { expected string makeExpectedAbs bool before func(dir string) billy.Filesystem - wantErr string + wantErr error }{ { name: "symlink: pointing to abs outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", expected: filepath.FromSlash("/etc/passwd"), @@ -341,18 +441,19 @@ func TestReadLink(t *testing.T) { { name: "file: rel pointing to abs above cwd", filename: "../../file", - wantErr: "path outside base dir", + wantErr: ErrPathEscapesParent, }, { name: "symlink: abs symlink pointing outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: true, expected: filepath.FromSlash("/etc/passwd"), + wantErr: ErrPathEscapesParent, }, { name: "symlink: dir pointing outside cwd", @@ -370,11 +471,11 @@ func TestReadLink(t *testing.T) { err = os.WriteFile(filepath.Join(outside, "file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(cwd, true) + return newBoundOS(cwd) }, filename: "current-dir/symlink/file", makeAbs: true, - wantErr: "path outside base dir", + wantErr: ErrPathEscapesParent, }, { name: "symlink: within cwd + baseDir symlink", @@ -394,7 +495,7 @@ func TestReadLink(t *testing.T) { require.NoError(t, err) err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(cwdAlt, "symlink-file")) require.NoError(t, err) - return newBoundOS(cwd, true) + return newBoundOS(cwd) }, filename: "symlink-file", expected: filepath.FromSlash("cwd-target/file"), @@ -421,17 +522,17 @@ func TestReadLink(t *testing.T) { require.NoError(t, err) err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(outside, "symlink-file")) require.NoError(t, err) - return newBoundOS(cwd, true) + return newBoundOS(cwd) }, filename: "symlink-outside/symlink-file", - wantErr: "path outside base dir", + wantErr: ErrPathEscapesParent, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) if tt.before != nil { fs = tt.before(dir) @@ -448,8 +549,8 @@ func TestReadLink(t *testing.T) { } got, err := fs.Readlink(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) assert.Empty(got) } else { require.NoError(t, err) @@ -465,14 +566,14 @@ func TestLstat(t *testing.T) { filename string makeAbs bool before func(dir string) billy.Filesystem - wantErr string + wantErr error }{ { name: "rel symlink: pointing to abs outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", }, @@ -481,7 +582,7 @@ func TestLstat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.Symlink("../../../../../../../../etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", }, @@ -490,7 +591,7 @@ func TestLstat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: true, @@ -500,7 +601,7 @@ func TestLstat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.Symlink("../../../../../../../../etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: false, @@ -523,7 +624,7 @@ func TestLstat(t *testing.T) { require.NoError(t, err) err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(cwdAlt, "symlink-file")) require.NoError(t, err) - return newBoundOS(cwd, true) + return newBoundOS(cwd) }, filename: "symlink-file", makeAbs: false, @@ -550,28 +651,28 @@ func TestLstat(t *testing.T) { err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(outside, "symlink-file")) require.NoError(t, err) - return newBoundOS(cwd, true) + return newBoundOS(cwd) }, filename: "symlink-outside/symlink-file", makeAbs: false, - wantErr: "path outside base dir", + wantErr: ErrPathEscapesParent, }, { name: "path: rel pointing to abs above cwd", filename: "../../file", - wantErr: "path outside base dir", + wantErr: ErrPathEscapesParent, }, { name: "path: abs pointing outside cwd", filename: "/etc/passwd", - wantErr: "path outside base dir", + wantErr: ErrPathEscapesParent, }, { name: "file: rel", before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", }, @@ -580,7 +681,7 @@ func TestLstat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "./test-file", }, @@ -589,7 +690,7 @@ func TestLstat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", makeAbs: true, @@ -599,7 +700,7 @@ func TestLstat(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) if tt.before != nil { fs = tt.before(dir) @@ -610,8 +711,8 @@ func TestLstat(t *testing.T) { filename = filepath.Join(dir, filename) } fi, err := fs.Lstat(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) assert.Nil(fi) } else { require.NoError(t, err) @@ -628,27 +729,27 @@ func TestStat(t *testing.T) { filename string makeAbs bool before func(dir string) billy.Filesystem - wantErr string + wantErr error }{ { name: "rel symlink: pointing to abs outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "rel symlink: pointing to rel path above cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("../../../../../../../../etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { @@ -656,46 +757,46 @@ func TestStat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: true, - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "abs symlink: pointing to rel outside cwd", before: func(dir string) billy.Filesystem { err := os.Symlink("../../../../../../../../etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: false, - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "path: rel pointing to abs above cwd", filename: "../../file", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "path: abs pointing outside cwd", filename: "/etc/passwd", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "rel file", before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", }, { name: "rel dot dir", before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: ".", }, @@ -704,7 +805,7 @@ func TestStat(t *testing.T) { before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", makeAbs: true, @@ -714,7 +815,7 @@ func TestStat(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) if tt.before != nil { fs = tt.before(dir) @@ -726,8 +827,8 @@ func TestStat(t *testing.T) { } fi, err := fs.Stat(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) assert.Nil(fi) } else { require.NoError(t, err) @@ -743,40 +844,41 @@ func TestRemove(t *testing.T) { filename string makeAbs bool before func(dir string) billy.Filesystem - wantErr string + after func(t *testing.T, dir string) + wantErr error }{ { name: "path: rel pointing outside cwd w forward slash", filename: "/some/path/outside/cwd", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "path: rel pointing outside cwd", filename: "../../../../path/outside/cwd", - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { name: "inexistent dir", before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "inexistent", - wantErr: notFoundError(), + wantErr: os.ErrNotExist, }, { name: "same dot dir", before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: ".", - wantErr: "base dir cannot be removed", + wantErr: billy.ErrBaseDirCannotBeRemoved, }, { name: "same dir file", before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", }, @@ -788,31 +890,49 @@ func TestRemove(t *testing.T) { require.NoError(t, err) err = os.Symlink(target, filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", }, + { + name: "rel path to file below cwd", + before: func(dir string) billy.Filesystem { + p := filepath.Join(dir, "sub") + err := os.MkdirAll(p, 0o777) + require.NoError(t, err) + + err = os.WriteFile(filepath.Join(dir, "sub", "rel-below-cwd"), []byte("anything"), 0o600) + require.NoError(t, err) + return newBoundOS(dir) + }, + filename: "./sub/rel-below-cwd", + }, { name: "rel path to file above cwd", before: func(dir string) billy.Filesystem { - err := os.WriteFile(filepath.Join(dir, "rel-above-cwd"), []byte("anything"), 0o600) + p := filepath.Join(dir, "sub") + err := os.MkdirAll(p, 0o777) require.NoError(t, err) - return newBoundOS(dir, true) + + err = os.WriteFile(filepath.Join(dir, "rel-above-cwd"), []byte("anything"), 0o600) + require.NoError(t, err) + return newBoundOS(p) }, - filename: "../../rel-above-cwd", + filename: "../rel-above-cwd", + wantErr: ErrPathEscapesParent, }, { name: "abs file", before: func(dir string) billy.Filesystem { err := os.WriteFile(filepath.Join(dir, "abs-test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "abs-test-file", makeAbs: true, }, { - name: "abs symlink: pointing outside is forced to descend", + name: "abs symlink: pointing outside is deleted", before: func(dir string) billy.Filesystem { cwd := filepath.Join(dir, "current-dir") outsideFile := filepath.Join(dir, "outside-cwd/file") @@ -825,16 +945,20 @@ func TestRemove(t *testing.T) { require.NoError(t, err) err = os.Symlink(outsideFile, filepath.Join(cwd, "remove-abs-symlink")) require.NoError(t, err) - return newBoundOS(cwd, true) + return newBoundOS(cwd) + }, + after: func(t *testing.T, dir string) { + t.Helper() + _, err := os.Stat(filepath.Join(dir, "outside-cwd/file")) + require.NoError(t, err) }, filename: "remove-abs-symlink", - wantErr: notFoundError(), }, { - name: "rel symlink: pointing outside is forced to descend", + name: "rel symlink: pointing outside is deleted", before: func(dir string) billy.Filesystem { cwd := filepath.Join(dir, "current-dir") - outsideFile := filepath.Join(dir, "outside-cwd", "file2") + outsideFile := filepath.Join(dir, "outside-cwd", "file") err := os.Mkdir(cwd, 0o700) require.NoError(t, err) @@ -842,18 +966,22 @@ func TestRemove(t *testing.T) { require.NoError(t, err) err = os.WriteFile(outsideFile, []byte("anything"), 0o600) require.NoError(t, err) - err = os.Symlink(filepath.Join("..", "outside-cwd", "file2"), filepath.Join(cwd, "remove-abs-symlink2")) + err = os.Symlink(filepath.Join("..", "outside-cwd", "file"), filepath.Join(cwd, "remove-rel-symlink")) + require.NoError(t, err) + return newBoundOS(cwd) + }, + after: func(t *testing.T, dir string) { + t.Helper() + _, err := os.Stat(filepath.Join(dir, "outside-cwd/file")) require.NoError(t, err) - return newBoundOS(cwd, true) }, filename: "remove-rel-symlink", - wantErr: notFoundError(), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) if tt.before != nil { fs = tt.before(dir) @@ -865,11 +993,15 @@ func TestRemove(t *testing.T) { } err := fs.Remove(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) } else { require.NoError(t, err) } + + if tt.after != nil { + tt.after(t, dir) + } }) } } @@ -888,7 +1020,7 @@ func TestRemoveAll(t *testing.T) { t.Helper() err := os.MkdirAll(filepath.Join(dir, "parent", "children"), 0o700) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "parent", }, @@ -902,7 +1034,7 @@ func TestRemoveAll(t *testing.T) { t.Helper() err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "test-file", }, @@ -915,7 +1047,7 @@ func TestRemoveAll(t *testing.T) { require.NoError(t, err) err = os.Symlink(target, filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", }, @@ -925,9 +1057,10 @@ func TestRemoveAll(t *testing.T) { t.Helper() err := os.WriteFile(filepath.Join(dir, "rel-above-cwd"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "../../rel-above-cwd", + wantErr: ErrPathEscapesParent.Error(), }, { name: "abs file", @@ -935,7 +1068,7 @@ func TestRemoveAll(t *testing.T) { t.Helper() err := os.WriteFile(filepath.Join(dir, "abs-test-file"), []byte("anything"), 0o600) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "abs-test-file", makeAbs: true, @@ -946,7 +1079,7 @@ func TestRemoveAll(t *testing.T) { t.Helper() err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) require.NoError(t, err) - return newBoundOS(dir, true) + return newBoundOS(dir) }, filename: "symlink", makeAbs: true, @@ -956,7 +1089,7 @@ func TestRemoveAll(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) + fs, ok := newBoundOS(dir).(*BoundOS) assert.True(ok) if tt.before != nil { @@ -1008,7 +1141,7 @@ func TestJoin(t *testing.T) { for _, tt := range tests { t.Run(tt.wanted, func(t *testing.T) { assert := assert.New(t) - fs := newBoundOS(t.TempDir(), true) + fs := newBoundOS(t.TempDir()) got := fs.Join(tt.elems...) assert.Equal(tt.wanted, got) @@ -1016,198 +1149,10 @@ func TestJoin(t *testing.T) { } } -func TestAbs(t *testing.T) { - tests := []struct { - name string - cwd string - filename string - makeAbs bool - expected string - makeExpectedAbs bool - wantErr string - deduplicatePath bool - before func(dir string) - }{ - { - name: "path: same dir rel file", - cwd: "/working/dir", - filename: "./file", - expected: filepath.FromSlash("/working/dir/file"), - }, - { - name: "path: descending rel file", - cwd: "/working/dir", - filename: "file", - expected: filepath.FromSlash("/working/dir/file"), - }, - { - name: "path: ascending rel file 1", - cwd: "/working/dir", - filename: "../file", - expected: filepath.FromSlash("/working/dir/file"), - }, - { - name: "path: ascending rel file 2", - cwd: "/working/dir", - filename: "../../file", - expected: filepath.FromSlash("/working/dir/file"), - }, - { - name: "path: ascending rel file 3", - cwd: "/working/dir", - filename: "/../../file", - expected: filepath.FromSlash("/working/dir/file"), - }, - { - name: "path: abs file within cwd", - cwd: filepath.FromSlash("/working/dir"), - filename: filepath.FromSlash("/working/dir/abs-file"), - expected: filepath.FromSlash("/working/dir/abs-file"), - deduplicatePath: true, - }, - { - name: "path: abs file within cwd wo deduplication", - cwd: filepath.FromSlash("/working/dir"), - filename: filepath.FromSlash("/working/dir/abs-file"), - expected: filepath.FromSlash("/working/dir/working/dir/abs-file"), - }, - { - name: "path: abs file within cwd", - cwd: "/working/dir", - filename: "/outside/dir/abs-file", - expected: filepath.FromSlash("/working/dir/outside/dir/abs-file"), - }, - { - name: "abs symlink: within cwd w abs descending target", - filename: "ln-cwd-cwd", - makeAbs: true, - expected: "within-cwd", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink(filepath.Join(dir, "within-cwd"), filepath.Join(dir, "ln-cwd-cwd")) - require.NoError(t, err) - }, - deduplicatePath: true, - }, - { - name: "abs symlink: within cwd w rel descending target", - filename: "ln-rel-cwd-cwd", - makeAbs: true, - expected: "within-cwd", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink("within-cwd", filepath.Join(dir, "ln-rel-cwd-cwd")) - require.NoError(t, err) - }, - deduplicatePath: true, - }, - { - name: "abs symlink: within cwd w abs ascending target", - filename: "ln-cwd-up", - makeAbs: true, - expected: "/some/outside/dir", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink("/some/outside/dir", filepath.Join(dir, "ln-cwd-up")) - require.NoError(t, err) - }, - deduplicatePath: true, - }, - { - name: "abs symlink: within cwd w rel ascending target", - filename: "ln-rel-cwd-up", - makeAbs: true, - expected: "outside-cwd", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink("../../outside-cwd", filepath.Join(dir, "ln-rel-cwd-up")) - require.NoError(t, err) - }, - deduplicatePath: true, - }, - { - name: "rel symlink: within cwd w abs descending target", - filename: "ln-cwd-cwd", - expected: "within-cwd", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink(filepath.Join(dir, "within-cwd"), filepath.Join(dir, "ln-cwd-cwd")) - require.NoError(t, err) - }, - deduplicatePath: true, - }, - { - name: "rel symlink: within cwd w rel descending target", - filename: "ln-rel-cwd-cwd2", - expected: "within-cwd", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink("within-cwd", filepath.Join(dir, "ln-rel-cwd-cwd2")) - require.NoError(t, err) - }, - }, - { - name: "rel symlink: within cwd w abs ascending target", - filename: "ln-cwd-up2", - expected: "/outside/path/up", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink("/outside/path/up", filepath.Join(dir, "ln-cwd-up2")) - require.NoError(t, err) - }, - }, - { - name: "rel symlink: within cwd w rel ascending target", - filename: "ln-rel-cwd-up2", - expected: "outside", - makeExpectedAbs: true, - before: func(dir string) { - err := os.Symlink("../../../../outside", filepath.Join(dir, "ln-rel-cwd-up2")) - require.NoError(t, err) - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - cwd := tt.cwd - if cwd == "" { - cwd = t.TempDir() - } - - fs, ok := newBoundOS(cwd, tt.deduplicatePath).(*BoundOS) - assert.True(ok) - - if tt.before != nil { - tt.before(cwd) - } - - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(cwd, filename) - } - - expected := tt.expected - if tt.makeExpectedAbs { - expected = filepath.Join(cwd, expected) - } - - got, err := fs.abs(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - - assert.Equal(expected, got) - }) - } -} - func TestReadDir(t *testing.T) { assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) f, err := os.Create(filepath.Join(dir, "file1")) require.NoError(t, err) @@ -1232,35 +1177,25 @@ func TestReadDir(t *testing.T) { err = os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink")) require.NoError(t, err) dirs, err = fs.ReadDir("symlink") - require.ErrorContains(t, err, notFoundError()) + require.ErrorIs(t, err, ErrPathEscapesParent) assert.Nil(dirs) } -func TestInsideBaseDirEval(t *testing.T) { - assert := assert.New(t) - - fs := BoundOS{baseDir: "/"} - b, err := fs.insideBaseDirEval("a") - assert.True(b) - require.NoError(t, err) - - fs = BoundOS{baseDir: ""} - b, err = fs.insideBaseDirEval(filepath.Join("a", "b", "c")) - assert.True(b) - require.NoError(t, err) -} - func TestMkdirAll(t *testing.T) { assert := assert.New(t) root := t.TempDir() cwd := filepath.Join(root, "cwd") + + err := os.MkdirAll(cwd, 0o700) + require.NoError(t, err) + target := "abc" targetAbs := filepath.Join(cwd, target) - fs := newBoundOS(cwd, true) + fs := newBoundOS(cwd) // Even if CWD is changed outside of the fs instance, // the base dir must still be observed. - err := os.Chdir(os.TempDir()) + err = os.Chdir(os.TempDir()) require.NoError(t, err) err = fs.MkdirAll(target, 0o700) @@ -1276,7 +1211,7 @@ func TestMkdirAll(t *testing.T) { require.NoError(t, err) err = fs.MkdirAll(filepath.Join(cwd, "symlink", "new-dir"), 0o700) - require.NoError(t, err) + require.ErrorIs(t, err, ErrPathEscapesParent) // For windows, the volume name must be removed from the path or // it will lead to an invalid path. @@ -1284,13 +1219,13 @@ func TestMkdirAll(t *testing.T) { root = root[len(vol):] } - mustExist(filepath.Join(cwd, root, "outside", "new-dir")) + fi, _ = os.Stat(filepath.Join(root, "outside", "new-dir")) + require.Nil(t, fi, "dir must not be created") } func TestRename(t *testing.T) { - assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) oldFile := "old-file" newFile := filepath.Join("newdir", "newfile") @@ -1312,12 +1247,11 @@ func TestRename(t *testing.T) { di, err := os.Stat(filepath.Dir(filepath.Join(dir, newFile))) require.NoError(t, err) - assert.NotNil(di) + assert.NotNil(t, di) expected := 0o775 actual := int(di.Mode().Perm()) - assert.Equal( - expected, actual, "Permission mismatch - expected: 0o%o, actual: 0o%o", expected, actual, - ) + assert.Equal(t, expected, actual, + "Permission mismatch - expected: 0o%o, actual: 0o%o", expected, actual) } else { err = fs.Rename(oldFile, newFile) require.NoError(t, err) @@ -1325,27 +1259,11 @@ func TestRename(t *testing.T) { fi, err := os.Stat(filepath.Join(dir, newFile)) require.NoError(t, err) - assert.NotNil(fi) + assert.NotNil(t, fi) err = fs.Rename(filepath.FromSlash("/tmp/outside/cwd/file1"), newFile) - require.ErrorIs(t, err, os.ErrNotExist) + assert.ErrorIs(t, err, ErrPathEscapesParent) err = fs.Rename(oldFile, filepath.FromSlash("/tmp/outside/cwd/file2")) - require.ErrorIs(t, err, os.ErrNotExist) -} - -func mustExist(filename string) { - fi, err := os.Stat(filename) - if err != nil || fi == nil { - panic(fmt.Sprintf("file %s should exist", filename)) - } -} - -func notFoundError() string { - switch runtime.GOOS { - case "windows": - return "The system cannot find the " // {path,file} specified - default: - return "no such file or directory" - } + assert.ErrorIs(t, err, ErrPathEscapesParent) } diff --git a/osfs/os_options.go b/osfs/os_options.go index 2f235c6..99b971d 100644 --- a/osfs/os_options.go +++ b/osfs/os_options.go @@ -1,3 +1,28 @@ package osfs type Option func(*options) + +// WithBoundOS returns the option of using a Bound filesystem OS. +func WithBoundOS() Option { + return func(o *options) { + o.Type = BoundOSFS + } +} + +// WithChrootOS returns the option of using a Chroot filesystem OS. +func WithChrootOS() Option { + return func(o *options) { + o.Type = ChrootOSFS + } +} + +type options struct { + Type +} + +type Type int + +const ( + ChrootOSFS Type = iota + BoundOSFS +)