Skip to content

Commit 1601fc0

Browse files
zaldihzaldih
committed
feat(files.service): improve isDangerous algorithm
1 parent 3a56179 commit 1601fc0

2 files changed

Lines changed: 162 additions & 40 deletions

File tree

‎__tests__/files.service.test.ts‎

Lines changed: 104 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -142,39 +142,112 @@ describe('File Service', () => {
142142
});
143143
});
144144

145-
describe('#isDangerous', () => {
146-
it('should return false for paths that are not considered dangerous', () => {
147-
expect(
148-
fileService.isDangerous('/home/apps/myapp/node_modules'),
149-
).toBeFalsy();
150-
expect(fileService.isDangerous('node_modules')).toBeFalsy();
151-
expect(
152-
fileService.isDangerous('/home/user/projects/a/node_modules'),
153-
).toBeFalsy();
154-
expect(
155-
fileService.isDangerous('/Applications/NotAnApp/node_modules'),
156-
).toBeFalsy();
157-
expect(
158-
fileService.isDangerous('C:\\Users\\User\\Documents\\node_modules'),
159-
).toBeFalsy();
145+
describe('isDangerous', () => {
146+
const originalEnv = { ...process.env };
147+
const originalCwd = process.cwd();
148+
149+
const mockCwd = (cwd: string) => {
150+
jest.spyOn(process, 'cwd').mockReturnValue(cwd);
151+
};
152+
153+
afterAll(() => {
154+
process.env = originalEnv;
160155
});
161156

162-
it('should return true for paths that are considered dangerous', () => {
163-
expect(
164-
fileService.isDangerous('/home/.config/myapp/node_modules'),
165-
).toBeTruthy();
166-
expect(fileService.isDangerous('.apps/node_modules')).toBeTruthy();
167-
expect(
168-
fileService.isDangerous('.apps/.sample/node_modules'),
169-
).toBeTruthy();
170-
expect(
171-
fileService.isDangerous('/Applications/MyApp.app/node_modules'),
172-
).toBeTruthy();
173-
expect(
174-
fileService.isDangerous(
175-
'C:\\Users\\User\\AppData\\Local\\node_modules',
176-
),
177-
).toBeTruthy();
157+
describe('POSIX paths', () => {
158+
beforeAll(() => {
159+
process.env.HOME = '/home/user';
160+
delete process.env.USERPROFILE;
161+
});
162+
163+
test('safe relative path', () => {
164+
mockCwd('/safe/path');
165+
expect(fileService.isDangerous('../project/node_modules')).toBe(false);
166+
});
167+
168+
test('hidden relative path', () => {
169+
mockCwd('/home/user/projects');
170+
expect(fileService.isDangerous('../.config/secret')).toBe(true);
171+
});
172+
173+
test('node_modules in ~/.cache', () => {
174+
expect(
175+
fileService.isDangerous('/home/user/.cache/project/node_modules'),
176+
).toBe(false);
177+
});
178+
179+
test('parent relative path (..)', () => {
180+
mockCwd('/home/user');
181+
expect(fileService.isDangerous('..')).toBe(false);
182+
});
183+
184+
test('current relative path (.)', () => {
185+
mockCwd('/home/user');
186+
expect(fileService.isDangerous('.')).toBe(false);
187+
});
188+
});
189+
190+
describe('Windows paths', () => {
191+
beforeAll(() => {
192+
process.env.USERPROFILE = 'C:\\Users\\user';
193+
process.env.HOME = '';
194+
});
195+
196+
test('safe relative path', () => {
197+
mockCwd('D:\\safe\\path');
198+
expect(fileService.isDangerous('..\\project\\node_modules')).toBe(
199+
false,
200+
);
201+
});
202+
203+
test('AppData relative path', () => {
204+
mockCwd('C:\\Users\\user\\Documents');
205+
expect(fileService.isDangerous('..\\AppData\\Roaming\\app')).toBe(true);
206+
});
207+
208+
test('Program Files (x86)', () => {
209+
expect(
210+
fileService.isDangerous('C:\\Program Files (x86)\\app\\node_modules'),
211+
).toBe(true);
212+
});
213+
214+
test('network paths', () => {
215+
expect(
216+
fileService.isDangerous('\\\\network\\share\\.hidden\\node_modules'),
217+
).toBe(true);
218+
});
219+
});
220+
221+
describe('Edge cases', () => {
222+
test('no home directory', () => {
223+
delete process.env.HOME;
224+
delete process.env.USERPROFILE;
225+
expect(fileService.isDangerous('/some/path')).toBe(false);
226+
});
227+
228+
test('whitelisted hidden segments', () => {
229+
expect(
230+
fileService.isDangerous('/tmp/.cache/project/node_modules'),
231+
).toBe(false);
232+
expect(fileService.isDangerous('/tmp/.npm/project/node_modules')).toBe(
233+
false,
234+
);
235+
});
236+
237+
test('macOS application bundle', () => {
238+
expect(
239+
fileService.isDangerous(
240+
'/Applications/MyApp.app/Contents/node_modules',
241+
),
242+
).toBe(true);
243+
});
244+
245+
test('Windows-style path on POSIX', () => {
246+
process.env.HOME = '/home/user';
247+
expect(fileService.isDangerous('/home/user/AppData/Local/.cache')).toBe(
248+
false,
249+
);
250+
});
178251
});
179252
});
180253

‎src/services/files/files.service.ts‎

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import fs, { accessSync, readFileSync, Stats, statSync } from 'fs';
2+
import path from 'path';
23
import {
34
IFileService,
45
IFileStat,
@@ -85,16 +86,64 @@ export abstract class FileService implements IFileService {
8586
* application-specific data or configurations that should not be tampered with. Deleting node_modules
8687
* from these locations could potentially disrupt the normal operation of these applications.
8788
*/
88-
isDangerous(path: string): boolean {
89-
const hiddenFilePattern = /(^|\/)\.[^/.]/g;
90-
const macAppsPattern = /(^|\/)Applications\/[^/]+\.app\//g;
91-
const windowsAppDataPattern = /(^|\\)AppData\\/g;
92-
93-
return (
94-
hiddenFilePattern.test(path) ||
95-
macAppsPattern.test(path) ||
96-
windowsAppDataPattern.test(path)
89+
isDangerous(originalPath: string): boolean {
90+
const absolutePath = path.resolve(originalPath);
91+
const normalizedPath = absolutePath.replace(/\\/g, '/').toLowerCase();
92+
93+
const home = process.env.HOME || process.env.USERPROFILE || '';
94+
let isInHome = false;
95+
96+
if (home) {
97+
const normalizedHome = path
98+
.resolve(home)
99+
.replace(/\\/g, '/')
100+
.toLowerCase();
101+
isInHome = normalizedPath.startsWith(normalizedHome);
102+
}
103+
104+
if (isInHome) {
105+
if (/\/\.config(\/|$)/.test(normalizedPath)) {
106+
return true;
107+
}
108+
if (/\/\.local\/share(\/|$)/.test(normalizedPath)) {
109+
return true;
110+
}
111+
if (/\/\.(cache|npm|pnpm)(\/|$)/.test(normalizedPath)) {
112+
return false;
113+
}
114+
}
115+
116+
if (/\/applications\/[^/]+\.app\//.test(normalizedPath)) {
117+
return true;
118+
}
119+
120+
if (normalizedPath.includes('/appdata/roaming')) {
121+
return true;
122+
}
123+
if (normalizedPath.includes('/appdata/local')) {
124+
if (/\/\.(cache|npm|pnpm)(\/|$)/.test(normalizedPath)) {
125+
return false;
126+
}
127+
return true;
128+
}
129+
if (/program files( \(x86\))?\//.test(normalizedPath)) {
130+
return true;
131+
}
132+
133+
const segments = normalizedPath.split('/');
134+
const hasUnsafeHiddenFolder = segments.some(
135+
(segment) =>
136+
segment.startsWith('.') &&
137+
segment !== '.' &&
138+
segment !== '..' &&
139+
!['.cache', '.npm', '.pnpm'].includes(segment),
97140
);
141+
142+
if (hasUnsafeHiddenFolder) {
143+
return true;
144+
}
145+
146+
return false;
98147
}
99148

100149
async getRecentModificationInDir(path: string): Promise<number> {

0 commit comments

Comments
 (0)