diff --git a/internal/worker/sdk/processor.go b/internal/worker/sdk/processor.go index acc6ae7..ae82593 100644 --- a/internal/worker/sdk/processor.go +++ b/internal/worker/sdk/processor.go @@ -784,24 +784,41 @@ func toJSONString(v any) string { // safeResolvePath resolves a path relative to cwd and validates it doesn't escape the cwd directory. // Returns the resolved absolute path and true if valid, or empty string and false if path traversal detected. +// This function is a security sanitizer for path traversal attacks. func safeResolvePath(path, cwd string) (string, bool) { - if filepath.IsAbs(path) { - // Absolute paths are allowed as-is - return filepath.Clean(path), true + // Clean the input path to normalize any .. or . components + cleanPath := filepath.Clean(path) + + // Reject paths that explicitly contain parent directory traversal after cleaning + if strings.Contains(cleanPath, "..") { + return "", false } + + if filepath.IsAbs(cleanPath) { + // For absolute paths, verify they're within cwd if cwd is specified + if cwd != "" { + cleanCwd := filepath.Clean(cwd) + if !strings.HasPrefix(cleanPath, cleanCwd+string(filepath.Separator)) && cleanPath != cleanCwd { + return "", false + } + } + return cleanPath, true + } + if cwd == "" { - return filepath.Clean(path), true + return cleanPath, true } // Clean the cwd first cleanCwd := filepath.Clean(cwd) // Join and clean the path - absPath := filepath.Clean(filepath.Join(cleanCwd, path)) + absPath := filepath.Join(cleanCwd, cleanPath) - // Verify the resolved path is still within cwd (prevents path traversal via ..) - // Use HasPrefix on cleaned paths to detect escapes - if !strings.HasPrefix(absPath, cleanCwd+string(filepath.Separator)) && absPath != cleanCwd { + // Use filepath.Rel to verify the path is actually within cwd + // If Rel returns a path starting with "..", it escapes the base + rel, err := filepath.Rel(cleanCwd, absPath) + if err != nil || strings.HasPrefix(rel, "..") { return "", false } @@ -905,12 +922,13 @@ func GetFileMtimes(paths []string, cwd string) map[string]int64 { // GetFileContent reads file content for verification purposes. // Returns content and ok status. func GetFileContent(path, cwd string) (string, bool) { - absPath := path - if !filepath.IsAbs(path) && cwd != "" { - absPath = filepath.Join(cwd, path) + absPath, ok := safeResolvePath(path, cwd) + if !ok { + // Reject paths that attempt directory traversal + return "", false } - content, err := os.ReadFile(absPath) // #nosec G304 -- intentional file read for verification + content, err := os.ReadFile(absPath) // #nosec G304 -- path validated by safeResolvePath if err != nil { return "", false } diff --git a/internal/worker/sdk/processor_test.go b/internal/worker/sdk/processor_test.go index 304bb94..04532c4 100644 --- a/internal/worker/sdk/processor_test.go +++ b/internal/worker/sdk/processor_test.go @@ -1081,3 +1081,100 @@ func BenchmarkSanitizePromptWithControlChars(b *testing.B) { sanitizePrompt(prompt) } } + +// TestSafeResolvePath tests the path traversal protection. +func TestSafeResolvePath(t *testing.T) { + // Create a temporary directory for testing + tmpDir := t.TempDir() + + tests := []struct { + name string + path string + cwd string + wantPath string + wantOk bool + }{ + { + name: "simple relative path", + path: "file.txt", + cwd: tmpDir, + wantOk: true, + wantPath: filepath.Join(tmpDir, "file.txt"), + }, + { + name: "nested relative path", + path: "subdir/file.txt", + cwd: tmpDir, + wantOk: true, + wantPath: filepath.Join(tmpDir, "subdir", "file.txt"), + }, + { + name: "path traversal with ..", + path: "../etc/passwd", + cwd: tmpDir, + wantOk: false, + }, + { + name: "path traversal with multiple ..", + path: "../../etc/passwd", + cwd: tmpDir, + wantOk: false, + }, + { + name: "path traversal hidden in middle", + path: "subdir/../../../etc/passwd", + cwd: tmpDir, + wantOk: false, + }, + { + name: "just parent directory", + path: "..", + cwd: tmpDir, + wantOk: false, + }, + { + name: "absolute path without cwd", + path: "/some/absolute/path", + cwd: "", + wantOk: true, + wantPath: "/some/absolute/path", + }, + { + name: "relative path without cwd", + path: "relative/path", + cwd: "", + wantOk: true, + wantPath: "relative/path", + }, + { + name: "current directory reference", + path: "./file.txt", + cwd: tmpDir, + wantOk: true, + wantPath: filepath.Join(tmpDir, "file.txt"), + }, + { + name: "absolute path outside cwd", + path: "/etc/passwd", + cwd: tmpDir, + wantOk: false, + }, + { + name: "absolute path inside cwd", + path: filepath.Join(tmpDir, "inside.txt"), + cwd: tmpDir, + wantOk: true, + wantPath: filepath.Join(tmpDir, "inside.txt"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotPath, gotOk := safeResolvePath(tt.path, tt.cwd) + assert.Equal(t, tt.wantOk, gotOk, "ok status mismatch") + if tt.wantPath != "" && gotOk { + assert.Equal(t, tt.wantPath, gotPath, "path mismatch") + } + }) + } +} diff --git a/internal/worker/service.go b/internal/worker/service.go index 6db9a8f..d14a051 100644 --- a/internal/worker/service.go +++ b/internal/worker/service.go @@ -1443,12 +1443,10 @@ func (s *Service) getRecentSearchQueries(project string, limit int) []RecentSear } // Filter by project (iterate from newest to oldest) - // Cap capacity to maxRecentQueries to prevent excessive allocation from user input - capacity := limit - if capacity > maxRecentQueries { - capacity = maxRecentQueries - } - result := make([]RecentSearchQuery, 0, capacity) + // Use constant capacity to prevent excessive allocation from user input + // limit is already bounded to maxRecentQueries above, but we use the constant + // directly here to satisfy static analysis tools + result := make([]RecentSearchQuery, 0, maxRecentQueries) for i := 0; i < s.recentQueriesLen; i++ { idx := (s.recentQueriesHead + i) % maxRecentQueries q := s.recentQueriesBuf[idx]