Conversation
…in path expression Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
Reviewer's GuideRefines path handling in the S3 file store to validate each user-supplied path component and enforce directory containment using filepath semantics instead of prefix checks, mitigating path traversal risks flagged by code scanning alert #10. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Adopt main's cleaner architecture: validPathComponent for strict segment validation (accountID, bucket), and objectPath as a separate method that allows slashes in S3 keys while still enforcing containment via filepath.Rel.
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
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.
Potential fix for https://github.com/skyoo2003/devcloud/security/code-scanning/10
General fix: validate untrusted path components as components (not full paths), reject absolute/empty/traversal-like segments, then build the full path and enforce containment using path semantics (
filepath.Rel).Best concrete fix here (in
internal/services/s3/store.go):validatePathComponent(part string) errorthat rejects:"."and".."/or\)safePath, validate allpartsfirst.HasPrefixcontainment check with:cleanBase := filepath.Clean(fs.baseDir)rel, err := filepath.Rel(cleanBase, cleaned)err != nil,rel == "..", orstrings.HasPrefix(rel, ".."+string(filepath.Separator))provider.go.No new external dependencies are needed.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.