diff --git a/.github/workflows/go-git-integration.yml b/.github/workflows/go-git-integration.yml new file mode 100644 index 0000000..3257bf4 --- /dev/null +++ b/.github/workflows/go-git-integration.yml @@ -0,0 +1,114 @@ +on: + pull_request: + workflow_dispatch: + +name: Go Git Integration +permissions: + contents: read + +jobs: + test: + name: ${{ matrix.os }} / Go stable + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: + - ubuntu-latest + - macos-latest + - windows-latest + defaults: + run: + shell: bash + steps: + - name: Checkout go-billy + uses: actions/checkout@v4 + with: + path: go-billy + + - name: Select go-git branch + id: target + run: | + set -e + module_path="$(awk '$1 == "module" { print $2 }' go-billy/go.mod)" + + case "${module_path}" in + github.com/go-git/go-billy/v6) + go_git_ref="main" + ;; + github.com/go-git/go-billy/v5) + go_git_ref="releases/v5.x" + ;; + *) + echo "::error::unsupported go-billy module path: ${module_path}" + exit 1 + ;; + esac + + echo "go_billy_module=${module_path}" >> "${GITHUB_OUTPUT}" + echo "go_git_ref=${go_git_ref}" >> "${GITHUB_OUTPUT}" + echo "Testing ${module_path} against go-git ${go_git_ref}" + + - name: Checkout go-git + uses: actions/checkout@v4 + with: + repository: go-git/go-git + ref: ${{ steps.target.outputs.go_git_ref }} + path: go-git + fetch-depth: 0 + + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version: stable + cache-dependency-path: | + go-billy/go.sum + go-git/**/go.sum + + - name: Use local go-billy + run: | + set -e + go_billy_dir="$(cd go-billy && pwd)" + go_git_dir="$(cd go-git && pwd)" + go_billy_module="${{ steps.target.outputs.go_billy_module }}" + + if command -v cygpath >/dev/null 2>&1; then + go_billy="$(cygpath -m "${go_billy_dir}")" + go_git="$(cygpath -m "${go_git_dir}")" + else + go_billy="${go_billy_dir}" + go_git="${go_git_dir}" + fi + + cd go-git + go_git_module="$(awk '$1 == "module" { print $2 }' go.mod)" + go mod edit -replace "${go_billy_module}=${go_billy}" + go mod tidy + + if [ -f cli/go-git/go.mod ]; then + cd cli/go-git + go mod edit -replace "${go_git_module}=${go_git}" + go mod edit -replace "${go_billy_module}=${go_billy}" + go mod tidy + fi + + - name: Configure known hosts + continue-on-error: true + if: matrix.os != 'ubuntu-latest' + run: | + mkdir -p ~/.ssh + ssh-keyscan -H github.com > ~/.ssh/known_hosts + + - name: Set Git config + run: | + git config --global user.email "gha@example.com" + git config --global user.name "GitHub Actions" + + - name: Test go-git + working-directory: go-git + run: go test ./... + + - name: Test go-git CLI + if: ${{ hashFiles('go-git/cli/go-git/go.mod') != '' }} + working-directory: go-git/cli/go-git + run: go test ./... diff --git a/embedfs/embed.go b/embedfs/embed.go index 978c0d0..a5519e3 100644 --- a/embedfs/embed.go +++ b/embedfs/embed.go @@ -14,10 +14,15 @@ import ( "github.com/go-git/go-billy/v6" ) +// Embed is a read-only billy.Filesystem backed by an [embed.FS]. Write +// operations return [billy.ErrReadOnly]; symlink, chroot, lstat and tempfile +// operations return [billy.ErrNotSupported]. type Embed struct { underlying iofs.FS } +// New returns a read-only [billy.Filesystem] backed by efs. A nil efs is +// treated as an empty [embed.FS]. func New(efs *embed.FS) billy.Filesystem { fs := &Embed{ underlying: efs, @@ -30,6 +35,9 @@ func New(efs *embed.FS) billy.Filesystem { return fs } +// Root returns the empty string. [Embed] has no notion of a base directory: +// paths are passed straight through to the wrapped [embed.FS], which uses +// forward-slash paths rooted at the embedding package. func (fs *Embed) Root() string { return "" } diff --git a/go.mod b/go.mod index 7f1daef..99c099e 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.25.0 require ( - github.com/cyphar/filepath-securejoin v0.6.1 github.com/stretchr/testify v1.11.1 golang.org/x/sys v0.44.0 ) diff --git a/go.sum b/go.sum index fac3532..c10d524 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/helper/chroot/chroot.go b/helper/chroot/chroot.go index f7618e9..f98c762 100644 --- a/helper/chroot/chroot.go +++ b/helper/chroot/chroot.go @@ -23,9 +23,9 @@ type ChrootHelper struct { //nolint const maxFollowedSymlinks = 8 // Aligns with POSIX_SYMLOOP_MAX -// New creates a new filesystem wrapping up the given 'fs'. -// The created filesystem has its base in the given ChrootHelperectory of the -// underlying filesystem. +// New creates a new filesystem wrapping the given 'fs', rooted at base. +// All paths passed to the returned filesystem are joined with base before +// being forwarded to the underlying filesystem. func New(fs billy.Basic, base string) billy.Filesystem { return &ChrootHelper{ underlying: polyfill.New(fs), @@ -358,6 +358,11 @@ func (fs *ChrootHelper) Lstat(filename string) (os.FileInfo, error) { return sl.Lstat(fullpath) } +// Symlink creates link with the given target. Slashes in target are +// normalised to the host separator. Absolute targets are rewritten to be +// rooted under the chroot base before being stored, so that the link is +// resolvable when the chroot is reopened from a different working directory. +// Relative targets are stored as provided. func (fs *ChrootHelper) Symlink(target, link string) error { target = filepath.FromSlash(target) @@ -379,6 +384,15 @@ func (fs *ChrootHelper) Symlink(target, link string) error { return sl.Symlink(target, link) } +// Readlink returns the target stored for link. +// +// Relative targets are returned unchanged, preserving the original separators +// as written by [ChrootHelper.Symlink] or the underlying filesystem. Absolute +// targets that resolve under the chroot base are translated back to be +// absolute relative to the chroot (using the host path separator), so callers +// see paths in the chroot's coordinate system rather than the underlying +// filesystem's. Absolute targets that resolve outside the base are returned +// as written. func (fs *ChrootHelper) Readlink(link string) (string, error) { fullpath, err := fs.underlyingPath(link) if err != nil { @@ -395,8 +409,16 @@ func (fs *ChrootHelper) Readlink(link string) (string, error) { return "", err } + rawTarget := target + target = filepath.FromSlash(target) + if filepath.Separator == '\\' && len(target) >= 3 && + target[0] == '\\' && target[2] == ':' && + filepath.VolumeName(fs.base) != "" { + target = target[1:] + } + if !filepath.IsAbs(target) && !strings.HasPrefix(target, string(filepath.Separator)) { - return target, nil + return rawTarget, nil } target, err = filepath.Rel(fs.base, target) diff --git a/helper/chroot/chroot_test.go b/helper/chroot/chroot_test.go index 60dc545..65d2240 100644 --- a/helper/chroot/chroot_test.go +++ b/helper/chroot/chroot_test.go @@ -135,7 +135,7 @@ func TestOpenFile(t *testing.T) { assert.Equal(t, f.Name(), filepath.Join("bar", "qux")) assert.Len(t, m.OpenFileArgs, 1) - assert.Equal(t, m.OpenFileArgs[0], [3]interface{}{"/foo/bar/qux", 42, os.FileMode(0o777)}) + assert.Equal(t, m.OpenFileArgs[0], [3]any{"/foo/bar/qux", 42, os.FileMode(0o777)}) } func TestOpenFileErrCrossedBoundary(t *testing.T) { @@ -268,7 +268,7 @@ func TestMkDirAll(t *testing.T) { require.NoError(t, err) assert.Len(t, m.MkdirAllArgs, 1) - assert.Equal(t, m.MkdirAllArgs[0], [2]interface{}{"/foo/bar", os.FileMode(0o777)}) + assert.Equal(t, m.MkdirAllArgs[0], [2]any{"/foo/bar", os.FileMode(0o777)}) } func TestMkdirAllErrCrossedBoundary(t *testing.T) { @@ -376,6 +376,63 @@ func TestReadlinkWithRelative(t *testing.T) { assert.Equal(t, m.ReadlinkArgs[0], "/foo/qux/bar") } +func TestReadlinkPreservesRelativeTarget(t *testing.T) { + m := &readlinkTargetMock{target: "qux/bar"} + + fs := New(m, "/foo") + link, err := fs.Readlink("link") + require.NoError(t, err) + assert.Equal(t, "qux/bar", link) +} + +func TestReadlinkNormalizesSlashAbsoluteTarget(t *testing.T) { + m := &readlinkTargetMock{target: "/foo/qux/bar"} + + fs := New(m, "/foo") + link, err := fs.Readlink("link") + require.NoError(t, err) + assert.Equal(t, string(os.PathSeparator)+filepath.Join("qux", "bar"), link) +} + +func TestReadlinkNormalizesSlashPrefixedWindowsDriveTarget(t *testing.T) { + if filepath.Separator != '\\' { + t.Skip("windows-only path form") + } + + m := &readlinkTargetMock{target: `/C:/repo/qux/bar`} + + fs := New(m, `C:\repo`) + link, err := fs.Readlink("link") + require.NoError(t, err) + assert.Equal(t, `\`+filepath.Join("qux", "bar"), link) +} + +func TestReadlinkPreservesDriveLookingTargetUnderWindowsSeparatorRoot(t *testing.T) { + if filepath.Separator != '\\' { + t.Skip("windows-only path form") + } + + m := &readlinkTargetMock{target: `\c:\test\123`} + + fs := New(m, `\`) + link, err := fs.Readlink("link") + require.NoError(t, err) + assert.Equal(t, `\c:\test\123`, link) +} + +func TestReadlinkNormalizesSlashAbsoluteTargetUnderWindowsSeparatorRoot(t *testing.T) { + if filepath.Separator != '\\' { + t.Skip("windows-only path form") + } + + m := &readlinkTargetMock{target: `/dir/file`} + + fs := New(m, `\`) + link, err := fs.Readlink("link") + require.NoError(t, err) + assert.Equal(t, `\`+filepath.Join("dir", "file"), link) +} + func TestReadlinkErrCrossedBoundary(t *testing.T) { m := &test.SymlinkMock{} @@ -407,3 +464,13 @@ func testCapabilities(t *testing.T, basic billy.Basic) { assert.Equal(t, capabilities, baseCapabilities) } + +type readlinkTargetMock struct { + test.SymlinkMock + target string +} + +func (fs *readlinkTargetMock) Readlink(link string) (string, error) { + fs.ReadlinkArgs = append(fs.ReadlinkArgs, link) + return fs.target, nil +} diff --git a/helper/mount/mount_test.go b/helper/mount/mount_test.go index aeced03..c59d91b 100644 --- a/helper/mount/mount_test.go +++ b/helper/mount/mount_test.go @@ -113,7 +113,7 @@ func TestOpenFile(t *testing.T) { assert.Len(t, underlying.OpenFileArgs, 1) assert.Equal(t, underlying.OpenFileArgs[0], - [3]interface{}{filepath.Join("bar", "qux"), 42, os.FileMode(0o777)}) + [3]any{filepath.Join("bar", "qux"), 42, os.FileMode(0o777)}) assert.Empty(t, source.OpenFileArgs) } @@ -133,7 +133,7 @@ func TestOpenFileInMount(t *testing.T) { assert.Empty(t, underlying.OpenFileArgs) assert.Len(t, source.OpenFileArgs, 1) assert.Equal(t, source.OpenFileArgs[0], - [3]interface{}{filepath.Join("bar", "qux"), 42, os.FileMode(0o777)}) + [3]any{filepath.Join("bar", "qux"), 42, os.FileMode(0o777)}) } func TestStat(t *testing.T) { @@ -266,7 +266,7 @@ func TestMkdirAll(t *testing.T) { assert.Len(t, underlying.MkdirAllArgs, 1) assert.Equal(t, underlying.MkdirAllArgs[0], - [2]interface{}{filepath.Join("bar", "qux"), os.FileMode(0o777)}) + [2]any{filepath.Join("bar", "qux"), os.FileMode(0o777)}) assert.Empty(t, source.MkdirAllArgs) } @@ -278,7 +278,7 @@ func TestMkdirAllInMount(t *testing.T) { assert.Empty(t, underlying.MkdirAllArgs) assert.Len(t, source.MkdirAllArgs, 1) assert.Equal(t, source.MkdirAllArgs[0], - [2]interface{}{filepath.Join("bar", "qux"), os.FileMode(0o777)}) + [2]any{filepath.Join("bar", "qux"), os.FileMode(0o777)}) } func TestLstat(t *testing.T) { diff --git a/internal/test/mock.go b/internal/test/mock.go index 0cc4aa6..7f96a15 100644 --- a/internal/test/mock.go +++ b/internal/test/mock.go @@ -21,7 +21,7 @@ func (l *CallLogger) Log(call string, args string) { type BasicMock struct { CreateArgs []string OpenArgs []string - OpenFileArgs [][3]interface{} + OpenFileArgs [][3]any StatArgs []string RenameArgs [][2]string RemoveArgs []string @@ -40,7 +40,7 @@ func (fs *BasicMock) Open(filename string) (billy.File, error) { } func (fs *BasicMock) OpenFile(filename string, flag int, mode fs.FileMode) (billy.File, error) { - fs.OpenFileArgs = append(fs.OpenFileArgs, [3]interface{}{filename, flag, mode}) + fs.OpenFileArgs = append(fs.OpenFileArgs, [3]any{filename, flag, mode}) return &FileMock{name: filename, callLogger: &fs.CallLogger}, nil } @@ -77,7 +77,7 @@ func (fs *TempFileMock) TempFile(dir, prefix string) (billy.File, error) { type DirMock struct { BasicMock ReadDirArgs []string - MkdirAllArgs [][2]interface{} + MkdirAllArgs [][2]any } func (fs *DirMock) ReadDir(path string) ([]fs.DirEntry, error) { @@ -86,7 +86,7 @@ func (fs *DirMock) ReadDir(path string) ([]fs.DirEntry, error) { } func (fs *DirMock) MkdirAll(filename string, perm fs.FileMode) error { - fs.MkdirAllArgs = append(fs.MkdirAllArgs, [2]interface{}{filename, perm}) + fs.MkdirAllArgs = append(fs.MkdirAllArgs, [2]any{filename, perm}) return nil } diff --git a/memfs/file.go b/memfs/file.go index 950150c..c46bdfb 100644 --- a/memfs/file.go +++ b/memfs/file.go @@ -160,7 +160,7 @@ func (fi *fileInfo) IsDir() bool { return fi.mode.IsDir() } -func (*fileInfo) Sys() interface{} { +func (*fileInfo) Sys() any { return nil } diff --git a/memfs/memory_test.go b/memfs/memory_test.go index 3af2a9a..c07d52f 100644 --- a/memfs/memory_test.go +++ b/memfs/memory_test.go @@ -120,7 +120,7 @@ func TestOrder(t *testing.T) { } attempts := 30 - for n := 0; n < attempts; n++ { + for range attempts { actual, err := fs.ReadDir("") require.NoError(t, err) @@ -589,7 +589,7 @@ func TestThreadSafety(t *testing.T) { wg.Done() } - for i := 0; i < files; i++ { + for i := range files { wg.Add(4) go fnc(i, "a", false) diff --git a/osfs/errors.go b/osfs/errors.go new file mode 100644 index 0000000..223e601 --- /dev/null +++ b/osfs/errors.go @@ -0,0 +1,12 @@ +package osfs + +import "errors" + +// ErrPathEscapesParent represents when an action leads to escaping from the +// dir the filesystem is bound to. +// +// On non-js builds this aligns with [os.Root]'s containment guarantees; the +// upstream error returned by os.Root is not exported, so this sentinel is +// exposed instead. On js/wasm the symbol is defined for API parity but is +// not returned by the in-memory implementation. +var ErrPathEscapesParent = errors.New("path escapes from parent") diff --git a/osfs/os.go b/osfs/os.go index f69d851..26263a2 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -4,7 +4,6 @@ package osfs import ( - "fmt" "io" "io/fs" "os" @@ -19,80 +18,47 @@ const ( ) // Default Filesystem representing the root of the os filesystem. -var Default = newBoundOS(string(os.PathSeparator), true) +var Default = newBoundOS(string(os.PathSeparator)) -// New returns a new OS filesystem. -// By default paths are deduplicated, but still enforced -// under baseDir. For more info refer to WithDeduplicatePath. +// New returns a new OS filesystem rooted at baseDir. +// +// The returned filesystem is always a [BoundOS]: containment is enforced +// via [os.Root], opened and closed per operation. For better performance +// with caller-managed lifecycle, use [FromRoot] instead. +// +// All [Option] values are accepted for API compatibility but have no +// effect on the returned implementation. func New(baseDir string, opts ...Option) billy.Filesystem { - o := &options{ - deduplicatePath: true, - } + o := &options{} for _, opt := range opts { opt(o) } - return newBoundOS(baseDir, o.deduplicatePath) + return newBoundOS(baseDir) } -// WithBoundOS returns the option of using a Bound filesystem OS. +// WithBoundOS selects the [BoundOS] implementation. +// +// [BoundOS] is the only OS-backed implementation returned by [New], so this +// option is the default and is kept for API compatibility. func WithBoundOS() Option { return func(o *options) { o.Type = BoundOSFS } } -// 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 identifies an osfs implementation. type Type int const ( - ChrootOSFS Type = iota - BoundOSFS + // BoundOSFS selects the [BoundOS] implementation. + BoundOSFS Type = iota ) -func tempFile(dir, prefix, name string) (billy.File, error) { - f, err := os.CreateTemp(dir, prefix) - if err != nil { - return nil, err - } - return &file{File: f, name: name}, nil -} - -func openFile(fn, name string, flag int, perm fs.FileMode, createDir func(string) error) (billy.File, error) { - if flag&os.O_CREATE != 0 { - if createDir == nil { - return nil, fmt.Errorf("createDir func cannot be nil if file needs to be opened in create mode") - } - if err := createDir(fn); err != nil { - return nil, err - } - } - - f, err := os.OpenFile(fn, flag, perm) - if err != nil { - return nil, err - } - return &file{File: f, name: name}, err -} - // file is a wrapper for an os.File which adds support for file locking. type file struct { *os.File diff --git a/osfs/os_bench_test.go b/osfs/os_bench_test.go index 443a418..e2ab497 100644 --- a/osfs/os_bench_test.go +++ b/osfs/os_bench_test.go @@ -1,3 +1,5 @@ +//go:build !wasm + package osfs_test import ( @@ -8,6 +10,7 @@ import ( "testing" "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/helper/iofs" "github.com/go-git/go-billy/v6/memfs" "github.com/go-git/go-billy/v6/osfs" "github.com/go-git/go-billy/v6/util" @@ -16,93 +19,203 @@ import ( const fileName = "foo.bar" -func BenchmarkOpen(b *testing.B) { +func mustFromRoot(tb testing.TB, root *os.Root) billy.Filesystem { + tb.Helper() + fs, err := osfs.FromRoot(root) + require.NoError(tb, err) + return fs +} + +type benchEnv struct { + baseDir string + root *os.Root + mem billy.Filesystem + bound billy.Filesystem + rootFS billy.Filesystem +} + +func newBenchEnv(b *testing.B, withFile bool) benchEnv { + b.Helper() b.StopTimer() + baseDir := b.TempDir() root, err := os.OpenRoot(baseDir) require.NoError(b, err) - - osfn := filepath.Join(baseDir, fileName) - - err = os.WriteFile(osfn, []byte("test"), 0o600) - require.NoError(b, err) + b.Cleanup(func() { root.Close() }) m := memfs.New() - err = util.WriteFile(m, fileName, []byte("test"), 0o600) - require.NoError(b, err) + if withFile { + err = os.WriteFile(filepath.Join(baseDir, fileName), []byte("test"), 0o600) + require.NoError(b, err) + err = util.WriteFile(m, fileName, []byte("test"), 0o600) + require.NoError(b, err) + } + b.StartTimer() + return benchEnv{ + baseDir: baseDir, + root: root, + mem: m, + bound: osfs.New(baseDir, osfs.WithBoundOS()), + rootFS: mustFromRoot(b, root), + } +} - b.Run("memfs", benchmarkOpen(m)) - b.Run("boundOS", benchmarkOpen(osfs.New(baseDir, osfs.WithBoundOS()))) +func BenchmarkOpen(b *testing.B) { + e := newBenchEnv(b, true) + b.Run("memfs", benchmarkOpen(e.mem)) + b.Run("boundOS", benchmarkOpen(e.bound)) + b.Run("rootOS", benchmarkOpen(e.rootFS)) b.Run("go-lib", func(b *testing.B) { for b.Loop() { - _, err := root.Open(fileName) + f, err := e.root.Open(fileName) if err != nil { b.Fatal("cannot open file", "error", err) } + f.Close() } }) } -func BenchmarkReaddir(b *testing.B) { - b.StopTimer() - baseDir := b.TempDir() - osfn := filepath.Join(baseDir, fileName) - - m := memfs.New() - for i := 0; i < 1000; i++ { - err := os.WriteFile(fmt.Sprint(osfn, i), []byte("test"), 0o600) - require.NoError(b, err) +func BenchmarkCreate(b *testing.B) { + e := newBenchEnv(b, false) + b.Run("memfs", benchmarkCreate(e.mem)) + b.Run("boundOS", benchmarkCreate(e.bound)) + b.Run("rootOS", benchmarkCreate(e.rootFS)) + b.Run("go-lib", func(b *testing.B) { + for b.Loop() { + f, err := e.root.Create(fileName) + if err != nil { + b.Fatal("cannot create file", "error", err) + } + f.Close() + } + }) +} - err = util.WriteFile(m, fmt.Sprint(fileName, i), []byte("test"), 0o600) - require.NoError(b, err) - } - b.StartTimer() +func BenchmarkStat(b *testing.B) { + e := newBenchEnv(b, true) + b.Run("memfs", benchmarkStat(e.mem)) + b.Run("boundOS", benchmarkStat(e.bound)) + b.Run("rootOS", benchmarkStat(e.rootFS)) + b.Run("go-lib", func(b *testing.B) { + for b.Loop() { + _, err := e.root.Stat(fileName) + if err != nil { + b.Fatal("cannot stat file", "error", err) + } + } + }) +} - b.Run("memfs", benchmarkReaddir(m, ".")) - b.Run("boundOS", benchmarkReaddir(osfs.New(baseDir, osfs.WithBoundOS()), ".")) +func BenchmarkLstat(b *testing.B) { + e := newBenchEnv(b, true) + b.Run("memfs", benchmarkLstat(e.mem)) + b.Run("boundOS", benchmarkLstat(e.bound)) + b.Run("rootOS", benchmarkLstat(e.rootFS)) b.Run("go-lib", func(b *testing.B) { for b.Loop() { - _, err := os.ReadDir(baseDir) + _, err := e.root.Lstat(fileName) if err != nil { - b.Fatal("cannot read dir", "error", err) + b.Fatal("cannot lstat file", "error", err) } } }) } -func BenchmarkWalkdir(b *testing.B) { +func newBenchEnvMany(b *testing.B, n int) benchEnv { + b.Helper() b.StopTimer() + baseDir := b.TempDir() root, err := os.OpenRoot(baseDir) require.NoError(b, err) - osfn := filepath.Join(baseDir, fileName) + b.Cleanup(func() { root.Close() }) + osfn := filepath.Join(baseDir, fileName) m := memfs.New() - for i := 0; i < 1000; i++ { + for i := range n { err = os.WriteFile(fmt.Sprint(osfn, i), []byte("test"), 0o600) require.NoError(b, err) - err = util.WriteFile(m, fmt.Sprint(fileName, i), []byte("test"), 0o600) require.NoError(b, err) } + b.StartTimer() + return benchEnv{ + baseDir: baseDir, + root: root, + mem: m, + bound: osfs.New(baseDir, osfs.WithBoundOS()), + rootFS: mustFromRoot(b, root), + } +} - b.Run("memfs", benchmarkReaddir(m, ".")) - b.Run("boundOS", benchmarkReaddir(osfs.New(baseDir, osfs.WithBoundOS()), ".")) +func BenchmarkReaddir(b *testing.B) { + e := newBenchEnvMany(b, 1000) + b.Run("memfs", benchmarkReaddir(e.mem, ".")) + b.Run("boundOS", benchmarkReaddir(e.bound, ".")) + b.Run("rootOS", benchmarkReaddir(e.rootFS, ".")) b.Run("go-lib", func(b *testing.B) { for b.Loop() { - i := 0 - err := fs.WalkDir(root.FS(), ".", func(_ string, _ fs.DirEntry, err error) error { - i++ - return err - }) + _, err := os.ReadDir(e.baseDir) if err != nil { - b.Fatal("cannot walk dir", "error", err) + b.Fatal("cannot read dir", "error", err) } + } + }) +} - if i != 1001 { // 1000 files + dir entry - b.Fatal("wrong walk number", "i", i) +func BenchmarkWalkdir(b *testing.B) { + e := newBenchEnvMany(b, 1000) + b.Run("memfs", benchmarkWalkdir(iofs.New(e.mem))) + b.Run("boundOS", benchmarkWalkdir(iofs.New(e.bound))) + b.Run("rootOS", benchmarkWalkdir(iofs.New(e.rootFS))) + b.Run("go-lib", benchmarkWalkdir(e.root.FS())) +} + +func BenchmarkRename(b *testing.B) { + e := newBenchEnv(b, false) + b.Run("memfs", benchmarkRename(e.mem)) + b.Run("boundOS", benchmarkRename(e.bound)) + b.Run("rootOS", benchmarkRename(e.rootFS)) + b.Run("go-lib", func(b *testing.B) { + for b.Loop() { + b.StopTimer() + _ = e.root.Remove("rename-dst") + f, err := e.root.Create("rename-src") + if err != nil { + b.Fatal(err) + } + f.Close() + b.StartTimer() + + err = e.root.Rename("rename-src", "rename-dst") + if err != nil { + b.Fatal("cannot rename file", "error", err) + } + } + }) +} + +func BenchmarkRemove(b *testing.B) { + e := newBenchEnv(b, false) + b.Run("memfs", benchmarkRemove(e.mem)) + b.Run("boundOS", benchmarkRemove(e.bound)) + b.Run("rootOS", benchmarkRemove(e.rootFS)) + b.Run("go-lib", func(b *testing.B) { + for b.Loop() { + b.StopTimer() + f, err := e.root.Create("remove-target") + if err != nil { + b.Fatal(err) + } + f.Close() + b.StartTimer() + + err = e.root.Remove("remove-target") + if err != nil { + b.Fatal("cannot remove file", "error", err) } } }) @@ -111,10 +224,63 @@ func BenchmarkWalkdir(b *testing.B) { func benchmarkOpen(fs billy.Filesystem) func(b *testing.B) { return func(b *testing.B) { for b.Loop() { - _, err := fs.Open(fileName) + f, err := fs.Open(fileName) if err != nil { b.Fatal("cannot open file", "error", err) } + f.Close() + } + } +} + +func benchmarkCreate(fs billy.Filesystem) func(b *testing.B) { + return func(b *testing.B) { + for b.Loop() { + f, err := fs.Create(fileName) + if err != nil { + b.Fatal("cannot create file", "error", err) + } + f.Close() + } + } +} + +func benchmarkStat(fs billy.Filesystem) func(b *testing.B) { + return func(b *testing.B) { + for b.Loop() { + _, err := fs.Stat(fileName) + if err != nil { + b.Fatal("cannot stat file", "error", err) + } + } + } +} + +func benchmarkLstat(fs billy.Filesystem) func(b *testing.B) { + return func(b *testing.B) { + for b.Loop() { + _, err := fs.Lstat(fileName) + if err != nil { + b.Fatal("cannot lstat file", "error", err) + } + } + } +} + +func benchmarkWalkdir(fsys fs.FS) func(b *testing.B) { + return func(b *testing.B) { + for b.Loop() { + i := 0 + err := fs.WalkDir(fsys, ".", func(_ string, _ fs.DirEntry, err error) error { + i++ + return err + }) + if err != nil { + b.Fatal("cannot walk dir", "error", err) + } + if i != 1001 { // 1000 files + dir entry + b.Fatal("wrong walk number", "i", i) + } } } } @@ -132,3 +298,42 @@ func benchmarkReaddir(fs billy.Filesystem, path string) func(b *testing.B) { } } } + +func benchmarkRename(fs billy.Filesystem) func(b *testing.B) { + return func(b *testing.B) { + for b.Loop() { + b.StopTimer() + _ = fs.Remove("rename-dst") + f, err := fs.Create("rename-src") + if err != nil { + b.Fatal(err) + } + f.Close() + b.StartTimer() + + err = fs.Rename("rename-src", "rename-dst") + if err != nil { + b.Fatal("cannot rename file", "error", err) + } + } + } +} + +func benchmarkRemove(fs billy.Filesystem) func(b *testing.B) { + return func(b *testing.B) { + for b.Loop() { + b.StopTimer() + f, err := fs.Create("remove-target") + if err != nil { + b.Fatal(err) + } + f.Close() + b.StartTimer() + + err = fs.Remove("remove-target") + if err != nil { + b.Fatal("cannot remove file", "error", err) + } + } + } +} diff --git a/osfs/os_bound.go b/osfs/os_bound.go index acd114b..6a3d11c 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -19,402 +19,350 @@ package osfs import ( - "io/fs" + "errors" + 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 = dotPathPrefixes() - dotSeparators = dotPathSeparators() -) - -func dotPathPrefixes() []string { - if filepath.Separator == '\\' { - return []string{"./", ".\\"} - } - return []string{"./"} +// BoundOS is a fs implementation based on the OS filesystem which relies on +// Go's [os.Root]. A new [os.Root] is opened and closed for each filesystem +// operation to avoid holding a directory handle open. +// +// For better performance, prefer [RootOS] via [FromRoot] with a +// caller-managed [os.Root]. +// +// Behaviours of note: +// 1. Read and write operations can only be directed to files which descend +// from the base dir. +// 2. Symlink targets are stored verbatim and not rewritten, so they may +// point outside the base dir or to non-existent paths. [BoundOS.Readlink] +// returns the stored target with path separators normalised to forward +// slashes (see [filepath.ToSlash]). +// 3. Operations leading to escapes to outside the [os.Root] location result +// in [ErrPathEscapesParent]. +type BoundOS struct { + baseDir string } -func dotPathSeparators() string { - if filepath.Separator == '\\' { - return `/\` +func newBoundOS(d string) billy.Filesystem { + if d == "" { + d = string(os.PathSeparator) } - return `/` + d = hostPath(d) + return &BoundOS{baseDir: d} } -// BoundOS is a fs implementation based on the OS filesystem which is bound to -// a base dir. -// Prefer this fs implementation over ChrootOS. -// -// 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. -type BoundOS struct { - baseDir string - deduplicatePath bool +// rootFS opens a temporary [RootOS] and returns a cleanup function that +// closes the underlying [os.Root]. +func (fs *BoundOS) rootFS() (*RootOS, func(), error) { + return fs.rootFSWithCreate("", false) } -func newBoundOS(d string, deduplicatePath bool) billy.Filesystem { - return &BoundOS{baseDir: d, deduplicatePath: deduplicatePath} +func (fs *BoundOS) rootFSWithCreate(name string, createBase bool) (*RootOS, func(), error) { + rootPath := fs.operationRoot(name) + r, err := openRootAt(rootPath, createBase) + if err != nil { + return nil, func() {}, err + } + return &RootOS{root: r}, func() { r.Close() }, nil } func (fs *BoundOS) Capabilities() billy.Capability { return boundCapabilities() } -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) { - name := fs.name(filename) - filename = fs.expandDot(filename) - fn, err := fs.abs(filename) +func (fs *BoundOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.File, error) { + rfs, cleanup, err := fs.rootFSWithCreate(name, flag&os.O_CREATE != 0) if err != nil { return nil, err } - - return openFile(fn, name, flag, perm, fs.createDir) + defer cleanup() + return rfs.OpenFile(name, flag, perm) } -func (fs *BoundOS) ReadDir(path string) ([]fs.DirEntry, error) { - path = fs.expandDot(path) - dir, err := fs.abs(path) +func (fs *BoundOS) ReadDir(name string) ([]gofs.DirEntry, error) { + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return nil, err } - - return os.ReadDir(dir) + defer cleanup() + return rfs.ReadDir(name) } func (fs *BoundOS) Rename(from, to string) error { - if fs.isBaseDir(from) { - return billy.ErrBaseDirCannotBeRenamed - } - - f, err := fs.absNoFollow(from) - if err != nil { - return err - } - if _, err := os.Lstat(f); err != nil { - return err - } - - t, err := fs.absNoFollow(to) + rfs, cleanup, err := fs.rootFSWithCreate(from, false) if err != nil { return err } - - // MkdirAll for target name. - if err := fs.createDir(t); err != nil { - return err - } - - return os.Rename(f, t) + defer cleanup() + return rfs.Rename(from, 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, perm gofs.FileMode) error { + rfs, cleanup, err := fs.rootFSWithCreate(name, true) if err != nil { return err } - return os.MkdirAll(dir, perm) + defer cleanup() + return rfs.MkdirAll(name, perm) } -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) { - name := filepath.Base(fs.name(filename)) - filename = fs.expandDot(filename) - filename, err := fs.abs(filename) - if err != nil { - return nil, err +func (fs *BoundOS) Stat(name string) (os.FileInfo, error) { + if fs.isBaseDir(name) { + return fs.baseInfo(false) } - fi, err := os.Stat(filename) + + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return nil, err } - return fileInfo{FileInfo: fi, name: name}, nil + defer cleanup() + return rfs.Stat(name) } -func (fs *BoundOS) Remove(filename string) error { - if fs.isBaseDir(filename) { - return billy.ErrBaseDirCannotBeRemoved - } - - fn, err := fs.absNoFollow(filename) +func (fs *BoundOS) Remove(name string) error { + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return err } - return os.Remove(fn) + defer cleanup() + return rfs.Remove(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. func (fs *BoundOS) TempFile(dir, prefix string) (billy.File, error) { - name := "" - if dir != "" { - name = fs.name(fs.Join(dir, prefix)) - } - if dir != "" { - var err error - dir = fs.expandDot(dir) - 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 - } - } - } - - f, err := tempFile(dir, prefix, "") - if err != nil { - return nil, err - } - if name != "" { - if osFile, ok := f.(*file); ok { - osFile.name = fs.Join(filepath.Dir(name), filepath.Base(osFile.File.Name())) - } - } - return f, nil + 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 fs.isBaseDir(path) { - return billy.ErrBaseDirCannotBeRemoved - } - - dir, err := fs.absNoFollow(path) +func (fs *BoundOS) RemoveAll(name string) error { + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return err } - return os.RemoveAll(dir) + defer cleanup() + return rfs.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 { + rfs, cleanup, err := fs.rootFSWithCreate(newname, true) if err != nil { return err } - // MkdirAll for containing dir. - if err := fs.createDir(ln); err != nil { - return err - } - return os.Symlink(target, ln) + defer cleanup() + return rfs.Symlink(oldname, newname) } -func (fs *BoundOS) name(p string) string { - name := fs.rootRelative(p) - if name == "" { - return "." +func (fs *BoundOS) Lstat(name string) (os.FileInfo, error) { + if fs.isBaseDir(name) { + return fs.baseInfo(true) } - return name -} -func (fs *BoundOS) expandDot(p string) string { - if p == "." { - return fs.baseDir - } - for _, prefix := range dotPrefixes { - if strings.HasPrefix(p, prefix) { - p = strings.TrimLeft(strings.TrimPrefix(p, prefix), dotSeparators) - if p == "" { - return fs.baseDir - } - return p - } + rfs, cleanup, err := fs.rootFSWithCreate(name, false) + if err != nil { + return nil, err } - return p + defer cleanup() + return rfs.Lstat(name) } -func (fs *BoundOS) isBaseDir(path string) bool { - if path == "" || filepath.Clean(path) == "." { - return true - } - path = fs.expandDot(path) - if filepath.Clean(path) == filepath.Clean(fs.baseDir) { - return true +func (fs *BoundOS) Readlink(name string) (string, error) { + rfs, cleanup, err := fs.rootFSWithCreate(name, false) + if err != nil { + return "", err } - abspath, err := fs.abs(path) + defer cleanup() + return rfs.Readlink(name) +} + +func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { + rfs, cleanup, err := fs.rootFSWithCreate(path, false) if err != nil { - return false + return err } - return filepath.Clean(abspath) == filepath.Clean(fs.baseDir) + defer cleanup() + return rfs.Chmod(path, mode) } -func (fs *BoundOS) absNoFollow(filename string) (string, error) { - if fs.baseDir == "" { - return filepath.Clean(fs.expandDot(filename)), nil +// 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) { + if hostPath, ok := fs.hostAbsolutePath(path); ok { + return newBoundOS(hostPath), nil } - rel := fs.rootRelative(filename) - if rel == "" { - return fs.baseDir, nil + rfs, cleanup, err := fs.rootFS() + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return newBoundOS(fs.chrootPath(path)), nil + } + return nil, err } + defer cleanup() - parent, err := fs.secureJoin(filepath.Dir(rel)) + rel := rfs.toRelative(path) + if rel == "" { + rel = "." + } + childRoot, err := rfs.root.OpenRoot(rel) if err != nil { - return "", err + if errors.Is(err, os.ErrNotExist) { + return newBoundOS(fs.chrootPath(path)), nil + } + return nil, err } + defer childRoot.Close() - return filepath.Join(parent, filepath.Base(rel)), nil + return newBoundOS(filepath.Clean(childRoot.Name())), nil +} + +// Root returns the current base dir of the billy.Filesystem. +func (fs *BoundOS) Root() string { + return fs.baseDir } -func (fs *BoundOS) rootRelative(filename string) string { - filename = fs.expandDot(filename) - filename = filepath.Clean(filename) - if filepath.Clean(filename) == filepath.Clean(fs.baseDir) { - return "" +func (fs *BoundOS) isBaseDir(name string) bool { + if name == "" { + return true } - if filepath.IsAbs(filename) { - if isLocalToBase(fs.baseDir, filename) { - rel, _ := filepath.Rel(fs.baseDir, filename) - return cleanUnderRoot(rel) + + name = hostPath(name) + if filepath.IsAbs(name) { + if rel, ok := relativeInsideBase(fs.baseDir, name); ok { + return cleanUnderRoot(rel) == "" } - return cleanUnderRoot(filename) } - return cleanUnderRoot(filename) -} -func isLocalToBase(base, name string) bool { - rel, err := filepath.Rel(base, name) - return err == nil && (rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)))) + return cleanUnderRoot(name) == "" } -func cleanUnderRoot(filename string) string { - vol := filepath.VolumeName(filename) - filename = filename[len(vol):] - filename = filepath.Join(string(filepath.Separator), filename) - return strings.TrimLeft(filename, string(filepath.Separator)) -} +func (fs *BoundOS) chrootPath(path string) string { + if hostPath, ok := fs.hostAbsolutePath(path); ok { + return filepath.Clean(hostPath) + } -func (fs *BoundOS) Lstat(filename string) (os.FileInfo, error) { - filename, err := fs.absNoFollow(filename) - if err != nil { - return nil, err + path = hostPath(path) + if filepath.IsAbs(path) { + if rel, ok := relativeInsideBase(fs.baseDir, path); ok { + return filepath.Clean(filepath.Join(fs.baseDir, rel)) + } } - return os.Lstat(filename) + + return filepath.Clean(filepath.Join(fs.baseDir, cleanUnderRoot(path))) } -func (fs *BoundOS) Readlink(link string) (string, error) { - link, err := fs.absNoFollow(link) - if err != nil { - return "", err +func (fs *BoundOS) hostAbsolutePath(path string) (string, bool) { + if fs.baseDir != string(os.PathSeparator) { + return "", false } - return os.Readlink(link) -} -func (fs *BoundOS) secureJoin(path string) (string, error) { - if filepath.Separator != '\\' { - return securejoin.SecureJoin(fs.baseDir, path) + path = hostPath(path) + if filepath.VolumeName(path) != "" && filepath.IsAbs(path) { + return path, true } - return securejoin.SecureJoinVFS(fs.baseDir, path, boundOSVFS{}) + + return "", false } -type boundOSVFS struct{} +func (fs *BoundOS) operationRoot(path string) string { + if hostPath, ok := fs.hostAbsolutePath(path); ok { + vol := filepath.VolumeName(hostPath) + if vol != "" { + return vol + string(filepath.Separator) + } + } -func (boundOSVFS) Lstat(name string) (os.FileInfo, error) { - return os.Lstat(name) + return fs.baseDir } -func (boundOSVFS) Readlink(name string) (string, error) { - target, err := os.Readlink(name) - if err != nil { - return "", err - } - if filepath.Separator == '\\' && filepath.VolumeName(target) == "" && (strings.HasPrefix(target, `\`) || strings.HasPrefix(target, `/`)) { - if vol := filepath.VolumeName(name); vol != "" { - return vol + filepath.FromSlash(target), nil +func (fs *BoundOS) baseInfo(lstat bool) (os.FileInfo, error) { + base := filepath.Clean(fs.baseDir) + parent := filepath.Dir(base) + name := filepath.Base(base) + + if parent == base { + root, err := openRootAt(base, false) + if err != nil { + return nil, err } + defer root.Close() + if lstat { + return root.Lstat(".") + } + return root.Stat(".") } - return target, nil -} -func (fs *BoundOS) Chmod(path string, mode fs.FileMode) error { - abspath, err := fs.abs(path) + root, err := openRootAt(parent, false) if err != nil { - return err + return nil, err } - return os.Chmod(abspath, mode) + defer root.Close() + if lstat { + return root.Lstat(name) + } + return root.Stat(name) } -// 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 := fs.secureJoin(path) +func openRootAt(path string, create bool) (*os.Root, error) { + path = filepath.Clean(path) + root, err := os.OpenRoot(path) + if err == nil || !create || !errors.Is(err, os.ErrNotExist) { + return root, err + } + + ancestor, rel, err := openExistingAncestor(path) if err != nil { return nil, err } - return &BoundOS{ - baseDir: joined, - deduplicatePath: fs.deduplicatePath, - }, nil -} - -// Root returns the current base dir of the billy.Filesystem. -// This is required in order for this implementation to be a drop-in -// replacement for other upstream implementations (e.g. memory and osfs). -func (fs *BoundOS) Root() string { - return fs.baseDir -} + defer ancestor.Close() -func (fs *BoundOS) createDir(fullpath string) error { - dir := filepath.Dir(fullpath) - if dir != "." { - if err := os.MkdirAll(dir, defaultDirectoryMode); err != nil { - return err + if rel != "." { + if err := ancestor.MkdirAll(rel, defaultDirectoryMode); err != nil { + return nil, err } } - return nil + return os.OpenRoot(path) } -// 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) - } +func openExistingAncestor(path string) (*os.Root, string, error) { + path = filepath.Clean(path) + ancestorPath := path - path, err := fs.secureJoin(filename) - if err != nil { - return "", err - } + for { + root, err := os.OpenRoot(ancestorPath) + if err == nil { + rel, relErr := filepath.Rel(ancestorPath, path) + if relErr != nil { + root.Close() + return nil, "", relErr + } + return root, rel, nil + } + if !errors.Is(err, os.ErrNotExist) { + return nil, "", err + } - 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):]) + parent := filepath.Dir(ancestorPath) + if parent == ancestorPath { + return nil, "", err } + ancestorPath = parent } - return path, nil } diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 6be22e4..ca58b5d 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -19,6 +19,7 @@ package osfs import ( + "errors" "fmt" "os" "path/filepath" @@ -26,20 +27,95 @@ import ( "strings" "testing" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestBoundOSCapabilities(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) - _, ok := fs.(billy.Capable) - assert.True(t, ok) + fs := newBoundOS(dir) + c, ok := fs.(billy.Capable) + require.True(t, ok) + + assert.Equal(t, boundCapabilities(), c.Capabilities()) +} + +func TestFromRoot(t *testing.T) { + t.Parallel() + + t.Run("valid root", func(t *testing.T) { + t.Parallel() + root, err := os.OpenRoot(t.TempDir()) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, root.Close()) }) + + fs, err := FromRoot(root) + require.NoError(t, err) + assert.IsType(t, &RootOS{}, fs) + assert.Equal(t, root.Name(), fs.Root()) + + f, err := fs.Create("test-file") + require.NoError(t, err) + require.NoError(t, f.Close()) + + _, err = fs.Stat("test-file") + require.NoError(t, err) + }) + + t.Run("nil root", func(t *testing.T) { + t.Parallel() + _, err := FromRoot(nil) + require.Error(t, err) + }) + + t.Run("closed root", func(t *testing.T) { + t.Parallel() + root, err := os.OpenRoot(t.TempDir()) + require.NoError(t, err) + require.NoError(t, root.Close()) + + fs, err := FromRoot(root) + require.NoError(t, err) + assert.Equal(t, root.Name(), fs.Root()) + + _, err = fs.Stat(".") + require.ErrorContains(t, err, "file already closed") + }) +} + +// TestOpenAbsSymlinkInsideRoot verifies that Open can follow a symlink whose +// target is an absolute path pointing inside the root. os.Root rejects such +// symlinks because the absolute target appears to escape the root. BoundOS +// detects this, resolves the link to a relative path, and retries. +func TestOpenAbsSymlinkInsideRoot(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + absTarget := filepath.Join(dir, "target") + require.NoError(t, os.WriteFile(absTarget, []byte("content"), 0o600)) + require.NoError(t, os.Symlink(absTarget, filepath.Join(dir, "link"))) + + // Prove os.Root alone rejects the absolute symlink target. + root, err := os.OpenRoot(dir) + require.NoError(t, err) + t.Cleanup(func() { root.Close() }) + + _, err = root.Open("link") + require.Error(t, err, "os.Root.Open must reject an absolute symlink target inside the root") + require.ErrorContains(t, err, ErrPathEscapesParent.Error()) - caps := billy.Capabilities(fs) - assert.Equal(t, billy.AllCapabilities, caps) + // BoundOS resolves the absolute target to a relative path and succeeds. + bfs := newBoundOS(dir) + f, err := bfs.Open("link") + require.NoError(t, err) + + got := make([]byte, 7) + _, err = f.Read(got) + require.NoError(t, err) + assert.Equal(t, "content", string(got)) + require.NoError(t, f.Close()) } func TestOpen(t *testing.T) { @@ -47,117 +123,95 @@ func TestOpen(t *testing.T) { name string filename string makeAbs bool - before func(dir string) billy.Filesystem - wantErr string + before func(t *testing.T, dir string) billy.Filesystem + wantErr error }{ { name: "file: rel same dir", - 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) + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600)) + return newBoundOS(dir) }, filename: "test-file", }, { - name: "file: dot rel same dir", - 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) + name: "file: absolute fs path", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "abs-test-file"), []byte("anything"), 0o600)) + return newBoundOS(dir) }, - filename: "./test-file", + filename: "/abs-test-file", }, { - name: "file: dot rel same dir without deduplication", - before: func(dir string) billy.Filesystem { - err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) - require.NoError(t, err) - return newBoundOS(dir, false) + name: "file: host absolute path inside root", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "abs-test-file"), []byte("anything"), 0o600)) + return newBoundOS(dir) }, - filename: "./test-file", + filename: "abs-test-file", + makeAbs: true, }, { - name: "file: rel path to above cwd", - 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) + name: "file: parent traversal clamps to root", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "rel-above-cwd"), []byte("anything"), 0o600)) + return newBoundOS(dir) }, filename: "../../rel-above-cwd", }, { - name: "file: rel path to below cwd", - before: func(dir string) billy.Filesystem { - err := os.Mkdir(filepath.Join(dir, "sub"), 0o700) - 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) - }, - filename: "sub/rel-below-cwd", - }, - { - name: "file: abs inside cwd", - 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) - }, - filename: "abs-test-file", - makeAbs: true, - }, - { - name: "file: abs outside cwd", - before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) + name: "symlink: absolute target inside root", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + target := filepath.Join(dir, "target-file") + require.NoError(t, os.WriteFile(target, []byte("anything"), 0o600)) + require.NoError(t, os.Symlink(target, filepath.Join(dir, "symlink"))) + return newBoundOS(dir) }, - filename: "/some/path/outside/cwd", - wantErr: notFoundError(), + filename: "symlink", }, { - name: "symlink: same dir", - 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) + name: "symlink: relative target inside root", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "target-file"), []byte("anything"), 0o600)) + require.NoError(t, os.Symlink("target-file", filepath.Join(dir, "symlink"))) + return newBoundOS(dir) }, filename: "symlink", }, { - 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) + name: "symlink: absolute target outside root", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink"))) + return newBoundOS(dir) }, filename: "symlink", - makeAbs: true, - wantErr: notFoundError(), + wantErr: ErrPathEscapesParent, }, { - 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) + name: "symlink: relative target outside root", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.Symlink("../../../../../../outside/cwd", filepath.Join(dir, "symlink"))) + return newBoundOS(dir) }, filename: "symlink", - makeAbs: true, - wantErr: notFoundError(), + 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) + fs = tt.before(t, dir) } filename := tt.filename @@ -165,15 +219,16 @@ func TestOpen(t *testing.T) { filename = filepath.Join(dir, filename) } - fi, err := fs.Open(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - assert.Nil(fi) - } else { - require.NoError(t, err) - assert.NotNil(fi) - require.NoError(t, fi.Close()) + f, err := fs.Open(filename) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + assert.Nil(t, f) + return } + + require.NoError(t, err) + require.NotNil(t, f) + require.NoError(t, f.Close()) }) } } @@ -184,7 +239,7 @@ func TestOpenPreservesBackslashFilenamesOnNonWindows(t *testing.T) { } dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) tests := []struct { filename string @@ -195,1522 +250,439 @@ func TestOpenPreservesBackslashFilenamesOnNonWindows(t *testing.T) { } for _, tt := range tests { t.Run(tt.openPath, func(t *testing.T) { - err := os.WriteFile(filepath.Join(dir, tt.filename), []byte("anything"), 0o600) - require.NoError(t, err) - - fi, err := fs.Open(tt.openPath) - require.NoError(t, err) - require.NotNil(t, fi) - require.NoError(t, fi.Close()) - }) - } -} - -func TestWindowsOpenBackslashDotPaths(t *testing.T) { - if filepath.Separator != '\\' { - t.Skip("backslash is only a path separator on windows") - } - - dir := t.TempDir() - fs := newBoundOS(dir, true) + require.NoError(t, os.WriteFile(filepath.Join(dir, tt.filename), []byte("anything"), 0o600)) - err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) - require.NoError(t, err) - - tests := []struct { - name string - openPath string - wantName string - }{ - {name: "dot backslash file", openPath: `.\test-file`, wantName: "test-file"}, - {name: "dot backslash parent", openPath: `.\..`, wantName: "."}, - {name: "absolute parent", openPath: `\..`, wantName: "."}, - {name: "absolute child parent", openPath: `\foo\..`, wantName: "."}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { f, err := fs.Open(tt.openPath) require.NoError(t, err) require.NotNil(t, f) - assert.Equal(t, filepath.Clean(tt.wantName), filepath.Clean(f.Name())) require.NoError(t, f.Close()) }) } } -func Test_Symlink(t *testing.T) { - // The umask value set at OS level can impact this test, so - // it is set to 0 during the duration of this test and then - // reverted back to the original value. - // Outside of linux this is a no-op. +func TestSymlink(t *testing.T) { defer umask(0)() tests := []struct { - name string - link string - target string - before func(dir string) billy.Filesystem - wantStatErr string + name string + link string + target string + makeAbs bool }{ - { - name: "link to abs valid target", - link: "symlink", - target: filepath.FromSlash("/etc/passwd"), - }, - { - name: "dot link to abs valid target", - link: "./symlink", - target: filepath.FromSlash("/etc/passwd"), - }, - { - name: "link to abs inexistent target", - link: "symlink", - target: filepath.FromSlash("/some/random/path"), - }, - { - name: "link to rel valid target", - link: "symlink", - target: filepath.FromSlash("../../../../../../../../../etc/passwd"), - }, - { - name: "link to rel inexistent target", - link: "symlink", - target: filepath.FromSlash("../../../some/random/path"), - }, - { - name: "auto create dir", - link: "new-dir/symlink", - target: filepath.FromSlash("../../../some/random/path"), - }, - { - name: "keep dir filemode if exists", - link: "new-dir/symlink", - before: func(dir string) billy.Filesystem { - err := os.Mkdir(filepath.Join(dir, "new-dir"), 0o701) - require.NoError(t, err) - return newBoundOS(dir, true) - }, - target: filepath.FromSlash("../../../some/random/path"), - }, + {name: "link to abs valid target", 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", target: filepath.FromSlash("/etc/passwd")}, + {name: "auto create dir", link: "new-dir/symlink", target: filepath.FromSlash("../../../some/random/path")}, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) - - if tt.before != nil { - fs = tt.before(dir) - } - - // Even if CWD is changed outside of the fs instance, - // the base dir must still be observed. - err := os.Chdir(os.TempDir()) - require.NoError(t, err) - - link := filepath.Join(dir, tt.link) - - diBefore, _ := os.Lstat(filepath.Dir(link)) + fs := newBoundOS(dir) - err = fs.Symlink(tt.target, tt.link) - require.NoError(t, err) - - fi, err := os.Lstat(link) - if tt.wantStatErr != "" { - require.ErrorContains(t, err, tt.wantStatErr) - } else { - require.NoError(t, err) - assert.NotNil(fi) + link := tt.link + if tt.makeAbs { + link = filepath.Join(dir, tt.link) } - got, err := os.Readlink(link) - require.NoError(t, err) - assert.Equal(tt.target, got) + require.NoError(t, fs.Symlink(tt.target, link)) - diAfter, err := os.Lstat(filepath.Dir(link)) + got, err := os.Readlink(filepath.Join(dir, tt.link)) require.NoError(t, err) - - if diBefore != nil { - assert.Equal(diBefore.Mode(), diAfter.Mode()) - } + assert.Equal(t, tt.target, got) }) } } func TestTempFile(t *testing.T) { - assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) f, err := fs.TempFile("", "prefix") require.NoError(t, err) - assert.NotNil(f) - assert.Contains(f.Name(), os.TempDir()) + assert.True(t, strings.HasPrefix(f.Name(), filepath.Join(".tmp", "prefix")), f.Name()) 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("above", "cwd", "prefix")) + assert.True(t, strings.HasPrefix(f.Name(), filepath.Join("above", "cwd", "prefix")), f.Name()) require.NoError(t, f.Close()) - dir = os.TempDir() - // For windows, volume name must be removed. - if v := filepath.VolumeName(dir); v != "" { - dir = strings.TrimPrefix(dir, v) - } + f, err = fs.TempFile("../../../above/cwd", "prefix") + require.NoError(t, err) + assert.True(t, strings.HasPrefix(f.Name(), filepath.Join("above", "cwd", "prefix")), f.Name()) + require.NoError(t, f.Close()) +} - f, err = fs.TempFile(dir, "prefix") +func TestDefaultTempFileNameCanBeReopened(t *testing.T) { + f, err := Default.TempFile("", "go-billy-") require.NoError(t, err) - assert.NotNil(f) - assert.Contains(f.Name(), filepath.Join(strings.TrimLeft(dir, `/\`), "prefix")) + + name := f.Name() + require.True(t, filepath.IsAbs(hostPath(name)), name) require.NoError(t, f.Close()) + t.Cleanup(func() { _ = Default.Remove(name) }) + + reopened, err := Default.Open(name) + require.NoError(t, err) + require.NoError(t, reopened.Close()) } func TestChroot(t *testing.T) { - assert := assert.New(t) tmp := t.TempDir() - fs := newBoundOS(tmp, true) + fs := newBoundOS(tmp) + + chrooted, err := fs.Chroot("test") + require.NoError(t, err) + assert.IsType(t, &BoundOS{}, chrooted) + assert.Equal(t, filepath.Join(tmp, "test"), chrooted.Root()) +} + +func TestChrootMissingPathDoesNotCreate(t *testing.T) { + tmp := t.TempDir() + fs := newBoundOS(tmp) + + chrooted, err := fs.Chroot("missing") + require.NoError(t, err) + assert.Equal(t, filepath.Join(tmp, "missing"), chrooted.Root()) + + _, err = os.Stat(filepath.Join(tmp, "missing")) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func TestChrootDotKeepsCleanRoot(t *testing.T) { + tmp := t.TempDir() + fs := newBoundOS(tmp) + + chrooted, err := fs.Chroot(".") + require.NoError(t, err) + assert.Equal(t, tmp, chrooted.Root()) +} + +func TestChrootClampsParentTraversalToChildRoot(t *testing.T) { + tmp := t.TempDir() + fs := newBoundOS(tmp) + require.NoError(t, util.WriteFile(fs, "bar", []byte("root"), 0o644)) + require.NoError(t, util.WriteFile(fs, "foo/bar", []byte("chroot"), 0o644)) + + chrooted, err := fs.Chroot("foo") + require.NoError(t, err) - f, err := fs.Chroot("test") + got, err := util.ReadFile(chrooted, "../bar") require.NoError(t, err) - assert.NotNil(f) - assert.Equal(filepath.Join(tmp, "test"), f.Root()) - assert.IsType(&BoundOS{}, f) + assert.Equal(t, []byte("chroot"), got) } func TestRoot(t *testing.T) { - assert := assert.New(t) dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) + + assert.Equal(t, dir, fs.Root()) +} + +func TestStatBaseFile(t *testing.T) { + dir := t.TempDir() + name := filepath.Join(dir, "file") + require.NoError(t, os.WriteFile(name, []byte("content"), 0o600)) + + fs := newBoundOS(name) + for _, p := range []string{"", ".", "./", "./.", "/"} { + t.Run(fmt.Sprintf("path=%q", p), func(t *testing.T) { + fi, err := fs.Stat(p) + require.NoError(t, err) + assert.False(t, fi.IsDir()) + }) + } +} + +func TestEmptyBaseUsesOSRoot(t *testing.T) { + dir := t.TempDir() + name := filepath.Join(dir, "file") + require.NoError(t, os.WriteFile(name, []byte("content"), 0o600)) + + fs := newBoundOS("") + assert.Equal(t, string(os.PathSeparator), fs.Root()) + + f, err := fs.Open(name) + require.NoError(t, err) + require.NoError(t, f.Close()) +} + +func TestMkdirAllCreatesMissingBaseDir(t *testing.T) { + base := filepath.Join(t.TempDir(), "repo.git") + fs := newBoundOS(base) + + require.NoError(t, fs.MkdirAll(filepath.Join("objects", "info"), 0o700)) + mustExist(t, filepath.Join(base, "objects", "info")) +} + +func TestCreateCreatesMissingBaseDir(t *testing.T) { + base := filepath.Join(t.TempDir(), "repo.git") + fs := newBoundOS(base) + + f, err := fs.Create("config") + require.NoError(t, err) + require.NoError(t, f.Close()) + mustExist(t, filepath.Join(base, "config")) +} + +func TestCreateCreatesMissingBaseDirAncestors(t *testing.T) { + root := t.TempDir() + base := filepath.Join(root, "missing", "parents", "repo.git") + fs := newBoundOS(base) + + _, err := os.Stat(filepath.Join(root, "missing")) + require.ErrorIs(t, err, os.ErrNotExist) + + f, err := fs.Create(filepath.Join("objects", "info", "alternates")) + require.NoError(t, err) + require.NoError(t, f.Close()) + + mustExist(t, filepath.Join(base, "objects", "info", "alternates")) +} + +func TestReadlink(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) + target := filepath.FromSlash("/etc/passwd") + require.NoError(t, os.Symlink(target, filepath.Join(dir, "symlink"))) + + got, err := fs.Readlink("symlink") + require.NoError(t, err) + assert.Equal(t, filepath.ToSlash(target), got) + + _, err = fs.Readlink("../../symlink") + require.NoError(t, err) +} + +func TestLstat(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) + require.NoError(t, os.Symlink("target-file", filepath.Join(dir, "link"))) + + fi, err := fs.Lstat("link") + require.NoError(t, err) + assert.Equal(t, "link", fi.Name()) + assert.NotZero(t, fi.Mode()&os.ModeSymlink) +} + +func TestStatPreventsSymlinkEscape(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) + require.NoError(t, os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink"))) - root := fs.Root() - assert.Equal(dir, root) + fi, err := fs.Stat("symlink") + require.ErrorIs(t, err, ErrPathEscapesParent) + assert.Nil(t, fi) } -func TestReadLink(t *testing.T) { +func TestRemove(t *testing.T) { tests := []struct { - name string - filename string - makeAbs bool - expected string - makeExpectedAbs bool - before func(dir string) billy.Filesystem - wantErr string + name string + filename string + before func(t *testing.T, dir string) billy.Filesystem + 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) - }, - filename: "symlink", - expected: filepath.FromSlash("/etc/passwd"), - }, - { - name: "file: rel parent traversal is clamped to cwd", - filename: "../../file", - wantErr: notFoundError(), - }, - { - 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) - }, - filename: "symlink", - makeAbs: true, - expected: filepath.FromSlash("/etc/passwd"), + name: "inexistent dir", + filename: "inexistent", + wantErr: os.ErrNotExist, }, { - name: "symlink: dir pointing outside cwd", - before: func(dir string) billy.Filesystem { - cwd := filepath.Join(dir, "current-dir") - outside := filepath.Join(dir, "outside-cwd") - - err := os.Mkdir(cwd, 0o700) - require.NoError(t, err) - err = os.Mkdir(outside, 0o700) - require.NoError(t, err) - - err = os.Symlink(outside, filepath.Join(cwd, "symlink")) - require.NoError(t, err) - err = os.WriteFile(filepath.Join(outside, "file"), []byte("anything"), 0o600) - require.NoError(t, err) - - return newBoundOS(cwd, true) - }, - filename: "current-dir/symlink/file", - makeAbs: true, - wantErr: notFoundError(), + name: "base dir as dot", + filename: ".", + wantErr: billy.ErrBaseDirCannotBeRemoved, }, { - name: "symlink: within cwd + baseDir symlink", - before: func(dir string) billy.Filesystem { - cwd := filepath.Join(dir, "symlink-dir") - cwdAlt := filepath.Join(dir, "symlink-altdir") - cwdTarget := filepath.Join(dir, "cwd-target") - - err := os.MkdirAll(cwdTarget, 0o700) - require.NoError(t, err) - - err = os.WriteFile(filepath.Join(cwdTarget, "file"), []byte{}, 0o600) - require.NoError(t, err) - err = os.Symlink(cwdTarget, cwd) - require.NoError(t, err) - err = os.Symlink(cwdTarget, cwdAlt) - require.NoError(t, err) - err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(cwdAlt, "symlink-file")) - require.NoError(t, err) - return newBoundOS(cwd, true) + name: "same dir file", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600)) + return newBoundOS(dir) }, - filename: "symlink-file", - expected: filepath.FromSlash("cwd-target/file"), - makeExpectedAbs: true, + filename: "test-file", }, { - name: "symlink: outside cwd + baseDir symlink", - before: func(dir string) billy.Filesystem { //nolint - cwd := filepath.Join(dir, "symlink-dir") - outside := filepath.Join(cwd, "symlink-outside") - cwdTarget := filepath.Join(dir, "cwd-target") - outsideDir := filepath.Join(dir, "outside") - - err := os.Mkdir(cwdTarget, 0o700) - require.NoError(t, err) - err = os.Mkdir(outsideDir, 0o700) - require.NoError(t, err) - - err = os.WriteFile(filepath.Join(cwdTarget, "file"), []byte{}, 0o600) - require.NoError(t, err) - err = os.Symlink(cwdTarget, cwd) - require.NoError(t, err) - err = os.Symlink(outsideDir, outside) - require.NoError(t, err) - err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(outside, "symlink-file")) - require.NoError(t, err) - return newBoundOS(cwd, true) + name: "symlink is removed without touching target", + before: func(t *testing.T, dir string) billy.Filesystem { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(dir, "target-file"), []byte("target"), 0o600)) + require.NoError(t, os.Symlink("target-file", filepath.Join(dir, "link"))) + return newBoundOS(dir) }, - filename: "symlink-outside/symlink-file", - wantErr: notFoundError(), + filename: "link", }, } + 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) + fs = tt.before(t, dir) } - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(dir, filename) + err := fs.Remove(tt.filename) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return } + require.NoError(t, err) + }) + } +} - expected := tt.expected - if tt.makeExpectedAbs { - expected = filepath.Join(dir, expected) - } +func TestRemoveAll(t *testing.T) { + dir := t.TempDir() + fs := newTestBoundOS(t, dir) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "parent", "child"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "parent", "child", "file"), []byte("anything"), 0o600)) - got, err := fs.Readlink(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - assert.Empty(got) - } else { - require.NoError(t, err) - assert.Equal(expected, got) - } + require.NoError(t, fs.RemoveAll("parent")) + _, err := os.Stat(filepath.Join(dir, "parent")) + require.True(t, errors.Is(err, os.ErrNotExist)) +} + +func TestRemoveAllRemovesSymlink(t *testing.T) { + dir := t.TempDir() + fs := newTestBoundOS(t, dir) + require.NoError(t, os.Mkdir(filepath.Join(dir, "target-dir"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "target-dir", "file"), []byte("target"), 0o600)) + require.NoError(t, os.Symlink("target-dir", filepath.Join(dir, "link"))) + + require.NoError(t, fs.RemoveAll("link")) + _, err := os.Lstat(filepath.Join(dir, "link")) + require.ErrorIs(t, err, os.ErrNotExist) + requireFileContents(t, filepath.Join(dir, "target-dir", "file"), []byte("target")) +} + +func TestRemoveBaseDir(t *testing.T) { + tests := []string{"", ".", "./", "/..", "/foo/.."} + + for _, path := range tests { + t.Run(path, func(t *testing.T) { + dir := t.TempDir() + fs := newTestBoundOS(t, dir) + require.ErrorIs(t, fs.Remove(path), billy.ErrBaseDirCannotBeRemoved) + require.ErrorIs(t, fs.RemoveAll(path), billy.ErrBaseDirCannotBeRemoved) + mustExist(t, dir) }) } } -func TestReadlinkReadsSymlink(t *testing.T) { +func TestRemoveBaseDirAbsolutePath(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) - target := filepath.Join(dir, "target-file") - link := filepath.Join(dir, "link") + fs := newTestBoundOS(t, dir) - err := os.WriteFile(target, []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-file", link) - require.NoError(t, err) + require.ErrorIs(t, fs.Remove(dir), billy.ErrBaseDirCannotBeRemoved) + require.ErrorIs(t, fs.RemoveAll(dir), billy.ErrBaseDirCannotBeRemoved) + mustExist(t, dir) +} - got, err := fs.Readlink("link") - require.NoError(t, err) - assert.Equal(t, "target-file", got) - requireFileContents(t, target, []byte("target")) +func TestJoin(t *testing.T) { + fs := newBoundOS(t.TempDir()) + + assert.Equal(t, filepath.FromSlash("/a/b/c"), fs.Join("/a", "b/c")) + assert.Equal(t, "", fs.Join()) } -func TestLstat(t *testing.T) { - tests := []struct { - name string - filename string - makeAbs bool - before func(dir string) billy.Filesystem - wantErr string - }{ - { - 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) - }, - filename: "symlink", - }, - { - 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) - }, - filename: "symlink", - }, - { - name: "abs 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) - }, - filename: "symlink", - makeAbs: true, - }, - { - 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) - }, - filename: "symlink", - makeAbs: false, - }, - { - name: "symlink: within cwd + baseDir symlink", - before: func(dir string) billy.Filesystem { - cwd := filepath.Join(dir, "symlink-dir") - cwdAlt := filepath.Join(dir, "symlink-altdir") - cwdTarget := filepath.Join(dir, "cwd-target") - - err := os.MkdirAll(cwdTarget, 0o700) - require.NoError(t, err) - - err = os.WriteFile(filepath.Join(cwdTarget, "file"), []byte{}, 0o600) - require.NoError(t, err) - err = os.Symlink(cwdTarget, cwd) - require.NoError(t, err) - err = os.Symlink(cwdTarget, cwdAlt) - require.NoError(t, err) - err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(cwdAlt, "symlink-file")) - require.NoError(t, err) - return newBoundOS(cwd, true) - }, - filename: "symlink-file", - makeAbs: false, - }, - { - name: "symlink: outside cwd + baseDir symlink", - before: func(dir string) billy.Filesystem { //nolint - cwd := filepath.Join(dir, "symlink-dir") - outside := filepath.Join(cwd, "symlink-outside") - cwdTarget := filepath.Join(dir, "cwd-target") - outsideDir := filepath.Join(dir, "outside") - - err := os.Mkdir(cwdTarget, 0o700) - require.NoError(t, err) - err = os.Mkdir(outsideDir, 0o700) - require.NoError(t, err) - - err = os.WriteFile(filepath.Join(cwdTarget, "file"), []byte{}, 0o600) - require.NoError(t, err) - err = os.Symlink(cwdTarget, cwd) - require.NoError(t, err) - err = os.Symlink(outsideDir, outside) - require.NoError(t, err) - err = os.Symlink(filepath.Join(cwdTarget, "file"), filepath.Join(outside, "symlink-file")) - require.NoError(t, err) - - return newBoundOS(cwd, true) - }, - filename: "symlink-outside/symlink-file", - makeAbs: false, - wantErr: notFoundError(), - }, - { - name: "path: rel parent traversal is clamped to cwd", - filename: "../../file", - wantErr: notFoundError(), - }, - { - name: "path: abs pointing outside cwd", - filename: "/etc/passwd", - wantErr: notFoundError(), - }, - { - 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) - }, - filename: "test-file", - }, - { - name: "file: dot 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) - }, - filename: "./test-file", - }, - { - name: "file: abs", - 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) - }, - filename: "test-file", - makeAbs: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - dir := t.TempDir() - fs := newBoundOS(dir, true) - - if tt.before != nil { - fs = tt.before(dir) - } - - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(dir, filename) - } - fi, err := fs.Lstat(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - assert.Nil(fi) - } else { - require.NoError(t, err) - assert.NotNil(fi) - assert.Equal(filepath.Base(tt.filename), fi.Name()) - } - }) - } -} - -func TestLstatStatsSymlink(t *testing.T) { - dir := t.TempDir() - fs := newBoundOS(dir, true) - target := filepath.Join(dir, "target-file") - link := filepath.Join(dir, "link") - - err := os.WriteFile(target, []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-file", link) - require.NoError(t, err) - - fi, err := fs.Lstat("link") - require.NoError(t, err) - assert.Equal(t, "link", fi.Name()) - assert.NotZero(t, fi.Mode()&os.ModeSymlink) - requireFileContents(t, target, []byte("target")) -} - -func TestNoFollowOperationsAllowMissingPathUnderSymlinkedBase(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("symlink creation requires additional privileges on windows") - } - - dir := t.TempDir() - target := filepath.Join(dir, "target") - base := filepath.Join(dir, "base") - err := os.Mkdir(target, 0o700) - require.NoError(t, err) - err = os.Symlink(target, base) - require.NoError(t, err) - - fs := newBoundOS(base, true) - - _, err = fs.Lstat("/etc/passwd") - require.ErrorContains(t, err, notFoundError()) - assert.NotContains(t, err.Error(), "path outside base dir") - - err = fs.Remove("/some/path/outside/cwd") - require.ErrorContains(t, err, notFoundError()) - assert.NotContains(t, err.Error(), "path outside base dir") - - err = os.WriteFile(filepath.Join(target, "old-file"), []byte("renamed"), 0o600) - require.NoError(t, err) - err = fs.Rename("old-file", filepath.Join("newdir", "newfile")) - require.NoError(t, err) - requireFileContents(t, filepath.Join(target, "newdir", "newfile"), []byte("renamed")) - mustNotExist(filepath.Join(target, "old-file")) -} - -func TestNoFollowOperationsContainSymlinkedParentOutsideBase(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("symlink creation requires additional privileges on windows") - } - - dir := t.TempDir() - base := filepath.Join(dir, "base") - outside := filepath.Join(dir, "outside") - err := os.Mkdir(base, 0o700) - require.NoError(t, err) - err = os.Mkdir(outside, 0o700) - require.NoError(t, err) - err = os.Symlink(outside, filepath.Join(base, "link")) - require.NoError(t, err) - err = os.WriteFile(filepath.Join(base, "file"), []byte("source"), 0o600) - require.NoError(t, err) - - fs := newBoundOS(base, true) - - _, err = fs.Lstat(filepath.Join("link", "missing", "file")) - require.ErrorContains(t, err, notFoundError()) - assert.NotContains(t, err.Error(), "path outside base dir") - - err = fs.Remove(filepath.Join("link", "missing", "file")) - require.ErrorContains(t, err, notFoundError()) - assert.NotContains(t, err.Error(), "path outside base dir") - - err = fs.Rename("file", filepath.Join("link", "missing", "file")) - require.NoError(t, err) - - parent, err := securejoin.SecureJoin(base, filepath.Join("link", "missing")) - require.NoError(t, err) - requireFileContents(t, filepath.Join(parent, "file"), []byte("source")) - mustNotExist(filepath.Join(base, "file")) - mustNotExist(filepath.Join(outside, "missing", "file")) -} - -func TestStat(t *testing.T) { - tests := []struct { - name string - filename string - makeAbs bool - before func(dir string) billy.Filesystem - wantErr string - }{ - { - 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) - }, - filename: "symlink", - wantErr: notFoundError(), - }, - { - 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) - }, - filename: "symlink", - wantErr: notFoundError(), - }, - - { - name: "abs 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) - }, - filename: "symlink", - makeAbs: true, - wantErr: notFoundError(), - }, - { - 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) - }, - filename: "symlink", - makeAbs: false, - wantErr: notFoundError(), - }, - { - name: "path: rel pointing to abs above cwd", - filename: "../../file", - wantErr: notFoundError(), - }, - { - name: "path: abs pointing outside cwd", - filename: "/etc/passwd", - wantErr: notFoundError(), - }, - { - 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) - }, - filename: "test-file", - }, - { - name: "rel dot dir", - before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) - }, - filename: ".", - }, - { - name: "abs 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) - }, - filename: "test-file", - makeAbs: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - dir := t.TempDir() - fs := newBoundOS(dir, true) - - if tt.before != nil { - fs = tt.before(dir) - } - - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(dir, filename) - } - - fi, err := fs.Stat(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - assert.Nil(fi) - } else { - require.NoError(t, err) - assert.NotNil(fi) - } - }) - } -} - -func TestRemove(t *testing.T) { - tests := []struct { - name string - filename string - makeAbs bool - before func(dir string) billy.Filesystem - wantErr string - }{ - { - name: "path: rel pointing outside cwd w forward slash", - filename: "/some/path/outside/cwd", - wantErr: notFoundError(), - }, - { - name: "path: rel parent traversal is clamped to cwd", - filename: "../../../../path/outside/cwd", - wantErr: notFoundError(), - }, - { - name: "inexistent dir", - before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) - }, - filename: "inexistent", - wantErr: notFoundError(), - }, - { - name: "base dir as empty path", - filename: "", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as dot", - before: func(dir string) billy.Filesystem { - return newBoundOS(dir, true) - }, - filename: ".", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as dot slash", - filename: "./", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as absolute parent", - filename: "/..", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as absolute child parent", - filename: "/foo/..", - wantErr: "base dir cannot be removed", - }, - { - 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) - }, - filename: "test-file", - }, - { - name: "symlink: same dir", - 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) - }, - filename: "symlink", - }, - { - name: "rel parent traversal removes file under cwd", - 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) - }, - filename: "../../rel-above-cwd", - }, - { - 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) - }, - filename: "abs-test-file", - makeAbs: true, - }, - { - name: "abs symlink: pointing outside removes link", - before: func(dir string) billy.Filesystem { - cwd := filepath.Join(dir, "current-dir") - outsideFile := filepath.Join(dir, "outside-cwd/file") - - err := os.Mkdir(cwd, 0o700) - require.NoError(t, err) - err = os.MkdirAll(filepath.Dir(outsideFile), 0o700) - require.NoError(t, err) - err = os.WriteFile(outsideFile, []byte("anything"), 0o600) - require.NoError(t, err) - err = os.Symlink(outsideFile, filepath.Join(cwd, "remove-abs-symlink")) - require.NoError(t, err) - return newBoundOS(cwd, true) - }, - filename: "remove-abs-symlink", - }, - { - name: "rel symlink: pointing outside is forced to descend", - before: func(dir string) billy.Filesystem { - cwd := filepath.Join(dir, "current-dir") - outsideFile := filepath.Join(dir, "outside-cwd", "file2") - - err := os.Mkdir(cwd, 0o700) - require.NoError(t, err) - err = os.MkdirAll(filepath.Dir(outsideFile), 0o700) - 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")) - 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) - - if tt.before != nil { - fs = tt.before(dir) - } - - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(dir, filename) - } - - err := fs.Remove(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestRemoveRemovesSymlink(t *testing.T) { - dir := t.TempDir() - fs := newBoundOS(dir, true) - target := filepath.Join(dir, "target-file") - link := filepath.Join(dir, "link") - - err := os.WriteFile(target, []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-file", link) - require.NoError(t, err) - - err = fs.Remove("link") - require.NoError(t, err) - _, err = os.Lstat(link) - require.ErrorIs(t, err, os.ErrNotExist) - requireFileContents(t, target, []byte("target")) -} - -func TestWindowsRemoveBackslashDotPaths(t *testing.T) { - if filepath.Separator != '\\' { - t.Skip("backslash is only a path separator on windows") - } - - t.Run("dot backslash file", func(t *testing.T) { - dir := t.TempDir() - fs := newBoundOS(dir, true) - - filename := filepath.Join(dir, "test-file") - err := os.WriteFile(filename, []byte("anything"), 0o600) - require.NoError(t, err) - - err = fs.Remove(`.\test-file`) - require.NoError(t, err) - mustNotExist(filename) - }) - - for _, path := range windowsBackslashBaseDirAliases() { - t.Run(path, func(t *testing.T) { - dir := t.TempDir() - fs := newBoundOS(dir, true) - - err := fs.Remove(path) - require.ErrorIs(t, err, billy.ErrBaseDirCannotBeRemoved) - mustExist(dir) - }) - } -} - -func TestRemoveAll(t *testing.T) { - tests := []struct { - name string - filename string - makeAbs bool - before func(t *testing.T, dir string) billy.Filesystem - wantErr string - }{ - { - name: "parent with children", - before: func(t *testing.T, dir string) billy.Filesystem { - t.Helper() - err := os.MkdirAll(filepath.Join(dir, "parent", "children"), 0o700) - require.NoError(t, err) - return newBoundOS(dir, true) - }, - filename: "parent", - }, - { - name: "inexistent dir", - filename: "inexistent", - }, - { - name: "base dir as empty path", - filename: "", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as dot", - filename: ".", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as dot slash", - filename: "./", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as absolute parent", - filename: "/..", - wantErr: "base dir cannot be removed", - }, - { - name: "base dir as absolute child parent", - filename: "/foo/..", - wantErr: "base dir cannot be removed", - }, - { - name: "same dir file", - before: func(t *testing.T, dir string) billy.Filesystem { - t.Helper() - err := os.WriteFile(filepath.Join(dir, "test-file"), []byte("anything"), 0o600) - require.NoError(t, err) - return newBoundOS(dir, true) - }, - filename: "test-file", - }, - { - name: "same dir symlink", - before: func(t *testing.T, dir string) billy.Filesystem { - t.Helper() - 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) - }, - filename: "symlink", - }, - { - name: "rel parent traversal removes file under cwd", - before: func(t *testing.T, dir string) billy.Filesystem { - t.Helper() - err := os.WriteFile(filepath.Join(dir, "rel-above-cwd"), []byte("anything"), 0o600) - require.NoError(t, err) - return newBoundOS(dir, true) - }, - filename: "../../rel-above-cwd", - }, - { - name: "abs file", - before: func(t *testing.T, dir string) billy.Filesystem { - t.Helper() - err := os.WriteFile(filepath.Join(dir, "abs-test-file"), []byte("anything"), 0o600) - require.NoError(t, err) - return newBoundOS(dir, true) - }, - filename: "abs-test-file", - makeAbs: true, - }, - { - name: "abs symlink", - before: func(t *testing.T, dir string) billy.Filesystem { - t.Helper() - err := os.Symlink("/etc/passwd", filepath.Join(dir, "symlink")) - require.NoError(t, err) - return newBoundOS(dir, true) - }, - filename: "symlink", - makeAbs: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - assert.True(ok) - - if tt.before != nil { - fs, ok = tt.before(t, dir).(*BoundOS) - assert.True(ok) - } - - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(dir, filename) - } - - err := fs.RemoveAll(filename) - if tt.wantErr != "" { - require.ErrorContains(t, err, tt.wantErr) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestRemoveAllRemovesSymlink(t *testing.T) { +func TestReadDir(t *testing.T) { dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - require.True(t, ok) - target := filepath.Join(dir, "target-dir") - link := filepath.Join(dir, "link") + fs := newBoundOS(dir) - err := os.Mkdir(target, 0o700) - require.NoError(t, err) - err = os.WriteFile(filepath.Join(target, "file"), []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-dir", link) - require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, "file1"), []byte{}, 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "file2"), []byte{}, 0o600)) - err = fs.RemoveAll("link") + dirs, err := fs.ReadDir(".") require.NoError(t, err) - _, err = os.Lstat(link) - require.ErrorIs(t, err, os.ErrNotExist) - requireFileContents(t, filepath.Join(target, "file"), []byte("target")) -} - -func TestWindowsRemoveAllBackslashDotPaths(t *testing.T) { - if filepath.Separator != '\\' { - t.Skip("backslash is only a path separator on windows") - } - - t.Run("dot backslash file", func(t *testing.T) { - dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - require.True(t, ok) - - filename := filepath.Join(dir, "test-file") - err := os.WriteFile(filename, []byte("anything"), 0o600) - require.NoError(t, err) - - err = fs.RemoveAll(`.\test-file`) - require.NoError(t, err) - mustNotExist(filename) - }) - - for _, path := range windowsBackslashBaseDirAliases() { - t.Run(path, func(t *testing.T) { - dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - require.True(t, ok) - - err := fs.RemoveAll(path) - require.ErrorIs(t, err, billy.ErrBaseDirCannotBeRemoved) - mustExist(dir) - }) - } -} - -func TestRemoveBaseDirAbsolutePath(t *testing.T) { - tests := []struct { - name string - remove func(fs *BoundOS, path string) error - }{ - { - name: "remove", - remove: func(fs *BoundOS, path string) error { - return fs.Remove(path) - }, - }, - { - name: "remove all", - remove: func(fs *BoundOS, path string) error { - return fs.RemoveAll(path) - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - require.True(t, ok) + assert.Len(t, dirs, 2) - err := tt.remove(fs, dir) - require.ErrorIs(t, err, billy.ErrBaseDirCannotBeRemoved) - mustExist(dir) - }) - } + dirs, err = fs.ReadDir("/") + require.NoError(t, err) + assert.Len(t, dirs, 2) } -func TestJoin(t *testing.T) { - tests := []struct { - elems []string - wanted string - }{ - { - elems: []string{}, - wanted: "", - }, - { - elems: []string{"/a", "b", "c"}, - wanted: filepath.FromSlash("/a/b/c"), - }, - { - elems: []string{"/a", "b/c"}, - wanted: filepath.FromSlash("/a/b/c"), - }, - { - elems: []string{"/a", ""}, - wanted: filepath.FromSlash("/a"), - }, - { - elems: []string{"/a", "/", "b"}, - wanted: filepath.FromSlash("/a/b"), - }, - } - for _, tt := range tests { - t.Run(tt.wanted, func(t *testing.T) { - assert := assert.New(t) - fs := newBoundOS(t.TempDir(), true) +func TestReadDirPreventsSymlinkEscape(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) + require.NoError(t, os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink"))) - got := fs.Join(tt.elems...) - assert.Equal(tt.wanted, got) - }) - } + dirs, err := fs.ReadDir("symlink") + require.ErrorIs(t, err, ErrPathEscapesParent) + assert.Nil(t, dirs) } -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) - } +func TestMkdirAll(t *testing.T) { + root := t.TempDir() + cwd := filepath.Join(root, "cwd") + require.NoError(t, os.MkdirAll(cwd, 0o700)) + fs := newBoundOS(cwd) - assert.Equal(expected, got) - }) - } + require.NoError(t, fs.MkdirAll("abc", 0o700)) + mustExist(t, filepath.Join(cwd, "abc")) } -func TestAbsReturnsSecureJoinError(t *testing.T) { +func TestMkdirAllPreventsSymlinkEscape(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink creation requires additional privileges on windows") } - dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - require.True(t, ok) - - err := os.Symlink("loop", filepath.Join(dir, "loop")) - require.NoError(t, err) - - got, err := fs.abs(filepath.Join("loop", "file")) - require.Error(t, err) - assert.Empty(t, got) -} - -func TestReadDir(t *testing.T) { - assert := assert.New(t) - dir := t.TempDir() - fs := newBoundOS(dir, true) - - f, err := os.Create(filepath.Join(dir, "file1")) - require.NoError(t, err) - assert.NotNil(f) - require.NoError(t, f.Close()) - - f, err = os.Create(filepath.Join(dir, "file2")) - require.NoError(t, err) - assert.NotNil(f) - require.NoError(t, f.Close()) - - dirs, err := fs.ReadDir(dir) - require.NoError(t, err) - assert.NotNil(dirs) - assert.Len(dirs, 2) - - dirs, err = fs.ReadDir(".") - require.NoError(t, err) - assert.NotNil(dirs) - assert.Len(dirs, 2) - - 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()) - assert.Nil(dirs) -} - -func TestAbsNoFollowRootRelativePath(t *testing.T) { - root := t.TempDir() - fs, ok := newBoundOS(root, true).(*BoundOS) - require.True(t, ok) - - got, err := fs.absNoFollow(filepath.Join("a", "..", "b", "file")) - require.NoError(t, err) - assert.Equal(t, filepath.Join(root, "b", "file"), got) -} - -func TestMkdirAll(t *testing.T) { - assert := assert.New(t) root := t.TempDir() cwd := filepath.Join(root, "cwd") - target := "abc" - targetAbs := filepath.Join(cwd, target) - fs := newBoundOS(cwd, true) - - // Even if CWD is changed outside of the fs instance, - // the base dir must still be observed. - err := os.Chdir(os.TempDir()) - require.NoError(t, err) - - err = fs.MkdirAll(target, 0o700) - require.NoError(t, err) - - fi, err := os.Stat(targetAbs) - require.NoError(t, err) - assert.NotNil(fi) - - err = os.Mkdir(filepath.Join(root, "outside"), 0o700) - require.NoError(t, err) - err = os.Symlink(filepath.Join(root, "outside"), filepath.Join(cwd, "symlink")) - require.NoError(t, err) - - err = fs.MkdirAll(filepath.Join(cwd, "symlink", "new-dir"), 0o700) - require.NoError(t, err) - - // For windows, the volume name must be removed from the path or - // it will lead to an invalid path. - if vol := filepath.VolumeName(root); vol != "" { - root = root[len(vol):] - } + outside := filepath.Join(root, "outside") + require.NoError(t, os.MkdirAll(cwd, 0o700)) + require.NoError(t, os.Mkdir(outside, 0o700)) + require.NoError(t, os.Symlink(outside, filepath.Join(cwd, "symlink"))) - mustExist(filepath.Join(cwd, root, "outside", "new-dir")) + fs := newBoundOS(cwd) + err := fs.MkdirAll(filepath.Join(cwd, "symlink", "new-dir"), 0o700) + require.ErrorIs(t, err, ErrPathEscapesParent) } 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") - // Even if CWD is changed outside of the fs instance, - // the base dir must still be observed. - err := os.Chdir(os.TempDir()) - require.NoError(t, err) - f, err := fs.Create(oldFile) require.NoError(t, err) require.NoError(t, f.Close()) - if runtime.GOOS != "windows" { - resetUmask := umask(2) - err = fs.Rename(oldFile, newFile) - require.NoError(t, err) - resetUmask() + require.NoError(t, fs.Rename(oldFile, newFile)) + mustExist(t, filepath.Join(dir, newFile)) +} - di, err := os.Stat(filepath.Dir(filepath.Join(dir, newFile))) - require.NoError(t, err) - assert.NotNil(di) - expected := 0o775 - actual := int(di.Mode().Perm()) - assert.Equal( - expected, actual, "Permission mismatch - expected: 0o%o, actual: 0o%o", expected, actual, - ) - } else { - err = fs.Rename(oldFile, newFile) - require.NoError(t, err) - } +func TestRenameAbsoluteSource(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) - fi, err := os.Stat(filepath.Join(dir, newFile)) + f, err := fs.TempFile("", "tmp-") + require.NoError(t, err) + _, err = f.Write([]byte("data")) require.NoError(t, err) - assert.NotNil(fi) + require.NoError(t, f.Close()) - err = fs.Rename(filepath.FromSlash("/tmp/outside/cwd/file1"), newFile) - require.ErrorIs(t, err, os.ErrNotExist) + dst := filepath.Join("objects", "a8", "finalfile") + require.NoError(t, fs.Rename(f.Name(), dst)) - err = fs.Rename(oldFile, filepath.FromSlash("/tmp/outside/cwd/file2")) - require.ErrorIs(t, err, os.ErrNotExist) + got, err := os.ReadFile(filepath.Join(dir, dst)) + require.NoError(t, err) + assert.Equal(t, "data", string(got)) } func TestRenameRenamesSymlink(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) target := filepath.Join(dir, "target-file") link := filepath.Join(dir, "link") renamed := filepath.Join(dir, "renamed-link") - err := os.WriteFile(target, []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-file", link) - require.NoError(t, err) + require.NoError(t, os.WriteFile(target, []byte("target"), 0o600)) + require.NoError(t, os.Symlink("target-file", link)) - err = fs.Rename("link", "renamed-link") - require.NoError(t, err) - _, err = os.Lstat(link) + require.NoError(t, fs.Rename("link", "renamed-link")) + _, err := os.Lstat(link) require.ErrorIs(t, err, os.ErrNotExist) fi, err := os.Lstat(renamed) @@ -1723,63 +695,41 @@ func TestRenameRenamesSymlink(t *testing.T) { } func TestRenameBaseDir(t *testing.T) { - dir := t.TempDir() - fs := newBoundOS(dir, true) - - for _, from := range []string{"", ".", "./", "/..", "/foo/..", dir} { - err := fs.Rename(from, "renamed") - require.ErrorIs(t, err, billy.ErrBaseDirCannotBeRenamed) - mustExist(dir) - } -} - -func TestWindowsRenameBackslashDotPaths(t *testing.T) { - if filepath.Separator != '\\' { - t.Skip("backslash is only a path separator on windows") - } - - t.Run("dot backslash file", func(t *testing.T) { - dir := t.TempDir() - fs := newBoundOS(dir, true) - - oldPath := filepath.Join(dir, "test-file") - newPath := filepath.Join(dir, "renamed") - err := os.WriteFile(oldPath, []byte("anything"), 0o600) - require.NoError(t, err) - - err = fs.Rename(`.\test-file`, "renamed") - require.NoError(t, err) - mustNotExist(oldPath) - mustExist(newPath) - }) - - for _, path := range windowsBackslashBaseDirAliases() { - t.Run(path, func(t *testing.T) { + for _, from := range []string{"", ".", "./", "/..", "/foo/.."} { + t.Run(from, func(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) - err := fs.Rename(path, "renamed") + err := fs.Rename(from, "renamed") require.ErrorIs(t, err, billy.ErrBaseDirCannotBeRenamed) - mustExist(dir) + mustExist(t, dir) }) } } -func windowsBackslashBaseDirAliases() []string { - return []string{`.\..`, `\..`, `\foo\..`} -} - -func mustExist(filename string) { - fi, err := os.Stat(filename) - if err != nil || fi == nil { - panic(fmt.Sprintf("file %s should exist", filename)) +func TestRenamePreventsSymlinkEscape(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires additional privileges on windows") } + + root := t.TempDir() + cwd := filepath.Join(root, "cwd") + outside := filepath.Join(root, "outside") + require.NoError(t, os.MkdirAll(cwd, 0o700)) + require.NoError(t, os.Mkdir(outside, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(cwd, "old-file"), []byte("data"), 0o600)) + require.NoError(t, os.Symlink(outside, filepath.Join(cwd, "symlink"))) + + fs := newBoundOS(cwd) + err := fs.Rename("old-file", filepath.Join("symlink", "new-file")) + require.ErrorIs(t, err, ErrPathEscapesParent) } -func mustNotExist(filename string) { - if _, err := os.Stat(filename); !os.IsNotExist(err) { - panic(fmt.Sprintf("file %s should not exist", filename)) - } +func mustExist(t *testing.T, filename string) { + t.Helper() + fi, err := os.Stat(filename) + require.NoError(t, err) + require.NotNil(t, fi) } func requireFileContents(t *testing.T, filename string, want []byte) { @@ -1789,11 +739,10 @@ func requireFileContents(t *testing.T, filename string, want []byte) { assert.Equal(t, want, got) } -func notFoundError() string { - switch runtime.GOOS { - case "windows": - return "The system cannot find the " // {path,file} specified - default: - return "no such file or directory" - } +func newTestBoundOS(t *testing.T, dir string) *BoundOS { + t.Helper() + + fs, ok := newBoundOS(dir).(*BoundOS) + require.True(t, ok) + return fs } diff --git a/osfs/os_bound_windows_test.go b/osfs/os_bound_windows_test.go new file mode 100644 index 0000000..360811c --- /dev/null +++ b/osfs/os_bound_windows_test.go @@ -0,0 +1,39 @@ +//go:build windows + +package osfs + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestChrootPathKeepsDriveAbsolutePathFromEmptyBase(t *testing.T) { + fs := newTestBoundOS(t, "") + + want := filepath.Clean(`C:\Users\runner\repo.git`) + assert.Equal(t, want, fs.chrootPath(`C:\Users\runner\repo.git`)) + assert.Equal(t, want, fs.chrootPath(`/C:/Users/runner/repo.git`)) +} + +func TestNewBoundOSNormalizesSlashPrefixedDriveBase(t *testing.T) { + fs := newTestBoundOS(t, `/C:/Users/runner/repo.git`) + + assert.Equal(t, filepath.Clean(`C:\Users\runner\repo.git`), fs.Root()) +} + +func TestOperationRootUsesDriveForAbsolutePathFromEmptyBase(t *testing.T) { + fs := newTestBoundOS(t, "") + + assert.Equal(t, `C:\`, fs.operationRoot(`C:\Users\runner\.gitconfig`)) + assert.Equal(t, `C:\`, fs.operationRoot(`/C:/Users/runner/.gitconfig`)) +} + +func TestSlashPrefixedDrivePathIsCleanedUnderDriveRoot(t *testing.T) { + assert.Equal(t, filepath.Join("Users", "runner", ".gitconfig"), cleanUnderRoot(`/C:/Users/runner/.gitconfig`)) + + rel, ok := relativeInsideBase(`C:\`, `/C:/Users/runner/.gitconfig`) + assert.True(t, ok) + assert.Equal(t, filepath.Join("Users", "runner", ".gitconfig"), rel) +} diff --git a/osfs/os_js.go b/osfs/os_js.go index e55ae98..0a65286 100644 --- a/osfs/os_js.go +++ b/osfs/os_js.go @@ -18,45 +18,38 @@ var globalMemFs = memfs.New() // js/wasm environment. var Default = memfs.New() -// New returns a new OS filesystem. -// By default paths are deduplicated, but still enforced -// under baseDir. For more info refer to WithDeduplicatePath. +// New returns a new OS filesystem rooted at baseDir, backed by the +// js/wasm in-memory filesystem. 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 newChrootOS(baseDir) -} - -func newChrootOS(baseDir string) billy.Filesystem { - return chroot.New(Default, Default.Join("/", baseDir)) + return newBoundOS(baseDir) } // BoundOS is a fs implementation based on the js/wasm in-memory filesystem // which is bound to a base dir. type BoundOS struct { billy.Filesystem - baseDir string - deduplicatePath bool + baseDir string } -func newBoundOS(d string, deduplicatePath bool) billy.Filesystem { +func newBoundOS(d string) billy.Filesystem { baseDir := globalMemFs.Join("/", d) return &BoundOS{ - Filesystem: chroot.New(globalMemFs, baseDir), - baseDir: baseDir, - deduplicatePath: deduplicatePath, + Filesystem: chroot.New(globalMemFs, baseDir), + baseDir: baseDir, } } +// Capabilities implements the billy.Capable interface, delegating to the +// underlying in-memory filesystem. +func (fs *BoundOS) Capabilities() billy.Capability { + return billy.Capabilities(fs.Filesystem) +} + // 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) { @@ -64,7 +57,7 @@ func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { if err != nil { return nil, err } - return New(joined, WithBoundOS(), WithDeduplicatePath(fs.deduplicatePath)), nil + return New(joined, WithBoundOS()), nil } // Root returns the current base dir of the billy.Filesystem. @@ -94,30 +87,14 @@ func WithBoundOS() Option { } } -// 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 option is accepted for API parity with non-js builds and has no effect -// on the js/wasm in-memory implementation. -func WithDeduplicatePath(enabled bool) Option { - return func(o *options) { - o.deduplicatePath = enabled - } -} +type Option func(*options) type options struct { Type - deduplicatePath bool } type Type int const ( - ChrootOSFS Type = iota - BoundOSFS + BoundOSFS Type = iota ) diff --git a/osfs/os_js_test.go b/osfs/os_js_test.go index ca6aa4c..abca640 100644 --- a/osfs/os_js_test.go +++ b/osfs/os_js_test.go @@ -48,14 +48,9 @@ func TestWithBoundOSReturnsBoundOS(t *testing.T) { assert.IsType(t, &BoundOS{}, got) } -func TestWithChrootOSReturnsChrootHelper(t *testing.T) { - got := New(t.TempDir(), WithChrootOS()) - assert.IsType(t, &chroot.ChrootHelper{}, got) -} - -func TestDefaultTypeIsChrootOSFS(t *testing.T) { +func TestDefaultTypeIsBoundOS(t *testing.T) { got := New(t.TempDir()) - assert.IsType(t, &chroot.ChrootHelper{}, got) + assert.IsType(t, &BoundOS{}, got) } func TestBoundOSRoot(t *testing.T) { @@ -96,23 +91,12 @@ func TestBoundOSRejectsParentTraversal(t *testing.T) { assert.ErrorIs(t, err, billy.ErrCrossedBoundary) } -func TestWithDeduplicatePathIsAccepted(t *testing.T) { - // WithDeduplicatePath has no effect on the in-memory js/wasm - // implementation, but must remain callable for API parity with the - // non-js build so go-git and other consumers compile unchanged. - fs := New(t.TempDir(), WithBoundOS(), WithDeduplicatePath(false)) - assert.IsType(t, &BoundOS{}, fs) -} - // API call assertions. These ensure the exported option constructors remain // available under GOOS=js so downstream code (e.g. go-git) keeps compiling. var _ = New("/") var _ = New("/", WithBoundOS()) -var _ = New("/", WithChrootOS()) -var _ = New("/", WithDeduplicatePath(false)) // Type constants must stay exported for any consumer that switches on them. var ( - _ Type = ChrootOSFS _ Type = BoundOSFS ) diff --git a/osfs/os_options.go b/osfs/os_options.go index 2f235c6..72fbfb0 100644 --- a/osfs/os_options.go +++ b/osfs/os_options.go @@ -1,3 +1,5 @@ +//go:build !js + package osfs type Option func(*options) diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go new file mode 100644 index 0000000..db3cfc5 --- /dev/null +++ b/osfs/os_rootfs.go @@ -0,0 +1,413 @@ +//go:build !js + +package osfs + +import ( + "errors" + "fmt" + gofs "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/helper/chroot" + "github.com/go-git/go-billy/v6/util" +) + +// FromRoot creates a new [RootOS] from an [os.Root]. +// The provided root is used directly for all operations and the caller +// is responsible for its lifecycle. Root must not be nil. +func FromRoot(root *os.Root) (*RootOS, error) { + if root == nil { + return nil, errors.New("root must not be nil") + } + return &RootOS{root: root}, nil +} + +// RootOS is a high-performance fs implementation based on a caller-managed +// [os.Root]. Since the root is reused across all operations, this avoids +// the overhead of opening and closing it on every call. The caller is +// responsible for the root's lifecycle. +// +// For automatic lifecycle management at the cost of per-operation overhead, +// use [BoundOS] via [New] instead. +// +// Behaviours of note: +// 1. Read and write operations can only be directed to files which descend +// from the root dir. +// 2. Symlink targets are stored verbatim and not rewritten, so they may +// point outside the root dir or to non-existent paths. [RootOS.Readlink] +// returns the stored target with path separators normalised to forward +// slashes (see [filepath.ToSlash]). +// 3. Operations leading to escapes to outside the [os.Root] location result +// in [ErrPathEscapesParent]. +type RootOS struct { + root *os.Root +} + +func (fs *RootOS) Capabilities() billy.Capability { + return boundCapabilities() +} + +func (fs *RootOS) Create(name string) (billy.File, error) { + return fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, defaultCreateMode) +} + +func (fs *RootOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.File, error) { + rel := fs.toRelative(name) + display := fs.displayName(name) + + if flag&os.O_CREATE != 0 { + if err := createDir(fs.root, rel); err != nil { + return nil, translateError(err, rel) + } + } + + openName := rel + if openName == "" { + openName = "." + } + f, err := fs.root.OpenFile(openName, flag, perm) + if err == nil { + return &file{File: f, name: display}, nil + } + + // os.Root rejects an absolute symlink target even when it points back + // inside the same root. Resolve that one case to keep existing Billy + // symlink behavior without allowing a real root escape. + if flag&os.O_CREATE == 0 && isPathEscapeError(err) { + escapeErr := err + fi, lstatErr := fs.root.Lstat(openName) + if lstatErr == nil && fi.Mode()&gofs.ModeSymlink != 0 { + if target, readlinkErr := fs.root.Readlink(openName); readlinkErr == nil { + if targetRel, ok := fs.absoluteSymlinkTarget(target); ok { + if f, err = fs.root.OpenFile(targetRel, flag, perm); err == nil { + return &file{File: f, name: display}, nil + } + } + } + } + err = escapeErr + } + + return nil, translateError(err, rel) +} + +func (fs *RootOS) ReadDir(name string) ([]gofs.DirEntry, error) { + rel := fs.toRelative(name) + if rel == "" { + rel = "." + } + + f, err := fs.root.Open(rel) + if err != nil { + return nil, translateError(err, rel) + } + defer f.Close() + + e, err := f.ReadDir(-1) + if err != nil { + return nil, translateError(err, rel) + } + return e, nil +} + +func (fs *RootOS) Rename(from, to string) error { + if fs.isBaseDir(from) { + return billy.ErrBaseDirCannotBeRenamed + } + + fromRel := fs.toRelative(from) + toRel := fs.toRelative(to) + + if err := createDir(fs.root, toRel); err != nil { + return translateError(err, toRel) + } + + return translateError(fs.root.Rename(fromRel, toRel), fmt.Sprintf("%s -> %s", fromRel, toRel)) +} + +func (fs *RootOS) MkdirAll(name string, perm gofs.FileMode) error { + rel := fs.toRelative(name) + if rel == "" { + rel = "." + } + + // os.Root errors when perm contains bits other than the nine least-significant bits (0o777). + err := fs.root.MkdirAll(rel, perm&0o777) + return translateError(err, rel) +} + +func (fs *RootOS) Open(name string) (billy.File, error) { + return fs.OpenFile(name, os.O_RDONLY, 0) +} + +func (fs *RootOS) Stat(name string) (os.FileInfo, error) { + rel := fs.toRelative(name) + if rel == "" { + rel = "." + } + + fi, err := fs.root.Stat(rel) + if err != nil { + return nil, translateError(err, rel) + } + + return fileInfo{FileInfo: fi, name: filepath.Base(fs.displayName(name))}, nil +} + +func (fs *RootOS) Remove(name string) error { + if fs.isBaseDir(name) { + return billy.ErrBaseDirCannotBeRemoved + } + + rel := fs.toRelative(name) + err := fs.root.Remove(rel) + if err == nil { + return nil + } + + return translateError(err, rel) +} + +// TempFile creates a temporary file. If dir is empty, the file +// will be created within a .tmp dir. +func (fs *RootOS) TempFile(dir, prefix string) (billy.File, error) { + return util.TempFile(fs, dir, prefix) +} + +func (fs *RootOS) Join(elem ...string) string { + return filepath.Join(elem...) +} + +func (fs *RootOS) RemoveAll(name string) error { + if fs.isBaseDir(name) { + return billy.ErrBaseDirCannotBeRemoved + } + + rel := fs.toRelative(name) + return translateError(fs.root.RemoveAll(rel), rel) +} + +func (fs *RootOS) Symlink(oldname, newname string) error { + newRel := fs.toRelative(newname) + + if err := createDir(fs.root, newRel); err != nil { + return translateError(err, newRel) + } + + return translateError(fs.root.Symlink(oldname, newRel), newRel) +} + +func (fs *RootOS) Lstat(name string) (os.FileInfo, error) { + rel := fs.toRelative(name) + if rel == "" { + rel = "." + } + + fi, err := fs.root.Lstat(rel) + if err != nil { + return nil, translateError(err, rel) + } + + return fileInfo{FileInfo: fi, name: filepath.Base(fs.displayName(name))}, nil +} + +func (fs *RootOS) Readlink(name string) (string, error) { + rel := fs.toRelative(name) + if rel == "" { + rel = "." + } + + lnk, err := fs.root.Readlink(rel) + if err != nil { + return "", translateError(err, rel) + } + + return filepath.ToSlash(lnk), nil +} + +func (fs *RootOS) Chmod(path string, mode gofs.FileMode) error { + rel := fs.toRelative(path) + if rel == "" { + rel = "." + } + return translateError(fs.root.Chmod(rel, mode), rel) +} + +// Chroot returns a logical sub-filesystem rooted at the result of joining +// the provided path with the underlying root dir. The returned filesystem +// reuses this [RootOS]'s [os.Root] (no new file descriptor is opened), +// so its lifecycle is tied to the parent root managed by the caller. +// +// If path does not yet exist under the parent root it is created (with +// [defaultDirectoryMode]). If path exists but is not a directory an error +// is returned. +// +// Containment is enforced by the parent [os.Root], not by the chroot +// path: operations cannot escape the parent root, but a symlink within +// the chroot may resolve to a sibling location elsewhere under the +// parent root. For a tighter boundary at the chroot path, open a new +// [os.Root] via [os.OpenRoot] and wrap it with [FromRoot]. +func (fs *RootOS) Chroot(path string) (billy.Filesystem, error) { + rel := fs.toRelative(path) + if rel == "" { + rel = "." + } + + if err := ensureDir(fs.root, rel); err != nil { + return nil, err + } + + return chroot.New(fs, filepath.Join(fs.root.Name(), rel)), nil +} + +// Root returns the current root dir of the filesystem. +func (fs *RootOS) Root() string { + return fs.root.Name() +} + +func (fs *RootOS) displayName(name string) string { + name = hostPath(name) + if filepath.IsAbs(name) && isHostRoot(fs.root.Name()) { + return filepath.Clean(name) + } + + rel := fs.toRelative(name) + if rel == "" { + return "." + } + return rel +} + +func (fs *RootOS) isBaseDir(name string) bool { + rel := fs.toRelative(name) + return rel == "" || rel == "." +} + +func (fs *RootOS) toRelative(name string) string { + if name == "" { + return "" + } + + name = hostPath(name) + if filepath.IsAbs(name) { + if rel, ok := relativeInsideBase(fs.root.Name(), name); ok { + return cleanUnderRoot(rel) + } + } + + return cleanUnderRoot(name) +} + +func (fs *RootOS) absoluteSymlinkTarget(target string) (string, bool) { + if !isRootedPath(target) { + return "", false + } + target = hostPath(target) + rel, ok := relativeInsideBase(fs.root.Name(), target) + if !ok { + rel = cleanUnderRoot(target) + } + if rel == "" { + return ".", true + } + return cleanUnderRoot(rel), true +} + +func relativeInsideBase(base, name string) (string, bool) { + base = hostPath(base) + name = hostPath(name) + rel, err := filepath.Rel(filepath.Clean(base), filepath.Clean(name)) + if err != nil { + return "", false + } + if rel == "." { + return "", true + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", false + } + return rel, true +} + +func isRootedPath(name string) bool { + name = hostPath(name) + if filepath.IsAbs(name) { + return true + } + return strings.HasPrefix(name, string(filepath.Separator)) +} + +func cleanUnderRoot(name string) string { + name = hostPath(name) + vol := filepath.VolumeName(name) + name = name[len(vol):] + name = filepath.Join(string(filepath.Separator), name) + return strings.TrimLeft(name, string(filepath.Separator)) +} + +func hostPath(name string) string { + name = filepath.FromSlash(name) + if filepath.Separator == '\\' && len(name) >= 3 && + name[0] == '\\' && name[2] == ':' { + return name[1:] + } + return name +} + +func isHostRoot(name string) bool { + name = filepath.Clean(hostPath(name)) + if !filepath.IsAbs(name) && !strings.HasPrefix(name, string(filepath.Separator)) { + return false + } + return filepath.Dir(name) == name +} + +func ensureDir(root *os.Root, path string) error { + fi, err := root.Lstat(path) + if errors.Is(err, os.ErrNotExist) { + if err := root.MkdirAll(path, defaultDirectoryMode); err != nil { + return fmt.Errorf("failed to auto create dir: %w", translateError(err, path)) + } + return nil + } + if err != nil { + return translateError(err, path) + } + if fi != nil && !fi.IsDir() { + return fmt.Errorf("cannot chroot: %q is not a dir (mode %s)", path, fi.Mode()) + } + return nil +} + +func createDir(root *os.Root, fullpath string) error { + dir := filepath.Dir(fullpath) + if dir != "." { + if err := root.MkdirAll(dir, defaultDirectoryMode); err != nil { + if errors.Is(err, os.ErrExist) { + return nil + } + return err + } + } + + return nil +} + +func isPathEscapeError(err error) bool { + return err != nil && strings.Contains(err.Error(), ErrPathEscapesParent.Error()) +} + +func translateError(err error, file string) error { + if err == nil { + return nil + } + + if isPathEscapeError(err) { + return fmt.Errorf("%w: %q", ErrPathEscapesParent, file) + } + + return err +} diff --git a/osfs/os_test.go b/osfs/os_test.go index 5ff16c9..cf5d4a1 100644 --- a/osfs/os_test.go +++ b/osfs/os_test.go @@ -14,7 +14,7 @@ func TestDefault(t *testing.T) { want := &BoundOS{} got := Default - if reflect.TypeOf(got) != reflect.TypeOf(want) { + if reflect.TypeOf(got) != reflect.TypeFor[*BoundOS]() { t.Errorf("wanted Default to be %T got %T", want, got) } } diff --git a/test/basic_test.go b/test/basic_test.go index 5901e6d..fab78b2 100644 --- a/test/basic_test.go +++ b/test/basic_test.go @@ -65,7 +65,7 @@ func TestCreateOverwrite(t *testing.T) { eachBasicFS(t, func(t *testing.T, fs Basic) { t.Helper() - for i := 0; i < 3; i++ { + for i := range 3 { f, err := fs.Create("foo") require.NoError(t, err) diff --git a/test/bench_test.go b/test/bench_test.go index aa54664..b14ef4b 100644 --- a/test/bench_test.go +++ b/test/bench_test.go @@ -1,3 +1,5 @@ +//go:build !js && !wasm && !wasip1 + package test import ( @@ -34,6 +36,13 @@ type btest struct { func BenchmarkCompare(b *testing.B) { b.ReportAllocs() + rootDir := b.TempDir() + root, err := os.OpenRoot(rootDir) + require.NoError(b, err) + b.Cleanup(func() { root.Close() }) + rootFS, err := osfs.FromRoot(root) + require.NoError(b, err) + tests := []btest{ { // provide baseline comparison against direct use of os. @@ -49,6 +58,13 @@ func BenchmarkCompare(b *testing.B) { openF: billyOpen, createF: billyCreate, }, + { + name: "osfs.rootOS", + fn: fn, + sut: rootFS, + openF: billyOpen, + createF: billyCreate, + }, { name: "memfs", fn: fn, diff --git a/test/dir_test.go b/test/dir_test.go index 8961d38..0f98f26 100644 --- a/test/dir_test.go +++ b/test/dir_test.go @@ -185,7 +185,7 @@ func TestDir_ReadDirNested(t *testing.T) { } path = "/" - for i := 0; i < maxNestedDirs; i++ { + for i := range maxNestedDirs { path = fs.Join(path, strconv.Itoa(i)) info, err := fs.ReadDir(path) require.NoError(t, err) diff --git a/test/fs_test.go b/test/fs_test.go index 450a56b..839fb3c 100644 --- a/test/fs_test.go +++ b/test/fs_test.go @@ -138,7 +138,7 @@ func TestFS_SymlinkWithChrootCrossBounders(t *testing.T) { if _, ok := qux.(*osfs.BoundOS); ok { fi, err = qux.Stat("qux/link") assert.Nil(t, fi, fs) - assert.ErrorIs(t, err, os.ErrNotExist) + assert.ErrorIs(t, err, osfs.ErrPathEscapesParent) } }) } diff --git a/test/symlink_test.go b/test/symlink_test.go index 13c0a11..faf423a 100644 --- a/test/symlink_test.go +++ b/test/symlink_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "os" + "path/filepath" "runtime" "testing" @@ -223,7 +224,7 @@ func TestReadlinkWithAbsolutePath(t *testing.T) { oldname, err := fs.Readlink("dir/link") require.NoError(t, err) - assert.Equal(t, oldname, expectedSymlinkTarget) + assert.Equal(t, expectedSymlinkTarget, filepath.FromSlash(oldname)) }) } diff --git a/test/tempfile_test.go b/test/tempfile_test.go index a5695ab..e0fb046 100644 --- a/test/tempfile_test.go +++ b/test/tempfile_test.go @@ -85,10 +85,10 @@ func TestRenameTempFile(t *testing.T) { func TestTempFileMany(t *testing.T) { eachTempFS(t, func(t *testing.T, fs tempFS) { t.Helper() - for i := 0; i < 1024; i++ { + for range 1024 { var files []billy.File - for j := 0; j < 100; j++ { + for range 100 { f, err := fs.TempFile("test-dir", "test-prefix") require.NoError(t, err) files = append(files, f) @@ -105,10 +105,10 @@ func TestTempFileMany(t *testing.T) { func TestTempFileManyWithUtil(t *testing.T) { eachTempFS(t, func(t *testing.T, fs tempFS) { t.Helper() - for i := 0; i < 1024; i++ { + for range 1024 { var files []billy.File - for j := 0; j < 100; j++ { + for range 100 { f, err := util.TempFile(fs, "test-dir", "test-prefix") require.NoError(t, err) files = append(files, f) diff --git a/util/util.go b/util/util.go index 191d4ef..ce23586 100644 --- a/util/util.go +++ b/util/util.go @@ -180,7 +180,7 @@ func TempFile(fs billy.Basic, dir, prefix string) (f billy.File, err error) { } nconflict := 0 - for i := 0; i < 10000; i++ { + for range 10000 { name := filepath.Join(dir, prefix+nextSuffix()) f, err = fs.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, tempFileMode) if errors.Is(err, os.ErrExist) { @@ -214,7 +214,7 @@ func TempDir(fs billy.Dir, dir, prefix string) (name string, err error) { } nconflict := 0 - for i := 0; i < 10000; i++ { + for range 10000 { try := filepath.Join(dir, prefix+nextSuffix()) err = fs.MkdirAll(try, tempDirectoryMode) if errors.Is(err, os.ErrExist) { diff --git a/util/util_test.go b/util/util_test.go index 2423bf9..80d84fd 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -139,7 +139,6 @@ func TestReadFileCases(t *testing.T) { } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() fs, name := tt.setup(t) @@ -208,7 +207,7 @@ type writeErrorSyncFS struct { } func (fs *writeErrorSyncFS) OpenFile(filename string, flag int, mode iofs.FileMode) (billy.File, error) { - fs.OpenFileArgs = append(fs.OpenFileArgs, [3]interface{}{filename, flag, mode}) + fs.OpenFileArgs = append(fs.OpenFileArgs, [3]any{filename, flag, mode}) return fs.file, nil }