From c22f75cb02a576a66a14e011c19877e2efa1e3d2 Mon Sep 17 00:00:00 2001 From: Sungkyu Yoo Date: Sun, 19 Apr 2026 22:32:35 +0900 Subject: [PATCH 1/2] Potential fix for code scanning alert no. 10: 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 | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/internal/services/s3/store.go b/internal/services/s3/store.go index 177754b..1c1db96 100644 --- a/internal/services/s3/store.go +++ b/internal/services/s3/store.go @@ -23,13 +23,35 @@ func NewFileStore(baseDir string) *FileStore { return &FileStore{baseDir: abs} } +// validatePathComponent ensures a user-provided path fragment is a single safe component. +func validatePathComponent(part string) error { + if part == "" { + return fmt.Errorf("invalid empty path component") + } + if part == "." || part == ".." { + return fmt.Errorf("invalid path component: %q", part) + } + if strings.ContainsRune(part, filepath.Separator) || strings.Contains(part, "/") || strings.Contains(part, "\\") { + return fmt.Errorf("invalid path component with separators: %q", part) + } + return nil +} + // 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...)...) + for _, part := range parts { + if err := validatePathComponent(part); err != nil { + return "", err + } + } + + cleanBase := filepath.Clean(fs.baseDir) + joined := filepath.Join(append([]string{cleanBase}, 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 { + + rel, err := filepath.Rel(cleanBase, cleaned) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return "", fmt.Errorf("path traversal detected: %s", cleaned) } return cleaned, nil From 3117d717dbc0f5f9feb67c42034b139cc17a3490 Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Mon, 20 Apr 2026 07:53:26 +0900 Subject: [PATCH 2/2] fix: allow slashes in S3 keys for path validation The previous validatePathComponent rejected any path component containing slashes, but S3 keys legitimately use "/" as a delimiter (e.g. "a/b/c/file.txt"). Path traversal protection is already enforced by the filepath.Rel containment check in safePath, so the slash rejection was redundant and overly strict. --- internal/services/s3/store.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/services/s3/store.go b/internal/services/s3/store.go index 1c1db96..e060909 100644 --- a/internal/services/s3/store.go +++ b/internal/services/s3/store.go @@ -23,7 +23,10 @@ func NewFileStore(baseDir string) *FileStore { return &FileStore{baseDir: abs} } -// validatePathComponent ensures a user-provided path fragment is a single safe component. +// validatePathComponent ensures a user-provided path fragment is safe. +// Empty strings and bare "." or ".." components are rejected. +// Slashes are allowed since S3 keys use "/" as a delimiter (e.g. "dir/file.txt"). +// Path traversal protection is enforced by the filepath.Rel containment check in safePath. func validatePathComponent(part string) error { if part == "" { return fmt.Errorf("invalid empty path component") @@ -31,9 +34,6 @@ func validatePathComponent(part string) error { if part == "." || part == ".." { return fmt.Errorf("invalid path component: %q", part) } - if strings.ContainsRune(part, filepath.Separator) || strings.Contains(part, "/") || strings.Contains(part, "\\") { - return fmt.Errorf("invalid path component with separators: %q", part) - } return nil }