Skip to content

Commit bc19ec8

Browse files
committed
vfs: fix bugs from analysis review
P0 fixes: - setup.js: use fileURLToPath() instead of path.pathname for file: URLs - file_handle.js: enforce read/write permissions and exclusive flags - memory.js: prevent auto-creation of parent dirs in openSync - memory.js: validate destination before removing source in renameSync P1 fixes: - memory.js: convert numeric O_* flags to string equivalents - dir.js: add callback support to read() and close() - watcher.js: fix unwatchFile leak by clearing internal listeners set - provider.js: pass options.flag through writeFile/appendFile - memory.js: implement recursive readdir - watcher.js: poll children for directory watch, rescan for new files - streams.js: support fd and start options in read/write streams P2 fixes: - provider.js: check R_OK/W_OK/X_OK permission bits in access() - provider.js: check COPYFILE_EXCL in copyFile() - stats.js: add createZeroStats() without S_IFREG for watchFile - errors.js: add createEACCES error factory
1 parent d96bc30 commit bc19ec8

10 files changed

Lines changed: 510 additions & 104 deletions

File tree

lib/internal/vfs/dir.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,29 @@ class VirtualDir {
4242
return this.#entries[this.#index++];
4343
}
4444

45-
async read() {
45+
async read(callback) {
46+
if (typeof callback === 'function') {
47+
try {
48+
const result = this.readSync();
49+
process.nextTick(callback, null, result);
50+
} catch (err) {
51+
process.nextTick(callback, err);
52+
}
53+
return;
54+
}
4655
return this.readSync();
4756
}
4857

4958
closeSync() {
5059
this.#closed = true;
5160
}
5261

53-
async close() {
62+
async close(callback) {
63+
if (typeof callback === 'function') {
64+
this.closeSync();
65+
process.nextTick(callback, null);
66+
return;
67+
}
5468
this.closeSync();
5569
}
5670

lib/internal/vfs/errors.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const {
1818
UV_EROFS,
1919
UV_EINVAL,
2020
UV_ELOOP,
21+
UV_EACCES,
2122
} = internalBinding('uv');
2223

2324
/**
@@ -162,6 +163,22 @@ function createELOOP(syscall, path) {
162163
return err;
163164
}
164165

166+
/**
167+
* Creates an EACCES error for permission denied.
168+
* @param {string} syscall The system call name
169+
* @param {string} path The path
170+
* @returns {Error}
171+
*/
172+
function createEACCES(syscall, path) {
173+
const err = new UVException({
174+
errno: UV_EACCES,
175+
syscall,
176+
path,
177+
});
178+
ErrorCaptureStackTrace(err, createEACCES);
179+
return err;
180+
}
181+
165182
module.exports = {
166183
createENOENT,
167184
createENOTDIR,
@@ -172,4 +189,5 @@ module.exports = {
172189
createEROFS,
173190
createEINVAL,
174191
createELOOP,
192+
createEACCES,
175193
};

lib/internal/vfs/file_handle.js

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,18 +277,48 @@ class MemoryFileHandle extends VirtualFileHandle {
277277
this.#getStats = getStats;
278278

279279
// Handle different open modes
280-
if (flags === 'w' || flags === 'w+') {
280+
if (flags === 'w' || flags === 'w+' ||
281+
flags === 'wx' || flags === 'wx+') {
281282
// Write mode: truncate
282283
this.#content = Buffer.alloc(0);
283284
if (entry) {
284285
entry.content = this.#content;
285286
}
286-
} else if (flags === 'a' || flags === 'a+') {
287+
} else if (flags === 'a' || flags === 'a+' ||
288+
flags === 'ax' || flags === 'ax+') {
287289
// Append mode: position at end
288290
this.position = this.#content.length;
289291
}
290292
}
291293

294+
/**
295+
* Throws EBADF if the handle was not opened for writing.
296+
*/
297+
#checkWritable() {
298+
if (this.flags === 'r') {
299+
throw createEBADF('write');
300+
}
301+
}
302+
303+
/**
304+
* Throws EBADF if the handle was not opened for reading.
305+
*/
306+
#checkReadable() {
307+
const f = this.flags;
308+
if (f === 'w' || f === 'a' || f === 'wx' || f === 'ax') {
309+
throw createEBADF('read');
310+
}
311+
}
312+
313+
/**
314+
* Returns true if this handle was opened in append mode.
315+
* @returns {boolean}
316+
*/
317+
#isAppend() {
318+
const f = this.flags;
319+
return f === 'a' || f === 'a+' || f === 'ax' || f === 'ax+';
320+
}
321+
292322
/**
293323
* Gets the current content synchronously.
294324
* For dynamic content providers, this gets fresh content from the entry.
@@ -325,6 +355,7 @@ class MemoryFileHandle extends VirtualFileHandle {
325355
*/
326356
readSync(buffer, offset, length, position) {
327357
this.#checkClosed();
358+
this.#checkReadable();
328359

329360
// Get content (resolves dynamic content providers)
330361
const content = this.content;
@@ -369,8 +400,12 @@ class MemoryFileHandle extends VirtualFileHandle {
369400
*/
370401
writeSync(buffer, offset, length, position) {
371402
this.#checkClosed();
403+
this.#checkWritable();
372404

373-
const writePos = position !== null && position !== undefined ? position : this.position;
405+
// In append mode, always write at the end
406+
const writePos = this.#isAppend() ?
407+
this.#content.length :
408+
(position !== null && position !== undefined ? position : this.position);
374409
const data = buffer.subarray(offset, offset + length);
375410

376411
// Expand content if needed
@@ -417,6 +452,7 @@ class MemoryFileHandle extends VirtualFileHandle {
417452
*/
418453
readFileSync(options) {
419454
this.#checkClosed();
455+
this.#checkReadable();
420456

421457
// Get content (resolves dynamic content providers)
422458
const content = this.content;
@@ -434,6 +470,7 @@ class MemoryFileHandle extends VirtualFileHandle {
434470
*/
435471
async readFile(options) {
436472
this.#checkClosed();
473+
this.#checkReadable();
437474

438475
// Get content asynchronously (supports async content providers)
439476
const content = await this.getContentAsync();
@@ -452,6 +489,7 @@ class MemoryFileHandle extends VirtualFileHandle {
452489
*/
453490
writeFileSync(data, options) {
454491
this.#checkClosed();
492+
this.#checkWritable();
455493

456494
const buffer = typeof data === 'string' ? Buffer.from(data, options?.encoding) : data;
457495

@@ -512,6 +550,7 @@ class MemoryFileHandle extends VirtualFileHandle {
512550
*/
513551
truncateSync(len = 0) {
514552
this.#checkClosed();
553+
this.#checkWritable();
515554

516555
if (len < this.#content.length) {
517556
this.#content = this.#content.subarray(0, len);

lib/internal/vfs/provider.js

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,19 @@ const {
66

77
const {
88
createEROFS,
9+
createEEXIST,
10+
createEACCES,
911
} = require('internal/vfs/errors');
1012

13+
const {
14+
fs: {
15+
R_OK,
16+
W_OK,
17+
X_OK,
18+
COPYFILE_EXCL,
19+
},
20+
} = internalBinding('constants');
21+
1122
/**
1223
* Base class for VFS providers.
1324
* Providers implement the essential primitives that the VFS delegates to.
@@ -267,7 +278,8 @@ class VirtualProvider {
267278
if (this.readonly) {
268279
throw createEROFS('open', path);
269280
}
270-
const handle = await this.open(path, 'w', options?.mode);
281+
const flag = options?.flag ?? 'w';
282+
const handle = await this.open(path, flag, options?.mode);
271283
try {
272284
await handle.writeFile(data, options);
273285
} finally {
@@ -285,7 +297,8 @@ class VirtualProvider {
285297
if (this.readonly) {
286298
throw createEROFS('open', path);
287299
}
288-
const handle = this.openSync(path, 'w', options?.mode);
300+
const flag = options?.flag ?? 'w';
301+
const handle = this.openSync(path, flag, options?.mode);
289302
try {
290303
handle.writeFileSync(data, options);
291304
} finally {
@@ -304,7 +317,8 @@ class VirtualProvider {
304317
if (this.readonly) {
305318
throw createEROFS('open', path);
306319
}
307-
const handle = await this.open(path, 'a', options?.mode);
320+
const flag = options?.flag ?? 'a';
321+
const handle = await this.open(path, flag, options?.mode);
308322
try {
309323
await handle.writeFile(data, options);
310324
} finally {
@@ -322,7 +336,8 @@ class VirtualProvider {
322336
if (this.readonly) {
323337
throw createEROFS('open', path);
324338
}
325-
const handle = this.openSync(path, 'a', options?.mode);
339+
const flag = options?.flag ?? 'a';
340+
const handle = this.openSync(path, flag, options?.mode);
326341
try {
327342
handle.writeFileSync(data, options);
328343
} finally {
@@ -369,6 +384,11 @@ class VirtualProvider {
369384
if (this.readonly) {
370385
throw createEROFS('copyfile', dest);
371386
}
387+
if ((mode & COPYFILE_EXCL) !== 0) {
388+
if (await this.exists(dest)) {
389+
throw createEEXIST('copyfile', dest);
390+
}
391+
}
372392
const content = await this.readFile(src);
373393
await this.writeFile(dest, content);
374394
}
@@ -383,6 +403,11 @@ class VirtualProvider {
383403
if (this.readonly) {
384404
throw createEROFS('copyfile', dest);
385405
}
406+
if ((mode & COPYFILE_EXCL) !== 0) {
407+
if (this.existsSync(dest)) {
408+
throw createEEXIST('copyfile', dest);
409+
}
410+
}
386411
const content = this.readFileSync(src);
387412
this.writeFileSync(dest, content);
388413
}
@@ -420,8 +445,8 @@ class VirtualProvider {
420445
* @returns {Promise<void>}
421446
*/
422447
async access(path, mode) {
423-
// Default: just check if the path exists
424-
await this.stat(path);
448+
const stats = await this.stat(path);
449+
this.#checkAccessMode(path, stats, mode);
425450
}
426451

427452
/**
@@ -430,8 +455,30 @@ class VirtualProvider {
430455
* @param {number} [mode] Access mode
431456
*/
432457
accessSync(path, mode) {
433-
// Default: just check if the path exists
434-
this.statSync(path);
458+
const stats = this.statSync(path);
459+
this.#checkAccessMode(path, stats, mode);
460+
}
461+
462+
/**
463+
* Checks access mode bits against file stats.
464+
* @param {string} path The path (for error messages)
465+
* @param {Stats} stats The file stats
466+
* @param {number} mode The requested access mode
467+
*/
468+
#checkAccessMode(path, stats, mode) {
469+
if (mode == null || mode === 0) return; // F_OK = 0, existence-only check
470+
471+
const fileMode = stats.mode & 0o777; // Permission bits
472+
// Check owner permissions (simplified: treat VFS user as owner)
473+
if ((mode & R_OK) !== 0 && (fileMode & 0o400) === 0) {
474+
throw createEACCES('access', path);
475+
}
476+
if ((mode & W_OK) !== 0 && (fileMode & 0o200) === 0) {
477+
throw createEACCES('access', path);
478+
}
479+
if ((mode & X_OK) !== 0 && (fileMode & 0o100) === 0) {
480+
throw createEACCES('access', path);
481+
}
435482
}
436483

437484
// === HARD LINK OPERATIONS (optional) ===

0 commit comments

Comments
 (0)