diff --git a/allowedpaths/portable_windows.go b/allowedpaths/portable_windows.go index 6f64b06d..888582fd 100644 --- a/allowedpaths/portable_windows.go +++ b/allowedpaths/portable_windows.go @@ -26,11 +26,11 @@ func IsErrIsDirectory(err error) bool { // GetFileInformationByHandle (volume serial + file index). // The path and sandbox are needed to open the file through the sandbox. func FileIdentity(absPath string, _ fs.FileInfo, sandbox *Sandbox) (uint64, uint64, bool) { - root, relPath, ok := sandbox.resolve(absPath) + ar, relPath, ok := sandbox.resolve(absPath) if !ok { return 0, 0, false } - f, err := root.OpenFile(relPath, os.O_RDONLY, 0) + f, err := ar.root.OpenFile(relPath, os.O_RDONLY, 0) if err != nil { return 0, 0, false } diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 4da1f32b..705dbe85 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -19,6 +19,13 @@ import ( "strings" ) +// Access mode bits for permission checks. +const ( + modeRead = 0x04 + modeWrite = 0x02 + modeExecute = 0x01 +) + // MaxGlobEntries is the maximum number of directory entries read per single // glob expansion step. ReadDirForGlob returns an error for directories that // exceed this limit to prevent memory exhaustion during pattern matching. @@ -65,25 +72,137 @@ func New(paths []string) (sb *Sandbox, warnings []byte, err error) { return &Sandbox{roots: roots}, buf.Bytes(), nil } -// resolve returns the matching os.Root and the path relative to it for the -// given absolute path. It returns false if no root matches. -func (s *Sandbox) resolve(absPath string) (*os.Root, string, bool) { +// isPathEscapeError reports whether err is the unexported "path escapes +// from parent" error from os.Root. Stable per Hyrum's Law. +func isPathEscapeError(err error) bool { + var pe *os.PathError + if errors.As(err, &pe) { + return pe.Err != nil && pe.Err.Error() == "path escapes from parent" + } + return false +} + +// maxSymlinkHops is the maximum number of symlink resolutions performed +// when following cross-root symlinks. Prevents infinite loops from +// circular symlinks. +const maxSymlinkHops = 10 + +// resolve returns the matching root entry and the path relative to it for +// the given absolute path. It returns false if no root matches. +func (s *Sandbox) resolve(absPath string) (*root, string, bool) { if s == nil { return nil, "", false } - for _, ar := range s.roots { - rel, err := filepath.Rel(ar.absPath, absPath) + for i := range s.roots { + rel, err := filepath.Rel(s.roots[i].absPath, absPath) if err != nil { continue } if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { continue } - return ar.root, rel, true + return &s.roots[i], rel, true } return nil, "", false } +// resolveRootFollowingSymlinks resolves absPath to a (root, relPath) pair, +// following symlinks that cross between allowed roots. It walks the +// relative path component by component; when a component is a symlink, +// its target is resolved to an absolute path and matched against all +// roots, then resolution continues with the remaining components. +// +// When preserveLast is true, the final path component is not resolved +// even if it is a symlink. This preserves lstat/readlink semantics. +// +// This is only called as a fallback when the primary os.Root operation +// fails, so there is no overhead on the happy path. +func (s *Sandbox) resolveRootFollowingSymlinks(absPath string, preserveLast bool) (*root, string, bool) { + // Clean trailing slashes so filepath.Dir computes the correct parent + // when resolving relative symlink targets. + absPath = filepath.Clean(absPath) + // N+1 iterations: up to N to resolve symlinks, 1 more to confirm + // the final path has no more symlinks and return success. + for range maxSymlinkHops + 1 { + ar, rel, ok := s.resolve(absPath) + if !ok { + return nil, "", false + } + + // Walk rel component by component looking for symlinks. + components := strings.Split(rel, string(filepath.Separator)) + symlinkFound := false + for i := range components { + if components[i] == "." { + continue + } + // When preserveLast is set, skip the final component so that + // Lstat and Readlink operate on the symlink itself. + if preserveLast && i == len(components)-1 { + break + } + partial := strings.Join(components[:i+1], string(filepath.Separator)) + info, err := ar.root.Lstat(partial) + if err != nil { + // Component doesn't exist or isn't accessible. It can't + // be a symlink we need to resolve, so return what we have + // and let the caller get the real error. + return ar, rel, true + } + if info.Mode()&fs.ModeSymlink == 0 { + continue + } + // Found a symlink — read its target. + target, err := ar.root.Readlink(partial) + if err != nil { + return nil, "", false + } + // Resolve target to absolute path. + if !filepath.IsAbs(target) { + parentAbs := absPath + for j := len(components) - 1; j >= i; j-- { + parentAbs = filepath.Dir(parentAbs) + } + target = filepath.Join(parentAbs, target) + } + // Append remaining components after the symlink. + if i+1 < len(components) { + remaining := strings.Join(components[i+1:], string(filepath.Separator)) + target = filepath.Join(target, remaining) + } + absPath = filepath.Clean(target) + symlinkFound = true + break + } + if !symlinkFound { + return ar, rel, true + } + } + return nil, "", false // too many hops +} + +// resolveFollowingSymlinks is like resolveRootFollowingSymlinks but returns +// the *os.Root handle instead of the internal root entry. +func (s *Sandbox) resolveFollowingSymlinks(absPath string, preserveLast bool) (*os.Root, string, bool) { + ar, rel, ok := s.resolveRootFollowingSymlinks(absPath, preserveLast) + if !ok { + return nil, "", false + } + return ar.root, rel, true +} + +// openWithSymlinkFallback opens relPath through root, falling back to +// cross-root symlink resolution if the open fails with a path escape. +func (s *Sandbox) openWithSymlinkFallback(root *os.Root, relPath, absPath string) (*os.File, error) { + f, err := root.Open(relPath) + if err != nil && isPathEscapeError(err) { + if r, rel, ok := s.resolveFollowingSymlinks(absPath, false); ok { + f, err = r.Open(rel) + } + } + return f, err +} + // Access checks whether the resolved path is accessible with the given mode. // All operations go through os.Root to stay within the sandbox. // Mode: 0x04 = read, 0x02 = write, 0x01 = execute. @@ -118,7 +237,23 @@ func (s *Sandbox) Access(path string, cwd string, mode uint32) error { // performs the permission check (fd-relative OpenFile with // O_NONBLOCK for reads on Unix, mode-bit inspection for // everything else). - _, err = ar.accessCheck(rel, mode&0x04 != 0, mode&0x02 != 0, mode&0x01 != 0) + checkRead := mode&modeRead != 0 + checkWrite := mode&modeWrite != 0 + checkExec := mode&modeExecute != 0 + + _, err = ar.accessCheck(rel, checkRead, checkWrite, checkExec) + if err == nil { + return nil + } + if !isPathEscapeError(err) { + return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission} + } + // Symlink escapes this root — resolve across all roots. + resolved, resolvedRel, ok := s.resolveRootFollowingSymlinks(absPath, false) + if !ok { + return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission} + } + _, err = resolved.accessCheck(resolvedRel, checkRead, checkWrite, checkExec) if err != nil { return &os.PathError{Op: "access", Path: path, Err: os.ErrPermission} } @@ -157,12 +292,24 @@ func (s *Sandbox) Open(path string, cwd string, flag int, perm os.FileMode) (io. absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return nil, &os.PathError{Op: "open", Path: path, Err: os.ErrPermission} } - f, err := root.OpenFile(relPath, flag, perm) + f, err := ar.root.OpenFile(relPath, flag, perm) + if err == nil { + return f, nil + } + if !isPathEscapeError(err) { + return nil, PortablePathError(err) + } + // Symlink escapes this root — resolve across all roots. + r, rel, ok := s.resolveFollowingSymlinks(absPath, false) + if !ok { + return nil, PortablePathError(err) + } + f, err = r.OpenFile(rel, flag, perm) if err != nil { return nil, PortablePathError(err) } @@ -191,12 +338,12 @@ func (s *Sandbox) ReadDirForGlob(path string, cwd string) ([]fs.DirEntry, error) func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEntry, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return nil, &os.PathError{Op: "readdir", Path: path, Err: os.ErrPermission} } - f, err := root.Open(relPath) + f, err := s.openWithSymlinkFallback(ar.root, relPath, absPath) if err != nil { return nil, PortablePathError(err) } @@ -232,12 +379,12 @@ func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEnt func (s *Sandbox) OpenDir(path string, cwd string) (fs.ReadDirFile, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return nil, &os.PathError{Op: "opendir", Path: path, Err: os.ErrPermission} } - f, err := root.Open(relPath) + f, err := s.openWithSymlinkFallback(ar.root, relPath, absPath) if err != nil { return nil, PortablePathError(err) } @@ -250,12 +397,12 @@ func (s *Sandbox) OpenDir(path string, cwd string) (fs.ReadDirFile, error) { func (s *Sandbox) IsDirEmpty(path string, cwd string) (bool, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return false, &os.PathError{Op: "readdir", Path: path, Err: os.ErrPermission} } - f, err := root.Open(relPath) + f, err := s.openWithSymlinkFallback(ar.root, relPath, absPath) if err != nil { return false, PortablePathError(err) } @@ -278,11 +425,11 @@ func (s *Sandbox) IsDirEmpty(path string, cwd string) (bool, error) { // O(n) memory regardless of offset value, where n = min(maxRead, entries). func (s *Sandbox) ReadDirLimited(path string, cwd string, offset, maxRead int) ([]fs.DirEntry, bool, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return nil, false, &os.PathError{Op: "readdir", Path: path, Err: os.ErrPermission} } - f, err := root.Open(relPath) + f, err := s.openWithSymlinkFallback(ar.root, relPath, absPath) if err != nil { return nil, false, PortablePathError(err) } @@ -372,12 +519,23 @@ func (s *Sandbox) Stat(path string, cwd string) (fs.FileInfo, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return nil, &os.PathError{Op: "stat", Path: path, Err: os.ErrPermission} } - info, err := root.Stat(relPath) + info, err := ar.root.Stat(relPath) + if err == nil { + return info, nil + } + if !isPathEscapeError(err) { + return nil, PortablePathError(err) + } + r, rel, ok := s.resolveFollowingSymlinks(absPath, false) + if !ok { + return nil, PortablePathError(err) + } + info, err = r.Stat(rel) if err != nil { return nil, PortablePathError(err) } @@ -395,12 +553,23 @@ func (s *Sandbox) Lstat(path string, cwd string) (fs.FileInfo, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return nil, &os.PathError{Op: "lstat", Path: path, Err: os.ErrPermission} } - info, err := root.Lstat(relPath) + info, err := ar.root.Lstat(relPath) + if err == nil { + return info, nil + } + if !isPathEscapeError(err) { + return nil, PortablePathError(err) + } + r, rel, ok := s.resolveFollowingSymlinks(absPath, true) + if !ok { + return nil, PortablePathError(err) + } + info, err = r.Lstat(rel) if err != nil { return nil, PortablePathError(err) } @@ -411,12 +580,23 @@ func (s *Sandbox) Lstat(path string, cwd string) (fs.FileInfo, error) { func (s *Sandbox) Readlink(path string, cwd string) (string, error) { absPath := toAbs(path, cwd) - root, relPath, ok := s.resolve(absPath) + ar, relPath, ok := s.resolve(absPath) if !ok { return "", &os.PathError{Op: "readlink", Path: path, Err: os.ErrPermission} } - target, err := root.Readlink(relPath) + target, err := ar.root.Readlink(relPath) + if err == nil { + return target, nil + } + if !isPathEscapeError(err) { + return "", PortablePathError(err) + } + r, rel, ok := s.resolveFollowingSymlinks(absPath, true) + if !ok { + return "", PortablePathError(err) + } + target, err = r.Readlink(rel) if err != nil { return "", PortablePathError(err) } diff --git a/allowedpaths/sandbox_unix_test.go b/allowedpaths/sandbox_unix_test.go index fdd1a6b3..f9568edc 100644 --- a/allowedpaths/sandbox_unix_test.go +++ b/allowedpaths/sandbox_unix_test.go @@ -187,6 +187,29 @@ func TestAccessSymlinkWithinSandbox(t *testing.T) { assert.NoError(t, sb.Access("link.txt", dir, 0x04)) } +// TestIsPathEscapeError is a regression test for isPathEscapeError. +// If Go changes the "path escapes from parent" error string in a future +// version, this test will fail and alert us to update the detection. +func TestIsPathEscapeError(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outside, "file.txt"), []byte("data"), 0644)) + require.NoError(t, os.Symlink(filepath.Join(outside, "file.txt"), filepath.Join(dir, "escape.txt"))) + + r, err := os.OpenRoot(dir) + require.NoError(t, err) + defer r.Close() + + _, err = r.OpenFile("escape.txt", os.O_RDONLY, 0) + require.Error(t, err) + assert.True(t, isPathEscapeError(err), "expected path escape error, got: %v", err) + + // Non-escape errors should not match. + _, err = r.OpenFile("nonexistent.txt", os.O_RDONLY, 0) + require.Error(t, err) + assert.False(t, isPathEscapeError(err), "ENOENT should not be a path escape error") +} + // TestAccessSymlinkEscapeBlocked verifies that Access blocks symlinks // that resolve outside the sandbox. func TestAccessSymlinkEscapeBlocked(t *testing.T) { @@ -414,3 +437,262 @@ func TestAccessFIFONonBlocking(t *testing.T) { t.Fatal("Access blocked on FIFO — O_NONBLOCK not effective") } } + +// --- Cross-root symlink tests --- + +// TestCrossRootSymlinkOpen verifies that a symlink in one allowed root +// pointing to a file in another allowed root can be opened. +func TestCrossRootSymlinkOpen(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir1, "file.txt"), []byte("hello"), 0644)) + require.NoError(t, os.Symlink(filepath.Join(dir1, "file.txt"), filepath.Join(dir2, "link.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + f, err := sb.Open("link.txt", dir2, os.O_RDONLY, 0) + require.NoError(t, err) + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "hello", string(buf[:n])) +} + +// TestCrossRootSymlinkStat verifies that Stat follows a cross-root symlink. +func TestCrossRootSymlinkStat(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir1, "file.txt"), []byte("hello"), 0644)) + require.NoError(t, os.Symlink(filepath.Join(dir1, "file.txt"), filepath.Join(dir2, "link.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + info, err := sb.Stat("link.txt", dir2) + require.NoError(t, err) + assert.Equal(t, int64(5), info.Size()) + assert.Zero(t, info.Mode()&os.ModeSymlink, "Stat should follow the symlink") +} + +// TestCrossRootSymlinkAccess verifies that Access works through a cross-root symlink. +func TestCrossRootSymlinkAccess(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir1, "file.txt"), []byte("hello"), 0644)) + require.NoError(t, os.Symlink(filepath.Join(dir1, "file.txt"), filepath.Join(dir2, "link.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + assert.NoError(t, sb.Access("link.txt", dir2, modeRead)) +} + +// TestCrossRootSymlinkRelativeTarget verifies cross-root resolution with +// a relative symlink target (e.g. ../dir1/file.txt). +func TestCrossRootSymlinkRelativeTarget(t *testing.T) { + parent := t.TempDir() + dir1 := filepath.Join(parent, "dir1") + dir2 := filepath.Join(parent, "dir2") + require.NoError(t, os.MkdirAll(dir1, 0755)) + require.NoError(t, os.MkdirAll(dir2, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dir1, "file.txt"), []byte("data"), 0644)) + require.NoError(t, os.Symlink("../dir1/file.txt", filepath.Join(dir2, "link.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + f, err := sb.Open("link.txt", dir2, os.O_RDONLY, 0) + require.NoError(t, err) + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "data", string(buf[:n])) +} + +// TestCrossRootSymlinkIntermediateDir verifies that an intermediate directory +// symlink crossing roots is resolved correctly. +func TestCrossRootSymlinkIntermediateDir(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + subdir := filepath.Join(dir1, "sub") + require.NoError(t, os.MkdirAll(subdir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(subdir, "file.txt"), []byte("deep"), 0644)) + // dir2/link -> dir1/sub (directory symlink) + require.NoError(t, os.Symlink(subdir, filepath.Join(dir2, "link"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + f, err := sb.Open(filepath.Join("link", "file.txt"), dir2, os.O_RDONLY, 0) + require.NoError(t, err) + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "deep", string(buf[:n])) +} + +// TestCrossRootSymlinkOutsideAllRoots verifies that a symlink pointing +// outside all allowed roots is still blocked. +func TestCrossRootSymlinkOutsideAllRoots(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret"), 0644)) + require.NoError(t, os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(dir1, "escape.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + _, err = sb.Open("escape.txt", dir1, os.O_RDONLY, 0) + assert.Error(t, err) +} + +// TestCrossRootSymlinkMissingTarget verifies that a cross-root symlink +// pointing to a non-existent file returns ENOENT, not the escape error. +func TestCrossRootSymlinkMissingTarget(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + // link points into dir1, but the target file doesn't exist. + require.NoError(t, os.Symlink(filepath.Join(dir1, "missing.txt"), filepath.Join(dir2, "link.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + _, err = sb.Open("link.txt", dir2, os.O_RDONLY, 0) + require.Error(t, err) + assert.Contains(t, err.Error(), "no such file or directory", "should report file not found, not path escape") +} + +// TestCrossRootSymlinkLstatPreservesTerminal verifies that Lstat through a +// cross-root intermediate symlink preserves the terminal symlink. +func TestCrossRootSymlinkLstatPreservesTerminal(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + // dir1/sub/leaf.txt -> some_target (a symlink) + subdir := filepath.Join(dir1, "sub") + require.NoError(t, os.MkdirAll(subdir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(subdir, "real.txt"), []byte("data"), 0644)) + require.NoError(t, os.Symlink("real.txt", filepath.Join(subdir, "leaf.txt"))) + // dir2/bridge -> dir1/sub (cross-root directory symlink) + require.NoError(t, os.Symlink(subdir, filepath.Join(dir2, "bridge"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + // Lstat should report leaf.txt as a symlink, not the target. + info, err := sb.Lstat(filepath.Join("bridge", "leaf.txt"), dir2) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSymlink, "Lstat should report symlink mode for terminal component") +} + +// TestCrossRootSymlinkReadlinkPreservesTerminal verifies that Readlink +// through a cross-root intermediate symlink reads the terminal symlink. +func TestCrossRootSymlinkReadlinkPreservesTerminal(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + subdir := filepath.Join(dir1, "sub") + require.NoError(t, os.MkdirAll(subdir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(subdir, "real.txt"), []byte("data"), 0644)) + require.NoError(t, os.Symlink("real.txt", filepath.Join(subdir, "leaf.txt"))) + require.NoError(t, os.Symlink(subdir, filepath.Join(dir2, "bridge"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + target, err := sb.Readlink(filepath.Join("bridge", "leaf.txt"), dir2) + require.NoError(t, err) + assert.Equal(t, "real.txt", target) +} + +// TestCrossRootSymlinkLoopBlocked verifies that circular symlinks between +// roots are detected and rejected after maxSymlinkHops. +func TestCrossRootSymlinkLoopBlocked(t *testing.T) { + dir1 := t.TempDir() + dir2 := t.TempDir() + // dir1/a.txt -> dir2/b.txt -> dir1/a.txt (circular) + require.NoError(t, os.Symlink(filepath.Join(dir2, "b.txt"), filepath.Join(dir1, "a.txt"))) + require.NoError(t, os.Symlink(filepath.Join(dir1, "a.txt"), filepath.Join(dir2, "b.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + _, err = sb.Open("a.txt", dir1, os.O_RDONLY, 0) + assert.Error(t, err, "circular cross-root symlinks should be rejected") + + _, err = sb.Stat("a.txt", dir1) + assert.Error(t, err, "circular cross-root symlinks should be rejected") +} + +// TestCrossRootSymlinkChainLimit verifies that a long chain of cross-root +// symlinks exceeding maxSymlinkHops is rejected. +func TestCrossRootSymlinkChainLimit(t *testing.T) { + dirs := make([]string, maxSymlinkHops+2) + for i := range dirs { + dirs[i] = t.TempDir() + } + // Last directory has the real file. + require.NoError(t, os.WriteFile(filepath.Join(dirs[len(dirs)-1], "file.txt"), []byte("end"), 0644)) + + // Each dir[i]/link.txt -> dir[i+1]/link.txt, except the penultimate + // which points to the real file. + for i := 0; i < len(dirs)-1; i++ { + target := filepath.Join(dirs[i+1], "link.txt") + if i == len(dirs)-2 { + target = filepath.Join(dirs[i+1], "file.txt") + } + require.NoError(t, os.Symlink(target, filepath.Join(dirs[i], "link.txt"))) + } + + sb, _, err := New(dirs) + require.NoError(t, err) + defer sb.Close() + + _, err = sb.Open("link.txt", dirs[0], os.O_RDONLY, 0) + assert.Error(t, err, "symlink chain exceeding maxSymlinkHops should be rejected") +} + +// TestCrossRootSymlinkSiblingDirs verifies that a symlink in one sibling +// directory pointing into another sibling directory can be read when both +// are in AllowedPaths. +func TestCrossRootSymlinkSiblingDirs(t *testing.T) { + parent := t.TempDir() + dir1 := filepath.Join(parent, "dir1") + dir2 := filepath.Join(parent, "dir2") + require.NoError(t, os.MkdirAll(dir1, 0755)) + require.NoError(t, os.MkdirAll(dir2, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(dir1, "file.txt"), []byte("abc"), 0644)) + // dir2/sym.txt -> ../dir1/file.txt + require.NoError(t, os.Symlink("../dir1/file.txt", filepath.Join(dir2, "sym.txt"))) + + sb, _, err := New([]string{dir1, dir2}) + require.NoError(t, err) + defer sb.Close() + + f, err := sb.Open(filepath.Join(dir2, "sym.txt"), "/", os.O_RDONLY, 0) + require.NoError(t, err, "cross-root symlink between sibling dirs should be readable") + defer f.Close() + + buf := make([]byte, 64) + n, _ := f.Read(buf) + assert.Equal(t, "abc", string(buf[:n])) + + info, err := sb.Stat(filepath.Join(dir2, "sym.txt"), "/") + require.NoError(t, err) + assert.Equal(t, int64(3), info.Size()) + + assert.NoError(t, sb.Access(filepath.Join(dir2, "sym.txt"), "/", modeRead)) +} diff --git a/analysis/symbols_allowedpaths.go b/analysis/symbols_allowedpaths.go index c129e39d..73cc7c7b 100644 --- a/analysis/symbols_allowedpaths.go +++ b/analysis/symbols_allowedpaths.go @@ -27,6 +27,7 @@ var allowedpathsAllowedSymbols = []string{ "io.EOF", // 🟢 sentinel error value; pure constant. "io.ReadWriteCloser", // 🟢 combined interface type; no side effects. "io/fs.DirEntry", // 🟢 interface type for directory entries; no side effects. + "io/fs.ModeSymlink", // 🟢 file mode bit for symlinks; pure constant. "io/fs.ErrExist", // 🟢 sentinel error for "already exists"; pure constant. "io/fs.ErrNotExist", // 🟢 sentinel error for "does not exist"; pure constant. "io/fs.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. @@ -35,6 +36,7 @@ var allowedpathsAllowedSymbols = []string{ "io/fs.ReadDirFile", // 🟢 read-only directory handle interface; no write capability. "os.DevNull", // 🟢 platform null device path constant; pure constant. "os.ErrPermission", // 🟢 sentinel error for permission denied; pure constant. + "os.File", // 🟠 file handle returned by os.Root.Open; needed for cross-root symlink fallback. "os.FileMode", // 🟢 file permission bits type; pure type. "os.Getgid", // 🟠 returns the numeric group id of the caller; read-only syscall. "os.Getgroups", // 🟠 returns supplementary group ids; read-only syscall. @@ -45,6 +47,8 @@ var allowedpathsAllowedSymbols = []string{ "os.Root", // 🟠 sandboxed directory root type; core of the filesystem sandbox. "os.Stat", // 🟠 returns file info for a path; needed for sandbox path validation. "path/filepath.Abs", // 🟢 returns absolute path; pure path computation. + "path/filepath.Clean", // 🟢 normalizes a path; pure function, no I/O. + "path/filepath.Dir", // 🟢 returns directory portion of a path; pure function, no I/O. "path/filepath.IsAbs", // 🟢 checks if path is absolute; pure function, no I/O. "path/filepath.Join", // 🟢 joins path elements; pure function, no I/O. "path/filepath.Rel", // 🟢 returns relative path; pure path computation. @@ -54,6 +58,8 @@ var allowedpathsAllowedSymbols = []string{ "strings.Compare", // 🟢 compares two strings lexicographically; pure function, no I/O. "strings.EqualFold", // 🟢 case-insensitive string comparison; pure function, no I/O. "strings.HasPrefix", // 🟢 pure function for prefix matching; no I/O. + "strings.Join", // 🟢 joins string slices; pure function, no I/O. + "strings.Split", // 🟢 splits a string by separator; pure function, no I/O. "syscall.ByHandleFileInformation", // 🟢 Windows file identity structure; pure type for file metadata. "syscall.EISDIR", // 🟢 "is a directory" errno constant; pure constant. "syscall.Errno", // 🟢 system call error number type; pure type. diff --git a/tests/scenarios/shell/allowed_paths/cross_root_symlink_dir.yaml b/tests/scenarios/shell/allowed_paths/cross_root_symlink_dir.yaml new file mode 100644 index 00000000..bd4efaee --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/cross_root_symlink_dir.yaml @@ -0,0 +1,18 @@ +description: Symlink to a directory in another allowed root can be traversed +# skip: allowed_paths sandbox restriction is an rshell-specific feature not present in bash +skip_assert_against_bash: true +setup: + files: + - path: dir1/sub/file.txt + content: "deep content\n" + - path: dir2/link + symlink: ../dir1/sub +input: + allowed_paths: ["dir1", "dir2"] + script: |+ + cat dir2/link/file.txt +expect: + stdout: |+ + deep content + stderr: |+ + exit_code: 0 diff --git a/tests/scenarios/shell/allowed_paths/cross_root_symlink_outside_blocked.yaml b/tests/scenarios/shell/allowed_paths/cross_root_symlink_outside_blocked.yaml new file mode 100644 index 00000000..95fe373c --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/cross_root_symlink_outside_blocked.yaml @@ -0,0 +1,18 @@ +description: Symlink pointing outside all allowed roots is still blocked +# skip: allowed_paths sandbox restriction is an rshell-specific feature not present in bash +skip_assert_against_bash: true +setup: + files: + - path: outside/secret.txt + content: "secret\n" + - path: dir1/escape.txt + symlink: ../outside/secret.txt +input: + allowed_paths: ["dir1"] + script: |+ + cat dir1/escape.txt +expect: + stdout: |+ + stderr_contains: + - "path escapes from parent" + exit_code: 1 diff --git a/tests/scenarios/shell/allowed_paths/cross_root_symlink_read.yaml b/tests/scenarios/shell/allowed_paths/cross_root_symlink_read.yaml new file mode 100644 index 00000000..c6e137fc --- /dev/null +++ b/tests/scenarios/shell/allowed_paths/cross_root_symlink_read.yaml @@ -0,0 +1,18 @@ +description: Symlink in one allowed root pointing to a file in another allowed root can be read +# skip: allowed_paths sandbox restriction is an rshell-specific feature not present in bash +skip_assert_against_bash: true +setup: + files: + - path: dir1/file.txt + content: "cross-root content\n" + - path: dir2/link.txt + symlink: ../dir1/file.txt +input: + allowed_paths: ["dir1", "dir2"] + script: |+ + cat dir2/link.txt +expect: + stdout: |+ + cross-root content + stderr: |+ + exit_code: 0