k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-19 05:26:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(plugins): keep a plugin id from pointing outside its own directory

Browse Source 
`POST /api/admin/plugins/:id/uninstall` walks its route parameter into
pluginCodeDir() and hands the result to a recursive remove. Express decodes
`%2F` after routing, so `..%2F..%2Fuploads` matches the route and reaches the
helper as `../../uploads` — path.join then lands in the uploads tree, and
`deleteData` gets the data root the same way. Verified against the express
version in the lockfile.

pluginCodeDir and pluginDataDir now reject an id that could leave its parent.
Narrower than the manifest's ID_RE on purpose: that rule wants 3-40 lowercase
characters and belongs at install time, while this one only has to make the id
unable to steer a path, so it stays permissive about length and case. Every
other filesystem sink in the backend already has its guard — storage keys,
extraction, the wiki, backup filenames. This one did not.
This commit is contained in:
Maurice
2026-08-25 10:58:32 +02:00
committed by Maurice
parent 40957e909b
commit bc59289f73
2 changed files with 66 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/tests/unit/plugins/protocol-paths.test.ts
+42 -1
View File
@@ -5,7 +5,7 @@
import { describe, it, expect, afterEach } from 'vitest';
import { isKnownPermission, METHOD_PERMISSION, KNOWN_METHODS, HOOK_PERMISSION } from '../../../src/nest/plugins/protocol/envelope';
import path from 'node:path';
import { pluginsCodeRoot, pluginsDataRoot, pluginCodeDir, pluginDbFile, resolveChildEntry, serverCodeRoot, pluginPermissionArgs, pluginRealCodeDir, ensurePluginModuleType } from '../../../src/nest/plugins/paths';
import { pluginsCodeRoot, pluginsDataRoot, pluginCodeDir, pluginDataDir, pluginDbFile, resolveChildEntry, serverCodeRoot, pluginPermissionArgs, pluginRealCodeDir, ensurePluginModuleType } from '../../../src/nest/plugins/paths';
afterEach(() => {
delete process.env.TREK_PLUGINS_DIR;
@@ -101,3 +101,44 @@ describe('paths', () => {
}
});
});
/**
* The id these helpers get comes off a route parameter — POST
* /api/admin/plugins/:id/uninstall walks it into pluginCodeDir and hands the result
* to a recursive remove. Express decodes %2F only AFTER routing, so a traversal
* still matches the route and arrives here decoded.
*/
describe('plugin id containment', () => {
const traversals = [
'../../uploads',
'../..',
'..',
'.',
'a/b',
'a\b',
'C:\Windows',
'.hidden',
'',
];
it('refuses any id that could point outside its parent', () => {
for (const id of traversals) {
expect(() => pluginCodeDir(id), id).toThrow(/invalid plugin id/);
expect(() => pluginDataDir(id), id).toThrow(/invalid plugin id/);
}
});
it('still accepts the ids plugins actually use', () => {
for (const id of ['x', 'a1', '001', 'flight-tracker', 'Bad-Id', 'trip.todos', 'koffi_and_friends']) {
expect(path.basename(pluginCodeDir(id))).toBe(id);
expect(path.basename(pluginDataDir(id))).toBe(id);
}
});
it('keeps every accepted id inside the plugin roots', () => {
for (const id of ['x', 'flight-tracker', 'a.b', 'a-b_c']) {
expect(pluginCodeDir(id).startsWith(pluginsCodeRoot() + path.sep)).toBe(true);
expect(pluginDataDir(id).startsWith(pluginsDataRoot() + path.sep)).toBe(true);
}
});
});