Reject paths outside the repo root in repo drivers and AI file tools

runtime/ai/develop_file.go

@@ -3,7 +3,6 @@ package ai
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 	aiv1 "github.com/rilldata/rill/proto/gen/rill/ai/v1"
@@ -56,9 +55,11 @@ func (t *DevelopFile) Handler(ctx context.Context, args *DevelopFileArgs) (*Deve
 	if args.Path == "" || args.Prompt == "" {
 		return nil, fmt.Errorf("invalid input: path and prompt are required")
 	}
-	if !strings.HasPrefix(args.Path, "/") {
-		args.Path = "/" + args.Path
+	path, err := normalizeFilePath(args.Path)
+	if err != nil {
+		return nil, err
 	}
+	args.Path = path
 
 	// Prepare the system prompts
 	generalInstructions, err := instructions.Load("development.md", instructions.Options{})

runtime/ai/file_read.go

@@ -2,7 +2,6 @@ package ai
 
 import (
 	"context"
-	"strings"
 
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 	"github.com/rilldata/rill/runtime"
@@ -49,9 +48,11 @@ func (t *ReadFile) CheckAccess(ctx context.Context) (bool, error) {
 func (t *ReadFile) Handler(ctx context.Context, args *ReadFileArgs) (*ReadFileResult, error) {
 	s := GetSession(ctx)
 
-	if !strings.HasPrefix(args.Path, "/") {
-		args.Path = "/" + args.Path
+	path, err := normalizeFilePath(args.Path)
+	if err != nil {
+		return nil, err
 	}
+	args.Path = path
 
 	blob, _, err := t.Runtime.GetFile(ctx, s.InstanceID(), args.Path)
 	if err != nil {

runtime/ai/file_read_test.go

@@ -0,0 +1,40 @@
+package ai_test
+
+import (
+	"testing"
+
+	"github.com/rilldata/rill/runtime/ai"
+	"github.com/rilldata/rill/runtime/testruntime"
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadFile(t *testing.T) {
+	// Setup a project with a file and a test session
+	rt, instanceID := testruntime.NewInstanceWithOptions(t, testruntime.InstanceOptions{
+		Files: map[string]string{
+			"models/test_model.sql": "SELECT 1 AS val",
+		},
+	})
+	s := newSession(t, rt, instanceID)
+
+	// Read file with and without a leading slash
+	for _, path := range []string{"models/test_model.sql", "/models/test_model.sql"} {
+		var res *ai.ReadFileResult
+		_, err := s.CallTool(t.Context(), ai.RoleUser, ai.ReadFileName, &res, &ai.ReadFileArgs{Path: path})
+		require.NoError(t, err)
+		require.NotNil(t, res)
+		require.Equal(t, "SELECT 1 AS val", res.Contents)
+	}
+
+	// Read non-existent file
+	var res *ai.ReadFileResult
+	_, err := s.CallTool(t.Context(), ai.RoleUser, ai.ReadFileName, &res, &ai.ReadFileArgs{Path: "models/non_existent.sql"})
+	require.Error(t, err)
+
+	// Reject paths that traverse outside the project directory
+	for _, path := range []string{"../secret.txt", "/../secret.txt", "models/../../secret.txt", "../../../../etc/passwd", "..\\..\\secret.txt"} {
+		res = nil
+		_, err := s.CallTool(t.Context(), ai.RoleUser, ai.ReadFileName, &res, &ai.ReadFileArgs{Path: path})
+		require.ErrorContains(t, err, "must not contain", "path %q", path)
+	}
+}

runtime/ai/file_write.go

@@ -58,9 +58,11 @@ func (t *WriteFile) CheckAccess(ctx context.Context) (bool, error) {
 func (t *WriteFile) Handler(ctx context.Context, args *WriteFileArgs) (*WriteFileResult, error) {
 	s := GetSession(ctx)
 
-	if !strings.HasPrefix(args.Path, "/") {
-		args.Path = "/" + args.Path
+	path, err := normalizeFilePath(args.Path)
+	if err != nil {
+		return nil, err
 	}
+	args.Path = path
 
 	// Read existing content before writing (for diff computation)
 	var isNewFile bool

runtime/ai/file_write_test.go

@@ -80,4 +80,21 @@ func TestWriteFile(t *testing.T) {
 		Remove: true,
 	})
 	require.Error(t, err)
+
+	// Reject paths that traverse outside the project directory
+	for _, path := range []string{"../escape.sql", "/../escape.sql", "models/../../escape.sql", "..\\escape.sql"} {
+		res = nil
+		_, err = s.CallTool(t.Context(), ai.RoleUser, ai.WriteFileName, &res, &ai.WriteFileArgs{
+			Path:     path,
+			Contents: "SELECT 1 AS val",
+		})
+		require.ErrorContains(t, err, "must not contain", "path %q", path)
+
+		res = nil
+		_, err = s.CallTool(t.Context(), ai.RoleUser, ai.WriteFileName, &res, &ai.WriteFileArgs{
+			Path:   path,
+			Remove: true,
+		})
+		require.ErrorContains(t, err, "must not contain", "path %q", path)
+	}
 }

runtime/ai/util.go

@@ -63,6 +63,21 @@ func resolverResultToTabular(res runtime.ResolverResult) ([]SchemaField, [][]any
 	return fields, data, nil
 }
 
+// normalizeFilePath converts a model-provided file path to an absolute path within the project (e.g. "models/foo.sql" to "/models/foo.sql").
+// It returns an error for paths with ".." segments, which could otherwise traverse outside the project directory.
+func normalizeFilePath(p string) (string, error) {
+	segments := strings.FieldsFunc(p, func(r rune) bool { return r == '/' || r == '\\' })
+	for _, segment := range segments {
+		if segment == ".." {
+			return "", fmt.Errorf("invalid path %q: must not contain %q segments", p, "..")
+		}
+	}
+	if !strings.HasPrefix(p, "/") {
+		p = "/" + p
+	}
+	return p, nil
+}
+
 var templateFuncs = template.FuncMap{
 	"backticks": func() string {
 		return "```"

runtime/drivers/admin/repo.go

@@ -147,7 +147,10 @@ func (r *repo) Get(ctx context.Context, path string) (string, error) {
 
 	var readErr error
 	for _, root := range r.roots() { // Search in every underlying file system.
-		fp := filepath.Join(root, path)
+		fp, err := drivers.ResolveRepoPath(root, path)
+		if err != nil {
+			return "", err
+		}
 		b, err := os.ReadFile(fp)
 		if err != nil {
 			// Keep searching if it's a not exist error. Otherwise break and return the error immediately.
@@ -185,7 +188,10 @@ func (r *repo) Hash(ctx context.Context, paths []string) (string, error) {
 		if drivers.IsIgnored(path, r.ignorePaths) {
 			continue // Skip if file does not exist
 		}
-		fp := filepath.Join(root, path)
+		fp, err := drivers.ResolveRepoPath(root, path)
+		if err != nil {
+			return "", err
+		}
 		file, err := os.Open(fp)
 		if err != nil {
 			if os.IsNotExist(err) {
@@ -218,7 +224,10 @@ func (r *repo) Stat(ctx context.Context, path string) (*drivers.FileInfo, error)
 
 	var statErr error
 	for _, root := range r.roots() { // Search in every underlying file system.
-		fp := filepath.Join(root, path)
+		fp, err := drivers.ResolveRepoPath(root, path)
+		if err != nil {
+			return nil, err
+		}
 		info, err := os.Stat(fp)
 		if err != nil {
 			// Keep searching if it's a not exist error. Otherwise break and return the error immediately.
@@ -254,7 +263,10 @@ func (r *repo) Put(ctx context.Context, path string, reader io.Reader) error {
 		return fmt.Errorf("can't write to ignored path %q", path)
 	}
 
-	fp := filepath.Join(root, path)
+	fp, err := drivers.ResolveRepoPath(root, path)
+	if err != nil {
+		return err
+	}
 
 	err = os.MkdirAll(filepath.Dir(fp), os.ModePerm)
 	if err != nil {
@@ -292,7 +304,10 @@ func (r *repo) MkdirAll(ctx context.Context, path string) error {
 		return fmt.Errorf("can't write to ignored path %q", path)
 	}
 
-	fp := filepath.Join(root, path)
+	fp, err := drivers.ResolveRepoPath(root, path)
+	if err != nil {
+		return err
+	}
 
 	err = os.MkdirAll(fp, os.ModePerm)
 	if err != nil {
@@ -322,8 +337,14 @@ func (r *repo) Rename(ctx context.Context, fromPath, toPath string) error {
 		return fmt.Errorf("can't write to ignored path %q", toPath)
 	}
 
-	fromPath = filepath.Join(root, fromPath)
-	toPath = filepath.Join(root, toPath)
+	fromPath, err = drivers.ResolveRepoPath(root, fromPath)
+	if err != nil {
+		return err
+	}
+	toPath, err = drivers.ResolveRepoPath(root, toPath)
+	if err != nil {
+		return err
+	}
 
 	if _, err := os.Stat(toPath); !strings.EqualFold(fromPath, toPath) && err == nil {
 		return os.ErrExist
@@ -358,7 +379,10 @@ func (r *repo) Delete(ctx context.Context, path string, force bool) error {
 		return fmt.Errorf("can't write to ignored path %q", path)
 	}
 
-	fp := filepath.Join(root, path)
+	fp, err := drivers.ResolveRepoPath(root, path)
+	if err != nil {
+		return err
+	}
 
 	if force {
 		err = os.RemoveAll(fp)

runtime/drivers/file/repo.go

@@ -71,7 +71,10 @@ func (c *connection) ListGlob(ctx context.Context, glob string, skipDirs bool) (
 
 // Get implements drivers.RepoStore.
 func (c *connection) Get(ctx context.Context, filePath string) (string, error) {
-	fp := filepath.Join(c.root, filePath)
+	fp, err := drivers.ResolveRepoPath(c.root, filePath)
+	if err != nil {
+		return "", err
+	}
 
 	b, err := os.ReadFile(fp)
 	if err != nil {
@@ -90,8 +93,11 @@ func (c *connection) Get(ctx context.Context, filePath string) (string, error) {
 func (c *connection) Hash(ctx context.Context, paths []string) (string, error) {
 	hasher := md5.New()
 	for _, path := range paths {
-		path = filepath.Join(c.root, path)
-		file, err := os.Open(path)
+		fp, err := drivers.ResolveRepoPath(c.root, path)
+		if err != nil {
+			return "", err
+		}
+		file, err := os.Open(fp)
 		if err != nil {
 			if os.IsNotExist(err) {
 				continue
@@ -110,7 +116,10 @@ func (c *connection) Hash(ctx context.Context, paths []string) (string, error) {
 
 // Stat implements drivers.RepoStore.
 func (c *connection) Stat(ctx context.Context, filePath string) (*drivers.FileInfo, error) {
-	filePath = filepath.Join(c.root, filePath)
+	filePath, err := drivers.ResolveRepoPath(c.root, filePath)
+	if err != nil {
+		return nil, err
+	}
 
 	info, err := os.Stat(filePath)
 	if err != nil {
@@ -125,9 +134,12 @@ func (c *connection) Stat(ctx context.Context, filePath string) (*drivers.FileIn
 
 // Put implements drivers.RepoStore.
 func (c *connection) Put(ctx context.Context, filePath string, reader io.Reader) error {
-	filePath = filepath.Join(c.root, filePath)
+	filePath, err := drivers.ResolveRepoPath(c.root, filePath)
+	if err != nil {
+		return err
+	}
 
-	err := os.MkdirAll(filepath.Dir(filePath), os.ModePerm)
+	err = os.MkdirAll(filepath.Dir(filePath), os.ModePerm)
 	if err != nil {
 		return err
 	}
@@ -148,9 +160,12 @@ func (c *connection) Put(ctx context.Context, filePath string, reader io.Reader)
 
 // MkdirAll implements drivers.RepoStore.
 func (c *connection) MkdirAll(ctx context.Context, dirPath string) error {
-	dirPath = filepath.Join(c.root, dirPath)
+	dirPath, err := drivers.ResolveRepoPath(c.root, dirPath)
+	if err != nil {
+		return err
+	}
 
-	err := os.MkdirAll(dirPath, os.ModePerm)
+	err = os.MkdirAll(dirPath, os.ModePerm)
 	if err != nil {
 		return err
 	}
@@ -160,13 +175,19 @@ func (c *connection) MkdirAll(ctx context.Context, dirPath string) error {
 
 // Rename implements drivers.RepoStore.
 func (c *connection) Rename(ctx context.Context, fromPath, toPath string) error {
-	toPath = filepath.Join(c.root, toPath)
+	toPath, err := drivers.ResolveRepoPath(c.root, toPath)
+	if err != nil {
+		return err
+	}
 
-	fromPath = filepath.Join(c.root, fromPath)
+	fromPath, err = drivers.ResolveRepoPath(c.root, fromPath)
+	if err != nil {
+		return err
+	}
 	if _, err := os.Stat(toPath); !strings.EqualFold(fromPath, toPath) && err == nil {
 		return os.ErrExist
 	}
-	err := os.Rename(fromPath, toPath)
+	err = os.Rename(fromPath, toPath)
 	if err != nil {
 		return err
 	}
@@ -175,7 +196,10 @@ func (c *connection) Rename(ctx context.Context, fromPath, toPath string) error
 
 // Delete implements drivers.RepoStore.
 func (c *connection) Delete(ctx context.Context, filePath string, force bool) error {
-	filePath = filepath.Join(c.root, filePath)
+	filePath, err := drivers.ResolveRepoPath(c.root, filePath)
+	if err != nil {
+		return err
+	}
 	if force {
 		return os.RemoveAll(filePath)
 	}

runtime/drivers/repo.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -146,6 +147,18 @@ func IsIgnored(path string, additionalIgnoredPaths []string) bool {
 	return false
 }
 
+// ResolveRepoPath resolves a repo-relative path against the given root directory and returns the resulting file system path.
+// It returns an error if the resolved path falls outside the root, which prevents path traversal using ".." segments.
+// Repo drivers must use it instead of joining untrusted paths onto the root directly.
+func ResolveRepoPath(root, path string) (string, error) {
+	root = filepath.Clean(root)
+	fp := filepath.Join(root, path)
+	if fp != root && !strings.HasPrefix(fp, root+string(filepath.Separator)) {
+		return "", fmt.Errorf("path %q is outside the repo root", path)
+	}
+	return fp, nil
+}
+
 type RepoStatus struct {
 	// IsGitRepo indicates if the repo is backed by a Git repository.
 	IsGitRepo     bool