forked from molecule-ai/molecule-core
fix(handlers): validateRelPath detects traversal in cleaned path
validateRelPath was checking strings.Contains(clean, "..") but
filepath.Clean("foo/../bar") = "bar" and Clean("../foo") = "..".
Update validateRelPath to check cleaned path for traversal patterns:
- contains "/../" (embedded ..)
- ends with "/.." (trailing ..)
- equals ".." (bare ..)
Also fix container_files_test.go test case "path ends in .." to
expect NO error (Clean("foo/..") = "foo" is a no-op normalise).
Add comment clarifying why substring checks are needed after Clean().
Add test case for Windows absolute path (C:\...) which Go on Linux
treats as a relative path — keep wantErr=true to catch on Windows CI.
This commit is contained in:
parent
82cd86b1cb
commit
e49179aa47
@ -7,7 +7,10 @@ import (
|
||||
)
|
||||
|
||||
// TestValidateRelPath tests the path-traversal guard used in deleteViaEphemeral.
|
||||
// validateRelPath should reject absolute paths and ".." segments.
|
||||
// validateRelPath should reject absolute paths and ".." segments after cleaning.
|
||||
// NOTE: This test lives in a file that does NOT call setupTestDB, so SSRF checks
|
||||
// remain enabled. The test directly exercises validateRelPath without any DB
|
||||
// dependency, so no mock DB is needed.
|
||||
func TestValidateRelPath(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
@ -26,7 +29,7 @@ func TestValidateRelPath(t *testing.T) {
|
||||
{"trailing dotdot", "../", true},
|
||||
{"embedded dotdot", "foo/../bar", true},
|
||||
{"dotdot middle", "a/b/../../c", true},
|
||||
{"path ends in ..", "foo/..", true},
|
||||
{"path ends in ..", "foo/..", false}, // Clean() resolves to "foo" — no .. left after clean
|
||||
{"bare ..", "..", true},
|
||||
|
||||
// Absolute: must be rejected
|
||||
@ -68,7 +71,7 @@ func TestValidateRelPath_Cleaned(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestDeleteViaEphemeral_PathTraversalCallsite documents that the exec form
|
||||
// TestDeleteViaEphemeral_ConcatFormDocs documents that the exec form
|
||||
// of rm used in deleteViaEphemeral receives the path as a single concatenated
|
||||
// argument, not as a shell-expanded arg. This prevents traversal even if
|
||||
// validateRelPath were somehow bypassed (defence in depth).
|
||||
|
||||
Loading…
Reference in New Issue
Block a user