From a3a2b6e3f5066f1fa655e617afe5fe77d50329fc Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 6 May 2026 10:46:13 +0100 Subject: [PATCH 01/23] build: Add go-git integration test Signed-off-by: Paulo Gomes --- .github/workflows/go-git-integration.yml | 114 +++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 .github/workflows/go-git-integration.yml 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 ./... From e7951312af00a204223851c1b737287a3c4d8728 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 22 Oct 2025 19:30:23 +0100 Subject: [PATCH 02/23] osfs: Replace filepath-securejoin with os.Root This change removes the previous on-demand costly evaluation of paths with the Go's traversal resistent primitive os.Root. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The benchmarks in /test/ indicate that in most scenarios this represent a positive performance: │ base.txt │ pr.txt │ │ sec/op │ sec/op vs base │ Compare/osfs.boundOS_open-4 15.78µ ± 4% 13.38µ ± 2% -15.22% (p=0.002 n=6) Compare/osfs.boundOS_read-4 88.45µ ± 1% 91.08µ ± 0% +2.97% (p=0.002 n=6) Compare/osfs.boundOS_write-4 924.4µ ± 23% 867.2µ ± 18% ~ (p=0.180 n=6) Compare/osfs.boundOS_create-4 31.39µ ± 51% 23.10µ ± 72% ~ (p=0.065 n=6) Compare/osfs.boundOS_stat-4 9.493µ ± 2% 4.244µ ± 2% -55.30% (p=0.002 n=6) Compare/osfs.boundOS_rename-4 68.14µ ± 1% 42.76µ ± 3% -37.26% (p=0.002 n=6) Compare/osfs.boundOS_remove-4 30.14µ ± 2% 24.31µ ± 3% -19.34% (p=0.002 n=6) Compare/osfs.boundOS_mkdirall-4 18.25µ ± 351% 17.74µ ± 303% ~ (p=0.699 n=6) Compare/osfs.boundOS_tempfile-4 39.63µ ± 5% 34.35µ ± 2% -13.31% (p=0.002 n=6) │ base.txt │ pr.txt │ │ B/op │ B/op vs base │ Compare/osfs.boundOS_open-4 1032.0 ± 0% 424.0 ± 0% -58.91% (p=0.002 n=6) Compare/osfs.boundOS_read-4 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=6) ¹ Compare/osfs.boundOS_write-4 1507.0 ± 3% 261.0 ± 0% -82.68% (p=0.002 n=6) Compare/osfs.boundOS_create-4 1536.5 ± 3% 278.0 ± 1% -81.91% (p=0.002 n=6) Compare/osfs.boundOS_stat-4 1120.0 ± 0% 240.0 ± 0% -78.57% (p=0.002 n=6) Compare/osfs.boundOS_rename-4 4845.0 ± 0% 122.0 ± 1% -97.48% (p=0.002 n=6) Compare/osfs.boundOS_remove-4 1055.00 ± 0% 63.00 ± 0% -94.03% (p=0.002 n=6) Compare/osfs.boundOS_mkdirall-4 2123.5 ± 20% 199.0 ± 8% -90.63% (p=0.002 n=6) Compare/osfs.boundOS_tempfile-4 183.0 ± 1% 336.0 ± 0% +83.61% (p=0.002 n=6) │ base.txt │ pr.txt │ │ allocs/op │ allocs/op vs base │ Compare/osfs.boundOS_open-4 21.000 ± 0% 9.000 ± 0% -57.14% (p=0.002 n=6) Compare/osfs.boundOS_read-4 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=6) ¹ Compare/osfs.boundOS_write-4 25.000 ± 4% 8.000 ± 0% -68.00% (p=0.002 n=6) Compare/osfs.boundOS_create-4 26.000 ± 4% 8.000 ± 0% -69.23% (p=0.002 n=6) Compare/osfs.boundOS_stat-4 19.000 ± 0% 3.000 ± 0% -84.21% (p=0.002 n=6) Compare/osfs.boundOS_rename-4 68.000 ± 0% 8.000 ± 0% -88.24% (p=0.002 n=6) Compare/osfs.boundOS_remove-4 19.000 ± 0% 3.000 ± 0% -84.21% (p=0.002 n=6) Compare/osfs.boundOS_mkdirall-4 32.500 ± 14% 8.000 ± 12% -75.38% (p=0.002 n=6) Compare/osfs.boundOS_tempfile-4 6.000 ± 0% 14.000 ± 0% +133.33% (p=0.002 n=6) A new ErrPathEscapesParent was introduced to represent when an operation is attempting to escape the root/bound dir used for the bound OS. Signed-off-by: Paulo Gomes --- go.mod | 2 - go.sum | 2 - osfs/os.go | 14 +- osfs/os_bound.go | 388 ++------- osfs/os_bound_test.go | 1849 ++++++++--------------------------------- osfs/os_rootfs.go | 392 +++++++++ test/fs_test.go | 2 +- 7 files changed, 815 insertions(+), 1834 deletions(-) create mode 100644 osfs/os_rootfs.go 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/osfs/os.go b/osfs/os.go index f69d851..4b42a19 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -19,11 +19,9 @@ 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. func New(baseDir string, opts ...Option) billy.Filesystem { o := &options{ deduplicatePath: true, @@ -32,7 +30,7 @@ func New(baseDir string, opts ...Option) billy.Filesystem { opt(o) } - return newBoundOS(baseDir, o.deduplicatePath) + return newBoundOS(baseDir) } // WithBoundOS returns the option of using a Bound filesystem OS. @@ -43,13 +41,9 @@ func WithBoundOS() Option { } // 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. +// BoundOS now relies on os.Root for path containment, so this option is kept +// for API compatibility and has no effect. func WithDeduplicatePath(enabled bool) Option { return func(o *options) { o.deduplicatePath = enabled diff --git a/osfs/os_bound.go b/osfs/os_bound.go index acd114b..ea839c6 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -19,402 +19,190 @@ package osfs import ( - "io/fs" + gofs "io/fs" "os" "path/filepath" - "strings" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/util" ) -var ( - dotPrefixes = dotPathPrefixes() - dotSeparators = dotPathSeparators() -) - -func dotPathPrefixes() []string { - if filepath.Separator == '\\' { - return []string{"./", ".\\"} - } - return []string{"./"} -} - -func dotPathSeparators() string { - if filepath.Separator == '\\' { - return `/\` - } - return `/` -} - -// BoundOS is a fs implementation based on the OS filesystem which is bound to -// a base dir. -// Prefer this fs implementation over ChrootOS. +// BoundOS is a fs implementation based on the OS filesystem which relies on +// Go's [os.Root]. 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 descends +// 1. Read and write operations can only be directed to files which descend // from the base dir. // 2. Symlinks don't have their targets modified, and therefore can point // to locations outside the base dir or to non-existent paths. -// 3. Readlink and Lstat ensures that the link file is located within the base -// dir, evaluating any symlinks that file or base dir may contain. +// 3. Operations leading to escapes to outside the [os.Root] location result +// in [ErrPathEscapesParent]. type BoundOS struct { - baseDir string - deduplicatePath bool + baseDir string } -func newBoundOS(d string, deduplicatePath bool) billy.Filesystem { - return &BoundOS{baseDir: d, deduplicatePath: deduplicatePath} +func newBoundOS(d string, _ ...bool) billy.Filesystem { + return &BoundOS{baseDir: d} +} + +// rootFS opens a temporary [RootOS] and returns a cleanup function that +// closes the underlying [os.Root]. +func (fs *BoundOS) rootFS() (*RootOS, func(), error) { + r, err := os.OpenRoot(fs.baseDir) + 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.rootFS() 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.rootFS() 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.rootFS() 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.rootFS() 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) +func (fs *BoundOS) Stat(name string) (os.FileInfo, error) { + rfs, cleanup, err := fs.rootFS() if err != nil { return nil, err } - fi, err := os.Stat(filename) - 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.rootFS() 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.rootFS() 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.rootFS() 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 "." - } - 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 - } - } - return p -} - -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 - } - abspath, err := fs.abs(path) - if err != nil { - return false - } - return filepath.Clean(abspath) == filepath.Clean(fs.baseDir) -} - -func (fs *BoundOS) absNoFollow(filename string) (string, error) { - if fs.baseDir == "" { - return filepath.Clean(fs.expandDot(filename)), nil - } - - rel := fs.rootRelative(filename) - if rel == "" { - return fs.baseDir, nil - } - - parent, err := fs.secureJoin(filepath.Dir(rel)) - if err != nil { - return "", err - } - - return filepath.Join(parent, filepath.Base(rel)), nil -} - -func (fs *BoundOS) rootRelative(filename string) string { - filename = fs.expandDot(filename) - filename = filepath.Clean(filename) - if filepath.Clean(filename) == filepath.Clean(fs.baseDir) { - return "" - } - if filepath.IsAbs(filename) { - if isLocalToBase(fs.baseDir, filename) { - rel, _ := filepath.Rel(fs.baseDir, filename) - 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)))) -} - -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) Lstat(filename string) (os.FileInfo, error) { - filename, err := fs.absNoFollow(filename) +func (fs *BoundOS) Lstat(name string) (os.FileInfo, error) { + rfs, cleanup, err := fs.rootFS() if err != nil { return nil, err } - return os.Lstat(filename) -} - -func (fs *BoundOS) Readlink(link string) (string, error) { - link, err := fs.absNoFollow(link) - if err != nil { - return "", err - } - return os.Readlink(link) -} - -func (fs *BoundOS) secureJoin(path string) (string, error) { - if filepath.Separator != '\\' { - return securejoin.SecureJoin(fs.baseDir, path) - } - return securejoin.SecureJoinVFS(fs.baseDir, path, boundOSVFS{}) + defer cleanup() + return rfs.Lstat(name) } -type boundOSVFS struct{} - -func (boundOSVFS) Lstat(name string) (os.FileInfo, error) { - return os.Lstat(name) -} - -func (boundOSVFS) Readlink(name string) (string, error) { - target, err := os.Readlink(name) +func (fs *BoundOS) Readlink(name string) (string, error) { + rfs, cleanup, err := fs.rootFS() 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 - } - } - return target, nil + defer cleanup() + return rfs.Readlink(name) } -func (fs *BoundOS) Chmod(path string, mode fs.FileMode) error { - abspath, err := fs.abs(path) +func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { + rfs, cleanup, err := fs.rootFS() if err != nil { return err } - return os.Chmod(abspath, mode) + defer cleanup() + return rfs.Chmod(path, mode) } -// Chroot returns a new BoundOS filesystem, with the base dir set to the +// 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) + rfs, cleanup, err := fs.rootFS() 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 -} - -func (fs *BoundOS) createDir(fullpath string) error { - dir := filepath.Dir(fullpath) - if dir != "." { - if err := os.MkdirAll(dir, defaultDirectoryMode); err != nil { - return err - } - } + defer cleanup() - return nil -} - -// abs transforms filename to an absolute path, taking into account the base dir. -// Relative paths won't be allowed to ascend the base dir, so `../file` will become -// `/working-dir/file`. -// -// Note that if filename is a symlink, the returned address will be the target of the -// symlink. -func (fs *BoundOS) abs(filename string) (string, error) { - if filename == fs.baseDir { - filename = string(filepath.Separator) + rel := rfs.toRelative(path) + if rel == "" { + rel = "." } - - path, err := fs.secureJoin(filename) + childRoot, err := openChildRoot(rfs.root, rel) if err != nil { - return "", err + return nil, err } + defer childRoot.Close() - 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):]) - } - } - return path, nil + return newBoundOS(childRoot.Name()), nil +} + +// Root returns the current base dir of the billy.Filesystem. +func (fs *BoundOS) Root() string { + return fs.baseDir } diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 6be22e4..288470f 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -19,27 +19,69 @@ package osfs import ( - "fmt" + "errors" "os" "path/filepath" "runtime" "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()) - caps := billy.Capabilities(fs) - assert.Equal(t, billy.AllCapabilities, caps) + _, err = fs.Stat(".") + require.ErrorContains(t, err, "file already closed") + }) } func TestOpen(t *testing.T) { @@ -47,117 +89,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 +185,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 +205,7 @@ func TestOpenPreservesBackslashFilenamesOnNonWindows(t *testing.T) { } dir := t.TempDir() - fs := newBoundOS(dir, true) + fs := newBoundOS(dir) tests := []struct { filename string @@ -195,1522 +216,343 @@ 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()) - }) - } -} + require.NoError(t, os.WriteFile(filepath.Join(dir, tt.filename), []byte("anything"), 0o600)) -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) - - 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) - } + fs := newBoundOS(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)) - - 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(dir, "prefix") + f, err = fs.TempFile("../../../above/cwd", "prefix") require.NoError(t, err) - assert.NotNil(f) - assert.Contains(f.Name(), filepath.Join(strings.TrimLeft(dir, `/\`), "prefix")) + assert.True(t, strings.HasPrefix(f.Name(), filepath.Join("above", "cwd", "prefix")), f.Name()) require.NoError(t, f.Close()) } func TestChroot(t *testing.T) { - assert := assert.New(t) tmp := t.TempDir() - fs := newBoundOS(tmp, true) + fs := newBoundOS(tmp) - f, err := fs.Chroot("test") + chrooted, err := fs.Chroot("test") require.NoError(t, err) - assert.NotNil(f) - assert.Equal(filepath.Join(tmp, "test"), f.Root()) - assert.IsType(&BoundOS{}, f) + assert.IsType(t, &BoundOS{}, chrooted) + assert.Equal(t, filepath.Join(tmp, "test"), chrooted.Root()) } -func TestRoot(t *testing.T) { - assert := assert.New(t) - dir := t.TempDir() - fs := newBoundOS(dir, true) +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) - root := fs.Root() - assert.Equal(dir, root) + got, err := util.ReadFile(chrooted, "../bar") + require.NoError(t, err) + assert.Equal(t, []byte("chroot"), got) } -func TestReadLink(t *testing.T) { - tests := []struct { - name string - filename string - makeAbs bool - expected string - makeExpectedAbs bool - before func(dir string) billy.Filesystem - wantErr string - }{ - { - 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: "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: "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", - expected: filepath.FromSlash("cwd-target/file"), - makeExpectedAbs: true, - }, - { - 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", - wantErr: notFoundError(), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert := assert.New(t) - dir := t.TempDir() - fs := newBoundOS(dir, true) +func TestRoot(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) - if tt.before != nil { - fs = tt.before(dir) - } + assert.Equal(t, dir, fs.Root()) +} - filename := tt.filename - if tt.makeAbs { - filename = filepath.Join(dir, filename) - } +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"))) - expected := tt.expected - if tt.makeExpectedAbs { - expected = filepath.Join(dir, expected) - } + got, err := fs.Readlink("symlink") + require.NoError(t, err) + assert.Equal(t, target, got) - 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) - } - }) - } + _, err = fs.Readlink("../../symlink") + require.NoError(t, err) } -func TestReadlinkReadsSymlink(t *testing.T) { +func TestLstat(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) - target := filepath.Join(dir, "target-file") - link := filepath.Join(dir, "link") + fs := newBoundOS(dir) + require.NoError(t, os.Symlink("target-file", filepath.Join(dir, "link"))) - err := os.WriteFile(target, []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-file", link) + fi, err := fs.Lstat("link") require.NoError(t, err) + assert.Equal(t, "link", fi.Name()) + assert.NotZero(t, fi.Mode()&os.ModeSymlink) +} - got, err := fs.Readlink("link") - require.NoError(t, err) - assert.Equal(t, "target-file", got) - requireFileContents(t, target, []byte("target")) +func TestStatPreventsSymlinkEscape(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) + require.NoError(t, os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink"))) + + fi, err := fs.Stat("symlink") + require.ErrorIs(t, err, ErrPathEscapesParent) + assert.Nil(t, fi) } -func TestLstat(t *testing.T) { +func TestRemove(t *testing.T) { tests := []struct { 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: "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: "inexistent dir", + filename: "inexistent", + wantErr: os.ErrNotExist, }, { - name: "path: abs pointing outside cwd", - filename: "/etc/passwd", - wantErr: notFoundError(), + name: "base dir as dot", + filename: ".", + wantErr: billy.ErrBaseDirCannotBeRemoved, }, { - 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) + 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: "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) + 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: "test-file", - makeAbs: true, + 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) - } - 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()) + err := fs.Remove(tt.filename) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return } + require.NoError(t, err) }) } } -func TestLstatStatsSymlink(t *testing.T) { +func TestRemoveAll(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir, true) - target := filepath.Join(dir, "target-file") - link := filepath.Join(dir, "link") + fs := newBoundOS(dir).(*BoundOS) + 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)) - err := os.WriteFile(target, []byte("target"), 0o600) - require.NoError(t, err) - err = os.Symlink("target-file", link) - require.NoError(t, err) + require.NoError(t, fs.RemoveAll("parent")) + _, err := os.Stat(filepath.Join(dir, "parent")) + require.True(t, errors.Is(err, os.ErrNotExist)) +} - 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 TestRemoveAllRemovesSymlink(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir).(*BoundOS) + 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 TestNoFollowOperationsAllowMissingPathUnderSymlinkedBase(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("symlink creation requires additional privileges on windows") +func TestRemoveBaseDir(t *testing.T) { + tests := []string{"", ".", "./", "/..", "/foo/.."} + + for _, path := range tests { + t.Run(path, func(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir).(*BoundOS) + require.ErrorIs(t, fs.Remove(path), billy.ErrBaseDirCannotBeRemoved) + require.ErrorIs(t, fs.RemoveAll(path), billy.ErrBaseDirCannotBeRemoved) + mustExist(t, dir) + }) } +} +func TestRemoveBaseDirAbsolutePath(t *testing.T) { 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(dir).(*BoundOS) + + require.ErrorIs(t, fs.Remove(dir), billy.ErrBaseDirCannotBeRemoved) + require.ErrorIs(t, fs.RemoveAll(dir), billy.ErrBaseDirCannotBeRemoved) + mustExist(t, dir) +} + +func TestJoin(t *testing.T) { + fs := newBoundOS(t.TempDir()) - fs := newBoundOS(base, true) + assert.Equal(t, filepath.FromSlash("/a/b/c"), fs.Join("/a", "b/c")) + assert.Equal(t, "", fs.Join()) +} - _, err = fs.Lstat("/etc/passwd") - require.ErrorContains(t, err, notFoundError()) - assert.NotContains(t, err.Error(), "path outside base dir") +func TestReadDir(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir) - err = fs.Remove("/some/path/outside/cwd") - require.ErrorContains(t, err, notFoundError()) - assert.NotContains(t, err.Error(), "path outside base dir") + require.NoError(t, os.WriteFile(filepath.Join(dir, "file1"), []byte{}, 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "file2"), []byte{}, 0o600)) - err = os.WriteFile(filepath.Join(target, "old-file"), []byte("renamed"), 0o600) + dirs, err := fs.ReadDir(".") require.NoError(t, err) - err = fs.Rename("old-file", filepath.Join("newdir", "newfile")) + assert.Len(t, dirs, 2) + + dirs, err = fs.ReadDir("/") require.NoError(t, err) - requireFileContents(t, filepath.Join(target, "newdir", "newfile"), []byte("renamed")) - mustNotExist(filepath.Join(target, "old-file")) + assert.Len(t, dirs, 2) } -func TestNoFollowOperationsContainSymlinkedParentOutsideBase(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("symlink creation requires additional privileges on windows") - } - +func TestReadDirPreventsSymlinkEscape(t *testing.T) { 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) { - dir := t.TempDir() - fs, ok := newBoundOS(dir, true).(*BoundOS) - require.True(t, ok) - target := filepath.Join(dir, "target-dir") - link := filepath.Join(dir, "link") - - 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) - - err = fs.RemoveAll("link") - 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) - - err := tt.remove(fs, dir) - require.ErrorIs(t, err, billy.ErrBaseDirCannotBeRemoved) - mustExist(dir) - }) - } -} + fs := newBoundOS(dir) + require.NoError(t, os.Symlink("/some/path/outside/cwd", filepath.Join(dir, "symlink"))) -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) - - 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) + 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"))) - // 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):] - } - - 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) - assert.NotNil(fi) + _, err = f.Write([]byte("data")) + require.NoError(t, err) + 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 +565,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) { @@ -1788,12 +608,3 @@ func requireFileContents(t *testing.T, filename string, want []byte) { require.NoError(t, err) 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" - } -} diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go new file mode 100644 index 0000000..ad064bc --- /dev/null +++ b/osfs/os_rootfs.go @@ -0,0 +1,392 @@ +//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" +) + +// ErrPathEscapesParent represents when an action leads to escaping from the +// given dir the filesystem is bound to. +// +// The upstream version of this error used by [os.Root] is not public. +var ErrPathEscapesParent = errors.New("path escapes from parent") + +// FromRoot creates a new [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. Symlinks don't have their targets modified, and therefore can point +// to locations outside the root dir or to non-existent paths. +// 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) + + err := createDir(fs.root, toRel) + if err == nil { + err = fs.root.Rename(fromRel, toRel) + } + + return translateError(err, 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 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. +// +// 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 { + 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 "" + } + + 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 !filepath.IsAbs(target) { + return "", false + } + 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) { + 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 cleanUnderRoot(name string) string { + vol := filepath.VolumeName(name) + name = name[len(vol):] + name = filepath.Join(string(filepath.Separator), name) + return strings.TrimLeft(name, string(filepath.Separator)) +} + +// openChildRoot validates path and opens a child [os.Root]. +func openChildRoot(root *os.Root, path string) (*os.Root, error) { + if err := ensureDir(root, path); err != nil { + return nil, err + } + + childRoot, err := root.OpenRoot(path) + if err != nil { + return nil, fmt.Errorf("unable to chroot: %w", translateError(err, path)) + } + return childRoot, nil +} + +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: path is not dir") + } + 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/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) } }) } From 0fd9efdd22b99c62adba8f6bb1bb71a40a30214a Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 11 Apr 2026 23:34:47 +0100 Subject: [PATCH 03/23] osfs: Fix BoundOS capabilities, error handling and abs path logic Capabilities() used bitwise AND instead of OR, reporting no capabilities. translateError() swallowed errors when Unwrap returned nil and relied on exact string matching. Stat() created the base dir outside os.Root as a side-effect. Abs-to-relative conversion was duplicated across six methods. MkdirAll() silently discarded the caller's permission mode. Chroot() downgraded FromRoot instances to per-operation mode. Assisted-by: Claude Opus 4.6 Signed-off-by: Paulo Gomes From 6f22d2d86a0d1e6d0c01282b018c3db9967bee29 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sun, 12 Apr 2026 21:44:43 +0100 Subject: [PATCH 04/23] osfs: Improve BoundOS performance and expand benchmarks Cache os.Root in newBoundOS to eliminate per-operation OpenRoot/Close syscall pairs. Defer symlink resolution in OpenFile to a retry path that only triggers on path-escape errors, removing an Lstat from every non-create open. Simplify fsRoot and Chroot now that os.Root is always cached. Add benchmarks for Create, Stat, Lstat, Rename and Remove. Fix WalkDir benchmark to use fs.WalkDir via iofs adapter for all implementations. Exclude setup cost from Rename and Remove benchmarks. Add TestOpenAbsSymlinkInsideRoot to prove the symlink fallback in OpenFile is needed: os.Root rejects absolute symlink targets that point inside the root, and BoundOS resolves them to relative paths. Assisted-by: Claude Opus 4.6 Signed-off-by: Paulo Gomes --- osfs/os_bench_test.go | 291 +++++++++++++++++++++++++++++++++++------- osfs/os_bound_test.go | 33 +++++ 2 files changed, 281 insertions(+), 43 deletions(-) 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_test.go b/osfs/os_bound_test.go index 288470f..2f514a1 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -84,6 +84,39 @@ func TestFromRoot(t *testing.T) { }) } +// 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()) + + // 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) { tests := []struct { name string From 2a33d7623477dc910d3195f3a87b0f26cc528a14 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 13 Apr 2026 11:53:11 +0100 Subject: [PATCH 05/23] osfs: Extract RootOS from BoundOS for caller-managed os.Root Introduce RootOS, a high-performance filesystem backed by a caller-managed os.Root that is reused across all operations. BoundOS becomes a thin wrapper that opens and closes an os.Root per operation, avoiding leaked directory handles on Windows. FromRoot now returns (*RootOS, error), validating the root upfront rather than checking on every operation. RootOS.Chroot returns a child RootOS; BoundOS.Chroot returns a child BoundOS. Add rootOS sub-benchmarks alongside the existing implementations. Assisted-by: Claude Opus 4.6 Signed-off-by: Paulo Gomes From d768e0f22f492c4e64802ad7568c33579a16d5bd Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Tue, 28 Apr 2026 06:53:59 +0100 Subject: [PATCH 06/23] build: Remove benchmark from wasm Signed-off-by: Paulo Gomes --- osfs/os_js.go | 2 ++ osfs/os_options.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/osfs/os_js.go b/osfs/os_js.go index e55ae98..5520981 100644 --- a/osfs/os_js.go +++ b/osfs/os_js.go @@ -110,6 +110,8 @@ func WithDeduplicatePath(enabled bool) Option { } } +type Option func(*options) + type options struct { Type deduplicatePath bool 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) From 2d7db04c513317573043caf1263d04ed35ec3816 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sun, 3 May 2026 14:38:30 +0100 Subject: [PATCH 07/23] osfs: Avoid leaking os.Root in RootOS.Chroot Chroot previously opened a new os.Root for the child path that was never closed, leaking a file descriptor on every call. Wrap the existing RootOS via helper/chroot instead, reusing the caller-managed parent os.Root for all operations. Document that containment remains anchored at the parent root rather than the chroot path; callers needing a tighter boundary should open a new os.Root and wrap it with FromRoot. Assisted-by: Claude Opus 4.7 Signed-off-by: Paulo Gomes Entire-Checkpoint: 3c52307e49ba From c03000b19c46144b85ea498ad2a3e143d08292fd Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 8 May 2026 16:38:13 +0100 Subject: [PATCH 08/23] osfs: Remove unused tempFile and openFile Signed-off-by: Paulo Gomes --- osfs/os.go | 26 -------------------------- osfs/os_bound_test.go | 16 ++++++++++++---- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/osfs/os.go b/osfs/os.go index 4b42a19..faf0c63 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -4,7 +4,6 @@ package osfs import ( - "fmt" "io" "io/fs" "os" @@ -62,31 +61,6 @@ const ( BoundOSFS ) -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_bound_test.go b/osfs/os_bound_test.go index 2f514a1..61cd6ae 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -437,7 +437,7 @@ func TestRemove(t *testing.T) { func TestRemoveAll(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir).(*BoundOS) + 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)) @@ -448,7 +448,7 @@ func TestRemoveAll(t *testing.T) { func TestRemoveAllRemovesSymlink(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir).(*BoundOS) + 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"))) @@ -465,7 +465,7 @@ func TestRemoveBaseDir(t *testing.T) { for _, path := range tests { t.Run(path, func(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir).(*BoundOS) + fs := newTestBoundOS(t, dir) require.ErrorIs(t, fs.Remove(path), billy.ErrBaseDirCannotBeRemoved) require.ErrorIs(t, fs.RemoveAll(path), billy.ErrBaseDirCannotBeRemoved) mustExist(t, dir) @@ -475,7 +475,7 @@ func TestRemoveBaseDir(t *testing.T) { func TestRemoveBaseDirAbsolutePath(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir).(*BoundOS) + fs := newTestBoundOS(t, dir) require.ErrorIs(t, fs.Remove(dir), billy.ErrBaseDirCannotBeRemoved) require.ErrorIs(t, fs.RemoveAll(dir), billy.ErrBaseDirCannotBeRemoved) @@ -641,3 +641,11 @@ func requireFileContents(t *testing.T, filename string, want []byte) { require.NoError(t, err) assert.Equal(t, want, got) } + +func newTestBoundOS(t *testing.T, dir string) *BoundOS { + t.Helper() + + fs, ok := newBoundOS(dir).(*BoundOS) + require.True(t, ok) + return fs +} From c2029c2dd53dfcbc81e84e0c558e53b925578d99 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 8 May 2026 17:29:09 +0100 Subject: [PATCH 09/23] osfs: Normalise path Signed-off-by: Paulo Gomes --- osfs/os_bound.go | 25 ++++++++++++++++++++----- osfs/os_bound_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/osfs/os_bound.go b/osfs/os_bound.go index ea839c6..d6f46d9 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -46,13 +46,28 @@ type BoundOS struct { } func newBoundOS(d string, _ ...bool) billy.Filesystem { + if d == "" { + d = string(os.PathSeparator) + } return &BoundOS{baseDir: d} } // 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 (fs *BoundOS) rootFSWithCreate(createBase bool) (*RootOS, func(), error) { r, err := os.OpenRoot(fs.baseDir) + if err != nil { + if createBase && os.IsNotExist(err) { + if mkErr := os.MkdirAll(fs.baseDir, defaultDirectoryMode); mkErr != nil { + return nil, func() {}, mkErr + } + r, err = os.OpenRoot(fs.baseDir) + } + } if err != nil { return nil, func() {}, err } @@ -68,7 +83,7 @@ func (fs *BoundOS) Create(name string) (billy.File, error) { } func (fs *BoundOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.File, error) { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(flag&os.O_CREATE != 0) if err != nil { return nil, err } @@ -95,7 +110,7 @@ func (fs *BoundOS) Rename(from, to string) error { } func (fs *BoundOS) MkdirAll(name string, perm gofs.FileMode) error { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(true) if err != nil { return err } @@ -145,7 +160,7 @@ func (fs *BoundOS) RemoveAll(name string) error { } func (fs *BoundOS) Symlink(oldname, newname string) error { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(true) if err != nil { return err } @@ -183,7 +198,7 @@ func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { // 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) { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(true) if err != nil { return nil, err } @@ -199,7 +214,7 @@ func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { } defer childRoot.Close() - return newBoundOS(childRoot.Name()), nil + return newBoundOS(filepath.Clean(childRoot.Name())), nil } // Root returns the current base dir of the billy.Filesystem. diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 61cd6ae..1fd01d8 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -323,6 +323,15 @@ func TestChroot(t *testing.T) { assert.Equal(t, filepath.Join(tmp, "test"), chrooted.Root()) } +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) @@ -344,6 +353,37 @@ func TestRoot(t *testing.T) { assert.Equal(t, dir, fs.Root()) } +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 TestReadlink(t *testing.T) { dir := t.TempDir() fs := newBoundOS(dir) From 83fbcf6751fcf1ca0e8c0933710819a7c5c2deab Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 8 May 2026 20:51:07 +0100 Subject: [PATCH 10/23] osfs: Check for rooted path Signed-off-by: Paulo Gomes --- osfs/os_bound_test.go | 2 +- osfs/os_rootfs.go | 14 ++++++++++++-- test/common_windows.go | 2 +- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 1fd01d8..50276d2 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -392,7 +392,7 @@ func TestReadlink(t *testing.T) { got, err := fs.Readlink("symlink") require.NoError(t, err) - assert.Equal(t, target, got) + assert.Equal(t, filepath.ToSlash(target), got) _, err = fs.Readlink("../../symlink") require.NoError(t, err) diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go index ad064bc..e54afe8 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -230,7 +230,7 @@ func (fs *RootOS) Readlink(name string) (string, error) { return "", translateError(err, rel) } - return lnk, nil + return filepath.ToSlash(lnk), nil } func (fs *RootOS) Chmod(path string, mode gofs.FileMode) error { @@ -297,9 +297,10 @@ func (fs *RootOS) toRelative(name string) string { } func (fs *RootOS) absoluteSymlinkTarget(target string) (string, bool) { - if !filepath.IsAbs(target) { + if !isRootedPath(target) { return "", false } + target = filepath.FromSlash(target) rel, ok := relativeInsideBase(fs.root.Name(), target) if !ok { rel = cleanUnderRoot(target) @@ -324,7 +325,16 @@ func relativeInsideBase(base, name string) (string, bool) { return rel, true } +func isRootedPath(name string) bool { + if filepath.IsAbs(name) { + return true + } + name = filepath.FromSlash(name) + return strings.HasPrefix(name, string(filepath.Separator)) +} + func cleanUnderRoot(name string) string { + name = filepath.FromSlash(name) vol := filepath.VolumeName(name) name = name[len(vol):] name = filepath.Join(string(filepath.Separator), name) diff --git a/test/common_windows.go b/test/common_windows.go index b238d38..da0f656 100644 --- a/test/common_windows.go +++ b/test/common_windows.go @@ -12,7 +12,7 @@ import ( var ( customMode fs.FileMode = 0o666 - expectedSymlinkTarget = "\\dir\\file" + expectedSymlinkTarget = "/dir/file" ) func allFS(tempDir func() string) []billy.Filesystem { From 50c96410b08f82c636ff412cd2b0e655c15d5392 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 04:28:17 +0100 Subject: [PATCH 11/23] osfs: Ensure basedir does not need to exist Signed-off-by: Paulo Gomes --- osfs/os_bound.go | 157 +++++++++++++++++++++++++++++++--- osfs/os_bound_test.go | 38 ++++++++ osfs/os_bound_windows_test.go | 18 ++++ 3 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 osfs/os_bound_windows_test.go diff --git a/osfs/os_bound.go b/osfs/os_bound.go index d6f46d9..f1ac026 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -19,6 +19,7 @@ package osfs import ( + "errors" gofs "io/fs" "os" "path/filepath" @@ -59,15 +60,7 @@ func (fs *BoundOS) rootFS() (*RootOS, func(), error) { } func (fs *BoundOS) rootFSWithCreate(createBase bool) (*RootOS, func(), error) { - r, err := os.OpenRoot(fs.baseDir) - if err != nil { - if createBase && os.IsNotExist(err) { - if mkErr := os.MkdirAll(fs.baseDir, defaultDirectoryMode); mkErr != nil { - return nil, func() {}, mkErr - } - r, err = os.OpenRoot(fs.baseDir) - } - } + r, err := openRootAt(fs.baseDir, createBase) if err != nil { return nil, func() {}, err } @@ -123,6 +116,10 @@ func (fs *BoundOS) Open(name string) (billy.File, error) { } func (fs *BoundOS) Stat(name string) (os.FileInfo, error) { + if fs.isBaseDir(name) { + return fs.baseInfo(false) + } + rfs, cleanup, err := fs.rootFS() if err != nil { return nil, err @@ -169,6 +166,10 @@ func (fs *BoundOS) Symlink(oldname, newname string) error { } func (fs *BoundOS) Lstat(name string) (os.FileInfo, error) { + if fs.isBaseDir(name) { + return fs.baseInfo(true) + } + rfs, cleanup, err := fs.rootFS() if err != nil { return nil, err @@ -198,8 +199,15 @@ func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { // 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) { - rfs, cleanup, err := fs.rootFSWithCreate(true) + if hostPath, ok := fs.hostAbsolutePath(path); ok { + return newBoundOS(hostPath), 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() @@ -208,8 +216,11 @@ func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { if rel == "" { rel = "." } - childRoot, err := openChildRoot(rfs.root, rel) + childRoot, err := rfs.root.OpenRoot(rel) if err != nil { + if errors.Is(err, os.ErrNotExist) { + return newBoundOS(fs.chrootPath(path)), nil + } return nil, err } defer childRoot.Close() @@ -221,3 +232,127 @@ func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { func (fs *BoundOS) Root() string { return fs.baseDir } + +func (fs *BoundOS) isBaseDir(name string) bool { + if name == "" { + return true + } + + name = filepath.FromSlash(name) + if filepath.IsAbs(name) { + if rel, ok := relativeInsideBase(fs.baseDir, name); ok { + return cleanUnderRoot(rel) == "" + } + } + + return cleanUnderRoot(name) == "" +} + +func (fs *BoundOS) chrootPath(path string) string { + if hostPath, ok := fs.hostAbsolutePath(path); ok { + return filepath.Clean(hostPath) + } + + path = filepath.FromSlash(path) + if filepath.IsAbs(path) { + if rel, ok := relativeInsideBase(fs.baseDir, path); ok { + return filepath.Clean(filepath.Join(fs.baseDir, rel)) + } + } + + return filepath.Clean(filepath.Join(fs.baseDir, cleanUnderRoot(path))) +} + +func (fs *BoundOS) hostAbsolutePath(path string) (string, bool) { + if fs.baseDir != string(os.PathSeparator) { + return "", false + } + + path = filepath.FromSlash(path) + if filepath.VolumeName(path) != "" && filepath.IsAbs(path) { + return path, true + } + + if filepath.Separator == '\\' && len(path) >= 3 && + (path[0] == '\\' || path[0] == '/') && path[2] == ':' { + return path[1:], true + } + + return "", false +} + +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(".") + } + + root, err := openRootAt(parent, false) + if err != nil { + return nil, err + } + defer root.Close() + if lstat { + return root.Lstat(name) + } + return root.Stat(name) +} + +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 + } + defer ancestor.Close() + + if rel != "." { + if err := ancestor.MkdirAll(rel, defaultDirectoryMode); err != nil { + return nil, err + } + } + + return os.OpenRoot(path) +} + +func openExistingAncestor(path string) (*os.Root, string, error) { + path = filepath.Clean(path) + ancestorPath := path + + 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 + } + + parent := filepath.Dir(ancestorPath) + if parent == ancestorPath { + return nil, "", err + } + ancestorPath = parent + } +} diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 50276d2..e03676f 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -323,6 +323,18 @@ func TestChroot(t *testing.T) { 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) @@ -353,6 +365,17 @@ func TestRoot(t *testing.T) { 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) + fi, err := fs.Stat("/") + require.NoError(t, err) + assert.False(t, fi.IsDir()) +} + func TestEmptyBaseUsesOSRoot(t *testing.T) { dir := t.TempDir() name := filepath.Join(dir, "file") @@ -384,6 +407,21 @@ func TestCreateCreatesMissingBaseDir(t *testing.T) { 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) diff --git a/osfs/os_bound_windows_test.go b/osfs/os_bound_windows_test.go new file mode 100644 index 0000000..bf2bd01 --- /dev/null +++ b/osfs/os_bound_windows_test.go @@ -0,0 +1,18 @@ +//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`)) +} From 76b2fb6b78e7ad9bfc1a0131b7672e09217fd947 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 05:01:43 +0100 Subject: [PATCH 12/23] osfs: Normalise host path Signed-off-by: Paulo Gomes --- osfs/os_bound.go | 50 ++++++++++++++++++++--------------- osfs/os_bound_windows_test.go | 21 +++++++++++++++ osfs/os_rootfs.go | 25 +++++++++--------- 3 files changed, 62 insertions(+), 34 deletions(-) diff --git a/osfs/os_bound.go b/osfs/os_bound.go index f1ac026..a272927 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -50,17 +50,19 @@ func newBoundOS(d string, _ ...bool) billy.Filesystem { if d == "" { d = string(os.PathSeparator) } + d = hostPath(d) return &BoundOS{baseDir: d} } // 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) + return fs.rootFSWithCreate("", false) } -func (fs *BoundOS) rootFSWithCreate(createBase bool) (*RootOS, func(), error) { - r, err := openRootAt(fs.baseDir, createBase) +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 } @@ -76,7 +78,7 @@ func (fs *BoundOS) Create(name string) (billy.File, error) { } func (fs *BoundOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.File, error) { - rfs, cleanup, err := fs.rootFSWithCreate(flag&os.O_CREATE != 0) + rfs, cleanup, err := fs.rootFSWithCreate(name, flag&os.O_CREATE != 0) if err != nil { return nil, err } @@ -85,7 +87,7 @@ func (fs *BoundOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.Fi } func (fs *BoundOS) ReadDir(name string) ([]gofs.DirEntry, error) { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return nil, err } @@ -94,7 +96,7 @@ func (fs *BoundOS) ReadDir(name string) ([]gofs.DirEntry, error) { } func (fs *BoundOS) Rename(from, to string) error { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(from, false) if err != nil { return err } @@ -103,7 +105,7 @@ func (fs *BoundOS) Rename(from, to string) error { } func (fs *BoundOS) MkdirAll(name string, perm gofs.FileMode) error { - rfs, cleanup, err := fs.rootFSWithCreate(true) + rfs, cleanup, err := fs.rootFSWithCreate(name, true) if err != nil { return err } @@ -120,7 +122,7 @@ func (fs *BoundOS) Stat(name string) (os.FileInfo, error) { return fs.baseInfo(false) } - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return nil, err } @@ -129,7 +131,7 @@ func (fs *BoundOS) Stat(name string) (os.FileInfo, error) { } func (fs *BoundOS) Remove(name string) error { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return err } @@ -148,7 +150,7 @@ func (fs *BoundOS) Join(elem ...string) string { } func (fs *BoundOS) RemoveAll(name string) error { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return err } @@ -157,7 +159,7 @@ func (fs *BoundOS) RemoveAll(name string) error { } func (fs *BoundOS) Symlink(oldname, newname string) error { - rfs, cleanup, err := fs.rootFSWithCreate(true) + rfs, cleanup, err := fs.rootFSWithCreate(newname, true) if err != nil { return err } @@ -170,7 +172,7 @@ func (fs *BoundOS) Lstat(name string) (os.FileInfo, error) { return fs.baseInfo(true) } - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return nil, err } @@ -179,7 +181,7 @@ func (fs *BoundOS) Lstat(name string) (os.FileInfo, error) { } func (fs *BoundOS) Readlink(name string) (string, error) { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(name, false) if err != nil { return "", err } @@ -188,7 +190,7 @@ func (fs *BoundOS) Readlink(name string) (string, error) { } func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { - rfs, cleanup, err := fs.rootFS() + rfs, cleanup, err := fs.rootFSWithCreate(path, false) if err != nil { return err } @@ -238,7 +240,7 @@ func (fs *BoundOS) isBaseDir(name string) bool { return true } - name = filepath.FromSlash(name) + name = hostPath(name) if filepath.IsAbs(name) { if rel, ok := relativeInsideBase(fs.baseDir, name); ok { return cleanUnderRoot(rel) == "" @@ -253,7 +255,7 @@ func (fs *BoundOS) chrootPath(path string) string { return filepath.Clean(hostPath) } - path = filepath.FromSlash(path) + path = hostPath(path) if filepath.IsAbs(path) { if rel, ok := relativeInsideBase(fs.baseDir, path); ok { return filepath.Clean(filepath.Join(fs.baseDir, rel)) @@ -268,17 +270,23 @@ func (fs *BoundOS) hostAbsolutePath(path string) (string, bool) { return "", false } - path = filepath.FromSlash(path) + path = hostPath(path) if filepath.VolumeName(path) != "" && filepath.IsAbs(path) { return path, true } - if filepath.Separator == '\\' && len(path) >= 3 && - (path[0] == '\\' || path[0] == '/') && path[2] == ':' { - return path[1:], true + return "", false +} + +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) + } } - return "", false + return fs.baseDir } func (fs *BoundOS) baseInfo(lstat bool) (os.FileInfo, error) { diff --git a/osfs/os_bound_windows_test.go b/osfs/os_bound_windows_test.go index bf2bd01..360811c 100644 --- a/osfs/os_bound_windows_test.go +++ b/osfs/os_bound_windows_test.go @@ -16,3 +16,24 @@ func TestChrootPathKeepsDriveAbsolutePathFromEmptyBase(t *testing.T) { 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_rootfs.go b/osfs/os_rootfs.go index e54afe8..d005911 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -287,6 +287,7 @@ func (fs *RootOS) toRelative(name string) string { return "" } + name = hostPath(name) if filepath.IsAbs(name) { if rel, ok := relativeInsideBase(fs.root.Name(), name); ok { return cleanUnderRoot(rel) @@ -300,7 +301,7 @@ func (fs *RootOS) absoluteSymlinkTarget(target string) (string, bool) { if !isRootedPath(target) { return "", false } - target = filepath.FromSlash(target) + target = hostPath(target) rel, ok := relativeInsideBase(fs.root.Name(), target) if !ok { rel = cleanUnderRoot(target) @@ -312,6 +313,8 @@ func (fs *RootOS) absoluteSymlinkTarget(target string) (string, bool) { } 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 @@ -326,32 +329,28 @@ func relativeInsideBase(base, name string) (string, bool) { } func isRootedPath(name string) bool { + name = hostPath(name) if filepath.IsAbs(name) { return true } - name = filepath.FromSlash(name) return strings.HasPrefix(name, string(filepath.Separator)) } func cleanUnderRoot(name string) string { - name = filepath.FromSlash(name) + 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)) } -// openChildRoot validates path and opens a child [os.Root]. -func openChildRoot(root *os.Root, path string) (*os.Root, error) { - if err := ensureDir(root, path); err != nil { - return nil, err - } - - childRoot, err := root.OpenRoot(path) - if err != nil { - return nil, fmt.Errorf("unable to chroot: %w", translateError(err, path)) +func hostPath(name string) string { + name = filepath.FromSlash(name) + if filepath.Separator == '\\' && len(name) >= 3 && + name[0] == '\\' && name[2] == ':' { + return name[1:] } - return childRoot, nil + return name } func ensureDir(root *os.Root, path string) error { From 56ae0168d848867b24ab6108ab2eb816ddb26cb5 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 05:21:23 +0100 Subject: [PATCH 13/23] chroot: Improve handling of Windows paths Signed-off-by: Paulo Gomes --- helper/chroot/chroot.go | 10 +++++- helper/chroot/chroot_test.go | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/helper/chroot/chroot.go b/helper/chroot/chroot.go index f7618e9..a9e8167 100644 --- a/helper/chroot/chroot.go +++ b/helper/chroot/chroot.go @@ -395,8 +395,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..80e78ca 100644 --- a/helper/chroot/chroot_test.go +++ b/helper/chroot/chroot_test.go @@ -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 +} From 37a480edeacbb16d73950a1ff20701e963a40437 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 05:23:00 +0100 Subject: [PATCH 14/23] osfs: Improve handling of Windows paths Signed-off-by: Paulo Gomes --- osfs/os_bound_test.go | 14 ++++++++++++++ osfs/os_rootfs.go | 13 +++++++++++++ test/common_windows.go | 2 +- test/symlink_test.go | 3 ++- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index e03676f..516b26e 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -313,6 +313,20 @@ func TestTempFile(t *testing.T) { require.NoError(t, f.Close()) } +func TestDefaultTempFileNameCanBeReopened(t *testing.T) { + f, err := Default.TempFile("", "go-billy-") + require.NoError(t, err) + + 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) { tmp := t.TempDir() fs := newBoundOS(tmp) diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go index d005911..cd9258f 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -270,6 +270,11 @@ func (fs *RootOS) Root() string { } 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 "." @@ -353,6 +358,14 @@ func hostPath(name string) string { 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) { diff --git a/test/common_windows.go b/test/common_windows.go index da0f656..b238d38 100644 --- a/test/common_windows.go +++ b/test/common_windows.go @@ -12,7 +12,7 @@ import ( var ( customMode fs.FileMode = 0o666 - expectedSymlinkTarget = "/dir/file" + expectedSymlinkTarget = "\\dir\\file" ) func allFS(tempDir func() string) []billy.Filesystem { 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)) }) } From 0bc0ee952043add6c1ba333dc5f725225f1b73b7 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 11:43:32 +0100 Subject: [PATCH 15/23] Improve documentation Signed-off-by: Paulo Gomes --- embedfs/embed.go | 8 ++++++++ helper/chroot/chroot.go | 17 ++++++++++++++--- osfs/os.go | 14 ++++++++++++-- osfs/os_bound.go | 6 ++++-- osfs/os_rootfs.go | 6 ++++-- 5 files changed, 42 insertions(+), 9 deletions(-) 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/helper/chroot/chroot.go b/helper/chroot/chroot.go index a9e8167..d2ea8fc 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,12 @@ func (fs *ChrootHelper) Symlink(target, link string) error { return sl.Symlink(target, link) } +// Readlink returns the target stored for link. The returned target uses the +// host path separator. Targets that resolve outside the chroot base, or that +// were stored as relative paths, are returned as-is. Targets stored under the +// chroot base (typically those rewritten by [ChrootHelper.Symlink]) are +// translated back to be absolute relative to the chroot, so callers see paths +// in the chroot's coordinate system rather than the underlying filesystem's. func (fs *ChrootHelper) Readlink(link string) (string, error) { fullpath, err := fs.underlyingPath(link) if err != nil { diff --git a/osfs/os.go b/osfs/os.go index faf0c63..7929879 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -20,7 +20,14 @@ const ( // Default Filesystem representing the root of the os filesystem. var Default = newBoundOS(string(os.PathSeparator)) -// New returns a new OS filesystem. +// 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, @@ -32,7 +39,10 @@ func New(baseDir string, opts ...Option) billy.Filesystem { 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 diff --git a/osfs/os_bound.go b/osfs/os_bound.go index a272927..e5e24f3 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -38,8 +38,10 @@ import ( // Behaviours of note: // 1. Read and write operations can only be directed to files which descend // 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. +// 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 { diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go index cd9258f..150697c 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -42,8 +42,10 @@ func FromRoot(root *os.Root) (*RootOS, error) { // Behaviours of note: // 1. Read and write operations can only be directed to files which descend // from the root dir. -// 2. Symlinks don't have their targets modified, and therefore can point -// to locations outside the root dir or to non-existent paths. +// 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 { From 978e59d292ddfa075ecd2ce96212d199a93fc443 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 12:50:35 +0100 Subject: [PATCH 16/23] osfs: Fix build for js/wasm Signed-off-by: Paulo Gomes --- osfs/errors.go | 12 ++++++++++++ osfs/os_rootfs.go | 6 ------ 2 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 osfs/errors.go 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_rootfs.go b/osfs/os_rootfs.go index 150697c..1616282 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -15,12 +15,6 @@ import ( "github.com/go-git/go-billy/v6/util" ) -// ErrPathEscapesParent represents when an action leads to escaping from the -// given dir the filesystem is bound to. -// -// The upstream version of this error used by [os.Root] is not public. -var ErrPathEscapesParent = errors.New("path escapes from parent") - // FromRoot creates a new [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. From 9f7f71c13ce2c17b91ec3f747210e5d58c199069 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 12:51:42 +0100 Subject: [PATCH 17/23] tests: Add RootOS to comparison benchmarks Signed-off-by: Paulo Gomes --- test/bench_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/bench_test.go b/test/bench_test.go index aa54664..bc3ee2e 100644 --- a/test/bench_test.go +++ b/test/bench_test.go @@ -34,6 +34,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 +56,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, From 4b90a98537f11fb202aec93b15b1c3e925c22f26 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 12:52:10 +0100 Subject: [PATCH 18/23] Apply go fix ./... Signed-off-by: Paulo Gomes --- helper/chroot/chroot_test.go | 4 ++-- helper/mount/mount_test.go | 8 ++++---- internal/test/mock.go | 8 ++++---- memfs/file.go | 2 +- memfs/memory_test.go | 4 ++-- osfs/os_test.go | 2 +- test/basic_test.go | 2 +- test/dir_test.go | 2 +- test/tempfile_test.go | 8 ++++---- util/util.go | 4 ++-- util/util_test.go | 3 +-- 11 files changed, 23 insertions(+), 24 deletions(-) diff --git a/helper/chroot/chroot_test.go b/helper/chroot/chroot_test.go index 80e78ca..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) { 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/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/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/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 } From 17bae945218ace9abb8b55ca9a28c4cd1984c285 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 15:40:43 +0100 Subject: [PATCH 19/23] Address review feedback on osfs and chroot helper Adjusts ChrootHelper.Readlink doc to match the implementation (relative targets are returned unchanged), enriches the "not a dir" and Rename error messages with the offending paths, documents RootOS.Chroot's auto-create side effect, reintroduces WithChrootOS as a deprecated no-op for API compatibility with go-git, and expands TestStatBaseFile to cover ".", "./" and "./.". Assisted-by: Claude Opus 4.7 Signed-off-by: Paulo Gomes --- helper/chroot/chroot.go | 15 +++++++++------ osfs/os.go | 18 ++++++++++++++++++ osfs/os_bound_test.go | 11 ++++++++--- osfs/os_rootfs.go | 13 ++++++++----- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/helper/chroot/chroot.go b/helper/chroot/chroot.go index d2ea8fc..f98c762 100644 --- a/helper/chroot/chroot.go +++ b/helper/chroot/chroot.go @@ -384,12 +384,15 @@ func (fs *ChrootHelper) Symlink(target, link string) error { return sl.Symlink(target, link) } -// Readlink returns the target stored for link. The returned target uses the -// host path separator. Targets that resolve outside the chroot base, or that -// were stored as relative paths, are returned as-is. Targets stored under the -// chroot base (typically those rewritten by [ChrootHelper.Symlink]) are -// translated back to be absolute relative to the chroot, so callers see paths -// in the chroot's coordinate system rather than the underlying filesystem's. +// 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 { diff --git a/osfs/os.go b/osfs/os.go index 7929879..db6b7f0 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -49,6 +49,18 @@ func WithBoundOS() Option { } } +// WithChrootOS previously selected the path-traversal-based ChrootOS +// implementation. ChrootOS has been removed in favour of [BoundOS]; this +// option is now a no-op and is kept for API compatibility with downstream +// callers (notably go-git). +// +// Deprecated: use [WithBoundOS] (the default) or omit the option entirely. +func WithChrootOS() Option { + return func(o *options) { + o.Type = ChrootOSFS + } +} + // WithDeduplicatePath toggles the deduplication of the base dir in the path. // // BoundOS now relies on os.Root for path containment, so this option is kept @@ -64,10 +76,16 @@ type options struct { deduplicatePath bool } +// Type identifies an osfs implementation. The constants are retained for +// API compatibility; only [BoundOSFS] is selectable on non-js builds. type Type int const ( + // ChrootOSFS used to select the path-traversal-based ChrootOS. It is + // retained as a constant for source compatibility but no longer has an + // associated implementation; [New] always returns a [BoundOS]. ChrootOSFS Type = iota + // BoundOSFS selects the [BoundOS] implementation. BoundOSFS ) diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 516b26e..ca58b5d 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -20,6 +20,7 @@ package osfs import ( "errors" + "fmt" "os" "path/filepath" "runtime" @@ -385,9 +386,13 @@ func TestStatBaseFile(t *testing.T) { require.NoError(t, os.WriteFile(name, []byte("content"), 0o600)) fs := newBoundOS(name) - fi, err := fs.Stat("/") - require.NoError(t, err) - assert.False(t, fi.IsDir()) + 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) { diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go index 1616282..db3cfc5 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -121,12 +121,11 @@ func (fs *RootOS) Rename(from, to string) error { fromRel := fs.toRelative(from) toRel := fs.toRelative(to) - err := createDir(fs.root, toRel) - if err == nil { - err = fs.root.Rename(fromRel, toRel) + if err := createDir(fs.root, toRel); err != nil { + return translateError(err, toRel) } - 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 { @@ -242,6 +241,10 @@ func (fs *RootOS) Chmod(path string, mode gofs.FileMode) error { // 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 @@ -374,7 +377,7 @@ func ensureDir(root *os.Root, path string) error { return translateError(err, path) } if fi != nil && !fi.IsDir() { - return fmt.Errorf("cannot chroot: path is not dir") + return fmt.Errorf("cannot chroot: %q is not a dir (mode %s)", path, fi.Mode()) } return nil } From 8bf27914053bb292a398ff14a1b7e4bf5e3df735 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sat, 9 May 2026 15:44:37 +0100 Subject: [PATCH 20/23] tests: Skip bench comparison in js/wasm Signed-off-by: Paulo Gomes --- test/bench_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/bench_test.go b/test/bench_test.go index bc3ee2e..b14ef4b 100644 --- a/test/bench_test.go +++ b/test/bench_test.go @@ -1,3 +1,5 @@ +//go:build !js && !wasm && !wasip1 + package test import ( From 4cc2e1ee7bc73900485d34f81dbdd4fc8ff44c07 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sun, 10 May 2026 05:57:29 +0100 Subject: [PATCH 21/23] osfs: Remove ChrootOS references ChrootOS no longer exists; drop the WithChrootOS option, the ChrootOSFS type constant, and the js/wasm newChrootOS shim, along with the tests and API-compat assertions that referenced them. Assisted-by: Claude Opus 4.7 Signed-off-by: Paulo Gomes --- osfs/os.go | 21 ++------------------- osfs/os_js.go | 20 ++------------------ osfs/os_js_test.go | 11 ++--------- 3 files changed, 6 insertions(+), 46 deletions(-) diff --git a/osfs/os.go b/osfs/os.go index db6b7f0..500f77f 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -49,18 +49,6 @@ func WithBoundOS() Option { } } -// WithChrootOS previously selected the path-traversal-based ChrootOS -// implementation. ChrootOS has been removed in favour of [BoundOS]; this -// option is now a no-op and is kept for API compatibility with downstream -// callers (notably go-git). -// -// Deprecated: use [WithBoundOS] (the default) or omit the option entirely. -func WithChrootOS() Option { - return func(o *options) { - o.Type = ChrootOSFS - } -} - // WithDeduplicatePath toggles the deduplication of the base dir in the path. // // BoundOS now relies on os.Root for path containment, so this option is kept @@ -76,17 +64,12 @@ type options struct { deduplicatePath bool } -// Type identifies an osfs implementation. The constants are retained for -// API compatibility; only [BoundOSFS] is selectable on non-js builds. +// Type identifies an osfs implementation. type Type int const ( - // ChrootOSFS used to select the path-traversal-based ChrootOS. It is - // retained as a constant for source compatibility but no longer has an - // associated implementation; [New] always returns a [BoundOS]. - ChrootOSFS Type = iota // BoundOSFS selects the [BoundOS] implementation. - BoundOSFS + BoundOSFS Type = iota ) // file is a wrapper for an os.File which adds support for file locking. diff --git a/osfs/os_js.go b/osfs/os_js.go index 5520981..2666d1b 100644 --- a/osfs/os_js.go +++ b/osfs/os_js.go @@ -29,15 +29,7 @@ func New(baseDir string, opts ...Option) billy.Filesystem { 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, o.deduplicatePath) } // BoundOS is a fs implementation based on the js/wasm in-memory filesystem @@ -94,13 +86,6 @@ 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. @@ -120,6 +105,5 @@ type options struct { 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..fc21314 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) { @@ -108,11 +103,9 @@ func TestWithDeduplicatePathIsAccepted(t *testing.T) { // 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 ) From ca513f9d15907684fd92a9369aae94c83ab45bac Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sun, 10 May 2026 06:02:38 +0100 Subject: [PATCH 22/23] osfs: Remove unused WithDeduplicatePath option The option was a no-op in both build tags: on non-js the value was never read, and on js it was stored on BoundOS and propagated through Chroot but never consulted by path resolution. Containment is now handled by os.Root (non-js) and the chroot helper (js), making deduplication moot. Drops the option, the deduplicatePath fields, and the variadic bool on newBoundOS, along with the related test and API-compat assertion. Assisted-by: Claude Opus 4.7 Signed-off-by: Paulo Gomes --- osfs/os.go | 15 +-------------- osfs/os_bound.go | 2 +- osfs/os_js.go | 33 +++++++++------------------------ osfs/os_js_test.go | 9 --------- 4 files changed, 11 insertions(+), 48 deletions(-) diff --git a/osfs/os.go b/osfs/os.go index 500f77f..26263a2 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -29,9 +29,7 @@ var Default = newBoundOS(string(os.PathSeparator)) // 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) } @@ -49,19 +47,8 @@ func WithBoundOS() Option { } } -// WithDeduplicatePath toggles the deduplication of the base dir in the path. -// -// BoundOS now relies on os.Root for path containment, so this option is kept -// for API compatibility and has no effect. -func WithDeduplicatePath(enabled bool) Option { - return func(o *options) { - o.deduplicatePath = enabled - } -} - type options struct { Type - deduplicatePath bool } // Type identifies an osfs implementation. diff --git a/osfs/os_bound.go b/osfs/os_bound.go index e5e24f3..6a3d11c 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -48,7 +48,7 @@ type BoundOS struct { baseDir string } -func newBoundOS(d string, _ ...bool) billy.Filesystem { +func newBoundOS(d string) billy.Filesystem { if d == "" { d = string(os.PathSeparator) } diff --git a/osfs/os_js.go b/osfs/os_js.go index 2666d1b..5291d43 100644 --- a/osfs/os_js.go +++ b/osfs/os_js.go @@ -18,34 +18,29 @@ 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) } - return newBoundOS(baseDir, o.deduplicatePath) + 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, } } @@ -56,7 +51,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. @@ -86,20 +81,10 @@ func WithBoundOS() Option { } } -// 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 diff --git a/osfs/os_js_test.go b/osfs/os_js_test.go index fc21314..abca640 100644 --- a/osfs/os_js_test.go +++ b/osfs/os_js_test.go @@ -91,19 +91,10 @@ 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("/", WithDeduplicatePath(false)) // Type constants must stay exported for any consumer that switches on them. var ( From 64d3e12cdfa321fd3a52491fcb24d3c36a27441a Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sun, 10 May 2026 06:24:21 +0100 Subject: [PATCH 23/23] osfs: Align capabilities in os_js Signed-off-by: Paulo Gomes --- osfs/os_js.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osfs/os_js.go b/osfs/os_js.go index 5291d43..0a65286 100644 --- a/osfs/os_js.go +++ b/osfs/os_js.go @@ -44,6 +44,12 @@ func newBoundOS(d string) billy.Filesystem { } } +// 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) {