fix(ns-api-server): guard file endpoints against path traversal#1797
Merged
Conversation
Validate the resolved path in DownloadFile and DeleteFile stays within the configured download directory before any filesystem access, and reject with HTTP 400 otherwise. UploadFile is unaffected (it ignores the client name and generates a UUID). Addresses SonarCloud issues AZ9aszoIS1IsCbTKZ7OU, AZ9aszoIS1IsCbTKZ7OS and AZ9aszoIS1IsCbTKZ7OT (rule gosecurity:S2083, CWE-22), all on files.go. NOTE: this is a defense-in-depth hardening, not a real exploitable flaw. The routes are administrator-authenticated (JWT), and gin's default path cleaning already collapses "../" and decoded "%2f" before the handler runs, so no traversal could be triggered through the deployed nginx+gin stack (verified on a live device: every crafted request returned 404/400). The fix removes the reliance on that framework behavior so a future gin/config/route change cannot re-expose it, and clears the static-analysis blocker. Assisted-by: Claude Code:claude-opus-4-8
|
Tbaile
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds explicit path-containment validation to
DownloadFileandDeleteFileinpackages/ns-api-server/files/src/methods/files.go. A newsafeFilePath()helper resolves the path withfilepath.Joinand verifies it stays under the configured download directory (trailing-separator prefix check); traversal attempts now return HTTP 400.UploadFileis unchanged (it already ignores the client-supplied name and generates a UUID).SonarCloud issues addressed
Clears three BLOCKER findings (rule
gosecurity:S2083, CWE-22), all onfiles.go:AZ9aszoIS1IsCbTKZ7OU— line 77 (os.Open)AZ9aszoIS1IsCbTKZ7OS— line 118 (c.File)AZ9aszoIS1IsCbTKZ7OT— line 129 (os.Remove)This is defense-in-depth hardening, not a security incident:
/files/:filenameinsideauthGroup). An attacker would already need an admin token — and an admin has root-equivalent control by other means.../and decoded%2fbefore the handler runs. On a live device every crafted traversal request (..%2fsecret_jwt,../../../etc/shadow, double-encoded variants, direct-to-backend on127.0.0.1:8090) returned 404/400 — no traversal was reachable through the nginx + gin stack.E:U, admin-only) it drops to ~2–4 (Low).The value of the change is removing the reliance on undocumented framework behavior: a future gin upgrade,
UseRawPathtoggle, or route refactor could otherwise re-expose the sink. It also clears the static-analysis blocker.Verification
gofmtclean,go vetclean.methods/files_test.go(go test ./methods/→ ok): rejects..,../secret_jwt,../../../etc/shadow,../downloads-evil(prefix-sibling); allows legit / subdir / contained-absolute names.