fix: path traversal false positive on filenames containing ..#35644
Open
Maxwell Calkin (MaxwellCalkin) wants to merge 1 commit intolangchain-ai:masterfrom
Open
Conversation
…detection The path traversal check in _validate_and_resolve_path() used '".." in path' which is a substring match. This incorrectly rejected filenames containing ".." such as Next.js catch-all routes like ["...nextauth].ts". Replace with Path(path).parts membership check so that ".." is only rejected when it appears as a discrete path segment (actual traversal) rather than as part of a filename. The resolve() + relative_to() check below already provides the primary security boundary; this is defense-in-depth. Fixes langchain-ai#34961
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.
Description
Fixes #34961
FilesystemFileSearchMiddleware._validate_and_resolve_path()checks for path traversal using".." in path, which is a substring match. This incorrectly rejects any path where..appears anywhere in the string — including legitimate filenames like Next.js catch-all routes ([...nextauth].ts).Fix
Replace the substring check with a
Path(path).partsmembership check so that..is only rejected when it appears as a discrete path segment (i.e., actual directory traversal like/../or/foo/../bar), not when it appears inside a filename.Before:
After:
The same fix is applied to the
~check for consistency.Security is maintained by three layers of defense:
..and~as path segmentsPath.resolve()— canonicalizes the path, collapsing any..segmentsrelative_to()containment check — ensures the resolved path is within the root directoryTests
Added 3 tests to
TestPathTraversalSecurity:test_path_with_dots_in_filename_not_blocked—[...nextauth].tsglob workstest_path_with_dots_in_directory_name_not_blocked—my..folderdirectory workstest_grep_path_with_dots_in_filename— grep on[...nextauth].tsworksAll existing path traversal security tests continue to pass since
/../,~/, etc. contain../~as discrete path segments.