fix(bundle,lockfile): require every flat file key to be canonical - #185
Merged
Merged
Conversation
Bundle.parse and Lockfile.parse check a bucket dir and each file name for
escaping the root, but not the flat key they join into. An empty dir joins
with any name into an absolute key: `sources: { '': { files: { 'etc/passwd':
... } } }` parsed as the file `/etc/passwd`. Both parsers now also check the
joined key (stasis only ever writes the root bucket as '.').
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u
…ng joined keys Review of the joined-key check: it missed '' with a '' root listing (key ''), and the cause is wider than absolute keys -- bucket dirs were never required to be canonical, so 'src' and 'src/.' (or 'src/', './src') alias one directory with two payloads past the duplicate-key check, in bundles and lockfiles alike. A bucket dir must now be '.' or a relative path without empty, '.' or '..' segments (isCanonicalDir), checked once per bucket in Bundle.parse and Lockfile.parse, and on the write paths -- mergeModuleMaps and both serialize()s -- so stasis never writes an artifact its own parse rejects. It replaces the per-file joined-key check: with a canonical dir, a file name that doesn't escape can't make the key escape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u
exo-nikita
changed the title
Second review: making only bucket dirs canonical left file-name aliases open -- '.' holding 'src/x.js' next to 'src' holding './x.js' (or 'src/a/../x.js' in one bucket) still gave one file two payloads -- and serialize() still wrote artifacts its own parse rejects. One shared artifact-util helper, flatFileKeys, now flattens each file's key (dir and name joined), rejects any key with an empty, '.' or '..' segment (which covers the empty dir, absolute keys and every alias), and reports duplicates. Bundle.parse, Lockfile.parse, mergeModuleMaps and both serialize()s use it instead of their own loops, so what stasis writes always parses. It replaces isCanonicalDir and the per-name escape checks it subsumes; the Lockfile passes its keys to parseExecutable instead of recomputing them. Errors name the key (JSON-quoted); a non-string bucket dir gets a proper message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u
exo-nikita
changed the title
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
3 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.
Bundle.parseandLockfile.parsechecked a bucket dir and a file name each for escaping the project root. They never checked the flat key the two join into, and never required a canonical spelling of it. That leaves two problems.sources: { '': { files: { 'etc/passwd': … } } }parsed as the file/etc/passwd. With a root listing, it parsed as the key''.srcandsrc/..holdingsrc/x.jsnext tosrcholding./x.jssrc/x.jsandsrc/a/../x.jsserialize()could also write artifacts that the matching parse then rejects.Change
One shared helper in
artifact-util.js,flatFileKeys(modules, what, onDuplicate), joins each file's dir and name into its flat key. It then:.or..segment, which covers the empty dir, absolute keys and every alias aboveonDuplicate, so each caller keeps its existing messageBundle.parse,Lockfile.parse,mergeModuleMapsand bothserialize()methods use it in place of their own flatten-and-dedup loops. So whatever stasis writes, it can parse again.It also replaces the per-file escape checks it subsumes. The lockfile passes the resulting key set to
parseExecutableinstead of recomputing it. Errors name the key, JSON-quoted (bundle: non-canonical file key "src/./x.js"), and a non-string bucket dir gets its own message.Names made of dots stay ordinary names:
..foo,...,.pnpm/…andnode_modules/.bin/…are accepted. stasis builds keys frompath.relative, so it never writes a non-canonical one; the full suite, including many runs that build and write bundles and lockfiles, never hits the new check. A hand-edited artifact with such a key now fails to parse.Tests
src/,src/.,./src,src//libanda/../srcas dirs./x.js,a/../x.js,x//y.js,/x.jsandx/as file names./x.jsalias, and a v0./src/x.jsserialize()refuses a non-canonical key and a cross-bucket duplicate, andmerge()refuses a bad bucket on the other side, for bothBundleandLockfile.main.Relation to #184
#184 (the streaming bundle reader) has its own narrower per-file check in
stasis-core/src/bundle.js. Whichever PR merges second needs a small conflict resolved there. After this merges, #184's streaming pre-check can use the same rule.🤖 Generated with Claude Code
https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u