diff --git a/stasis-core/src/artifact-util.js b/stasis-core/src/artifact-util.js index 619ba3b7..ebeee6d2 100644 --- a/stasis-core/src/artifact-util.js +++ b/stasis-core/src/artifact-util.js @@ -261,6 +261,26 @@ export function mergeExecutableSets(a, b, bModules, scope) { return out } +// 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() @@ -302,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 5378891f..0aae0ed4 100644 --- a/stasis-core/src/bundle.js +++ b/stasis-core/src/bundle.js @@ -4,6 +4,7 @@ import { fileMapToObject, fileSetToObject, fromEntries, + flatFileKeys, hasNodeModulesSegment, isPlainObject, mergeFormatMaps, @@ -29,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 } @@ -152,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)) { @@ -168,19 +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) { - 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) @@ -237,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 31ddc1fc..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, 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,22 +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) { 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() @@ -123,15 +116,17 @@ 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 = [] diff --git a/tests/public-exports.test.js b/tests/public-exports.test.js index de7984ad..cad66330 100644 --- a/tests/public-exports.test.js +++ b/tests/public-exports.test.js @@ -328,6 +328,54 @@ 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 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 = (spec) => JSON.stringify({ + version: 0, config: { scope: 'full' }, entries: [], sources: sources(spec, 'sha512-x'), modules: {}, formats: {}, imports: {}, + }) + 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)) + } + 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 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) => { // `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).