Skip to content

Commit 2a4e348

Browse files
committed
Fix path traversal in ResolveLocalPath sandbox
Clean the resolved path for both absolute and relative inputs, and use a separator-aware prefix check so sibling directories sharing the root's prefix no longer pass the sandbox. Claude-Session: https://claude.ai/code/session_01XWGRxG2FDSw2NgJEn7NmeE
1 parent 9c94a59 commit 2a4e348

2 files changed

Lines changed: 59 additions & 4 deletions

File tree

runtime/pkg/fileutil/fileutil.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,18 @@ func ResolveLocalPath(path, root string, allowHostAccess bool) (string, error) {
324324
if !filepath.IsAbs(path) {
325325
finalPath = filepath.Join(root, path)
326326
}
327-
328-
if !allowHostAccess && !strings.HasPrefix(finalPath, root) {
329-
// path is outside the repo root
330-
return "", fmt.Errorf("path is outside repo root")
327+
// Clean here so that ".." segments are resolved before the sandbox check below;
328+
// filepath.Join already cleans the relative branch, but an absolute path does not pass through it.
329+
finalPath = filepath.Clean(finalPath)
330+
331+
if !allowHostAccess {
332+
// Ensure the resolved path stays within the repo root.
333+
// The trailing separator prevents a sibling directory whose name shares the root's prefix
334+
// (e.g. root "/data/proj" matching "/data/proj-x") from passing the check.
335+
root = filepath.Clean(root)
336+
if finalPath != root && !strings.HasPrefix(finalPath, root+string(filepath.Separator)) {
337+
return "", fmt.Errorf("path is outside repo root")
338+
}
331339
}
332340
return finalPath, nil
333341
}

runtime/pkg/fileutil/fileutil_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package fileutil
22

33
import (
44
"os/user"
5+
"path/filepath"
56
"testing"
67

78
"github.com/stretchr/testify/require"
@@ -74,6 +75,52 @@ func TestExpandHome(t *testing.T) {
7475
}
7576
}
7677

78+
func TestResolveLocalPath(t *testing.T) {
79+
sep := string(filepath.Separator)
80+
root := filepath.Clean("/data/proj123")
81+
82+
variations := []struct {
83+
Name string
84+
Path string
85+
Root string
86+
AllowHostAccess bool
87+
Expected string
88+
ExpectErr bool
89+
}{
90+
// Relative paths within the root are joined and cleaned.
91+
{"relative within root", "sub/file.csv", root, false, filepath.Join(root, "sub", "file.csv"), false},
92+
{"relative to root itself", ".", root, false, root, false},
93+
94+
// Absolute paths within the root are allowed.
95+
{"absolute within root", filepath.Join(root, "file.csv"), root, false, filepath.Join(root, "file.csv"), false},
96+
97+
// Absolute path traversal must be rejected once cleaned.
98+
{"absolute traversal", root + sep + ".." + sep + ".." + sep + "etc" + sep + "passwd", root, false, "", true},
99+
100+
// Relative traversal escaping the root must be rejected.
101+
{"relative traversal", ".." + sep + ".." + sep + "etc" + sep + "passwd", root, false, "", true},
102+
103+
// Sibling directory sharing the root's prefix must not pass the check.
104+
{"sibling prefix collision", "/data/proj123-x/secret", root, false, "", true},
105+
{"relative sibling prefix collision", ".." + sep + "proj123-x" + sep + "secret", root, false, "", true},
106+
107+
// With host access enabled, any path is allowed (but still cleaned).
108+
{"host access bypasses sandbox", root + sep + ".." + sep + "etc" + sep + "passwd", root, true, filepath.Clean("/data/etc/passwd"), false},
109+
}
110+
111+
for _, tt := range variations {
112+
t.Run(tt.Name, func(t *testing.T) {
113+
got, err := ResolveLocalPath(tt.Path, tt.Root, tt.AllowHostAccess)
114+
if tt.ExpectErr {
115+
require.Error(t, err)
116+
return
117+
}
118+
require.NoError(t, err)
119+
require.Equal(t, tt.Expected, got)
120+
})
121+
}
122+
}
123+
77124
func TestIsGlob(t *testing.T) {
78125
variations := []struct {
79126
Path string

0 commit comments

Comments
 (0)