Skip to content

Commit 3452bcf

Browse files
Copilotstreamich
andcommitted
fix: handle chmod 0 permissions in existsSync and access methods
Co-authored-by: streamich <[email protected]>
1 parent 7cf12b9 commit 3452bcf

File tree

2 files changed

+114
-3
lines changed

2 files changed

+114
-3
lines changed
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { create } from '../../../__tests__/util';
2+
3+
describe('chmod 0 permission issue reproduction', () => {
4+
// ❌ FAILURE - should return false but returns true
5+
test('chmod on file and existsSync', () => {
6+
// Arrange
7+
const vol = create({ '/path/to/file.txt': 'some text' });
8+
vol.chmodSync('/path/to/file.txt', 0o0000);
9+
10+
// Act
11+
const exists = vol.existsSync('/path/to/file.txt');
12+
13+
// Assert
14+
expect(exists).toBe(false);
15+
});
16+
17+
// ✅ SUCCESS - this should already work
18+
test('chmod on directory and existsSync', () => {
19+
// Arrange
20+
const vol = create({ '/path/to/file.txt': 'some text' });
21+
vol.chmodSync('/path/to/', 0o0000);
22+
23+
// Act
24+
const exists = vol.existsSync('/path/to/file.txt');
25+
26+
// Assert
27+
expect(exists).toBe(false);
28+
});
29+
30+
// ❌ FAILURE - should throw but doesn't throw
31+
test.each([4 /* R_OK */, 2 /* W_OK */])('chmod on file and access mode %d', mode => {
32+
// Arrange
33+
const vol = create({ '/path/to/file.txt': 'some text' });
34+
vol.chmodSync('/path/to/file.txt', 0o0000);
35+
36+
// Act & Assert
37+
expect(() => {
38+
vol.accessSync('/path/to/file.txt', mode);
39+
}).toThrow();
40+
});
41+
42+
// ✅ SUCCESS - this should already work
43+
test.each([4 /* R_OK */, 2 /* W_OK */])('chmod on directory and access mode %d', mode => {
44+
// Arrange
45+
const vol = create({ '/path/to/file.txt': 'some text' });
46+
vol.chmodSync('/path/to/', 0o0000);
47+
48+
// Act & Assert
49+
expect(() => {
50+
vol.accessSync('/path/to/file.txt', mode);
51+
}).toThrow();
52+
});
53+
54+
// F_OK should NOT throw - it just checks existence
55+
test('chmod on file and access F_OK should not throw', () => {
56+
const vol = create({ '/path/to/file.txt': 'some text' });
57+
vol.chmodSync('/path/to/file.txt', 0o0000);
58+
59+
// F_OK should not throw - file exists
60+
expect(() => {
61+
vol.accessSync('/path/to/file.txt', 0 /* F_OK */);
62+
}).not.toThrow();
63+
});
64+
65+
// Test the exact scenario from the issue using promises API and fs constants
66+
test('chmod on file and promises access with fs.constants', async () => {
67+
const vol = create({ '/path/to/file.txt': 'some text' });
68+
69+
await vol.promises.chmod('/path/to/file.txt', 0o0000);
70+
71+
// Should throw for R_OK and W_OK
72+
await expect(vol.promises.access('/path/to/file.txt', 4 /* R_OK */)).rejects.toThrow();
73+
await expect(vol.promises.access('/path/to/file.txt', 2 /* W_OK */)).rejects.toThrow();
74+
75+
// F_OK should NOT throw - it just checks existence, which it does exist
76+
await expect(vol.promises.access('/path/to/file.txt', 0 /* F_OK */)).resolves.toBeUndefined();
77+
});
78+
});

src/node/volume.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ const {
7474
O_DIRECTORY,
7575
O_SYMLINK,
7676
F_OK,
77+
R_OK,
78+
W_OK,
79+
X_OK,
7780
COPYFILE_EXCL,
7881
COPYFILE_FICLONE_FORCE,
7982
} = constants;
@@ -872,7 +875,17 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
872875
};
873876

874877
private _exists(filename: string): boolean {
875-
return !!this._stat(filename);
878+
try {
879+
const link = this._core.getResolvedLinkOrThrow(filename, 'stat');
880+
const node = link.getNode();
881+
882+
// For existsSync, Node.js returns false if the file exists but has no read permissions
883+
// This matches the behavior described in the existing test comment
884+
return node.canRead();
885+
} catch (err) {
886+
// If we can't find the file or access the path, it doesn't exist
887+
return false;
888+
}
876889
}
877890

878891
public existsSync = (path: PathLike): boolean => {
@@ -896,8 +909,28 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
896909
};
897910

898911
private _access(filename: string, mode: number) {
899-
// TODO: Need to check mode?
900-
this._core.getLinkOrThrow(filename, 'access');
912+
const link = this._core.getLinkOrThrow(filename, 'access');
913+
const node = link.getNode();
914+
915+
// F_OK (0) just checks for existence, which we already confirmed above
916+
if (mode === F_OK) {
917+
return;
918+
}
919+
920+
// Check read permission
921+
if (mode & R_OK && !node.canRead()) {
922+
throw createError(ERROR_CODE.EACCES, 'access', filename);
923+
}
924+
925+
// Check write permission
926+
if (mode & W_OK && !node.canWrite()) {
927+
throw createError(ERROR_CODE.EACCES, 'access', filename);
928+
}
929+
930+
// Check execute permission
931+
if (mode & X_OK && !node.canExecute()) {
932+
throw createError(ERROR_CODE.EACCES, 'access', filename);
933+
}
901934
}
902935

903936
public accessSync = (path: PathLike, mode: number = F_OK) => {

0 commit comments

Comments
 (0)