From 3fc5c01e76b65d56935de6e60fa3b49bb76e1407 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 17:39:54 +0000 Subject: [PATCH 1/3] fix(bundle,lockfile): reject a file key that joins into an absolute path 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 Claude-Session: https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u --- stasis-core/src/bundle.js | 4 +++- stasis-core/src/lockfile.js | 4 +++- tests/public-exports.test.js | 28 ++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/stasis-core/src/bundle.js b/stasis-core/src/bundle.js index 5378891f..4e3b939d 100644 --- a/stasis-core/src/bundle.js +++ b/stasis-core/src/bundle.js @@ -172,8 +172,10 @@ export class Bundle { for (const [dir, { files }] of modules) { for (const rel of Object.keys(files)) { const key = moduleFileKey(dir, rel) + // The joined key must stay inside the root too, not only its dir and rel: an empty dir makes + // `rel` absolute. Messages built only on failure: this loop visits every bundled file. + if (posixPathEscapes(key)) assert(false, `bundle path escapes the root: ${key}`) if (flatKeys.has(key)) { - // Message built only on failure: this loop visits every bundled file. assert(false, `duplicate file key '${key}' across bundle buckets -- module bucketing ` + `changed between writes (a workspace package without a version now owns its own ` + `bucket); regenerate the artifact (bundle=replace)`) diff --git a/stasis-core/src/lockfile.js b/stasis-core/src/lockfile.js index 31ddc1fc..a77c9fce 100644 --- a/stasis-core/src/lockfile.js +++ b/stasis-core/src/lockfile.js @@ -70,8 +70,10 @@ export class Lockfile { for (const name of Object.keys(files)) { assert(!posixPathEscapes(name)) const key = moduleFileKey(dir, name) + // The joined key must stay inside the root too, not only its dir and name: an empty dir makes + // `name` absolute. Messages built only on failure: this loop visits every attested file. + if (posixPathEscapes(key)) assert(false, `lockfile path escapes the root: ${key}`) if (flatKeys.has(key)) { - // Message built only on failure: this loop visits every attested file. assert(false, `duplicate file key '${key}' across lockfile buckets -- module bucketing ` + `changed between writes (a workspace package without a version now owns its own ` + `bucket); regenerate the lockfile (lock=replace)`) diff --git a/tests/public-exports.test.js b/tests/public-exports.test.js index de7984ad..628a875e 100644 --- a/tests/public-exports.test.js +++ b/tests/public-exports.test.js @@ -328,6 +328,34 @@ test('Bundle.parse rejects a v0 flat source path that escapes the project root', t.assert.throws(() => Bundle.parse(v0('/etc/passwd'))) }) +test('Bundle.parse and Lockfile.parse reject a file key that joins into an absolute path', (t) => { + // The bucket dir and the file name each stay inside the root, but an empty dir joins with any + // name into an absolute flat key ('' + 'etc/passwd' -> '/etc/passwd'), so the joined key is + // checked too. + const bundle = (dir) => JSON.stringify({ + version: 1, + config: { scope: 'full' }, + entries: [], + sources: { [dir]: { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'x' } } }, + formats: {}, + imports: {}, + }) + const lockfile = (dir) => JSON.stringify({ + version: 0, + config: { scope: 'full' }, + entries: [], + sources: { [dir]: { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'sha512-x' } } }, + modules: {}, + formats: {}, + imports: {}, + }) + t.assert.throws(() => Bundle.parse(bundle('')), /escapes the root: \/etc\/passwd/) + t.assert.throws(() => Lockfile.parse(lockfile('')), /escapes the root: \/etc\/passwd/) + // The root bucket is spelled '.', which joins to plain relative keys. + t.assert.deepStrictEqual([...Bundle.parse(bundle('.')).sources.keys()], ['etc/passwd']) + t.assert.deepStrictEqual(Object.keys(Lockfile.parse(lockfile('.')).modules.get('.').files), ['etc/passwd']) +}) + test('Bundle.parse keeps accepting a directory listing keyed at a module root (rel === \'\')', (t) => { // `stasis run --fs` keys a readdir of a package root at rel '' inside its own // bucket (moduleFileKey collapses that to the dir itself, see fs.test.js). From 6b83ad1ad51a6cc6972b5f967c445cd6309cca19 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 18:17:20 +0000 Subject: [PATCH 2/3] fix(bundle,lockfile): require canonical bucket dirs instead of checking 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 Claude-Session: https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u --- stasis-core/src/artifact-util.js | 4 ++++ stasis-core/src/bundle.js | 8 ++++--- stasis-core/src/lockfile.js | 10 ++++----- tests/public-exports.test.js | 37 ++++++++++++++++++++++++-------- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/stasis-core/src/artifact-util.js b/stasis-core/src/artifact-util.js index 619ba3b7..494175d1 100644 --- a/stasis-core/src/artifact-util.js +++ b/stasis-core/src/artifact-util.js @@ -261,11 +261,15 @@ export function mergeExecutableSets(a, b, bModules, scope) { return out } +// '.' or a relative path without empty, '.' or '..' segments: one spelling per bucket. +export const isCanonicalDir = (dir) => dir === '.' || dir.split('/').every((s) => s !== '' && s !== '.' && s !== '..') + // Result `files` objects are null-prototype, so a `__proto__` file name is a plain own key. export function mergeModuleMaps(a, b, label) { const out = new Map() const absorb = (modules) => { for (const [dir, info] of modules) { + assert(isCanonicalDir(dir), `${label}: bucket dir '${dir}' is not canonical`) const existing = out.get(dir) if (existing === undefined) { out.set(dir, { diff --git a/stasis-core/src/bundle.js b/stasis-core/src/bundle.js index 4e3b939d..e61818d2 100644 --- a/stasis-core/src/bundle.js +++ b/stasis-core/src/bundle.js @@ -5,6 +5,7 @@ import { fileSetToObject, fromEntries, hasNodeModulesSegment, + isCanonicalDir, isPlainObject, mergeFormatMaps, mergeImportMaps, @@ -170,12 +171,12 @@ export class Bundle { // Flat keys must be unique across buckets: two different bucket splits can flatten to one path, and the `sources` getter would serve either payload. const flatKeys = new Set() for (const [dir, { files }] of modules) { + // An empty dir would make keys absolute; aliases like 'src/.' would dodge the duplicate check. + assert(isCanonicalDir(dir), `bundle bucket dir '${dir}' is not canonical`) for (const rel of Object.keys(files)) { const key = moduleFileKey(dir, rel) - // The joined key must stay inside the root too, not only its dir and rel: an empty dir makes - // `rel` absolute. Messages built only on failure: this loop visits every bundled file. - if (posixPathEscapes(key)) assert(false, `bundle path escapes the root: ${key}`) if (flatKeys.has(key)) { + // Message built only on failure: this loop visits every bundled file. assert(false, `duplicate file key '${key}' across bundle buckets -- module bucketing ` + `changed between writes (a workspace package without a version now owns its own ` + `bucket); regenerate the artifact (bundle=replace)`) @@ -227,6 +228,7 @@ export class Bundle { const sourceEntries = [] for (const [dir, { name, version, ecosystem, files }] of this.modules) { if (Object.keys(files).length === 0) continue + assert(isCanonicalDir(dir), `bundle bucket dir '${dir}' is not canonical`) const inNodeModules = hasNodeModulesSegment(dir) if (inNodeModules) assert(name && version && files) const sorted = fromEntries(Object.entries(files).toSorted((a, b) => sortPaths(a[0], b[0]))) diff --git a/stasis-core/src/lockfile.js b/stasis-core/src/lockfile.js index a77c9fce..d2d410f2 100644 --- a/stasis-core/src/lockfile.js +++ b/stasis-core/src/lockfile.js @@ -1,4 +1,4 @@ -import { KNOWN_FORMATS, assert, serializeExecutable, fileMapToObject, fileSetToObject, fromEntries, hasNodeModulesSegment, isPlainObject, mergeExecutableSets, mergeFormatMaps, mergeImportMaps, mergeModuleMaps, moduleFileKey, moduleFileKeys, parseExecutable, posixPathEscapes, sortPaths } from './artifact-util.js' +import { KNOWN_FORMATS, assert, serializeExecutable, fileMapToObject, fileSetToObject, fromEntries, hasNodeModulesSegment, isCanonicalDir, isPlainObject, mergeExecutableSets, mergeFormatMaps, mergeImportMaps, mergeModuleMaps, moduleFileKey, moduleFileKeys, parseExecutable, posixPathEscapes, sortPaths } from './artifact-util.js' const VERSION = 0 @@ -65,15 +65,14 @@ export class Lockfile { // to one path, and hashes/attestation lookups key on the flat path. const flatKeys = new Set() for (const [dir, { files }] of modules) { - assert(!posixPathEscapes(dir)) + // An empty dir would make keys absolute; aliases like 'src/.' would dodge the duplicate check. + assert(isCanonicalDir(dir), `lockfile bucket dir '${dir}' is not canonical`) assert(files) for (const name of Object.keys(files)) { assert(!posixPathEscapes(name)) const key = moduleFileKey(dir, name) - // The joined key must stay inside the root too, not only its dir and name: an empty dir makes - // `name` absolute. Messages built only on failure: this loop visits every attested file. - if (posixPathEscapes(key)) assert(false, `lockfile path escapes the root: ${key}`) if (flatKeys.has(key)) { + // Message built only on failure: this loop visits every attested file. assert(false, `duplicate file key '${key}' across lockfile buckets -- module bucketing ` + `changed between writes (a workspace package without a version now owns its own ` + `bucket); regenerate the lockfile (lock=replace)`) @@ -138,6 +137,7 @@ export class Lockfile { const modules = [] const sources = [] for (const [dir, { name, version, ecosystem, files }] of this.modules) { + assert(isCanonicalDir(dir), `lockfile bucket dir '${dir}' is not canonical`) const inNodeModules = hasNodeModulesSegment(dir) if (inNodeModules) assert(name && version && files) const type = inNodeModules ? modules : sources diff --git a/tests/public-exports.test.js b/tests/public-exports.test.js index 628a875e..a9cfec48 100644 --- a/tests/public-exports.test.js +++ b/tests/public-exports.test.js @@ -328,15 +328,13 @@ test('Bundle.parse rejects a v0 flat source path that escapes the project root', t.assert.throws(() => Bundle.parse(v0('/etc/passwd'))) }) -test('Bundle.parse and Lockfile.parse reject a file key that joins into an absolute path', (t) => { - // The bucket dir and the file name each stay inside the root, but an empty dir joins with any - // name into an absolute flat key ('' + 'etc/passwd' -> '/etc/passwd'), so the joined key is - // checked too. - const bundle = (dir) => JSON.stringify({ +test('Bundle.parse and Lockfile.parse reject a non-canonical bucket dir', (t) => { + // '' makes every key absolute; the rest are aliases of another spelling of the dir. + const bundle = (dir, files = { 'etc/passwd': 'x' }) => JSON.stringify({ version: 1, config: { scope: 'full' }, entries: [], - sources: { [dir]: { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'x' } } }, + sources: { [dir]: { name: 'x', version: '1.0.0', files } }, formats: {}, imports: {}, }) @@ -349,13 +347,34 @@ test('Bundle.parse and Lockfile.parse reject a file key that joins into an absol formats: {}, imports: {}, }) - t.assert.throws(() => Bundle.parse(bundle('')), /escapes the root: \/etc\/passwd/) - t.assert.throws(() => Lockfile.parse(lockfile('')), /escapes the root: \/etc\/passwd/) - // The root bucket is spelled '.', which joins to plain relative keys. + for (const dir of ['', 'src/', 'src/.', './src', 'src//lib', 'a/../src']) { + t.assert.throws(() => Bundle.parse(bundle(dir)), /is not canonical/, JSON.stringify(dir)) + t.assert.throws(() => Lockfile.parse(lockfile(dir)), /is not canonical/, JSON.stringify(dir)) + } + // Also when the only file is a root listing, whose key would be '' itself. + t.assert.throws(() => Bundle.parse(bundle('', { '': '[]' })), /is not canonical/) + t.assert.throws(() => Bundle.parse(JSON.stringify({ + version: 1, + config: { scope: 'node_modules' }, + modules: { 'node_modules/w/.': { name: 'w', version: '1.0.0', files: { 'i.js': 'x' } } }, + formats: {}, + imports: {}, + })), /is not canonical/) t.assert.deepStrictEqual([...Bundle.parse(bundle('.')).sources.keys()], ['etc/passwd']) + t.assert.deepStrictEqual([...Bundle.parse(bundle('src')).sources.keys()], ['src/etc/passwd']) t.assert.deepStrictEqual(Object.keys(Lockfile.parse(lockfile('.')).modules.get('.').files), ['etc/passwd']) }) +test('Bundle and Lockfile refuse to merge or serialize a non-canonical bucket dir', (t) => { + // So stasis never writes an artifact its own parse rejects. + const bad = () => new Map([['', { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'x' } }]]) + t.assert.throws(() => new Bundle({ modules: bad() }).merge(new Bundle()), /is not canonical/) + t.assert.throws(() => new Bundle({ modules: bad() }).serialize(), /is not canonical/) + const full = { config: { scope: 'full' } } + t.assert.throws(() => new Lockfile({ ...full, modules: bad() }).merge(new Lockfile(full)), /is not canonical/) + t.assert.throws(() => new Lockfile({ ...full, modules: bad() }).serialize(), /is not canonical/) +}) + test('Bundle.parse keeps accepting a directory listing keyed at a module root (rel === \'\')', (t) => { // `stasis run --fs` keys a readdir of a package root at rel '' inside its own // bucket (moduleFileKey collapses that to the dir itself, see fs.test.js). From 4be5bb3254a8845bc5dd85aeb59c5c274fd02ac3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 18:37:46 +0000 Subject: [PATCH 3/3] fix(bundle,lockfile): require every flat file key to be canonical 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 Claude-Session: https://claude.ai/code/session_01FinLHpaKeRRjstqzMnwj6u --- stasis-core/src/artifact-util.js | 41 +++++++++------- stasis-core/src/bundle.js | 29 +++-------- stasis-core/src/lockfile.js | 29 +++++------ tests/public-exports.test.js | 83 ++++++++++++++++---------------- 4 files changed, 84 insertions(+), 98 deletions(-) diff --git a/stasis-core/src/artifact-util.js b/stasis-core/src/artifact-util.js index 494175d1..ebeee6d2 100644 --- a/stasis-core/src/artifact-util.js +++ b/stasis-core/src/artifact-util.js @@ -261,15 +261,31 @@ export function mergeExecutableSets(a, b, bModules, scope) { return out } -// '.' or a relative path without empty, '.' or '..' segments: one spelling per bucket. -export const isCanonicalDir = (dir) => dir === '.' || dir.split('/').every((s) => s !== '' && s !== '.' && s !== '..') +// An empty, '.' or '..' path segment. +const NON_CANONICAL_SEGMENT = /(?:^|\/)\.{0,2}(?:\/|$)/u + +// Maps each file's flat key to its bucket; rejects non-canonical keys, reports duplicates to onDuplicate. +export function flatFileKeys(modules, what, onDuplicate) { + const owners = new Map() + for (const [dir, { files }] of modules) { + if (typeof dir !== 'string') assert(false, `${what}: bucket dir ${String(dir)} is not a string`) + for (const rel of Object.keys(files)) { + const key = moduleFileKey(dir, rel) + // Messages built only on failure: this loop visits every file. + if (key !== '.' && NON_CANONICAL_SEGMENT.test(key)) assert(false, `${what}: non-canonical file key ${JSON.stringify(key)}`) + const owner = owners.get(key) + if (owner !== undefined) onDuplicate(key, owner, dir) + owners.set(key, dir) + } + } + return owners +} // Result `files` objects are null-prototype, so a `__proto__` file name is a plain own key. export function mergeModuleMaps(a, b, label) { const out = new Map() const absorb = (modules) => { for (const [dir, info] of modules) { - assert(isCanonicalDir(dir), `${label}: bucket dir '${dir}' is not canonical`) const existing = out.get(dir) if (existing === undefined) { out.set(dir, { @@ -306,21 +322,10 @@ export function mergeModuleMaps(a, b, label) { // releases (a versionless workspace package used to fall through to a parent bucket and now owns // its own), and per-dir absorption cannot see that: without this check the merge would WRITE an // artifact that then fails its own next parse on the duplicate-file-key guard. - const owners = new Map() - for (const [dir, { files }] of out) { - for (const rel of Object.keys(files)) { - const key = moduleFileKey(dir, rel) - const owner = owners.get(key) - if (owner !== undefined) { - // Message built only on failure: this loop visits every merged file. - assert(false, - `${label}: file '${key}' is bucketed under both '${owner}' and '${dir}' -- module bucketing ` + - `changed between the artifacts (a workspace package without a version now owns its own ` + - `bucket); regenerate the artifact (bundle=replace / lock=replace)`) - } - owners.set(key, dir) - } - } + flatFileKeys(out, label, (key, owner, dir) => assert(false, + `${label}: file '${key}' is bucketed under both '${owner}' and '${dir}' -- module bucketing ` + + `changed between the artifacts (a workspace package without a version now owns its own ` + + `bucket); regenerate the artifact (bundle=replace / lock=replace)`)) return out } diff --git a/stasis-core/src/bundle.js b/stasis-core/src/bundle.js index e61818d2..0aae0ed4 100644 --- a/stasis-core/src/bundle.js +++ b/stasis-core/src/bundle.js @@ -4,8 +4,8 @@ import { fileMapToObject, fileSetToObject, fromEntries, + flatFileKeys, hasNodeModulesSegment, - isCanonicalDir, isPlainObject, mergeFormatMaps, mergeImportMaps, @@ -30,6 +30,10 @@ const normalize = ({ name, version, ecosystem, files }) => { return { name, version: version ?? undefined, ...(ecosystem === undefined ? {} : { ecosystem }), files: fromEntries(Object.entries(files)) } } +const duplicateKey = (key) => assert(false, `duplicate file key '${key}' across bundle buckets -- module bucketing ` + + `changed between writes (a workspace package without a version now owns its own ` + + `bucket); regenerate the artifact (bundle=replace)`) + const inferModuleDir = (path) => splitNodeModulesPath(path) ?? { dir: '.', rel: path, name: null } @@ -153,10 +157,6 @@ export class Bundle { assert(json.entries === undefined) assert(json.sources === undefined) } - for (const [, { files }] of modules) { - // posixPathEscapes (not a '..' prefix): also catches mid-path escapes and absolute paths. - for (const rel of Object.keys(files)) assert(!posixPathEscapes(rel)) - } } else { assert(json.sources) for (const [path, content] of Object.entries(json.sources)) { @@ -169,21 +169,7 @@ export class Bundle { } // Flat keys must be unique across buckets: two different bucket splits can flatten to one path, and the `sources` getter would serve either payload. - const flatKeys = new Set() - for (const [dir, { files }] of modules) { - // An empty dir would make keys absolute; aliases like 'src/.' would dodge the duplicate check. - assert(isCanonicalDir(dir), `bundle bucket dir '${dir}' is not canonical`) - for (const rel of Object.keys(files)) { - const key = moduleFileKey(dir, rel) - if (flatKeys.has(key)) { - // Message built only on failure: this loop visits every bundled file. - assert(false, `duplicate file key '${key}' across bundle buckets -- module bucketing ` + - `changed between writes (a workspace package without a version now owns its own ` + - `bucket); regenerate the artifact (bundle=replace)`) - } - flatKeys.add(key) - } - } + const flatKeys = flatFileKeys(modules, 'bundle', duplicateKey) // Reject paths escaping the root here (incl. mid-path `a/../../x`): getImport resolves against the root at load. const imports = objectToMaps(json.imports) @@ -228,7 +214,6 @@ export class Bundle { const sourceEntries = [] for (const [dir, { name, version, ecosystem, files }] of this.modules) { if (Object.keys(files).length === 0) continue - assert(isCanonicalDir(dir), `bundle bucket dir '${dir}' is not canonical`) const inNodeModules = hasNodeModulesSegment(dir) if (inNodeModules) assert(name && version && files) const sorted = fromEntries(Object.entries(files).toSorted((a, b) => sortPaths(a[0], b[0]))) @@ -241,6 +226,8 @@ export class Bundle { } serialize() { + // Never write an artifact that parse would reject. + flatFileKeys(this.modules, 'bundle', duplicateKey) const entries = fileSetToObject(this.entries) const { modules, sources } = this.#groupedFromModules() const formats = fileMapToObject(this.formats) diff --git a/stasis-core/src/lockfile.js b/stasis-core/src/lockfile.js index d2d410f2..2007b89a 100644 --- a/stasis-core/src/lockfile.js +++ b/stasis-core/src/lockfile.js @@ -1,7 +1,11 @@ -import { KNOWN_FORMATS, assert, serializeExecutable, fileMapToObject, fileSetToObject, fromEntries, hasNodeModulesSegment, isCanonicalDir, isPlainObject, mergeExecutableSets, mergeFormatMaps, mergeImportMaps, mergeModuleMaps, moduleFileKey, moduleFileKeys, parseExecutable, posixPathEscapes, sortPaths } from './artifact-util.js' +import { KNOWN_FORMATS, assert, serializeExecutable, fileMapToObject, fileSetToObject, fromEntries, flatFileKeys, hasNodeModulesSegment, isPlainObject, mergeExecutableSets, mergeFormatMaps, mergeImportMaps, mergeModuleMaps, parseExecutable, posixPathEscapes, sortPaths } from './artifact-util.js' const VERSION = 0 +const duplicateKey = (key) => assert(false, `duplicate file key '${key}' across lockfile buckets -- module bucketing ` + + `changed between writes (a workspace package without a version now owns its own ` + + `bucket); regenerate the lockfile (lock=replace)`) + const normalize = ({ name, version, ecosystem, files }) => { assert(ecosystem === undefined || typeof ecosystem === 'string') // An absent version has one spelling: null (hand-edited or legacy JSON) folds into undefined so @@ -63,23 +67,11 @@ export class Lockfile { // Flat keys must be unique across buckets (mirrors Bundle.parse): two bucket splits can flatten // to one path, and hashes/attestation lookups key on the flat path. - const flatKeys = new Set() for (const [dir, { files }] of modules) { - // An empty dir would make keys absolute; aliases like 'src/.' would dodge the duplicate check. - assert(isCanonicalDir(dir), `lockfile bucket dir '${dir}' is not canonical`) + assert(!posixPathEscapes(dir)) assert(files) - for (const name of Object.keys(files)) { - assert(!posixPathEscapes(name)) - const key = moduleFileKey(dir, name) - if (flatKeys.has(key)) { - // Message built only on failure: this loop visits every attested file. - assert(false, `duplicate file key '${key}' across lockfile buckets -- module bucketing ` + - `changed between writes (a workspace package without a version now owns its own ` + - `bucket); regenerate the lockfile (lock=replace)`) - } - flatKeys.add(key) - } } + const flatKeys = flatFileKeys(modules, 'lockfile', duplicateKey) assert(isPlainObject(json.imports)) const imports = new Map() @@ -124,20 +116,21 @@ export class Lockfile { formats.set(key, format) } - // Every executable must be a file this lockfile attests; the key index is built only when the (usually absent) key is present. + // Every executable must be a file this lockfile attests. const executable = json.executable === undefined ? new Set() - : parseExecutable(json.executable, { what: 'lockfile', files: moduleFileKeys(modules), formats, scope: json.config.scope }) + : parseExecutable(json.executable, { what: 'lockfile', files: flatKeys, formats, scope: json.config.scope }) return new Lockfile({ config: json.config, entries, modules, imports, formats, executable }) } serialize() { + // Never write an artifact that parse would reject. + flatFileKeys(this.modules, 'lockfile', duplicateKey) const entries = fileSetToObject(this.entries) const modules = [] const sources = [] for (const [dir, { name, version, ecosystem, files }] of this.modules) { - assert(isCanonicalDir(dir), `lockfile bucket dir '${dir}' is not canonical`) const inNodeModules = hasNodeModulesSegment(dir) if (inNodeModules) assert(name && version && files) const type = inNodeModules ? modules : sources diff --git a/tests/public-exports.test.js b/tests/public-exports.test.js index a9cfec48..cad66330 100644 --- a/tests/public-exports.test.js +++ b/tests/public-exports.test.js @@ -328,51 +328,52 @@ test('Bundle.parse rejects a v0 flat source path that escapes the project root', t.assert.throws(() => Bundle.parse(v0('/etc/passwd'))) }) -test('Bundle.parse and Lockfile.parse reject a non-canonical bucket dir', (t) => { - // '' makes every key absolute; the rest are aliases of another spelling of the dir. - const bundle = (dir, files = { 'etc/passwd': 'x' }) => JSON.stringify({ - version: 1, - config: { scope: 'full' }, - entries: [], - sources: { [dir]: { name: 'x', version: '1.0.0', files } }, - formats: {}, - imports: {}, +test('Bundle.parse and Lockfile.parse reject a non-canonical file key', (t) => { + // An empty, '.' or '..' segment spells a file more than one way, or makes its key absolute. + const sources = (spec, value) => Object.fromEntries(Object.entries(spec).map(([dir, names]) => + [dir, { name: 'x', version: '1.0.0', files: Object.fromEntries(names.map((name) => [name, value])) }])) + const bundle = (spec) => JSON.stringify({ + version: 1, config: { scope: 'full' }, entries: [], sources: sources(spec, 'x'), formats: {}, imports: {}, }) - const lockfile = (dir) => JSON.stringify({ - version: 0, - config: { scope: 'full' }, - entries: [], - sources: { [dir]: { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'sha512-x' } } }, - modules: {}, - formats: {}, - imports: {}, + const lockfile = (spec) => JSON.stringify({ + version: 0, config: { scope: 'full' }, entries: [], sources: sources(spec, 'sha512-x'), modules: {}, formats: {}, imports: {}, }) - for (const dir of ['', 'src/', 'src/.', './src', 'src//lib', 'a/../src']) { - t.assert.throws(() => Bundle.parse(bundle(dir)), /is not canonical/, JSON.stringify(dir)) - t.assert.throws(() => Lockfile.parse(lockfile(dir)), /is not canonical/, JSON.stringify(dir)) + const rejected = [ + { '': ['etc/passwd'] }, + { '': [''] }, + ...['src/', 'src/.', './src', 'src//lib', 'a/../src'].map((dir) => ({ [dir]: ['x.js'] })), + ...['./x.js', 'a/../x.js', 'x//y.js', '/x.js', 'x/'].map((name) => ({ '.': [name] })), + { '.': ['src/x.js'], src: ['./x.js'] }, + ] + for (const spec of rejected) { + t.assert.throws(() => Bundle.parse(bundle(spec)), /non-canonical file key/, JSON.stringify(spec)) + t.assert.throws(() => Lockfile.parse(lockfile(spec)), /non-canonical file key/, JSON.stringify(spec)) } - // Also when the only file is a root listing, whose key would be '' itself. - t.assert.throws(() => Bundle.parse(bundle('', { '': '[]' })), /is not canonical/) - t.assert.throws(() => Bundle.parse(JSON.stringify({ - version: 1, - config: { scope: 'node_modules' }, - modules: { 'node_modules/w/.': { name: 'w', version: '1.0.0', files: { 'i.js': 'x' } } }, - formats: {}, - imports: {}, - })), /is not canonical/) - t.assert.deepStrictEqual([...Bundle.parse(bundle('.')).sources.keys()], ['etc/passwd']) - t.assert.deepStrictEqual([...Bundle.parse(bundle('src')).sources.keys()], ['src/etc/passwd']) - t.assert.deepStrictEqual(Object.keys(Lockfile.parse(lockfile('.')).modules.get('.').files), ['etc/passwd']) -}) - -test('Bundle and Lockfile refuse to merge or serialize a non-canonical bucket dir', (t) => { - // So stasis never writes an artifact its own parse rejects. - const bad = () => new Map([['', { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'x' } }]]) - t.assert.throws(() => new Bundle({ modules: bad() }).merge(new Bundle()), /is not canonical/) - t.assert.throws(() => new Bundle({ modules: bad() }).serialize(), /is not canonical/) + const v0 = (paths) => JSON.stringify({ + version: 0, config: { scope: 'full' }, formats: {}, imports: {}, sources: Object.fromEntries(paths.map((path) => [path, 'x'])), + }) + t.assert.throws(() => Bundle.parse(v0(['src/x.js', './src/x.js'])), /non-canonical file key/) + // Dot-names are ordinary names. + const names = ['x.js', '..foo', '...', '.pnpm/x', 'node_modules/.bin/x'] + const accepted = { '.': names, src: ['a/b.js'] } + t.assert.deepStrictEqual([...Bundle.parse(bundle(accepted)).sources.keys()], [...names, 'src/a/b.js']) + t.assert.deepStrictEqual(Object.keys(Lockfile.parse(lockfile(accepted)).modules.get('.').files), names) +}) + +test('Bundle and Lockfile refuse to merge or serialize what their parse rejects', (t) => { + const absolute = () => new Map([['', { name: 'x', version: '1.0.0', files: { 'etc/passwd': 'x' } }]]) + const duplicate = () => new Map([ + ['.', { name: 'a', version: '1.0.0', files: { 'src/x.js': 'x' } }], + ['src', { name: 's', version: '1.0.0', files: { 'x.js': 'y' } }], + ]) const full = { config: { scope: 'full' } } - t.assert.throws(() => new Lockfile({ ...full, modules: bad() }).merge(new Lockfile(full)), /is not canonical/) - t.assert.throws(() => new Lockfile({ ...full, modules: bad() }).serialize(), /is not canonical/) + t.assert.throws(() => new Bundle({ modules: absolute() }).serialize(), /non-canonical file key "\/etc\/passwd"/) + t.assert.throws(() => new Bundle({ modules: duplicate() }).serialize(), /duplicate file key 'src\/x\.js'/) + t.assert.throws(() => new Bundle().merge(new Bundle({ modules: absolute() })), /bundle merge: non-canonical file key/) + t.assert.throws(() => new Lockfile({ ...full, modules: absolute() }).serialize(), /non-canonical file key/) + t.assert.throws(() => new Lockfile({ ...full, modules: duplicate() }).serialize(), /duplicate file key 'src\/x\.js'/) + t.assert.throws(() => new Lockfile(full).merge(new Lockfile({ ...full, modules: absolute() })), /non-canonical file key/) + t.assert.throws(() => new Bundle({ modules: new Map([[1, { name: 'x', files: { a: 'b' } }]]) }).serialize(), /bucket dir 1 is not a string/) }) test('Bundle.parse keeps accepting a directory listing keyed at a module root (rel === \'\')', (t) => {