From 70a5004f60112dd03f7186af2b16df61625f146f Mon Sep 17 00:00:00 2001 From: Sungkyu Yoo Date: Sun, 19 Apr 2026 22:32:17 +0900 Subject: [PATCH 1/2] Potential fix for code scanning alert no. 13: 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 | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/services/s3/store.go b/internal/services/s3/store.go index 177754b..ee6dbcd 100644 --- a/internal/services/s3/store.go +++ b/internal/services/s3/store.go @@ -26,13 +26,26 @@ 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...)...) + baseAbs, err := filepath.Abs(fs.baseDir) + if err != nil { + return "", fmt.Errorf("resolve base dir: %w", err) + } + + joined := filepath.Join(append([]string{baseAbs}, 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) + candidateAbs, err := filepath.Abs(cleaned) + if err != nil { + return "", fmt.Errorf("resolve candidate path: %w", err) + } + + rel, err := filepath.Rel(baseAbs, candidateAbs) + if err != nil { + return "", fmt.Errorf("resolve relative path: %w", err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path traversal detected: %s", candidateAbs) } - return cleaned, nil + return candidateAbs, nil } // objectPath returns the absolute filesystem path for the given object. From 024bf2b830410027abbef9f3bf65f121e6b7f507 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Mon, 20 Apr 2026 19:46:49 +0900 Subject: [PATCH 2/2] fix: harden safePath against symlinks and remove redundant Abs calls Resolve baseDir symlinks once in NewFileStore via EvalSymlinks, remove per-call filepath.Abs overhead in safePath, and resolve candidate symlinks before containment check to prevent escape via symlinks. --- internal/services/s3/store.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/services/s3/store.go b/internal/services/s3/store.go index 6147585..40d2c9a 100644 --- a/internal/services/s3/store.go +++ b/internal/services/s3/store.go @@ -20,7 +20,11 @@ func NewFileStore(baseDir string) *FileStore { if err != nil { abs = filepath.Clean(baseDir) } - return &FileStore{baseDir: filepath.Clean(abs)} + resolved, err := filepath.EvalSymlinks(abs) + if err != nil { + resolved = abs + } + return &FileStore{baseDir: resolved} } // validPathComponent checks that a single path segment contains no path @@ -40,26 +44,23 @@ func validPathComponent(part string) error { // escape the base directory. It returns an error on path traversal attempts. // All components must be single path segments (no separators). func (fs *FileStore) safePath(parts ...string) (string, error) { - baseAbs, err := filepath.Abs(fs.baseDir) - if err != nil { - return "", fmt.Errorf("resolve base dir: %w", err) - } - - joined := filepath.Join(append([]string{baseAbs}, parts...)...) + joined := filepath.Join(append([]string{fs.baseDir}, parts...)...) cleaned := filepath.Clean(joined) - candidateAbs, err := filepath.Abs(cleaned) - if err != nil { - return "", fmt.Errorf("resolve candidate path: %w", err) + + // Resolve symlinks if the candidate path exists; otherwise use cleaned. + candidate := cleaned + if resolved, err := filepath.EvalSymlinks(cleaned); err == nil { + candidate = resolved } - rel, err := filepath.Rel(baseAbs, candidateAbs) + rel, err := filepath.Rel(fs.baseDir, candidate) if err != nil { return "", fmt.Errorf("resolve relative path: %w", err) } if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return "", fmt.Errorf("path traversal detected: %s", candidateAbs) + return "", fmt.Errorf("path traversal detected: %s", candidate) } - return candidateAbs, nil + return candidate, nil } // objectPath returns the absolute filesystem path for the given object.