From c273f5d89cb5910263a2cb6a538c276f04312652 Mon Sep 17 00:00:00 2001 From: John McLear Date: Thu, 15 Oct 2026 07:56:20 +0100 Subject: [PATCH 1/3] fix(pluginfw): validate dependency names on plugin uninstall GHSA-92hr-gmr6-h8cp validated plugin dependency names before they were used to build filesystem paths, but only on the install side. The uninstall side (removeSubDependency, uninstallDependency, unlinkSubDependency, unlinkDependency) still joined the raw package.json key onto pluginInstallPath / node_modules and passed it to rmSync({recursive, force}) or unlinkSync. A plugin declaring a dependency such as "zzz/../../../../tmp/x" with a GitHub-spec version got past live-plugin-manager, and uninstalling it deleted a directory outside the plugin folder. Move the name check into dependencyPath.ts, add a containment check (resolveDependencyPath) and use it at every uninstall/unlink sink. Names whose segments start with "." are now rejected as well. Reported by @baeseungwon1010 (GHSA-g9xm-3pqr-wcph). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013dDCHnxem1jJTYtTcY4Hqe --- src/static/js/pluginfw/LinkInstaller.ts | 46 ++++++++----- src/static/js/pluginfw/dependencyPath.ts | 21 ++++++ .../backend-new/specs/dependencyPath.test.ts | 40 +++++++++++ src/tests/backend/specs/LinkInstaller.ts | 66 +++++++++++++++++++ 4 files changed, 156 insertions(+), 17 deletions(-) create mode 100644 src/static/js/pluginfw/dependencyPath.ts create mode 100644 src/tests/backend-new/specs/dependencyPath.test.ts diff --git a/src/static/js/pluginfw/LinkInstaller.ts b/src/static/js/pluginfw/LinkInstaller.ts index 430e1fa9171..25278fe2116 100644 --- a/src/static/js/pluginfw/LinkInstaller.ts +++ b/src/static/js/pluginfw/LinkInstaller.ts @@ -5,6 +5,7 @@ import {accessSync, constants, rmSync, symlinkSync, unlinkSync} from "node:fs"; import {dependencies, name} from '../../../package.json' import settings from '../../../node/utils/Settings'; import {readFileSync} from "fs"; +import {isValidDependencyName, resolveDependencyPath} from "./dependencyPath"; export class LinkInstaller { private livePluginManager: PluginManager; @@ -92,6 +93,11 @@ export class LinkInstaller { } private async removeSubDependency(_name: string, dependency:string) { + const dependencyPath = resolveDependencyPath(pluginInstallPath, dependency) + if (dependencyPath == null) { + console.error(`Skipping plugin dependency with invalid name: ${dependency}`) + return + } if (this.dependenciesMap.has(dependency)) { console.debug(`Dependency ${dependency} is still being used by other plugins`) return @@ -99,7 +105,7 @@ export class LinkInstaller { // Read sub dependencies try { const json:IPluginInfo = JSON.parse( - readFileSync(path.join(pluginInstallPath, dependency, 'package.json'), 'utf-8')); + readFileSync(path.join(dependencyPath, 'package.json'), 'utf-8')); if(json.dependencies){ for (let [subDependency] of Object.entries(json.dependencies)) { await this.removeSubDependency(dependency, subDependency) @@ -110,11 +116,16 @@ export class LinkInstaller { } private uninstallDependency(dependency: string) { + const dependencyPath = resolveDependencyPath(pluginInstallPath, dependency) + if (dependencyPath == null) { + console.error(`Skipping plugin dependency with invalid name: ${dependency}`) + return + } try { console.debug(`Uninstalling dependency ${dependency}`) // Check if the dependency is already installed - accessSync(path.join(pluginInstallPath, dependency), constants.F_OK) - rmSync(path.join(pluginInstallPath, dependency), { + accessSync(dependencyPath, constants.F_OK) + rmSync(dependencyPath, { force: true, recursive: true }) @@ -146,6 +157,11 @@ export class LinkInstaller { } private async unlinkSubDependency(plugin: string, dependency: string) { + const dependencyPath = resolveDependencyPath(pluginInstallPath, dependency) + if (dependencyPath == null) { + console.error(`Skipping plugin dependency with invalid name: ${dependency}`) + return + } if (this.dependenciesMap.has(dependency)) { this.dependenciesMap.get(dependency)?.delete(plugin) if (this.dependenciesMap.get(dependency)!.size > 0) { @@ -157,7 +173,7 @@ export class LinkInstaller { // Read sub dependencies try { const json:IPluginInfo = JSON.parse( - readFileSync(path.join(pluginInstallPath, dependency, 'package.json'), 'utf-8')); + readFileSync(path.join(dependencyPath, 'package.json'), 'utf-8')); if(json.dependencies){ for (let [subDependency] of Object.entries(json.dependencies)) { await this.unlinkSubDependency(dependency, subDependency) @@ -177,17 +193,8 @@ export class LinkInstaller { } } - // A dependency name comes from a plugin's package.json and is used to build - // filesystem paths (readFileSync / symlinkSync), so validate it is a plain npm - // package name (optional scope, no path separators or "..") before use. - private isValidDependencyName(dependency: string): boolean { - return typeof dependency === 'string' && - !dependency.includes('..') && - /^(@[a-z0-9-._~]+\/)?[a-z0-9-._~]+$/i.test(dependency); - } - private async addSubDependency(plugin: string, dependency: string) { - if (!this.isValidDependencyName(dependency)) { + if (!isValidDependencyName(dependency)) { console.error(`Skipping plugin dependency with invalid name: ${dependency}`) return } @@ -214,7 +221,7 @@ export class LinkInstaller { } private linkDependency(dependency: string) { - if (!this.isValidDependencyName(dependency)) { + if (!isValidDependencyName(dependency)) { console.error(`Skipping plugin dependency with invalid name: ${dependency}`) return } @@ -236,10 +243,15 @@ export class LinkInstaller { } private unlinkDependency(dependency: string) { + const linkPath = resolveDependencyPath(node_modules, dependency) + if (linkPath == null) { + console.error(`Skipping plugin dependency with invalid name: ${dependency}`) + return + } try { // Check if the dependency is already installed - accessSync(path.join(node_modules, dependency), constants.F_OK) - unlinkSync(path.join(node_modules, dependency)) + accessSync(linkPath, constants.F_OK) + unlinkSync(linkPath) } catch (err) { // Symlink does not exist // So nothing to do diff --git a/src/static/js/pluginfw/dependencyPath.ts b/src/static/js/pluginfw/dependencyPath.ts new file mode 100644 index 00000000000..94ee41fe60d --- /dev/null +++ b/src/static/js/pluginfw/dependencyPath.ts @@ -0,0 +1,21 @@ +import path from "path"; + +// A dependency name comes from a plugin's package.json and is used to build +// filesystem paths (readFileSync / symlinkSync / unlinkSync / rmSync), so it +// must be a plain npm package name (optional scope, no path separators, no +// segment starting with ".", no "..") before use. +export const isValidDependencyName = (dependency: unknown): dependency is string => + typeof dependency === 'string' && + !dependency.includes('..') && + /^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/i.test(dependency); + +// Returns the absolute path of `dependency` inside `base`, or null if the name +// is invalid or the resolved path would leave `base`. Callers must treat null +// as "skip": never fall back to the raw name. +export const resolveDependencyPath = (base: string, dependency: unknown): string | null => { + if (!isValidDependencyName(dependency)) return null; + const root = path.resolve(base); + const resolved = path.resolve(root, dependency); + if (!resolved.startsWith(root + path.sep)) return null; + return resolved; +}; diff --git a/src/tests/backend-new/specs/dependencyPath.test.ts b/src/tests/backend-new/specs/dependencyPath.test.ts new file mode 100644 index 00000000000..04d9424aa3c --- /dev/null +++ b/src/tests/backend-new/specs/dependencyPath.test.ts @@ -0,0 +1,40 @@ +'use strict'; + +import os from 'os'; +import path from 'path'; +import {describe, expect, it} from 'vitest'; +import { + isValidDependencyName, + resolveDependencyPath, +} from '../../../static/js/pluginfw/dependencyPath'; + +describe('resolveDependencyPath', () => { + const base = path.join(os.tmpdir(), 'ep-base'); + + it('accepts plain and scoped npm names', () => { + expect(resolveDependencyPath(base, 'ep_align')).toBe(path.join(base, 'ep_align')); + expect(resolveDependencyPath(base, '@scope/pkg')).toBe(path.join(base, '@scope', 'pkg')); + }); + + it('rejects traversal, absolute and malformed names', () => { + for (const bad of [ + 'zzz/../../../../tmp/victim', + '../victim', + '..', + '.', + '', + '/etc', + 'a/b', + 'a\\..\\b', + '@scope/../x', + '.hidden', + '@scope/.x', + null, + undefined, + 42, + ]) { + expect(isValidDependencyName(bad), String(bad)).toBe(false); + expect(resolveDependencyPath(base, bad), String(bad)).toBeNull(); + } + }); +}); diff --git a/src/tests/backend/specs/LinkInstaller.ts b/src/tests/backend/specs/LinkInstaller.ts index b9f7aae1da2..b96ce067279 100644 --- a/src/tests/backend/specs/LinkInstaller.ts +++ b/src/tests/backend/specs/LinkInstaller.ts @@ -139,4 +139,70 @@ describe(__filename, function () { assert(msg.includes('nonexistent'), 'error should mention the path'); }); }); + + // GHSA-g9xm-3pqr-wcph: uninstalling a plugin whose package.json declares a + // dependency key like "zzz/../../../../tmp/x" made LinkInstaller rmSync() / + // unlinkSync() a path outside pluginInstallPath / node_modules. Importing + // installer first lets the real LinkInstaller instance load despite the + // circular import. + describe('uninstall path traversal (GHSA-g9xm-3pqr-wcph)', function () { + let installer: any; + let li: any; + let victimDir: string; + let victimFile: string; + const traversal = (from: string, to: string) => + `zzz/${path.relative(path.join(from, 'zzz'), to)}`; + + before(async function () { + installer = await import('../../../static/js/pluginfw/installer'); + li = installer.linkInstaller; + }); + + beforeEach(function () { + victimDir = path.join(tmpDir, 'victim-dir'); + fs.mkdirSync(victimDir); + fs.writeFileSync(path.join(victimDir, 'marker.txt'), 'keep me'); + victimFile = path.join(tmpDir, 'victim-file.txt'); + fs.writeFileSync(victimFile, 'keep me'); + sinon.stub(console, 'error'); + sinon.stub(console, 'debug'); + }); + + it('uninstallPlugin does not delete a directory outside pluginInstallPath', async function () { + const evil = traversal(installer.pluginInstallPath, victimDir); + // This is the path the vulnerable code handed to rmSync(). + assert.equal(path.join(installer.pluginInstallPath, evil), victimDir); + sinon.stub(li.livePluginManager, 'getInfo').returns({ + name: 'ep_g9xm_poc', + version: '1.0.0', + location: path.join(installer.pluginInstallPath, 'ep_g9xm_poc'), + mainFile: 'index.js', + dependencies: {[evil]: 'jashkenas/underscore'}, + dependencyDetails: {}, + }); + sinon.stub(li.livePluginManager, 'uninstall').resolves(); + + await li.uninstallPlugin('ep_g9xm_poc'); + + assert(fs.existsSync(path.join(victimDir, 'marker.txt')), 'victim dir was deleted'); + }); + + it('sub-dependency removal does not follow a traversal name', async function () { + const evil = traversal(installer.pluginInstallPath, victimDir); + await li.removeSubDependency('ep_x', evil); + await li.unlinkSubDependency('ep_x', evil); + li.uninstallDependency(evil); + + assert(fs.existsSync(path.join(victimDir, 'marker.txt')), 'victim dir was deleted'); + }); + + it('unlinkDependency does not unlink a file outside node_modules', function () { + const evil = traversal(installer.node_modules, victimFile); + assert.equal(path.join(installer.node_modules, evil), victimFile); + + li.unlinkDependency(evil); + + assert(fs.existsSync(victimFile), 'victim file was unlinked'); + }); + }); }); From 60947c6b057f7d7a88d1ceca8e61f66f57345a2a Mon Sep 17 00:00:00 2001 From: John McLear Date: Sun, 18 Oct 2026 12:25:08 +0100 Subject: [PATCH 2/3] docs(changelog): 3.3.9 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Y5YroE8t1VhXCVTjJhx2kU --- CHANGELOG.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 820290c49bb..b416d66b094 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,30 @@ +# 3.3.9 + +3.3.9 is a security and feature release. It closes a path traversal in plugin uninstall (GHSA-g9xm-3pqr-wcph), adds built-in pad retention, and ships a batch of fixes. + +### Security + +- **Plugins — dependency names are validated on uninstall as well as install (GHSA-g9xm-3pqr-wcph).** GHSA-92hr-gmr6-h8cp made the plugin installer validate dependency names on install, but the uninstall path still passed the raw `package.json` dependency key to `rmSync`/`unlinkSync`. A plugin declaring a dependency named like `zzz/../../..` could therefore delete a directory outside the plugin folder when an admin uninstalled it. Every install and uninstall path now resolves dependency paths through one helper that refuses names that escape the plugin directory. Exploiting it needs an admin to install and then uninstall the malicious plugin. Reported by @baeseungwon1010. + +### New features + +- **Pad retention — delete pads nobody has edited for N days (#8340).** A new `padRetention` setting replaces the unmaintained `ep_delete_after_delay` plugin without its HTML archive copies or tombstone pads. It is off by default and, once enabled, only logs `would delete pad ` until `dryRun` is set to `false`. A periodic sweep reads each pad's last-edit time straight from the database without loading pads into the cache, and an overdue pad is also deleted when someone opens it instead of showing stale content. Deletion uses the same path as the `deletePad` API. Options: `deleteAfterDays`, `warnBeforeDays`, `sweepIntervalHours`, `excludeGroupPads` and `excludePattern`. +- **Settings — hide the deletion-token modal without widening deletion rights (#7996, #8319).** The new `suppressPadDeletionTokenModal` setting (`SUPPRESS_PAD_DELETION_TOKEN_MODAL` in the container image) stops the one-time "Save your pad deletion token" modal from appearing. Until now the only way to hide it was `allowPadDeletionByAllUsers: true`, which also let any editor delete the pad. The token is still issued and still works, and non-creators still can't delete. Thanks to @sirsqm. + +### Notable fixes + +- **Performance — fonts are no longer inlined into the minified CSS (#8268, #8314).** The pad stylesheet embedded every editor and icon font as base64, so `pad.css` was about 1.7 MB and every visitor downloaded all of it before the pad rendered. Fonts are now fetched separately and only when used, and `pad.css` is about 61 KB. Thanks to @sirsqm. +- **Toolbar — a popup opened from the toolbar keeps focus (#8345).** The editor grabbed focus back after every toolbar command, so in current Firefox Escape didn't close the settings or users popup and typing a name didn't reach the name field. +- **Colibris — dark mode colours apply everywhere (#8271).** The light `--text-color` and `--bg-color` values could override the dark ones on elements outside the editor, such as the history banner. Thanks to @bobmullut. +- **Session transfer — failed requests are reported as failures (#8172, #8273).** Errors from the session transfer endpoint were treated as success. They now show a localized error, and a retry clears it. Thanks to @toashu123. +- **Share dialog — "Read only" is now "Use read-only link" (#8181, #8326).** The checkbox switches which link and embed code you copy; it never locked the pad. The new label and an accessible description say so. Thanks to @jum-apzn. +- **Docker — `--build-arg PnpmVersion` is honoured again (#8210, #8320).** Thanks to @sirsqm. + +### Other changes + +- Localisation updates from translatewiki.net. +- Dependency updates, including express 5.3.0, ejs 7.0.1, nodemailer 10.1.1, surrealdb 2.1.1, mssql 12.7.9 and ueberdb2 6.1.18. + # 3.3.8 3.3.8 is a security release. It closes an HTTP API privilege escalation for instances using the built-in OIDC provider with API clients (GHSA-38vj-95q4-gwvx). From 00f515726cc753157cb9ddab0951fde8d555d14d Mon Sep 17 00:00:00 2001 From: John McLear Date: Sun, 18 Oct 2026 12:34:35 +0100 Subject: [PATCH 3/3] test(pluginfw): keep uninstall traversal victims on the repo's drive On Windows CI os.tmpdir() is on C: while the checkout is on D:, so no relative traversal path to the victim exists and the precondition assert failed. Create the victims next to the plugin folder instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Y5YroE8t1VhXCVTjJhx2kU --- src/tests/backend/specs/LinkInstaller.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/tests/backend/specs/LinkInstaller.ts b/src/tests/backend/specs/LinkInstaller.ts index b96ce067279..9037ad14892 100644 --- a/src/tests/backend/specs/LinkInstaller.ts +++ b/src/tests/backend/specs/LinkInstaller.ts @@ -148,6 +148,7 @@ describe(__filename, function () { describe('uninstall path traversal (GHSA-g9xm-3pqr-wcph)', function () { let installer: any; let li: any; + let victimRoot: string; let victimDir: string; let victimFile: string; const traversal = (from: string, to: string) => @@ -159,15 +160,24 @@ describe(__filename, function () { }); beforeEach(function () { - victimDir = path.join(tmpDir, 'victim-dir'); + // Keep the victims next to the plugin folder rather than in os.tmpdir(): + // on Windows CI the temp dir is on a different drive, so no relative + // traversal path to it exists. + victimRoot = fs.mkdtempSync( + path.join(path.dirname(installer.pluginInstallPath), '.g9xm-victim-')); + victimDir = path.join(victimRoot, 'victim-dir'); fs.mkdirSync(victimDir); fs.writeFileSync(path.join(victimDir, 'marker.txt'), 'keep me'); - victimFile = path.join(tmpDir, 'victim-file.txt'); + victimFile = path.join(victimRoot, 'victim-file.txt'); fs.writeFileSync(victimFile, 'keep me'); sinon.stub(console, 'error'); sinon.stub(console, 'debug'); }); + afterEach(function () { + fs.rmSync(victimRoot, {recursive: true, force: true}); + }); + it('uninstallPlugin does not delete a directory outside pluginInstallPath', async function () { const evil = traversal(installer.pluginInstallPath, victimDir); // This is the path the vulnerable code handed to rmSync().