diff --git a/src/node/handler/PadMessageHandler.ts b/src/node/handler/PadMessageHandler.ts index d0f34600ca0..006831f768d 100644 --- a/src/node/handler/PadMessageHandler.ts +++ b/src/node/handler/PadMessageHandler.ts @@ -109,6 +109,34 @@ function getActivePadCountFromSessionInfos() { } exports.getActivePadCountFromSessionInfos = getActivePadCountFromSessionInfos; +/** + * Build a sanitized copy of the plugins registry suitable for sending to the + * client as part of clientVars. The shape is preserved but each plugin's + * `package` field is reduced to `{name, version}` so internal paths (realPath, + * path, location) are not leaked to the browser. + * + * CRITICAL: this function MUST NOT mutate the shared server-side registry. + * Other components — notably `src/node/utils/Minify.ts` — read + * `plugins.plugins[x].package.realPath` on every static asset request to + * resolve `/static/plugins/ep_/...` URLs to disk. Mutating the shared object + * in place would clobber `realPath` and cause every such request to 500 with + * `ERR_INVALID_ARG_TYPE: The "path" argument must be of type string`. + */ +const sanitizePluginsForWire = ( + pluginsRegistry: MapArrayType, +): MapArrayType => { + const out: MapArrayType = {}; + for (const [name, plugin] of Object.entries(pluginsRegistry)) { + const p: any = plugin.package; + out[name] = { + ...plugin, + package: {name: p.name, version: p.version}, + }; + } + return out; +}; +exports.sanitizePluginsForWire = sanitizePluginsForWire; + stats.gauge('totalUsers', () => getTotalActiveUsers()); stats.gauge('activePads', () => { return getActivePadCountFromSessionInfos(); @@ -1068,11 +1096,7 @@ const handleClientReady = async (socket:any, message: ClientReadyMessage) => { throw new Error('corrupt pad'); } - let pluginsSanitized: any = plugins.plugins - Object.keys(plugins.plugins).forEach(function(element) { - const p: any = plugins.plugins[element].package - pluginsSanitized[element].package = {name: p.name, version: p.version}; - }); + const pluginsSanitized = sanitizePluginsForWire(plugins.plugins); // Warning: never ever send sessionInfo.padId to the client. If the client is read only you // would open a security hole 1 swedish mile wide... const canEditPadSettings = settings.enablePadWideSettings && diff --git a/src/tests/backend/specs/sanitizePluginsForWire.ts b/src/tests/backend/specs/sanitizePluginsForWire.ts new file mode 100644 index 00000000000..052a35be409 --- /dev/null +++ b/src/tests/backend/specs/sanitizePluginsForWire.ts @@ -0,0 +1,65 @@ +'use strict'; + +import {strict as assert} from 'assert'; +const {sanitizePluginsForWire} = require('../../../node/handler/PadMessageHandler'); + +describe(__filename, function () { + const makeRegistry = () => ({ + ep_example: { + parts: [{name: 'ep_example', plugin: 'ep_example'}], + package: { + name: 'ep_example', + version: '1.2.3', + realPath: '/real/path/to/ep_example', + path: '/node_modules/ep_example', + location: '/real/path/to/ep_example', + }, + }, + ep_other: { + parts: [{name: 'ep_other', plugin: 'ep_other'}], + package: { + name: 'ep_other', + version: '0.1.0', + realPath: '/real/path/to/ep_other', + path: '/node_modules/ep_other', + location: '/real/path/to/ep_other', + }, + }, + }); + + it('returns a sanitized registry with only name and version in package', function () { + const registry = makeRegistry(); + const sanitized = sanitizePluginsForWire(registry); + assert.deepEqual(Object.keys(sanitized).sort(), ['ep_example', 'ep_other']); + assert.deepEqual(sanitized.ep_example.package, {name: 'ep_example', version: '1.2.3'}); + assert.deepEqual(sanitized.ep_other.package, {name: 'ep_other', version: '0.1.0'}); + }); + + it('does not mutate the input registry (issue: realPath clobbered on pad join)', function () { + const registry = makeRegistry(); + sanitizePluginsForWire(registry); + // The original objects MUST still carry realPath and the other internal + // path fields — Minify.ts relies on them for every /static/plugins/... + // asset request. Before the fix, the sanitization mutated these in place + // and caused every such request to 500 after the first pad connection. + assert.equal(registry.ep_example.package.realPath, '/real/path/to/ep_example'); + assert.equal(registry.ep_example.package.path, '/node_modules/ep_example'); + assert.equal(registry.ep_other.package.realPath, '/real/path/to/ep_other'); + assert.equal(registry.ep_other.package.path, '/node_modules/ep_other'); + }); + + it('repeated calls remain non-destructive', function () { + const registry = makeRegistry(); + for (let i = 0; i < 5; i++) sanitizePluginsForWire(registry); + assert.equal(registry.ep_example.package.realPath, '/real/path/to/ep_example'); + assert.equal(registry.ep_other.package.realPath, '/real/path/to/ep_other'); + }); + + it('returned copy and input are independent (mutation of result does not affect input)', function () { + const registry = makeRegistry(); + const sanitized = sanitizePluginsForWire(registry); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (sanitized.ep_example as any).package.name = 'tampered'; + assert.equal(registry.ep_example.package.name, 'ep_example'); + }); +});