Skip to content

Commit 361192f

Browse files
committed
Harden module path normalization and traversal checks
1 parent d360107 commit 361192f

4 files changed

Lines changed: 95 additions & 9 deletions

File tree

ROADMAP.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,8 @@ Goal: make multi-file script projects easier to compose and maintain.
257257

258258
### Security and Isolation
259259

260-
- [ ] Tighten module root boundary checks and path normalization.
261-
- [ ] Add test coverage for path traversal attempts.
260+
- [x] Tighten module root boundary checks and path normalization.
261+
- [x] Add test coverage for path traversal attempts.
262262
- [ ] Add explicit policy hooks for module allow/deny lists.
263263

264264
### Developer UX

docs/integration.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ can be exported explicitly with `export def ...`; if no explicit exports are
8585
declared, public names are exported by default and names starting with `_`
8686
remain private. Exported names are only injected into globals when no binding
8787
already exists, so existing host/script globals keep precedence.
88+
Import paths are normalized across slash styles, and traversal/symlink escapes
89+
outside configured module roots are blocked.
8890

8991
### Capability Adapters
9092

vibes/modules.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,18 @@ func parseModuleRequest(name string) (moduleRequest, error) {
3131
if trimmed == "" {
3232
return moduleRequest{}, fmt.Errorf("require: module name must be non-empty")
3333
}
34+
normalizedName := strings.ReplaceAll(trimmed, "\\", string(filepath.Separator))
35+
normalizedName = strings.ReplaceAll(normalizedName, "/", string(filepath.Separator))
36+
3437
request := moduleRequest{
3538
raw: name,
3639
explicitRelative: isExplicitRelativeModulePath(trimmed),
3740
}
38-
if filepath.Ext(trimmed) == "" {
39-
trimmed += ".vibe"
41+
if filepath.Ext(normalizedName) == "" {
42+
normalizedName += ".vibe"
4043
}
4144

42-
request.normalized = filepath.Clean(trimmed)
45+
request.normalized = filepath.Clean(normalizedName)
4346
if request.normalized == "." {
4447
return moduleRequest{}, fmt.Errorf("require: module name %q resolves to current directory", name)
4548
}
@@ -61,8 +64,8 @@ func isExplicitRelativeModulePath(name string) bool {
6164
}
6265

6366
func containsPathTraversal(cleanPath string) bool {
64-
sep := string(filepath.Separator)
65-
return slices.Contains(strings.Split(cleanPath, sep), "..")
67+
normalized := strings.ReplaceAll(filepath.Clean(cleanPath), "\\", "/")
68+
return slices.Contains(strings.Split(normalized, "/"), "..")
6669
}
6770

6871
func moduleCacheKey(root, relative string) string {
@@ -244,11 +247,15 @@ func (e *Engine) loadSearchPathModule(request moduleRequest) (moduleEntry, error
244247

245248
for _, root := range e.modPaths {
246249
key := moduleCacheKey(root, request.normalized)
250+
candidate := filepath.Join(root, request.normalized)
251+
252+
if _, err := moduleRelativePath(root, candidate); err != nil {
253+
return moduleEntry{}, fmt.Errorf("require: module name %q escapes module root", request.raw)
254+
}
255+
247256
if entry, ok := e.getCachedModule(key); ok {
248257
return entry, nil
249258
}
250-
251-
candidate := filepath.Join(root, request.normalized)
252259
data, readErr := os.ReadFile(candidate)
253260
if readErr != nil {
254261
if errors.Is(readErr, fs.ErrNotExist) {

vibes/modules_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,47 @@ end`)
367367
}
368368
}
369369

370+
func TestRequireRejectsBackslashPathTraversal(t *testing.T) {
371+
engine := MustNewEngine(Config{ModulePaths: []string{filepath.Join("testdata", "modules")}})
372+
373+
script, err := engine.Compile(`def run()
374+
require("nested\\..\\..\\etc\\passwd")
375+
end`)
376+
if err != nil {
377+
t.Fatalf("compile failed: %v", err)
378+
}
379+
380+
if _, err := script.Call(context.Background(), "run", nil, CallOptions{}); err == nil {
381+
t.Fatalf("expected error for backslash path traversal")
382+
} else if !strings.Contains(err.Error(), "escapes search paths") {
383+
t.Fatalf("unexpected error: %v", err)
384+
}
385+
}
386+
387+
func TestRequireNormalizesPathSeparators(t *testing.T) {
388+
engine := MustNewEngine(Config{ModulePaths: []string{filepath.Join("testdata", "modules")}})
389+
390+
script, err := engine.Compile(`def run(value)
391+
unix_style = require("shared/math")
392+
windows_style = require("shared\\math")
393+
unix_style.double(value) + windows_style.double(value)
394+
end`)
395+
if err != nil {
396+
t.Fatalf("compile failed: %v", err)
397+
}
398+
399+
result, err := script.Call(context.Background(), "run", []Value{NewInt(3)}, CallOptions{})
400+
if err != nil {
401+
t.Fatalf("call failed: %v", err)
402+
}
403+
if result.Kind() != KindInt || result.Int() != 12 {
404+
t.Fatalf("expected 12, got %#v", result)
405+
}
406+
if len(engine.modules) != 1 {
407+
t.Fatalf("expected normalized requires to share cache entry, got %d modules", len(engine.modules))
408+
}
409+
}
410+
370411
func TestRequireRelativePathRequiresModuleCaller(t *testing.T) {
371412
engine := MustNewEngine(Config{ModulePaths: []string{filepath.Join("testdata", "modules")}})
372413

@@ -489,6 +530,42 @@ end`)
489530
}
490531
}
491532

533+
func TestRequireSearchPathRejectsSymlinkEscape(t *testing.T) {
534+
if runtime.GOOS == "windows" {
535+
t.Skip("symlink behavior is environment-specific on Windows")
536+
}
537+
538+
moduleRoot := t.TempDir()
539+
outsideRoot := t.TempDir()
540+
541+
secretModule := filepath.Join(outsideRoot, "secret.vibe")
542+
if err := os.WriteFile(secretModule, []byte(`def hidden()
543+
42
544+
end
545+
`), 0o644); err != nil {
546+
t.Fatalf("write secret module: %v", err)
547+
}
548+
549+
symlinkPath := filepath.Join(moduleRoot, "link")
550+
if err := os.Symlink(outsideRoot, symlinkPath); err != nil {
551+
t.Skipf("symlink unavailable: %v", err)
552+
}
553+
554+
engine := MustNewEngine(Config{ModulePaths: []string{moduleRoot}})
555+
script, err := engine.Compile(`def run()
556+
require("link/secret")
557+
end`)
558+
if err != nil {
559+
t.Fatalf("compile failed: %v", err)
560+
}
561+
562+
if _, err := script.Call(context.Background(), "run", nil, CallOptions{}); err == nil {
563+
t.Fatalf("expected symlink escape error")
564+
} else if !strings.Contains(err.Error(), "escapes module root") {
565+
t.Fatalf("unexpected error: %v", err)
566+
}
567+
}
568+
492569
func TestRequireRelativePathRejectsOutOfRootCachedModule(t *testing.T) {
493570
if runtime.GOOS == "windows" {
494571
t.Skip("symlink behavior is environment-specific on Windows")

0 commit comments

Comments
 (0)