Extract filer path handling into separate type (#138)

This makes it reusable for the DBFS filer.

libs/filer/root_path.go

@@ -0,0 +1,39 @@
+package filer
+
+import (
+	"fmt"
+	"path"
+	"strings"
+)
+
+// RootPath can be joined with a relative path and ensures that
+// the returned path is always a strict child of the root path.
+type RootPath struct {
+	rootPath string
+}
+
+// NewRootPath constructs and returns [RootPath].
+// The named path is cleaned on construction.
+func NewRootPath(name string) RootPath {
+	return RootPath{
+		rootPath: path.Clean(name),
+	}
+}
+
+// Join returns the specified path name joined to the root.
+// It returns an error if the resulting path is not a strict child of the root path.
+func (p *RootPath) 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) {
+		return "", fmt.Errorf("relative path escapes root: %s", name)
+	}
+
+	// Don't allow name to resolve to the root path.
+	if strings.TrimPrefix(absPath, p.rootPath) == "" {
+		return "", fmt.Errorf("relative path resolves to root: %s", name)
+	}
+
+	return absPath, nil
+}

libs/filer/root_path_test.go

@@ -0,0 +1,85 @@
+package filer
+
+import (
+	"path"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func testRootPath(t *testing.T, uncleanRoot string) {
+	cleanRoot := path.Clean(uncleanRoot)
+	rp := NewRootPath(uncleanRoot)
+
+	remotePath, err := rp.Join("a/b/c")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/a/b/c", remotePath)
+
+	remotePath, err = rp.Join("a/b/../d")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/a/d", remotePath)
+
+	remotePath, err = rp.Join("a/../c")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/c", remotePath)
+
+	remotePath, err = rp.Join("a/b/c/.")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/a/b/c", remotePath)
+
+	remotePath, err = rp.Join("a/b/c/d/./../../f/g")
+	assert.NoError(t, err)
+	assert.Equal(t, cleanRoot+"/a/b/f/g", remotePath)
+
+	_, err = rp.Join("..")
+	assert.ErrorContains(t, err, `relative path escapes root: ..`)
+
+	_, err = rp.Join("a/../..")
+	assert.ErrorContains(t, err, `relative path escapes root: a/../..`)
+
+	_, err = rp.Join("./../.")
+	assert.ErrorContains(t, err, `relative path escapes root: ./../.`)
+
+	_, err = rp.Join("/./.././..")
+	assert.ErrorContains(t, err, `relative path escapes root: /./.././..`)
+
+	_, err = rp.Join("./../.")
+	assert.ErrorContains(t, err, `relative path escapes root: ./../.`)
+
+	_, err = rp.Join("./..")
+	assert.ErrorContains(t, err, `relative path escapes root: ./..`)
+
+	_, err = rp.Join("./../../..")
+	assert.ErrorContains(t, err, `relative path escapes root: ./../../..`)
+
+	_, err = rp.Join("./../a/./b../../..")
+	assert.ErrorContains(t, err, `relative path escapes root: ./../a/./b../../..`)
+
+	_, err = rp.Join("../..")
+	assert.ErrorContains(t, err, `relative path escapes root: ../..`)
+
+	_, err = rp.Join(".//a/..//./b/..")
+	assert.ErrorContains(t, err, `relative path resolves to root: .//a/..//./b/..`)
+
+	_, err = rp.Join("a/b/../..")
+	assert.ErrorContains(t, err, "relative path resolves to root: a/b/../..")
+
+	_, err = rp.Join("")
+	assert.ErrorContains(t, err, "relative path resolves to root: ")
+
+	_, err = rp.Join(".")
+	assert.ErrorContains(t, err, "relative path resolves to root: .")
+
+	_, err = rp.Join("/")
+	assert.ErrorContains(t, err, "relative path resolves to root: /")
+}
+
+func TestRootPathClean(t *testing.T) {
+	testRootPath(t, "/some/root/path")
+}
+
+func TestRootPathUnclean(t *testing.T) {
+	testRootPath(t, "/some/root/path/")
+	testRootPath(t, "/some/root/path/.")
+	testRootPath(t, "/some/root/../path/")
+}

libs/filer/workspace_files_client.go

@@ -23,7 +23,7 @@ type WorkspaceFilesClient struct {
 	apiClient       *client.DatabricksClient
 
 	// File operations will be relative to this path.
-	root string
+	root RootPath
 }
 
 func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer, error) {
@@ -36,28 +36,12 @@ func NewWorkspaceFilesClient(w *databricks.WorkspaceClient, root string) (Filer,
 		workspaceClient: w,
 		apiClient:       apiClient,
 
-		root: path.Clean(root),
+		root: NewRootPath(root),
 	}, nil
 }
 
-func (w *WorkspaceFilesClient) absPath(name string) (string, error) {
-	absPath := path.Join(w.root, name)
-
-	// Don't allow escaping the specified root using relative paths.
-	if !strings.HasPrefix(absPath, w.root) {
-		return "", fmt.Errorf("relative path escapes root: %s", name)
-	}
-
-	// Don't allow name to resolve to the root path.
-	if strings.TrimPrefix(absPath, w.root) == "" {
-		return "", fmt.Errorf("relative path resolves to root: %s", name)
-	}
-
-	return absPath, nil
-}
-
 func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io.Reader, mode ...WriteMode) error {
-	absPath, err := w.absPath(name)
+	absPath, err := w.root.Join(name)
 	if err != nil {
 		return err
 	}
@@ -109,7 +93,7 @@ func (w *WorkspaceFilesClient) Write(ctx context.Context, name string, reader io
 }
 
 func (w *WorkspaceFilesClient) Read(ctx context.Context, name string) (io.Reader, error) {
-	absPath, err := w.absPath(name)
+	absPath, err := w.root.Join(name)
 	if err != nil {
 		return nil, err
 	}
@@ -131,7 +115,7 @@ func (w *WorkspaceFilesClient) Read(ctx context.Context, name string) (io.Reader
 }
 
 func (w *WorkspaceFilesClient) Delete(ctx context.Context, name string) error {
-	absPath, err := w.absPath(name)
+	absPath, err := w.root.Join(name)
 	if err != nil {
 		return err
 	}

libs/filer/workspace_files_client_test.go

@@ -1,74 +0,0 @@
-package filer
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestWorkspaceFilesClientPaths(t *testing.T) {
-	root := "/some/root/path"
-	f := WorkspaceFilesClient{root: root}
-
-	remotePath, err := f.absPath("a/b/c")
-	assert.NoError(t, err)
-	assert.Equal(t, root+"/a/b/c", remotePath)
-
-	remotePath, err = f.absPath("a/b/../d")
-	assert.NoError(t, err)
-	assert.Equal(t, root+"/a/d", remotePath)
-
-	remotePath, err = f.absPath("a/../c")
-	assert.NoError(t, err)
-	assert.Equal(t, root+"/c", remotePath)
-
-	remotePath, err = f.absPath("a/b/c/.")
-	assert.NoError(t, err)
-	assert.Equal(t, root+"/a/b/c", remotePath)
-
-	remotePath, err = f.absPath("a/b/c/d/./../../f/g")
-	assert.NoError(t, err)
-	assert.Equal(t, root+"/a/b/f/g", remotePath)
-
-	_, err = f.absPath("..")
-	assert.ErrorContains(t, err, `relative path escapes root: ..`)
-
-	_, err = f.absPath("a/../..")
-	assert.ErrorContains(t, err, `relative path escapes root: a/../..`)
-
-	_, err = f.absPath("./../.")
-	assert.ErrorContains(t, err, `relative path escapes root: ./../.`)
-
-	_, err = f.absPath("/./.././..")
-	assert.ErrorContains(t, err, `relative path escapes root: /./.././..`)
-
-	_, err = f.absPath("./../.")
-	assert.ErrorContains(t, err, `relative path escapes root: ./../.`)
-
-	_, err = f.absPath("./..")
-	assert.ErrorContains(t, err, `relative path escapes root: ./..`)
-
-	_, err = f.absPath("./../../..")
-	assert.ErrorContains(t, err, `relative path escapes root: ./../../..`)
-
-	_, err = f.absPath("./../a/./b../../..")
-	assert.ErrorContains(t, err, `relative path escapes root: ./../a/./b../../..`)
-
-	_, err = f.absPath("../..")
-	assert.ErrorContains(t, err, `relative path escapes root: ../..`)
-
-	_, err = f.absPath(".//a/..//./b/..")
-	assert.ErrorContains(t, err, `relative path resolves to root: .//a/..//./b/..`)
-
-	_, err = f.absPath("a/b/../..")
-	assert.ErrorContains(t, err, "relative path resolves to root: a/b/../..")
-
-	_, err = f.absPath("")
-	assert.ErrorContains(t, err, "relative path resolves to root: ")
-
-	_, err = f.absPath(".")
-	assert.ErrorContains(t, err, "relative path resolves to root: .")
-
-	_, err = f.absPath("/")
-	assert.ErrorContains(t, err, "relative path resolves to root: /")
-}