diff --git a/internal/handler/file.go b/internal/handler/file.go index 5055eec..3292bc9 100644 --- a/internal/handler/file.go +++ b/internal/handler/file.go @@ -297,9 +297,9 @@ func (h *FileHandler) serveFromDisk(ctx *fasthttp.RequestCtx, absPath, urlPath s // compressible and large enough to benefit from compression. if h.cfg.Compression.Enabled && h.cfg.Compression.Precompressed && compress.IsCompressible(ct) && len(data) >= h.cfg.Compression.MinSize { - cached.GzipData = loadSidecar(absPath + ".gz") - cached.BrData = loadSidecar(absPath + ".br") - cached.ZstdData = loadSidecar(absPath + ".zst") + cached.GzipData = h.LoadSidecar(absPath + ".gz") + cached.BrData = h.LoadSidecar(absPath + ".br") + cached.ZstdData = h.LoadSidecar(absPath + ".zst") } // Generate on-the-fly gzip if no sidecar and content is compressible. @@ -501,11 +501,72 @@ func detectContentType(path string, data []byte) string { return "application/octet-stream" } -// loadSidecar attempts to read a pre-compressed sidecar file. -// Returns nil if the sidecar does not exist or cannot be read. -func loadSidecar(path string) []byte { - data, err := os.ReadFile(path) +// ValidateSidecarPath validates that a sidecar file path is within the root directory. +// It uses filepath.Clean() to normalize the path (recognized by CodeQL as a sanitizer), +// then verifies the canonical path remains within the root via symlink resolution and +// prefix checking. Returns the validated path or an error if validation fails. +// This function is designed to be recognized by static analyzers as a path sanitizer. +func (h *FileHandler) ValidateSidecarPath(sidecarPath string) (string, error) { + // Step 1: Normalize the path using filepath.Clean(). + // This removes ".." and "." components and is recognized by CodeQL as a sanitizer. + cleanPath := filepath.Clean(sidecarPath) + + // Step 2: Ensure the path is absolute (relative to root). + // If it's relative, make it absolute relative to root. + var absPath string + if filepath.IsAbs(cleanPath) { + absPath = cleanPath + } else { + absPath = filepath.Join(h.absRoot, cleanPath) + } + + // Step 3: Resolve symlinks to get the canonical path. + // This prevents symlink escape attacks where a sidecar could point outside root. + realPath, err := filepath.EvalSymlinks(absPath) + if err != nil { + // File doesn't exist or can't be resolved — return error. + return "", err + } + + // Step 4: Resolve the root directory to its canonical path for comparison. + // This is important on platforms like macOS where /tmp → /private/tmp. + realRoot := h.absRoot + if r, err := filepath.EvalSymlinks(h.absRoot); err == nil { + realRoot = r + } + + // Step 5: Verify the resolved path is still within the root directory. + // Add a trailing separator to prevent prefix collisions like "/root" matching "/rootsuffix". + rootWithSep := realRoot + if !strings.HasSuffix(rootWithSep, string(filepath.Separator)) { + rootWithSep += string(filepath.Separator) + } + + // Reject if the sidecar path escapes the root directory. + if realPath != realRoot && !strings.HasPrefix(realPath, rootWithSep) { + // Sidecar path escapes the root — reject it. + return "", fmt.Errorf("sidecar path escapes root directory") + } + + return realPath, nil +} + +// LoadSidecar attempts to read a pre-compressed sidecar file. +// Returns nil if the sidecar does not exist, cannot be read, or fails validation. +// The path parameter must be constructed from a validated absolute filesystem path +// (e.g., absPath + ".gz") to ensure it remains within the root directory. +func (h *FileHandler) LoadSidecar(path string) []byte { + // Validate the sidecar path to prevent path traversal attacks. + validatedPath, err := h.ValidateSidecarPath(path) + if err != nil { + // Validation failed (symlink escape, doesn't exist, etc.) — return nil. + return nil + } + + // Path is validated and safe — read the file. + data, err := os.ReadFile(validatedPath) if err != nil { + // File doesn't exist or can't be read — return nil. return nil } return data diff --git a/internal/handler/file_test.go b/internal/handler/file_test.go index d0b99ab..260b191 100644 --- a/internal/handler/file_test.go +++ b/internal/handler/file_test.go @@ -1,6 +1,7 @@ package handler_test import ( + "bytes" "io" "log" "os" @@ -948,3 +949,187 @@ func BenchmarkHandler_CacheHitQuiet(b *testing.B) { h(ctx) } } + +// TestValidateSidecarPath tests the path validation logic for sidecar files. +// This test ensures that CodeQL path-injection alerts are properly addressed +// by verifying that filepath.Clean() + symlink resolution + prefix checking +// prevents all known path traversal attacks. +func TestValidateSidecarPath(t *testing.T) { + root := t.TempDir() + cfg := &config.Config{} + cfg.Files.Root = root + cfg.Cache.Enabled = false + c := cache.NewCache(cfg.Cache.MaxBytes) + h := handler.NewFileHandler(cfg, c) + + // Create test files + testFile := filepath.Join(root, "test.txt") + if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil { + t.Fatal(err) + } + + // Create a subdirectory with a file + subdir := filepath.Join(root, "subdir") + if err := os.MkdirAll(subdir, 0755); err != nil { + t.Fatal(err) + } + subFile := filepath.Join(subdir, "sub.txt") + if err := os.WriteFile(subFile, []byte("sub"), 0644); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + path string + wantErr bool + desc string + }{ + { + name: "valid absolute path", + path: testFile, + wantErr: false, + desc: "Should accept valid absolute path within root", + }, + { + name: "valid relative path", + path: "test.txt", + wantErr: false, + desc: "Should accept valid relative path within root", + }, + { + name: "valid subdirectory path", + path: filepath.Join(root, "subdir", "sub.txt"), + wantErr: false, + desc: "Should accept valid path in subdirectory", + }, + { + name: "traversal with ..", + path: filepath.Join(root, "..", "etc", "passwd"), + wantErr: true, + desc: "Should reject path traversal with .. components", + }, + { + name: "traversal with multiple ..", + path: filepath.Join(root, "..", "..", "..", "etc", "passwd"), + wantErr: true, + desc: "Should reject multiple .. traversal attempts", + }, + { + name: "absolute path outside root", + path: "/etc/passwd", + wantErr: true, + desc: "Should reject absolute path outside root", + }, + { + name: "nonexistent file", + path: filepath.Join(root, "nonexistent.txt"), + wantErr: true, + desc: "Should reject nonexistent files (EvalSymlinks fails)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := h.ValidateSidecarPath(tt.path) + if (err != nil) != tt.wantErr { + t.Errorf("%s: got error=%v, wantErr=%v", tt.desc, err, tt.wantErr) + } + if !tt.wantErr && err == nil { + // Verify the result is within root + realRoot := root + if r, err := filepath.EvalSymlinks(root); err == nil { + realRoot = r + } + if !strings.HasPrefix(result, realRoot) && result != realRoot { + t.Errorf("%s: result %q is not within root %q", tt.desc, result, realRoot) + } + } + }) + } +} + +// TestLoadSidecar tests the sidecar file loading logic. +// This test verifies that the CodeQL-compliant path validation +// allows legitimate sidecar files to be loaded while rejecting attacks. +func TestLoadSidecar(t *testing.T) { + root := t.TempDir() + cfg := &config.Config{} + cfg.Files.Root = root + cfg.Cache.Enabled = false + c := cache.NewCache(cfg.Cache.MaxBytes) + h := handler.NewFileHandler(cfg, c) + + // Create a test file and its sidecar + testFile := filepath.Join(root, "test.txt") + sidecarFile := filepath.Join(root, "test.txt.gz") + sidecarContent := []byte("compressed data") + + if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(sidecarFile, sidecarContent, 0644); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + path string + wantNil bool + desc string + }{ + { + name: "valid sidecar", + path: sidecarFile, + wantNil: false, + desc: "Should load valid sidecar file", + }, + { + name: "nonexistent sidecar", + path: filepath.Join(root, "nonexistent.gz"), + wantNil: true, + desc: "Should return nil for nonexistent sidecar", + }, + { + name: "traversal attempt", + path: filepath.Join(root, "..", "etc", "passwd"), + wantNil: true, + desc: "Should return nil for traversal attempts", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := h.LoadSidecar(tt.path) + if (result == nil) != tt.wantNil { + t.Errorf("%s: got nil=%v, wantNil=%v", tt.desc, result == nil, tt.wantNil) + } + if !tt.wantNil && result != nil { + if !bytes.Equal(result, sidecarContent) { + t.Errorf("%s: got %q, want %q", tt.desc, result, sidecarContent) + } + } + }) + } +} + +// BenchmarkValidateSidecarPath benchmarks the path validation logic. +func BenchmarkValidateSidecarPath(b *testing.B) { + root := b.TempDir() + cfg := &config.Config{} + cfg.Files.Root = root + cfg.Cache.Enabled = false + c := cache.NewCache(cfg.Cache.MaxBytes) + h := handler.NewFileHandler(cfg, c) + + // Create a test file + testFile := filepath.Join(root, "test.txt") + if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil { + b.Fatal(err) + } + + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + _, _ = h.ValidateSidecarPath(testFile) + } +}