Skip to content

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

Merged
bonheur15 merged 1 commit into
mainfrom
alert-autofix-15
Aug 16, 2026
Merged

Potential fix for code scanning alert no. 15: Uncontrolled data used in path expression#1
bonheur15 merged 1 commit into
mainfrom
alert-autofix-15

Conversation

@bonheur15

@bonheur15 bonheur15 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Potential fix for https://github.com/hubfly-space/go-php/security/code-scanning/15

The best fix is to canonicalize and validate request paths before any filesystem operation, then only operate on a safe, relative path guaranteed to stay inside the intended root.

  1. internal/filesystem/cache.go
    In setETag, replace direct filepath.Join(s.Root, filepath.Clean(path)) with:

    • normalize request path to slash form,
    • force relative form (strings.TrimPrefix(..., "/")),
    • reject unsafe values (.., absolute, empty edge cases as needed),
    • join with s.Root,
    • resolve absolute paths for both root and candidate and ensure candidate remains under root (prefix check with separator boundary),
    • only then call os.Stat.

    This preserves functionality (ETag generation for valid static paths) while blocking traversal.

  2. internal/ui/server.go
    In the static handler, sanitize r.URL.Path before staticFS.Open(path[1:]):

    • clean path (path.Clean from stdlib’s slash-based path package),
    • ensure it remains absolute-style with leading /,
    • derive relative candidate and reject if it contains .. traversal semantics,
    • use this sanitized relative path for staticFS.Open,
    • set r.URL.Path to sanitized absolute path before delegating to fileServer.

    This single sanitization flow addresses both additional variants where tainted r.URL influences filesystem path handling.

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

Summary by CodeRabbit

  • Bug Fixes
    • Improved static-file routing for normalized request paths.
    • Prevented invalid parent-traversal paths from accessing files outside the configured root.
    • Improved handling of missing files by serving the appropriate fallback page.
    • Added safer handling for invalid or unresolved file paths.

…in path expression

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

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 916b27e2-7f37-44aa-805b-eb856d3ceaed

📥 Commits

Reviewing files that changed from the base of the PR and between 344e7fd and 215a400.

📒 Files selected for processing (2)
  • internal/filesystem/cache.go
  • internal/ui/server.go

📝 Walkthrough

Walkthrough

The changes add path normalization and traversal checks to static-file routing and ETag generation. Static routing serves normalized existing files or index.html, while ETag generation rejects paths outside the configured root.

Changes

Static path safety

Layer / File(s) Summary
Static-file path normalization
internal/ui/server.go
Static-file routing cleans request paths, handles root and traversal-like paths, preserves normalized paths for existing files, and falls back to index.html for missing files.
ETag path containment
internal/filesystem/cache.go
setETag resolves root and target paths absolutely, rejects empty, parent-traversal, and out-of-root paths, and stops before os.Stat when resolution fails.

Estimated code review effort: 3 (Moderate) | ~20 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch alert-autofix-15

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bonheur15
bonheur15 marked this pull request as ready for review August 16, 2026 11:32
@bonheur15
bonheur15 merged commit 8bb4084 into main Aug 16, 2026
13 of 16 checks passed
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