fix(bundle): validate module dirs and file keys with posixPathEscapes, matching the lockfile parser#60
Open
exo-nikita wants to merge 1 commit into
Conversation
…, matching the lockfile parser Bundle.parse guarded v1 module/source bucket dirs, per-file keys, and v0 flat source paths with only a '..'-prefix check, while Lockfile.parse (and Bundle.parse's own formats/imports checks) already use posixPathEscapes, which also catches a mid-path '..' that pops above the root and absolute paths. Defense-in-depth: no known exploit (extract re-validates containment against the output dir and load serves by canonical key), but the schema boundary should fail closed the same way in both parsers. rel === '' (a `directory` capture keyed at a module root) and the v0 inferred '.' dir normalize to '.' and still pass, pinned by a new test. The two extract-cmd tests whose escaping fixtures relied on parse being weaker now assert the earlier parse-time rejection (and still assert nothing is written). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M16Lwt8cM1azwev94UP81X
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
Bundle.parsevalidated v1 module/source bucket dirs, per-filerelkeys, and v0 flat source paths with only a'..'-prefix check (!x.startsWith('..')), whileLockfile.parse— andBundle.parse's ownformatsandimportschecks — already use the strongerposixPathEscapes(), which also rejects a mid-path..that pops above the root (a/../../x) and absolute paths. This PR replaces the four remaining prefix checks instasis-core/src/bundle.jswithposixPathEscapes, so both parsers validate paths the same way.Why (defense-in-depth)
No known exploit:
stasis extractindependently re-validates every write for containment against the output dir, and the loader serves files by canonical key. But the schema boundary should fail closed — a tampered bundle carryingnode_modules/a/../../../evilor/node_modules/pkgas a bucket dir previously parsed successfully and only failed (if at all) at a downstream consumer. Now it is rejected at parse time, consistent withLockfile.parse.Invariant preserved:
rel === ''(adirectorycapture keyed at a module root, seemoduleFileKey) and v0's inferred'.'dir both normalize to'.'and still pass — pinned by a new test rather than assumed.Tests
New cases in
tests/public-exports.test.js(alongside the existing Bundle/Lockfile rejection tests):modulesdir escaping mid-path (node_modules/a/../../../evil— containsnode_modules, so only the path check can catch it) → rejectedmodulesdir (/node_modules/pkg) → rejectedsub/../../x) → rejectedsourcesdir escaping mid-path (src/../../evil) → rejected../x,a/../../x,/etc/passwd→ rejecteddirectory-format listing keyed at a module root (rel === '') still parses and survives a serialize → parse round-tripTwo tests in
tests/extract-cmd.test.jshand-crafted escaping bundles specifically because "Bundle.parse only rejects paths that start with.."; those fixtures are now rejected at parse, so the tests assert the earlier (wrapped) parse failure and still assert nothing is written to disk.extractCommand's own containment check stays in place as a second layer.Ran
node --testonpublic-exports,extract-cmd,sbom,diff,ecosystem,fs,state,state-load-proto,state-split-bundle— all pass.bundle-cmd/state-load/state-sidecar/fs-sidecar/cli-fsshow 41 failures both on this branch and on pristinemainin this environment (missing installed deps, e.g.oxc-parser, and node v22 vsengines >=24) — unrelated and unchanged by this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01M16Lwt8cM1azwev94UP81X
Generated by Claude Code