Skip to content

Potential fix for code scanning alert no. 1: Uncontrolled data used in path expression#17

Draft
Bajahaw wants to merge 2 commits intomainfrom
alert-autofix-1
Draft

Potential fix for code scanning alert no. 1: Uncontrolled data used in path expression#17
Bajahaw wants to merge 2 commits intomainfrom
alert-autofix-1

Conversation

@Bajahaw
Copy link
Copy Markdown
Owner

@Bajahaw Bajahaw commented Mar 25, 2026

Potential fix for https://github.com/Bajahaw/ai-ui/security/code-scanning/1

In general, the fix is to ensure that any filesystem path derived from r.URL.Path is constrained to a known safe directory. That typically means joining r.URL.Path to h.staticDir, resolving it to an absolute path, and confirming that the result is still under h.staticDir before using it. If the check fails, the handler should return an error (or the SPA shell) instead of touching the filesystem.

For this specific code, the simplest, non‑functional change is to: (1) derive the candidate path using filepath.Join(h.staticDir, r.URL.Path), (2) normalize it with filepath.Abs, (3) ensure it still lives under h.staticDir (using a normalized absolute version of h.staticDir), and (4) use that safe, normalized path for os.Stat. If the path escapes the static directory or normalization fails, we treat it as “not a real file” and fall back to index.html (the existing behavior for non‑existent files). This avoids leaking whether arbitrary files outside h.staticDir exist, and keeps the rest of the behavior intact. To implement this, we need to import "path/filepath" and compute staticAbs once per request before the check.

Concretely:

  • Add path/filepath to the imports in backend/cmd/main.go.
  • In spaHandler.ServeHTTP, replace the path := "." + r.URL.Path and direct os.Stat(h.staticDir + "/" + r.URL.Path) with logic that:
    • Computes staticAbs, err := filepath.Abs(h.staticDir) and requested, err := filepath.Abs(filepath.Join(staticAbs, r.URL.Path)).
    • If any error occurs or requested is not under staticAbs, treat it as not a real file and serve index.html.
    • Otherwise, os.Stat(requested) to decide whether to serve the SPA shell or let h.fs handle the static file.
  • The unused path variable can be removed.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
return
}

if _, err := os.Stat(requestedAbs); os.IsNotExist(err) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI 5 days ago

To fix this, keep using absolute, normalized paths but strengthen the containment check between requestedAbs and staticAbs in a clear, standard way. Specifically, use strings.HasPrefix and ensure that either requestedAbs is exactly staticAbs or that it starts with staticAbs followed by the OS-specific path separator. This guarantees that /var/www/static2 is not treated as contained in /var/www/static, and it makes the intent explicit. Additionally, use filepath.Join instead of concatenating h.staticDir + "/index.html" so that path construction is correct and OS-independent.

Concretely, in backend/cmd/main.go:

  • Import the standard library package strings.
  • In spaHandler.ServeHTTP:
    • Compute indexFile := filepath.Join(staticAbs, "index.html") (after you know staticAbs), and reuse it for all fallback calls to http.ServeFile.
    • Replace the manual prefix check len(requestedAbs) < len(staticAbs) || requestedAbs[:len(staticAbs)] != staticAbs with a more robust boundary-aware prefix check using strings.HasPrefix and string(os.PathSeparator).

This preserves existing behavior (serving static files when inside the designated directory and index.html otherwise) while removing the path traversal risk and aligning with common secure path-handling patterns.

Suggested changeset 1
backend/cmd/main.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/cmd/main.go b/backend/cmd/main.go
--- a/backend/cmd/main.go
+++ b/backend/cmd/main.go
@@ -17,6 +17,7 @@
 	"os"
 	"os/signal"
 	"path/filepath"
+	"strings"
 	"syscall"
 	"time"
 
@@ -130,27 +131,29 @@
 	staticAbs, err := filepath.Abs(h.staticDir)
 	if err != nil {
 		// If we cannot resolve the static directory, fall back to the SPA shell.
-		http.ServeFile(w, r, h.staticDir+"/index.html")
+		http.ServeFile(w, r, filepath.Join(h.staticDir, "index.html"))
 		return
 	}
 
+	indexFile := filepath.Join(staticAbs, "index.html")
+
 	requestedPath := filepath.Join(staticAbs, r.URL.Path)
 	requestedAbs, err := filepath.Abs(requestedPath)
 	if err != nil {
 		// On error resolving the requested path, fall back to the SPA shell.
-		http.ServeFile(w, r, h.staticDir+"/index.html")
+		http.ServeFile(w, r, indexFile)
 		return
 	}
 
 	// Ensure the requested path is within the static directory to prevent directory traversal.
-	if len(requestedAbs) < len(staticAbs) || requestedAbs[:len(staticAbs)] != staticAbs {
-		http.ServeFile(w, r, h.staticDir+"/index.html")
+	if !(requestedAbs == staticAbs || strings.HasPrefix(requestedAbs, staticAbs+string(os.PathSeparator))) {
+		http.ServeFile(w, r, indexFile)
 		return
 	}
 
 	if _, err := os.Stat(requestedAbs); os.IsNotExist(err) {
 		// Not a real file – serve the SPA shell
-		http.ServeFile(w, r, h.staticDir+"/index.html")
+		http.ServeFile(w, r, indexFile)
 		return
 	}
 
EOF
@@ -17,6 +17,7 @@
"os"
"os/signal"
"path/filepath"
"strings"
"syscall"
"time"

@@ -130,27 +131,29 @@
staticAbs, err := filepath.Abs(h.staticDir)
if err != nil {
// If we cannot resolve the static directory, fall back to the SPA shell.
http.ServeFile(w, r, h.staticDir+"/index.html")
http.ServeFile(w, r, filepath.Join(h.staticDir, "index.html"))
return
}

indexFile := filepath.Join(staticAbs, "index.html")

requestedPath := filepath.Join(staticAbs, r.URL.Path)
requestedAbs, err := filepath.Abs(requestedPath)
if err != nil {
// On error resolving the requested path, fall back to the SPA shell.
http.ServeFile(w, r, h.staticDir+"/index.html")
http.ServeFile(w, r, indexFile)
return
}

// Ensure the requested path is within the static directory to prevent directory traversal.
if len(requestedAbs) < len(staticAbs) || requestedAbs[:len(staticAbs)] != staticAbs {
http.ServeFile(w, r, h.staticDir+"/index.html")
if !(requestedAbs == staticAbs || strings.HasPrefix(requestedAbs, staticAbs+string(os.PathSeparator))) {
http.ServeFile(w, r, indexFile)
return
}

if _, err := os.Stat(requestedAbs); os.IsNotExist(err) {
// Not a real file – serve the SPA shell
http.ServeFile(w, r, h.staticDir+"/index.html")
http.ServeFile(w, r, indexFile)
return
}

Copilot is powered by AI and may make mistakes. Always verify output.
@Bajahaw Bajahaw committed this autofix suggestion 5 days ago.
…n path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant