From 3670ad350efba38f88583cc632408c64d8baaebf Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:43:33 -0700 Subject: [PATCH] vfs: validate ZipProvider parent directories Reject attempts to create archive entries beneath non-directory parents. Cover open, mkdir, and rename in the sync and async APIs. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex --- lib/internal/vfs/providers/ziparchive.js | 24 ++++++++++++++++ test/parallel/test-vfs-zip-provider.js | 36 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/lib/internal/vfs/providers/ziparchive.js b/lib/internal/vfs/providers/ziparchive.js index 9f4ecceee460..eaadcc8c2eb5 100644 --- a/lib/internal/vfs/providers/ziparchive.js +++ b/lib/internal/vfs/providers/ziparchive.js @@ -336,6 +336,24 @@ class ZipProvider extends VirtualProvider { return false; } + /** + * Throws when an ancestor of `name` is a file. Missing ancestors are valid: + * ZIP archives represent directories implicitly when they contain entries + * beneath them. + * @param {string} name + * @param {string} syscall + * @param {string} path + */ + #validateParentDirectories(name, syscall, path) { + let slash = StringPrototypeIndexOf(name, '/'); + while (slash !== -1) { + if (this.#source.has(StringPrototypeSlice(name, 0, slash))) { + throw createENOTDIR(syscall, path); + } + slash = StringPrototypeIndexOf(name, '/', slash + 1); + } + } + async open(path, flags, mode) { const name = normalize(path); const fileEntry = await this.#getEntry(name); @@ -352,6 +370,7 @@ class ZipProvider extends VirtualProvider { if (!exists && mustExist(flags)) { throw createENOENT('open', path); } + if (!exists) this.#validateParentDirectories(name, 'open', path); let initial = EMPTY_BUFFER; if (exists && !isWriteTruncate(flags)) { initial = await fileEntry.content(); @@ -375,6 +394,7 @@ class ZipProvider extends VirtualProvider { if (!exists && mustExist(flags)) { throw createENOENT('open', path); } + if (!exists) this.#validateParentDirectories(name, 'open', path); let initial = EMPTY_BUFFER; if (exists && !isWriteTruncate(flags)) { initial = fileEntry.contentSync(); @@ -457,6 +477,7 @@ class ZipProvider extends VirtualProvider { async mkdir(path, options) { if (this.readonly) throw createEROFS('mkdir', path); const name = normalize(path); + this.#validateParentDirectories(name, 'mkdir', path); if (await this.exists(path)) { // `{ recursive: true }` only tolerates an existing *directory*; an // existing file (or any non-directory) still collides with EEXIST. @@ -469,6 +490,7 @@ class ZipProvider extends VirtualProvider { mkdirSync(path, options) { if (this.readonly) throw createEROFS('mkdir', path); const name = normalize(path); + this.#validateParentDirectories(name, 'mkdir', path); if (this.existsSync(path)) { // `{ recursive: true }` only tolerates an existing *directory*; an // existing file (or any non-directory) still collides with EEXIST. @@ -541,6 +563,7 @@ class ZipProvider extends VirtualProvider { // A file cannot take a directory's name; the archive would otherwise // hold both under it. if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath); + this.#validateParentDirectories(newName, 'rename', newPath); const content = await entry.content(); await this.#source.add(newName, content, { mode: entry.mode || undefined, @@ -556,6 +579,7 @@ class ZipProvider extends VirtualProvider { const entry = this.#getEntrySync(oldName); if (entry === null) throw createENOENT('rename', oldPath); if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath); + this.#validateParentDirectories(newName, 'rename', newPath); const content = entry.contentSync(); this.#source.addSync(newName, content, { mode: entry.mode || undefined, diff --git a/test/parallel/test-vfs-zip-provider.js b/test/parallel/test-vfs-zip-provider.js index ad137457f7a3..3ef5b4d5111e 100644 --- a/test/parallel/test-vfs-zip-provider.js +++ b/test/parallel/test-vfs-zip-provider.js @@ -83,6 +83,24 @@ async function buildArchive(entries, comment) { assert.strictEqual(await archiveVfs.promises.readFile('/new.txt', 'utf8'), 'brand new'); assert.strictEqual(zip.has('new.txt'), true); + // Entries cannot be created beneath a file. + await assert.rejects( + archiveVfs.promises.writeFile('/a.txt/child.txt', 'child'), + { code: 'ENOTDIR' }, + ); + await assert.rejects( + archiveVfs.promises.mkdir('/a.txt/child'), + { code: 'ENOTDIR' }, + ); + await assert.rejects( + archiveVfs.promises.rename('/new.txt', '/a.txt/renamed.txt'), + { code: 'ENOTDIR' }, + ); + assert.strictEqual(zip.has('a.txt/child.txt'), false); + assert.strictEqual(zip.has('a.txt/child/'), false); + assert.strictEqual(zip.has('a.txt/renamed.txt'), false); + assert.strictEqual(zip.has('new.txt'), true); + // Overwriting an existing file. await archiveVfs.promises.writeFile('/a.txt', 'overwritten'); assert.strictEqual(await archiveVfs.promises.readFile('/a.txt', 'utf8'), 'overwritten'); @@ -195,6 +213,24 @@ async function buildArchive(entries, comment) { archiveVfs.appendFileSync('/new.txt', '!'); assert.strictEqual(archiveVfs.readFileSync('/new.txt', 'utf8'), 'brand new!'); + // Entries cannot be created beneath a file. + assert.throws( + () => archiveVfs.writeFileSync('/a.txt/child.txt', 'child'), + { code: 'ENOTDIR' }, + ); + assert.throws( + () => archiveVfs.mkdirSync('/a.txt/child'), + { code: 'ENOTDIR' }, + ); + assert.throws( + () => archiveVfs.renameSync('/new.txt', '/a.txt/renamed.txt'), + { code: 'ENOTDIR' }, + ); + assert.strictEqual(zip.has('a.txt/child.txt'), false); + assert.strictEqual(zip.has('a.txt/child/'), false); + assert.strictEqual(zip.has('a.txt/renamed.txt'), false); + assert.strictEqual(zip.has('new.txt'), true); + // mkdir/rmdir. archiveVfs.mkdirSync('/newdir'); assert.strictEqual(archiveVfs.statSync('/newdir').isDirectory(), true);