Fix prefix-boundary hole in filer root-escape checks (#5497)

## Why

Found during a full-repo review of the CLI. All filers guard against
relative paths escaping their root, but the guard in
`libs/filer/workspace_root_path.go` and `libs/filer/local_root_path.go`
was a bare string prefix check. A path that resolves to a sibling
directory sharing the root as a name prefix slipped through: with root
`/Users/me/proj`, joining `../proj-evil/x` resolves to
`/Users/me/proj-evil/x`, which passes `strings.HasPrefix`. Every filer
(workspace files, DBFS, UC volumes, local) shares these helpers, and
`bundle init` templates write template-author-controlled paths through
them.

## Changes

Before, a joined path only had to start with the root string; now it
must either be exactly the root or extend it past a separator boundary.

- `WorkspaceRootPath.Join` and `localRootPath.Join` compare against the
root with a trailing separator and explicitly allow the exact-root
result. Joins like `ReadDir(".")` resolve to exactly the root and keep
working.
- Filers rooted at `/` (used by the `fs` commands) keep working: the
separator is only appended when the cleaned root does not already end in
one. Same for Windows drive roots like `C:\`.
- The unrooted local filer (`NewLocalRootPath("")`, used by `fs` for
local paths) keeps accepting any path.

## Test plan

- [x] Added sibling-prefix escape cases (`../path-evil`,
`../path-evil/x`, `../pathx`) to
`libs/filer/workspace_root_path_test.go` and
`libs/filer/local_root_path_test.go` (Unix and Windows variants); these
fail without the fix
- [x] Added escape-and-re-enter cases asserting `../path/x` still joins
under the root
- [x] Added `TestLocalRootPathEmptyRoot` covering the unrooted local
filer passthrough
- [x] Existing exact-root cases (`Join("")`, `Join(".")`, `Join("/")`)
pass unchanged
- [x] `go test ./libs/filer/` passes
- [x] `go test ./libs/template/... ./libs/sync/...` (direct consumers of
these helpers) passes
- [x] `./task fmt-q`, `./task lint-q`, `./task checks` pass

This pull request and its description were written by Isaac.

Author: simonfaltum

libs/filer/local_root_path.go

@@ -19,7 +19,21 @@ func NewLocalRootPath(root string) localRootPath {
 
 func (rp *localRootPath) Join(name string) (string, error) {
 	absPath := filepath.Join(rp.rootPath, name)
-	if !strings.HasPrefix(absPath, rp.rootPath) {
+
+	// An empty root carries no restriction (cmd/fs constructs unrooted local filers).
+	if rp.rootPath == "" {
+		return absPath, nil
+	}
+
+	// Joining exactly the root must stay allowed: calls like ReadDir(".") resolve to it.
+	// Any other path must extend the root past a separator boundary; a plain prefix
+	// check would also accept siblings like "/root-evil" for root "/root".
+	// Cleaned roots like "/" or `C:\` already end in a separator.
+	root := rp.rootPath
+	if !strings.HasSuffix(root, string(filepath.Separator)) {
+		root += string(filepath.Separator)
+	}
+	if absPath != rp.rootPath && !strings.HasPrefix(absPath, root) {
 		return "", fmt.Errorf("relative path escapes root: %s", name)
 	}
 	return absPath, nil

libs/filer/local_root_path_test.go

@@ -52,6 +52,11 @@ func testUnixLocalRootPath(t *testing.T, uncleanRoot string) {
 	assert.NoError(t, err)
 	assert.Equal(t, cleanRoot, remotePath)
 
+	// All roots in this test end in the path element "path".
+	remotePath, err = rp.Join("../path/x")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/x", remotePath)
+
 	_, err = rp.Join("..")
 	assert.ErrorContains(t, err, `relative path escapes root: ..`)
 
@@ -78,6 +83,16 @@ func testUnixLocalRootPath(t *testing.T, uncleanRoot string) {
 
 	_, err = rp.Join("../..")
 	assert.ErrorContains(t, err, `relative path escapes root: ../..`)
+
+	// Siblings that share the root as a string prefix must be rejected.
+	_, err = rp.Join("../path-evil")
+	assert.ErrorContains(t, err, `relative path escapes root: ../path-evil`)
+
+	_, err = rp.Join("../path-evil/x")
+	assert.ErrorContains(t, err, `relative path escapes root: ../path-evil/x`)
+
+	_, err = rp.Join("../pathx")
+	assert.ErrorContains(t, err, `relative path escapes root: ../pathx`)
 }
 
 func TestUnixLocalRootPath(t *testing.T) {
@@ -128,6 +143,21 @@ func testWindowsLocalRootPath(t *testing.T, uncleanRoot string) {
 
 	_, err = rp.Join(`a\..\..`)
 	assert.ErrorContains(t, err, `relative path escapes root`)
+
+	// All roots in this test end in the path element "path".
+	remotePath, err = rp.Join(`..\path\x`)
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+`\x`, remotePath)
+
+	// Siblings that share the root as a string prefix must be rejected.
+	_, err = rp.Join(`..\path-evil`)
+	assert.ErrorContains(t, err, `relative path escapes root`)
+
+	_, err = rp.Join(`..\path-evil\x`)
+	assert.ErrorContains(t, err, `relative path escapes root`)
+
+	_, err = rp.Join(`..\pathx`)
+	assert.ErrorContains(t, err, `relative path escapes root`)
 }
 
 func TestWindowsLocalRootPath(t *testing.T) {
@@ -140,3 +170,16 @@ func TestWindowsLocalRootPath(t *testing.T) {
 	testWindowsLocalRootPath(t, `c:\some\root\path\.`)
 	testWindowsLocalRootPath(t, `C:\some\root\..\path\`)
 }
+
+func TestLocalRootPathEmptyRoot(t *testing.T) {
+	// An empty root is the unrooted local filer used by cmd/fs.
+	rp := NewLocalRootPath("")
+
+	p, err := rp.Join("a/b")
+	assert.NoError(t, err)
+	assert.Equal(t, filepath.Clean("a/b"), p)
+
+	p, err = rp.Join("../a")
+	assert.NoError(t, err)
+	assert.Equal(t, filepath.Clean("../a"), p)
+}

libs/filer/workspace_root_path.go

@@ -26,7 +26,15 @@ func (p *WorkspaceRootPath) Join(name string) (string, error) {
 	absPath := path.Join(p.rootPath, name)
 
 	// Don't allow escaping the specified root using relative paths.
-	if !strings.HasPrefix(absPath, p.rootPath) {
+	// Joining exactly the root must stay allowed: calls like ReadDir(".") resolve to it.
+	// Any other path must extend the root past a separator boundary; a plain prefix
+	// check would also accept siblings like "/root-evil" for root "/root".
+	// Cleaned roots like "/" (see cmd/fs) already end in a separator.
+	root := p.rootPath
+	if !strings.HasSuffix(root, "/") {
+		root += "/"
+	}
+	if absPath != p.rootPath && !strings.HasPrefix(absPath, root) {
 		return "", fmt.Errorf("relative path escapes root: %s", name)
 	}
 

libs/filer/workspace_root_path_test.go

@@ -51,6 +51,11 @@ func testRootPath(t *testing.T, uncleanRoot string) {
 	assert.NoError(t, err)
 	assert.Equal(t, cleanRoot, remotePath)
 
+	// All roots in this test end in the path element "path".
+	remotePath, err = rp.Join("../path/x")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/x", remotePath)
+
 	_, err = rp.Join("..")
 	assert.ErrorContains(t, err, `relative path escapes root: ..`)
 
@@ -77,6 +82,16 @@ func testRootPath(t *testing.T, uncleanRoot string) {
 
 	_, err = rp.Join("../..")
 	assert.ErrorContains(t, err, `relative path escapes root: ../..`)
+
+	// Siblings that share the root as a string prefix must be rejected.
+	_, err = rp.Join("../path-evil")
+	assert.ErrorContains(t, err, `relative path escapes root: ../path-evil`)
+
+	_, err = rp.Join("../path-evil/x")
+	assert.ErrorContains(t, err, `relative path escapes root: ../path-evil/x`)
+
+	_, err = rp.Join("../pathx")
+	assert.ErrorContains(t, err, `relative path escapes root: ../pathx`)
 }
 
 func TestRootPathClean(t *testing.T) {