Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion runtime/drivers/duckdb/model_executor_self_sqlite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"database/sql"
"fmt"
"path/filepath"
"testing"

"github.com/rilldata/rill/runtime/drivers"
Expand All @@ -18,7 +19,7 @@ import (
func Test_sqliteToDuckDB_Transfer(t *testing.T) {
tempDir := t.TempDir()

dbPath := fmt.Sprintf("%s.db", tempDir)
dbPath := filepath.Join(tempDir, "source.db")
db, err := sql.Open("sqlite", dbPath)
require.NoError(t, err)

Expand All @@ -41,6 +42,7 @@ func Test_sqliteToDuckDB_Transfer(t *testing.T) {
OutputConnector: "duckdb",
Env: &drivers.ModelEnv{
AllowHostAccess: false,
RepoRoot: tempDir,
StageChanges: true,
},
PreliminaryInputProperties: map[string]any{
Expand Down
1 change: 1 addition & 0 deletions runtime/drivers/duckdb/model_executor_self_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestDuckDBToDuckDBTransfer(t *testing.T) {
OutputConnector: "duckdb",
Env: &drivers.ModelEnv{
AllowHostAccess: false,
RepoRoot: tempDir,
StageChanges: true,
},
PreliminaryInputProperties: map[string]any{
Expand Down
16 changes: 12 additions & 4 deletions runtime/pkg/fileutil/fileutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,18 @@ func ResolveLocalPath(path, root string, allowHostAccess bool) (string, error) {
if !filepath.IsAbs(path) {
finalPath = filepath.Join(root, path)
}

if !allowHostAccess && !strings.HasPrefix(finalPath, root) {
// path is outside the repo root
return "", fmt.Errorf("path is outside repo root")
// Clean here so that ".." segments are resolved before the sandbox check below;
// filepath.Join already cleans the relative branch, but an absolute path does not pass through it.
finalPath = filepath.Clean(finalPath)

if !allowHostAccess {
// Ensure the resolved path stays within the repo root.
// The trailing separator prevents a sibling directory whose name shares the root's prefix
// (e.g. root "/data/proj" matching "/data/proj-x") from passing the check.
root = filepath.Clean(root)
if finalPath != root && !strings.HasPrefix(finalPath, root+string(filepath.Separator)) {
return "", fmt.Errorf("path is outside repo root")
}
}
return finalPath, nil
}
Expand Down
47 changes: 47 additions & 0 deletions runtime/pkg/fileutil/fileutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package fileutil

import (
"os/user"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -74,6 +75,52 @@ func TestExpandHome(t *testing.T) {
}
}

func TestResolveLocalPath(t *testing.T) {
sep := string(filepath.Separator)
root := filepath.Clean("/data/proj123")

variations := []struct {
Name string
Path string
Root string
AllowHostAccess bool
Expected string
ExpectErr bool
}{
// Relative paths within the root are joined and cleaned.
{"relative within root", "sub/file.csv", root, false, filepath.Join(root, "sub", "file.csv"), false},
{"relative to root itself", ".", root, false, root, false},

// Absolute paths within the root are allowed.
{"absolute within root", filepath.Join(root, "file.csv"), root, false, filepath.Join(root, "file.csv"), false},

// Absolute path traversal must be rejected once cleaned.
{"absolute traversal", root + sep + ".." + sep + ".." + sep + "etc" + sep + "passwd", root, false, "", true},

// Relative traversal escaping the root must be rejected.
{"relative traversal", ".." + sep + ".." + sep + "etc" + sep + "passwd", root, false, "", true},

// Sibling directory sharing the root's prefix must not pass the check.
{"sibling prefix collision", "/data/proj123-x/secret", root, false, "", true},
{"relative sibling prefix collision", ".." + sep + "proj123-x" + sep + "secret", root, false, "", true},

// With host access enabled, any path is allowed (but still cleaned).
{"host access bypasses sandbox", root + sep + ".." + sep + "etc" + sep + "passwd", root, true, filepath.Clean("/data/etc/passwd"), false},
}

for _, tt := range variations {
t.Run(tt.Name, func(t *testing.T) {
got, err := ResolveLocalPath(tt.Path, tt.Root, tt.AllowHostAccess)
if tt.ExpectErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tt.Expected, got)
})
}
}

func TestIsGlob(t *testing.T) {
variations := []struct {
Path string
Expand Down
Loading