From 9f19c11c92d9a3fef4d20b7a67d28fb03718fb0d Mon Sep 17 00:00:00 2001 From: Sungkyu Yoo Date: Sun, 19 Apr 2026 22:32:12 +0900 Subject: [PATCH 1/2] Potential fix for code scanning alert no. 14: Uncontrolled data used in path expression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- internal/services/s3/store.go | 38 +++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/internal/services/s3/store.go b/internal/services/s3/store.go index 177754b..693313b 100644 --- a/internal/services/s3/store.go +++ b/internal/services/s3/store.go @@ -26,13 +26,39 @@ func NewFileStore(baseDir string) *FileStore { // safePath joins the components under baseDir and verifies the result does not // escape the base directory. It returns an error on path traversal attempts. func (fs *FileStore) safePath(parts ...string) (string, error) { - joined := filepath.Join(append([]string{fs.baseDir}, parts...)...) - cleaned := filepath.Clean(joined) - // Ensure the resolved path is still under baseDir. - if !strings.HasPrefix(cleaned, fs.baseDir+string(filepath.Separator)) && cleaned != fs.baseDir { - return "", fmt.Errorf("path traversal detected: %s", cleaned) + cleanParts := make([]string, 0, len(parts)) + for _, part := range parts { + if part == "" { + return "", fmt.Errorf("invalid empty path segment") + } + if filepath.IsAbs(part) { + return "", fmt.Errorf("absolute path segment not allowed: %s", part) + } + cleanPart := filepath.Clean(part) + if cleanPart == ".." || strings.HasPrefix(cleanPart, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path traversal detected in segment: %s", part) + } + cleanParts = append(cleanParts, cleanPart) } - return cleaned, nil + + baseAbs, err := filepath.Abs(fs.baseDir) + if err != nil { + return "", err + } + candidateAbs, err := filepath.Abs(filepath.Join(append([]string{baseAbs}, cleanParts...)...)) + if err != nil { + return "", err + } + + rel, err := filepath.Rel(baseAbs, candidateAbs) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { + return "", fmt.Errorf("path traversal detected: %s", candidateAbs) + } + + return candidateAbs, nil } // objectPath returns the absolute filesystem path for the given object. From c4812cd5366d4cf2647eed4b5af15a7ee4d5a5c3 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Mon, 20 Apr 2026 19:56:11 +0900 Subject: [PATCH 2/2] fix: address review comments on safePath - skip empty segments and reuse resolved baseDir --- internal/services/s3/store.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/services/s3/store.go b/internal/services/s3/store.go index 1e81eec..f4a6e76 100644 --- a/internal/services/s3/store.go +++ b/internal/services/s3/store.go @@ -47,7 +47,7 @@ func (fs *FileStore) safePath(parts ...string) (string, error) { cleanParts := make([]string, 0, len(parts)) for _, part := range parts { if part == "" { - return "", fmt.Errorf("invalid empty path segment") + continue } if filepath.IsAbs(part) { return "", fmt.Errorf("absolute path segment not allowed: %s", part) @@ -59,18 +59,14 @@ func (fs *FileStore) safePath(parts ...string) (string, error) { cleanParts = append(cleanParts, cleanPart) } - baseAbs, err := filepath.Abs(fs.baseDir) - if err != nil { - return "", err - } - candidate := filepath.Join(append([]string{baseAbs}, cleanParts...)...) + candidate := filepath.Join(append([]string{fs.baseDir}, cleanParts...)...) // Resolve symlinks if the candidate path exists; otherwise use the joined path. if resolved, err := filepath.EvalSymlinks(candidate); err == nil { candidate = resolved } - rel, err := filepath.Rel(baseAbs, candidate) + rel, err := filepath.Rel(fs.baseDir, candidate) if err != nil { return "", fmt.Errorf("resolve relative path: %w", err) }