Skip to content

Commit c68909d

Browse files
committed
vfs: add comprehensive tests for bug fixes and lint cleanup
Add 18 feature-focused test files covering bug fixes from both analysis rounds, and fix lint issues in stats.js and setup.js. Test files added: - test-vfs-access.js: access validation and mode enforcement - test-vfs-buffer-encoding.js: buffer encodings and buffer path args - test-vfs-copyfile-mode.js: copyFile COPYFILE_EXCL support - test-vfs-dir-handle.js: Dir double-close and read/close callbacks - test-vfs-file-url.js: file: URL handling with fileURLToPath - test-vfs-mkdir-recursive-return.js: mkdirSync recursive return value - test-vfs-no-auto-mkdir.js: writes/opens don't auto-create parents - test-vfs-open-flags.js: read-only/write-only/exclusive/numeric flags - test-vfs-readdir-recursive.js: readdir recursive with names and dirents - test-vfs-readfile-fd.js: readFileSync with virtual fd - test-vfs-rename-safety.js: renameSync preserves source on failure - test-vfs-rm-edge-cases.js: rmSync dir/link edge cases - test-vfs-stats-bigint.js: BigInt stats support - test-vfs-stream-options.js: writestream start and stream fd option - test-vfs-symlink-edge-cases.js: broken/intermediate symlinks - test-vfs-watch-directory.js: directory and recursive watch - test-vfs-watchfile.js: unwatchFile cleanup and zero stats - test-vfs-writefile-flags.js: writeFile/appendFile flag support Lint fixes: - stats.js: use BigInt from primordials, add missing JSDoc @returns - setup.js: consolidate ERR_MODULE_NOT_FOUND import
1 parent 85e4ef5 commit c68909d

26 files changed

+1183
-64
lines changed

lib/fs.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3748,7 +3748,7 @@ function mkdtemp(prefix, options, callback) {
37483748
const h = vfsState.handlers;
37493749
if (h !== null) {
37503750
try {
3751-
const result = h.mkdtempSync(prefix);
3751+
const result = h.mkdtempSync(prefix, typeof options === 'function' ? undefined : options);
37523752
if (result !== undefined) {
37533753
process.nextTick(callback, null, result);
37543754
return;
@@ -3778,7 +3778,7 @@ function mkdtemp(prefix, options, callback) {
37783778
function mkdtempSync(prefix, options) {
37793779
const h = vfsState.handlers;
37803780
if (h !== null) {
3781-
const result = h.mkdtempSync(prefix);
3781+
const result = h.mkdtempSync(prefix, options);
37823782
if (result !== undefined) return result;
37833783
}
37843784

lib/internal/vfs/dir.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ class VirtualDir {
5656
}
5757

5858
closeSync() {
59+
if (this.#closed) {
60+
throw new ERR_DIR_CLOSED();
61+
}
5962
this.#closed = true;
6063
}
6164

lib/internal/vfs/file_system.js

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ const {
2929
createENOENT,
3030
createENOTDIR,
3131
createEBADF,
32+
createEISDIR,
3233
} = require('internal/vfs/errors');
3334
const { VirtualReadStream, VirtualWriteStream } = require('internal/vfs/streams');
3435
const { VirtualDir } = require('internal/vfs/dir');
@@ -572,18 +573,25 @@ class VirtualFileSystem {
572573

573574
let stats;
574575
try {
575-
stats = this.statSync(filePath);
576+
stats = this.lstatSync(filePath);
576577
} catch (err) {
577578
if (force && err?.code === 'ENOENT') return;
578579
throw err;
579580
}
580581

582+
// Symlinks should be unlinked directly, never recursed into
583+
if (stats.isSymbolicLink()) {
584+
this.unlinkSync(filePath);
585+
return;
586+
}
587+
581588
if (stats.isDirectory()) {
582-
if (recursive) {
583-
const entries = this.readdirSync(filePath);
584-
for (let i = 0; i < entries.length; i++) {
585-
this.rmSync(joinPath(filePath, entries[i]), options);
586-
}
589+
if (!recursive) {
590+
throw createEISDIR('rm', filePath);
591+
}
592+
const entries = this.readdirSync(filePath);
593+
for (let i = 0; i < entries.length; i++) {
594+
this.rmSync(joinPath(filePath, entries[i]), options);
587595
}
588596
this.rmdirSync(filePath);
589597
} else {
@@ -1281,24 +1289,30 @@ class VirtualFileSystem {
12811289
},
12821290

12831291
async rm(filePath, options) {
1284-
// Delegate to sync implementation via the VFS instance
12851292
const recursive = options?.recursive === true;
12861293
const force = options?.force === true;
12871294

12881295
let stats;
12891296
try {
1290-
stats = await provider.stat(toProviderPath(filePath));
1297+
stats = await provider.lstat(toProviderPath(filePath));
12911298
} catch (err) {
12921299
if (force && err?.code === 'ENOENT') return;
12931300
throw err;
12941301
}
12951302

1303+
// Symlinks should be unlinked directly, never recursed into
1304+
if (stats.isSymbolicLink()) {
1305+
await provider.unlink(toProviderPath(filePath));
1306+
return;
1307+
}
1308+
12961309
if (stats.isDirectory()) {
1297-
if (recursive) {
1298-
const entries = await provider.readdir(toProviderPath(filePath));
1299-
for (let i = 0; i < entries.length; i++) {
1300-
await this.rm(joinPath(filePath, entries[i]), options);
1301-
}
1310+
if (!recursive) {
1311+
throw createEISDIR('rm', filePath);
1312+
}
1313+
const entries = await provider.readdir(toProviderPath(filePath));
1314+
for (let i = 0; i < entries.length; i++) {
1315+
await this.rm(joinPath(filePath, entries[i]), options);
13021316
}
13031317
await provider.rmdir(toProviderPath(filePath));
13041318
} else {

lib/internal/vfs/providers/memory.js

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,8 @@ class MemoryProvider extends VirtualProvider {
266266
for (let i = 0; i < segments.length; i++) {
267267
const segment = segments[i];
268268

269-
// Follow symlinks for intermediate path components
270-
if (current.isSymbolicLink() && followSymlinks) {
269+
// Always follow symlinks for intermediate path components
270+
if (current.isSymbolicLink()) {
271271
if (depth >= kMaxSymlinkDepth) {
272272
return { entry: null, resolvedPath: null, eloop: true };
273273
}
@@ -380,6 +380,16 @@ class MemoryProvider extends VirtualProvider {
380380
current = entry;
381381
}
382382

383+
// Follow symlinks on the final parent entry
384+
if (current.isSymbolicLink()) {
385+
const targetPath = this.#resolveSymlinkTarget(parentPath, current.target);
386+
const result = this.#lookupEntry(targetPath, true, 0);
387+
if (!result.entry) {
388+
throw createENOENT(syscall, path);
389+
}
390+
current = result.entry;
391+
}
392+
383393
if (!current.isDirectory()) {
384394
throw createENOTDIR(syscall, path);
385395
}
@@ -396,12 +406,13 @@ class MemoryProvider extends VirtualProvider {
396406
* @param {number} [size] Override size for files
397407
* @returns {Stats}
398408
*/
399-
#createStats(entry, size) {
409+
#createStats(entry, size, bigint) {
400410
const options = {
401411
mode: entry.mode,
402412
mtimeMs: entry.mtime,
403413
ctimeMs: entry.ctime,
404414
birthtimeMs: entry.birthtime,
415+
bigint,
405416
};
406417

407418
if (entry.isFile()) {
@@ -510,7 +521,7 @@ class MemoryProvider extends VirtualProvider {
510521

511522
statSync(path, options) {
512523
const entry = this.#getEntry(path, 'stat', true);
513-
return this.#createStats(entry);
524+
return this.#createStats(entry, undefined, options?.bigint);
514525
}
515526

516527
async stat(path, options) {
@@ -519,7 +530,7 @@ class MemoryProvider extends VirtualProvider {
519530

520531
lstatSync(path, options) {
521532
const entry = this.#getEntry(path, 'lstat', false);
522-
return this.#createStats(entry);
533+
return this.#createStats(entry, undefined, options?.bigint);
523534
}
524535

525536
async lstat(path, options) {
@@ -632,6 +643,7 @@ class MemoryProvider extends VirtualProvider {
632643
const segments = this.#splitPath(normalized);
633644
let current = this[kRoot];
634645
let currentPath = '/';
646+
let firstCreated;
635647

636648
for (const segment of segments) {
637649
currentPath = pathPosix.join(currentPath, segment);
@@ -640,20 +652,23 @@ class MemoryProvider extends VirtualProvider {
640652
entry = new MemoryEntry(TYPE_DIR, { mode: options?.mode });
641653
entry.children = new SafeMap();
642654
current.children.set(segment, entry);
655+
if (firstCreated === undefined) {
656+
firstCreated = currentPath;
657+
}
643658
} else if (!entry.isDirectory()) {
644659
throw createENOTDIR('mkdir', path);
645660
}
646661
current = entry;
647662
}
648-
} else {
649-
const parent = this.#ensureParent(normalized, false, 'mkdir');
650-
const name = pathPosix.basename(normalized);
651-
const entry = new MemoryEntry(TYPE_DIR, { mode: options?.mode });
652-
entry.children = new SafeMap();
653-
parent.children.set(name, entry);
663+
return firstCreated;
654664
}
655665

656-
return recursive ? normalized : undefined;
666+
const parent = this.#ensureParent(normalized, false, 'mkdir');
667+
const name = pathPosix.basename(normalized);
668+
const entry = new MemoryEntry(TYPE_DIR, { mode: options?.mode });
669+
entry.children = new SafeMap();
670+
parent.children.set(name, entry);
671+
return undefined;
657672
}
658673

659674
async mkdir(path, options) {
@@ -805,7 +820,7 @@ class MemoryProvider extends VirtualProvider {
805820
throw createEEXIST('symlink', path);
806821
}
807822

808-
const parent = this.#ensureParent(normalized, true, 'symlink');
823+
const parent = this.#ensureParent(normalized, false, 'symlink');
809824
const name = pathPosix.basename(normalized);
810825
const entry = new MemoryEntry(TYPE_SYMLINK);
811826
entry.target = target;

lib/internal/vfs/providers/real.js

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class RealFSProvider extends VirtualProvider {
192192
* @returns {string} The real filesystem path
193193
* @private
194194
*/
195-
#resolvePath(vfsPath) {
195+
#resolvePath(vfsPath, followSymlinks = true) {
196196
// Normalize the VFS path (remove leading slash, handle . and ..)
197197
let normalized = vfsPath;
198198
if (normalized.startsWith('/')) {
@@ -211,9 +211,51 @@ class RealFSProvider extends VirtualProvider {
211211
throw createENOENT('open', vfsPath);
212212
}
213213

214+
// Resolve symlinks to prevent escape via symbolic links
215+
if (followSymlinks) {
216+
try {
217+
const resolved = fs.realpathSync(realPath);
218+
if (resolved !== this.#rootPath &&
219+
!StringPrototypeStartsWith(resolved, rootWithSep)) {
220+
throw createENOENT('open', vfsPath);
221+
}
222+
return resolved;
223+
} catch (err) {
224+
if (err?.code !== 'ENOENT') throw err;
225+
// Path doesn't exist yet - verify deepest existing ancestor
226+
this.#verifyAncestorInRoot(realPath, rootWithSep, vfsPath);
227+
return realPath;
228+
}
229+
}
230+
231+
// For lstat/readlink (no final symlink follow), check parent only
232+
this.#verifyAncestorInRoot(realPath, rootWithSep, vfsPath);
214233
return realPath;
215234
}
216235

236+
/**
237+
* Verifies that the deepest existing ancestor of a path is within rootPath.
238+
* @param {string} realPath The real filesystem path
239+
* @param {string} rootWithSep The rootPath with trailing separator
240+
* @param {string} vfsPath The original VFS path (for error messages)
241+
*/
242+
#verifyAncestorInRoot(realPath, rootWithSep, vfsPath) {
243+
let current = path.dirname(realPath);
244+
while (current.length >= this.#rootPath.length) {
245+
try {
246+
const resolved = fs.realpathSync(current);
247+
if (resolved !== this.#rootPath &&
248+
!StringPrototypeStartsWith(resolved, rootWithSep)) {
249+
throw createENOENT('open', vfsPath);
250+
}
251+
return;
252+
} catch (err) {
253+
if (err?.code !== 'ENOENT') throw err;
254+
current = path.dirname(current);
255+
}
256+
}
257+
}
258+
217259
openSync(vfsPath, flags, mode) {
218260
const realPath = this.#resolvePath(vfsPath);
219261
const fd = fs.openSync(realPath, flags, mode);
@@ -241,12 +283,12 @@ class RealFSProvider extends VirtualProvider {
241283
}
242284

243285
lstatSync(vfsPath, options) {
244-
const realPath = this.#resolvePath(vfsPath);
286+
const realPath = this.#resolvePath(vfsPath, false);
245287
return fs.lstatSync(realPath, options);
246288
}
247289

248290
async lstat(vfsPath, options) {
249-
const realPath = this.#resolvePath(vfsPath);
291+
const realPath = this.#resolvePath(vfsPath, false);
250292
return fs.promises.lstat(realPath, options);
251293
}
252294

@@ -303,12 +345,12 @@ class RealFSProvider extends VirtualProvider {
303345
}
304346

305347
readlinkSync(vfsPath, options) {
306-
const realPath = this.#resolvePath(vfsPath);
348+
const realPath = this.#resolvePath(vfsPath, false);
307349
return fs.readlinkSync(realPath, options);
308350
}
309351

310352
async readlink(vfsPath, options) {
311-
const realPath = this.#resolvePath(vfsPath);
353+
const realPath = this.#resolvePath(vfsPath, false);
312354
return fs.promises.readlink(realPath, options);
313355
}
314356

0 commit comments

Comments
 (0)