From 7d01952850e8b2133d131c12437034079998c439 Mon Sep 17 00:00:00 2001 From: Sungkyu Yoo Date: Sun, 19 Apr 2026 22:32:54 +0900 Subject: [PATCH 1/2] Potential fix for code scanning alert no. 7: 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/lambda/store.go | 40 +++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/internal/services/lambda/store.go b/internal/services/lambda/store.go index 1debbde..b2f7670 100644 --- a/internal/services/lambda/store.go +++ b/internal/services/lambda/store.go @@ -145,16 +145,46 @@ func (s *LambdaStore) Close() error { } // codePath returns the filesystem path for a function's code zip. -// It validates the result stays under codeDir to prevent path traversal. +// It validates path components and ensures the resolved path stays under codeDir. func (s *LambdaStore) codePath(accountID, functionName string) (string, error) { + isSafeComponent := func(v string) bool { + if v == "" || v == "." || v == ".." { + return false + } + if filepath.IsAbs(v) { + return false + } + if strings.Contains(v, "/") || strings.Contains(v, "\\") || strings.Contains(v, "..") { + return false + } + return true + } + + if !isSafeComponent(accountID) || !isSafeComponent(functionName) { + return "", fmt.Errorf("invalid path component") + } + joined := filepath.Join(s.codeDir, accountID, functionName, "code.zip") cleaned := filepath.Clean(joined) - absBase, _ := filepath.Abs(s.codeDir) - absCleaned, _ := filepath.Abs(cleaned) - if !strings.HasPrefix(absCleaned, absBase+string(filepath.Separator)) { + + absBase, err := filepath.Abs(s.codeDir) + if err != nil { + return "", fmt.Errorf("resolve base code directory: %w", err) + } + absCleaned, err := filepath.Abs(cleaned) + if err != nil { + return "", fmt.Errorf("resolve code path: %w", err) + } + + rel, err := filepath.Rel(absBase, absCleaned) + if err != nil { + return "", fmt.Errorf("compute relative code path: %w", err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) { return "", fmt.Errorf("path traversal detected: %s", cleaned) } - return cleaned, nil + + return absCleaned, nil } // CreateFunction saves function metadata to SQLite and writes the code zip From 38e5c18734e0238f4c38ed7cf64dfe6bd9bdaf9c Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Mon, 20 Apr 2026 04:57:50 +0900 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20extract=20isSafePathComponent,=20loosen=20substring=20check,?= =?UTF-8?q?=20return=20cleaned=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract isSafePathComponent as a package-level helper with a doc comment explaining why ".." as a substring is allowed (traversal is caught by Rel) - Drop the overly strict strings.Contains(v, "..") check that rejected valid names like "user..1" - Return cleaned (relative) instead of absCleaned to preserve original output semantics for callers - Apply the same isSafePathComponent + filepath.Rel containment check in DeleteFunction for consistency --- internal/services/lambda/store.go | 47 ++++++++++++++++++------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/internal/services/lambda/store.go b/internal/services/lambda/store.go index b2f7670..1bdf0f0 100644 --- a/internal/services/lambda/store.go +++ b/internal/services/lambda/store.go @@ -144,23 +144,19 @@ func (s *LambdaStore) Close() error { return s.store.Close() } +// isSafePathComponent reports whether v is a single path segment with no +// separators or absolute markers. It intentionally does not reject ".." as a +// substring (e.g. "user..1") since the containment check below handles traversal. +func isSafePathComponent(v string) bool { + return v != "" && v != "." && v != ".." && !filepath.IsAbs(v) && + !strings.ContainsAny(v, "/\\") +} + // codePath returns the filesystem path for a function's code zip. -// It validates path components and ensures the resolved path stays under codeDir. +// It validates that accountID and functionName are single path segments and that +// the resolved path stays within s.codeDir to prevent path traversal. func (s *LambdaStore) codePath(accountID, functionName string) (string, error) { - isSafeComponent := func(v string) bool { - if v == "" || v == "." || v == ".." { - return false - } - if filepath.IsAbs(v) { - return false - } - if strings.Contains(v, "/") || strings.Contains(v, "\\") || strings.Contains(v, "..") { - return false - } - return true - } - - if !isSafeComponent(accountID) || !isSafeComponent(functionName) { + if !isSafePathComponent(accountID) || !isSafePathComponent(functionName) { return "", fmt.Errorf("invalid path component") } @@ -184,7 +180,7 @@ func (s *LambdaStore) codePath(accountID, functionName string) (string, error) { return "", fmt.Errorf("path traversal detected: %s", cleaned) } - return absCleaned, nil + return cleaned, nil } // CreateFunction saves function metadata to SQLite and writes the code zip @@ -330,9 +326,22 @@ func (s *LambdaStore) DeleteFunction(accountID, functionName string) error { return ErrFunctionNotFound } - // Remove the code directory (best-effort; ignore errors). - codeDir := filepath.Join(s.codeDir, accountID, functionName) - _ = os.RemoveAll(codeDir) + // Remove the code directory (best-effort; ignore errors), but only if the + // path components and resolved location are within the base code directory. + if isSafePathComponent(accountID) && isSafePathComponent(functionName) { + codeDir := filepath.Join(s.codeDir, accountID, functionName) + cleaned := filepath.Clean(codeDir) + baseDirAbs, err := filepath.Abs(s.codeDir) + if err == nil { + absCleaned, err := filepath.Abs(cleaned) + if err == nil { + rel, err := filepath.Rel(baseDirAbs, absCleaned) + if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) && !filepath.IsAbs(rel) { + _ = os.RemoveAll(absCleaned) + } + } + } + } return nil }