You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
OSFileChecker and OSDirectoryChecker rejected any cleaned path containing the substring "..", which blocked valid paths whose file or directory names include two dots (for example names like foo..backup or config..yaml), not actual traversal
Change: Replace the substring check with a rule that only treats leading .. segments (whole path ".." or prefix ".." + filepath.Separator) as traversal, consistent with paths left by filepath.Clean
Tests: Extended tools/fxconfig/internal/validation/validator_test.go with cases for benign ..-in-name paths, regressions (.., ../…), and internal .. segments that Clean collapses
Additional details (Optional)
Suggested checks: go test -race ./tools/fxconfig/internal/validation/...
(or go test -race ./tools/fxconfig/... including integration tests if you prefer a full sweep)
What was wrong? fxconfig has checks that validate file paths and directory paths before doing things like loading certs or config.
Those checks tried to block path traversal (paths that sneak “upward” using .., like going outside the intended folder).
They did it in a clumsy way: “if the path string contains two dots (..), reject it.”
Why is that bad?
Lots of real, normal names accidentally contain .. together, without meaning “go up one folder”:
A backup folder named myproject..backup
A file named config..yaml
So the checker would say “path traversal not allowed” even when nothing dangerous was happening. Valid setups would be wrongly rejected.
What does the PR fix?
It narrows the rule:
Reject only real “go up” patterns (.. at the start of the path, like ../secret),
not every path that merely has .. somewhere in a name.
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
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.
Type of change
Description
OSFileCheckerandOSDirectoryCheckerrejected any cleaned path containing the substring"..", which blocked valid paths whose file or directory names include two dots (for example names likefoo..backuporconfig..yaml), not actual traversalChange: Replace the substring check with a rule that only treats leading
..segments (whole path".."or prefix".." + filepath.Separator) as traversal, consistent with paths left byfilepath.CleanTests: Extended
tools/fxconfig/internal/validation/validator_test.gowith cases for benign..-in-name paths, regressions (..,../…), and internal..segments thatCleancollapsesAdditional details (Optional)
Suggested checks:
go test -race ./tools/fxconfig/internal/validation/...(or
go test -race ./tools/fxconfig/...including integration tests if you prefer a full sweep)