Repository navigation
Harden toFSPath against path traversal - #229
Open
webharerob wants to merge 1 commit into
Open
webharerob wants to merge 1 commit into
webharerob wants to merge 1 commit into
Conversation
toFSPath now rejects '.' and '..' path segments by default. A new 'root' option (a resource path) permits them as long as the resolved path stays inside both that root and the module/storage folder it was resolved against, so eg. mod::modulename/../bla is never accepted.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The unmatched-input fallback bypasses dot-segment and root-containment validation, allowing paths outside the configured root or traversal segments to escape rejection.
1 open issue
What changed in this PR
This PR hardens resource-to-filesystem conversion in @webhare/services by rejecting dot segments by default and introducing an explicit containment root.
Changes:
- Reject
.and..segments in mapped resource paths unless a root is provided. - Add a
rootoption intended to constrain resolved paths to both the specified root and their module/storage folder. - Extend path tests for traversal rejection, root containment, and invalid roots.
| File | Description |
|---|---|
whtree/modules/webhare_testsuite/tests/system/nodejs/test_resources.ts |
Adds traversal and root-containment test cases. |
whtree/jssdk/services/src/resources.ts |
Adds root options, separates base resolution, and validates path containment. |
Review effort: Balanced
Comment on lines
+28
to
+29
| if (typeof resolved !== "object" || resolved === null) //unmatched, return as-is (null or the original resource) | ||
| return resolved; |
This branch has not been deployed
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.
Changes
toFSPathnow rejects.and..path segments (eg.mod::system/../x,mod::system/./lib/,@webhare/../x), also whenallowUnmatchedorkeepUnmatchedis setrootoption totoFSPath. It takes a resource path, eg.mod::mymodule/data/. Withrootset,.and..are allowed as long as the result stays inside both the root and the module/storage folder the path resolves against, somod::modulename/../blais still rejected. The root itself must not contain dot segmentsrootset, the result must be inside the root even when the path has no dot segmentsTesting
testPathsinsystem.nodejs.test_resources: rejection of dot segments in all namespaces, dot names that aren't segments (..x,.y), resolving inside a root, escaping the root, escaping the module/storage folder, and invalid roots. The test passes locallyDeprecations and API changes
toFSPathnow throws for resource paths that contain.or..segments. Callers that relied on these being resolved must pass arootor normalize the path first (resolveResourcealready does)rootoption fortoFSPathin@webhare/servicesTODOs / FIXMEs