diff --git a/libs/filer/root_path.go b/libs/filer/root_path.go new file mode 100644 index 00000000..65b26d53 --- /dev/null +++ b/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 +} diff --git a/libs/filer/root_path_test.go b/libs/filer/root_path_test.go new file mode 100644 index 00000000..3787ef36 --- /dev/null +++ b/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/") +} diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index 03aa1b97..512f5f25 100644 --- a/libs/filer/workspace_files_client.go +++ b/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 } diff --git a/libs/filer/workspace_files_client_test.go b/libs/filer/workspace_files_client_test.go deleted file mode 100644 index 3700882a..00000000 --- a/libs/filer/workspace_files_client_test.go +++ /dev/null @@ -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: /") -}