From 59d27f57f712603841ff813606b646c8cabe7cca Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Thu, 29 Oct 2026 09:27:59 +0100 Subject: [PATCH] fix(wpt): prevent fixture path traversal Reject unsafe suite paths and require fixture destinations to stay below the WPT fixture root before removing stale assets. Signed-off-by: Filip Skokan Assisted-by: Codex --- lib/wpt/index.js | 21 +++++++--- test/unit/wpt_updater.test.js | 78 +++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/lib/wpt/index.js b/lib/wpt/index.js index fe7e9bd5..8e49501c 100644 --- a/lib/wpt/index.js +++ b/lib/wpt/index.js @@ -9,8 +9,12 @@ import { } from '../utils.js'; export class WPTUpdater { - constructor(path, cli, request, nodedir, commit) { - this.path = path; + constructor(name, cli, request, nodedir, commit) { + if (typeof name !== 'string' || /[\\:\0]/.test(name) || + name.split('/').some(part => part === '' || part === '.' || part === '..')) { + throw new Error(`Invalid WPT path: ${name}`); + } + this.path = name; this.nodedir = nodedir; this.cli = cli; @@ -19,7 +23,7 @@ export class WPTUpdater { owner: 'web-platform-tests', repo: 'wpt', branch: 'master', - path, + path: name, commit }; this.tree = new GitHubTree(cli, request, this.treeParams); @@ -27,7 +31,14 @@ export class WPTUpdater { } fixtures(...args) { - return path.join(this.nodedir, 'test', 'fixtures', 'wpt', ...args); + const root = path.join(this.nodedir, 'test', 'fixtures', 'wpt'); + const filename = path.join(root, ...args); + const relative = path.relative(root, filename); + if (args.length && (relative === '' || relative === '..' || + relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative))) { + throw new Error(`Invalid WPT fixture path: ${filename}`); + } + return filename; } get versionsPath() { @@ -65,7 +76,7 @@ export class WPTUpdater { } this.cli.startSpinner('Removing stale assets...'); - await removeDirectory(this.fixtures(this.path)); + await removeDirectory(fixtures); this.cli.startSpinner('Pulling assets...'); // See https://github.com/nodejs/node-core-utils/issues/810 diff --git a/test/unit/wpt_updater.test.js b/test/unit/wpt_updater.test.js index 92065d55..6449b6e4 100644 --- a/test/unit/wpt_updater.test.js +++ b/test/unit/wpt_updater.test.js @@ -45,6 +45,84 @@ describe('WPTUpdater', function() { cli.clearCalls(); }); + it('rejects unsafe WPT names before making requests', () => { + const unsafeNames = [ + '../', '', '.', '..', '../outside', '../../..', + 'url/..', 'url/../outside', 'url/./resources', + '/url', '//server/share', 'url/', 'url//resources', + '..\\outside', 'url\\..\\outside', '\\url', + 'C:\\outside', 'C:/outside', 'C:outside', 'url:stream', + 'url\0outside' + ]; + const request = { gql: sinon.stub(), buffer: sinon.stub() }; + for (const name of unsafeNames) { + assert.throws( + () => new WPTUpdater(name, cli, request, '.', 'testcommit'), + /Invalid WPT path/, + JSON.stringify(name) + ); + } + assert.strictEqual(request.gql.callCount, 0); + assert.strictEqual(request.buffer.callCount, 0); + }); + + it('rejects fixture paths outside a strict subdirectory', () => { + const updater = new WPTUpdater('url', cli, request, '.'); + for (const name of ['../wpt-sibling', '', '.', '..', 'url/..']) { + assert.throws(() => updater.fixtures(name), /Invalid WPT fixture path/); + } + assert.strictEqual(updater.fixtures(), nodePath.join('test', 'fixtures', 'wpt')); + assert.strictEqual( + updater.fixtures('url', 'resources', 'test.js'), + nodePath.join('test', 'fixtures', 'wpt', 'url', 'resources', 'test.js') + ); + }); + + it('updates a nested suite without removing sibling fixtures', async() => { + const tempDir = fs.mkdtempSync(nodePath.join(os.tmpdir(), 'ncu-wpt-')); + try { + const fixtures = nodePath.join(tempDir, 'test', 'fixtures', 'wpt'); + const suite = nodePath.join(fixtures, 'html', 'dom'); + fs.mkdirSync(suite, { recursive: true }); + const stalePath = nodePath.join(suite, 'stale.js'); + const siblingPath = nodePath.join(fixtures, 'html', 'sibling.js'); + const outsidePath = nodePath.join(fixtures, '..', 'outside.txt'); + fs.writeFileSync(stalePath, 'stale'); + fs.writeFileSync(siblingPath, 'sibling'); + fs.writeFileSync(outsidePath, 'outside'); + + const request = { + gql: sinon.stub().resolves({ + repository: { + object: { entries: [{ name: 'test.js', type: 'blob' }] } + } + }), + buffer: sinon.stub().resolves(Buffer.from('downloaded')) + }; + const updater = new WPTUpdater('html/dom', new TestCLI(), request, tempDir, 'testcommit'); + await updater.update(); + await updater.updateLicense(); + + assert.strictEqual(fs.existsSync(stalePath), false); + assert.strictEqual(fs.readFileSync(nodePath.join(suite, 'test.js'), 'utf8'), 'downloaded'); + assert.strictEqual(fs.readFileSync(siblingPath, 'utf8'), 'sibling'); + assert.strictEqual(fs.readFileSync(outsidePath, 'utf8'), 'outside'); + const licensePath = nodePath.join(fixtures, 'LICENSE.md'); + const versionsPath = nodePath.join(fixtures, 'versions.json'); + assert.strictEqual(fs.readFileSync(licensePath, 'utf8'), 'downloaded'); + assert.deepStrictEqual(JSON.parse(fs.readFileSync(versionsPath, 'utf8')), { + 'html/dom': { commit: 'testcommit', path: 'html/dom' } + }); + assert.deepStrictEqual(request.gql.firstCall.args, ['TreeEntries', { + owner: 'web-platform-tests', + repo: 'wpt', + expression: 'testcommit:html/dom' + }]); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + it('exits with meaningful error when WPT name not found', async() => { wptUpdater = new WPTUpdater(path, cli, request, nodedir); let thrown;