Skip to content

Commit fa46f62

Browse files
authored
Merge pull request #1173 from streamich/copilot/fix-1172
fix: handle chmod 0 permissions in existsSync and access methods
2 parents 439d17e + c0b4b16 commit fa46f62

File tree

2 files changed

+108
-2
lines changed

2 files changed

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

src/node/volume.ts

Lines changed: 25 additions & 2 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;
@@ -896,8 +899,28 @@ export class Volume implements FsCallbackApi, FsSynchronousApi {
896899
};
897900

898901
private _access(filename: string, mode: number) {
899-
// TODO: Need to check mode?
900-
this._core.getLinkOrThrow(filename, 'access');
902+
const link = this._core.getLinkOrThrow(filename, 'access');
903+
const node = link.getNode();
904+
905+
// F_OK (0) just checks for existence, which we already confirmed above
906+
if (mode === F_OK) {
907+
return;
908+
}
909+
910+
// Check read permission
911+
if (mode & R_OK && !node.canRead()) {
912+
throw createError(ERROR_CODE.EACCES, 'access', filename);
913+
}
914+
915+
// Check write permission
916+
if (mode & W_OK && !node.canWrite()) {
917+
throw createError(ERROR_CODE.EACCES, 'access', filename);
918+
}
919+
920+
// Check execute permission
921+
if (mode & X_OK && !node.canExecute()) {
922+
throw createError(ERROR_CODE.EACCES, 'access', filename);
923+
}
901924
}
902925

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

0 commit comments

Comments
 (0)