From ed936cabe6f0c13ca307bf90e417357737b41532 Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 11:34:15 +0200 Subject: [PATCH 1/3] vfs: apply open(2) effects to ZipProvider handles A ZipProvider handle keeps its content in memory and adds the entry to the archive when it is closed, and only if something was written. The effects a real `open(2)` has at open time are therefore lost, and so is the metadata the entry already carried: * `open(path, 'w')` followed by `close()` neither truncates an existing entry nor creates a missing one; the same holds for "a" on a missing file. Tools that touch or truncate by open-then-close do nothing. * Rewriting an entry (append, or an in-place write through "r+") re-adds it with the `mode` argument `open()` received (fs's default 0o666), not the mode the entry had, so a 0o755 script silently loses its executable bit. * `fstat` on a handle reports that same `open()` mode and the current time instead of the entry's mode and modification time. * Renaming a file onto an existing directory succeeds and leaves a name that is both a file and a directory; real file systems refuse with EISDIR. This adds a test for each of these against a mounted ZipBuffer, stating the real-fs outcome as the expectation. Proposed solution: mark the handle dirty at open time when the flags imply creation or truncation, so close always commits; carry the existing entry's mode and modification time on the handle, use them for `fstat` and for the re-added entry, and only fall back to the `open()` mode for a newly created entry; and reject `rename` onto an existing directory with EISDIR before touching the archive. Signed-off-by: Philipp Dunkel --- test/parallel/test-vfs-zip-provider-commit.js | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 test/parallel/test-vfs-zip-provider-commit.js diff --git a/test/parallel/test-vfs-zip-provider-commit.js b/test/parallel/test-vfs-zip-provider-commit.js new file mode 100644 index 00000000000..6a2874be4ee --- /dev/null +++ b/test/parallel/test-vfs-zip-provider-commit.js @@ -0,0 +1,102 @@ +// Flags: --experimental-vfs +'use strict'; + +// A ZipProvider handle commits its content to the archive when it is closed. +// The effects `open(2)` has at open time, and the metadata an entry already +// carries, must survive that model: opening with "w" creates or truncates +// even without a write, rewriting an entry keeps its mode, fstat reports the +// entry's mode, and a file cannot be renamed onto a directory. Each case +// states the real-fs outcome as the expectation. Cases are independent so +// the runner reports each one. + +require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const zlib = require('zlib'); +const vfs = require('node:vfs'); +const { test } = require('node:test'); + +// Builds a writable in-memory archive from [name, content, options] triples +// and mounts it, returning the mount point. +function mountZip(entries) { + const list = entries.map(({ 0: name, 1: content, 2: options }) => + zlib.ZipEntry.createSync(name, Buffer.from(content), options)); + const chunks = []; + for (const chunk of zlib.createZipArchiveSync(list)) chunks.push(chunk); + const provider = new vfs.ZipProvider(new zlib.ZipBuffer(Buffer.concat(chunks))); + return vfs.create(provider).mount(); +} + +test('opening an existing file with "w" truncates it even without a write', () => { + const file = path.join(mountZip([['f.txt', 'hello']]), 'f.txt'); + fs.closeSync(fs.openSync(file, 'w')); + assert.strictEqual(fs.readFileSync(file, 'utf8'), ''); +}); + +test('opening a new file with "w" creates it even without a write', () => { + const file = path.join(mountZip([['f.txt', 'hello']]), 'new.txt'); + fs.closeSync(fs.openSync(file, 'w')); + assert.strictEqual(fs.existsSync(file), true); +}); + +test('opening a new file with "a" creates it even without a write', () => { + const file = path.join(mountZip([['f.txt', 'hello']]), 'log.txt'); + fs.closeSync(fs.openSync(file, 'a')); + assert.strictEqual(fs.existsSync(file), true); +}); + +test('appending keeps the entry mode', () => { + const file = path.join(mountZip([['x.sh', 'a', { mode: 0o755 }]]), 'x.sh'); + assert.strictEqual(fs.statSync(file).mode & 0o777, 0o755); + fs.appendFileSync(file, 'b'); + assert.strictEqual(fs.statSync(file).mode & 0o777, 0o755); + assert.strictEqual(fs.readFileSync(file, 'utf8'), 'ab'); +}); + +test('an in-place write keeps the entry mode', () => { + const file = path.join(mountZip([['x.sh', 'abc', { mode: 0o755 }]]), 'x.sh'); + const fd = fs.openSync(file, 'r+'); + fs.writeSync(fd, Buffer.from('Z'), 0, 1, 0); + fs.closeSync(fd); + assert.strictEqual(fs.statSync(file).mode & 0o777, 0o755); +}); + +test('a new file gets the mode passed to open', () => { + const file = path.join(mountZip([['f.txt', 'hello']]), 'new.sh'); + const fd = fs.openSync(file, 'w', 0o700); + fs.writeSync(fd, Buffer.from('#!')); + fs.closeSync(fd); + assert.strictEqual(fs.statSync(file).mode & 0o777, 0o700); +}); + +test('fstat reports the entry mode, not the open() mode argument', () => { + const file = path.join(mountZip([['x.sh', 'a', { mode: 0o755 }]]), 'x.sh'); + const fd = fs.openSync(file, 'r'); + try { + assert.strictEqual(fs.fstatSync(fd).mode & 0o777, 0o755); + } finally { + fs.closeSync(fd); + } +}); + +test('fstat reports the entry modification time', () => { + const modified = new Date('2020-01-02T03:04:05Z'); + const file = path.join(mountZip([['f.txt', 'a', { modified }]]), 'f.txt'); + const fd = fs.openSync(file, 'r'); + try { + // ZIP timestamps have two-second resolution, so compare at that grain. + assert.strictEqual(Math.floor(fs.fstatSync(fd).mtimeMs / 2000), + Math.floor(modified.getTime() / 2000)); + } finally { + fs.closeSync(fd); + } +}); + +test('renaming a file onto an existing directory fails with EISDIR', () => { + const mount = mountZip([['dir/', ''], ['f', 'x']]); + assert.throws(() => fs.renameSync(path.join(mount, 'f'), path.join(mount, 'dir')), + { code: 'EISDIR' }); + assert.strictEqual(fs.statSync(path.join(mount, 'dir')).isDirectory(), true); + assert.strictEqual(fs.readFileSync(path.join(mount, 'f'), 'utf8'), 'x'); +}); From 72598d8573728d2a575f65420cd4cd90b3b756b9 Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 13:09:41 +0200 Subject: [PATCH 2/3] vfs: commit ZipProvider handles the way open(2) does A ZipProvider handle adds its entry to the archive when it is closed. Give that model the effects a real open(2) has up front, and keep the metadata an entry already carries: * A handle whose flags create or truncate the file starts out dirty, so closing it without a write still creates the missing entry or truncates the existing one. * The handle remembers the entry's own mode and modification time. The re-added entry keeps that mode instead of taking the `mode` argument `open()` was given (fs's default 0o666), so a 0o755 script survives an append or an in-place write; only a newly created file takes the mode from `open()`. * `fstat` reports that mode and, until the handle has changed the file, that modification time. * `rename` refuses to move a file onto an existing directory with EISDIR before touching the archive. Signed-off-by: Philipp Dunkel --- lib/internal/vfs/providers/ziparchive.js | 32 +++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/internal/vfs/providers/ziparchive.js b/lib/internal/vfs/providers/ziparchive.js index f369cdd22f7..bd6b9dd1a48 100644 --- a/lib/internal/vfs/providers/ziparchive.js +++ b/lib/internal/vfs/providers/ziparchive.js @@ -115,6 +115,8 @@ class ZipFileHandle extends VirtualFileHandle { #buffer; #size; #dirty = false; + #entryMode; + #modified; /** * @param {string} path @@ -125,8 +127,15 @@ class ZipFileHandle extends VirtualFileHandle { * @param {Buffer} initial The entry's current decompressed content, or an * empty buffer for a new/truncated file */ - constructor(path, flags, mode, source, name, initial) { + constructor(path, flags, mode, source, name, initial, entry = null, dirty = false) { super(path, flags, mode); + // An existing entry keeps its own mode and modification time across a + // rewrite; only a newly created file takes the mode open() was given. + this.#entryMode = entry === null ? this.mode : (entry.mode || 0o644); + this.#modified = entry === null ? null : entry.modified; + // Creation and truncation take effect at open time on a real file, so + // such a handle is committed on close even when nothing is written. + this.#dirty = dirty; this.#source = source; this.#name = name; this.#buffer = initial; @@ -220,8 +229,13 @@ class ZipFileHandle extends VirtualFileHandle { this.#doWriteFile(data, options); } + // Reports the entry's own mode and, until the handle has changed the file, + // its own modification time; once dirty the file is as new as its close. #doStat() { - return createFileStats(this.#size, { mode: this.mode }); + return createFileStats(this.#size, { + mode: this.#entryMode, + mtimeMs: this.#dirty || this.#modified === null ? undefined : this.#modified.getTime(), + }); } async stat(options) { return this.#doStat(); @@ -245,13 +259,13 @@ class ZipFileHandle extends VirtualFileHandle { async close() { if (this.#dirty && isWritableFlag(this.flags)) { - await this.#source.add(this.#name, this.#buffer.subarray(0, this.#size), { mode: this.mode }); + await this.#source.add(this.#name, this.#buffer.subarray(0, this.#size), { mode: this.#entryMode }); } await super.close(); } closeSync() { if (this.#dirty && isWritableFlag(this.flags)) { - this.#source.addSync(this.#name, this.#buffer.subarray(0, this.#size), { mode: this.mode }); + this.#source.addSync(this.#name, this.#buffer.subarray(0, this.#size), { mode: this.#entryMode }); } super.closeSync(); } @@ -350,7 +364,8 @@ class ZipProvider extends VirtualProvider { if (exists && !isWriteTruncate(flags)) { initial = await fileEntry.content(); } - return new ZipFileHandle(path, flags, mode, this.#source, name, initial); + return new ZipFileHandle(path, flags, mode, this.#source, name, initial, + fileEntry, !exists || isWriteTruncate(flags)); } openSync(path, flags, mode) { flags = normalizeFlags(flags); @@ -373,7 +388,8 @@ class ZipProvider extends VirtualProvider { if (exists && !isWriteTruncate(flags)) { initial = fileEntry.contentSync(); } - return new ZipFileHandle(path, flags, mode, this.#source, name, initial); + return new ZipFileHandle(path, flags, mode, this.#source, name, initial, + fileEntry, !exists || isWriteTruncate(flags)); } async stat(path, options) { @@ -531,6 +547,9 @@ class ZipProvider extends VirtualProvider { const newName = normalize(newPath); const entry = await this.#getEntry(oldName); if (entry === null) throw createENOENT('rename', oldPath); + // A file cannot take a directory's name; the archive would otherwise + // hold both under it. + if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath); const content = await entry.content(); await this.#source.add(newName, content, { mode: entry.mode || undefined, @@ -545,6 +564,7 @@ class ZipProvider extends VirtualProvider { const newName = normalize(newPath); const entry = this.#getEntrySync(oldName); if (entry === null) throw createENOENT('rename', oldPath); + if (this.#isDirectory(newName)) throw createEISDIR('rename', newPath); const content = entry.contentSync(); this.#source.addSync(newName, content, { mode: entry.mode || undefined, From a88f5b868851245a512d413624c09d1ec57a7803 Mon Sep 17 00:00:00 2001 From: Philipp Dunkel Date: Sun, 6 Sep 2026 16:32:10 +0200 Subject: [PATCH 3/3] vfs: give ZipProvider option bags a null prototype The options passed to `createFileStats()` and to the archive's `add()` and `addSync()` are plain literals, so a property added to `Object.prototype` would reach those callees as if it had been passed on purpose. Create them with a null prototype so only the fields set here are visible. Refs: https://github.com/nodejs/node/pull/65853 Signed-off-by: Philipp Dunkel --- lib/internal/vfs/providers/ziparchive.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/internal/vfs/providers/ziparchive.js b/lib/internal/vfs/providers/ziparchive.js index bd6b9dd1a48..0b4a86160e1 100644 --- a/lib/internal/vfs/providers/ziparchive.js +++ b/lib/internal/vfs/providers/ziparchive.js @@ -233,6 +233,7 @@ class ZipFileHandle extends VirtualFileHandle { // its own modification time; once dirty the file is as new as its close. #doStat() { return createFileStats(this.#size, { + __proto__: null, mode: this.#entryMode, mtimeMs: this.#dirty || this.#modified === null ? undefined : this.#modified.getTime(), }); @@ -259,13 +260,15 @@ class ZipFileHandle extends VirtualFileHandle { async close() { if (this.#dirty && isWritableFlag(this.flags)) { - await this.#source.add(this.#name, this.#buffer.subarray(0, this.#size), { mode: this.#entryMode }); + await this.#source.add(this.#name, this.#buffer.subarray(0, this.#size), + { __proto__: null, mode: this.#entryMode }); } await super.close(); } closeSync() { if (this.#dirty && isWritableFlag(this.flags)) { - this.#source.addSync(this.#name, this.#buffer.subarray(0, this.#size), { mode: this.#entryMode }); + this.#source.addSync(this.#name, this.#buffer.subarray(0, this.#size), + { __proto__: null, mode: this.#entryMode }); } super.closeSync(); }