From b6b3ceef4de4b56482a67c7808c4be61308cbf22 Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Wed, 3 Jun 2020 10:45:28 +0100 Subject: [PATCH] Add some proper validation of module/url properties in install api --- .../@node-red/registry/lib/installer.js | 23 ++++++-- .../runtime/locales/en-US/runtime.json | 2 + .../@node-red/registry/lib/installer_spec.js | 58 ++++++++++++------- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/packages/node_modules/@node-red/registry/lib/installer.js b/packages/node_modules/@node-red/registry/lib/installer.js index dac100613..22fb2a549 100644 --- a/packages/node_modules/@node-red/registry/lib/installer.js +++ b/packages/node_modules/@node-red/registry/lib/installer.js @@ -30,7 +30,7 @@ var npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm'; var paletteEditorEnabled = false; var settings; -var moduleRe = /^(@[^/]+?[/])?[^/]+?$/; +var moduleRe = /^(@[^/@]+?[/])?[^/@]+?$/; var slashRe = process.platform === "win32" ? /\\|[/]/ : /[/]/; var pkgurlRe = /^(https?|git(|\+https?|\+ssh|\+file)):\/\//; @@ -78,15 +78,24 @@ function checkExistingModule(module,version) { return false; } function installModule(module,version,url) { + module = module || ""; activePromise = activePromise.then(() => { //TODO: ensure module is 'safe' return new Promise((resolve,reject) => { var installName = module; var isUpgrade = false; try { - if (url && pkgurlRe.test(url)) { - // Git remote url or Tarball url - check the valid package url - installName = url; + if (url) { + if (pkgurlRe.test(url)) { + // Git remote url or Tarball url - check the valid package url + installName = url; + } else { + log.warn(log._("server.install.install-failed-url",{name:module,url:url})); + e = new Error("Invalid url"); + e.code = "invalid_module_url"; + reject(e); + return; + } } else if (moduleRe.test(module)) { // Simple module name - assume it can be npm installed if (version) { @@ -96,6 +105,12 @@ function installModule(module,version,url) { // A path - check if there's a valid package.json installName = module; module = checkModulePath(module); + } else { + log.warn(log._("server.install.install-failed-name",{name:module})); + e = new Error("Invalid module name"); + e.code = "invalid_module_name"; + reject(e); + return; } isUpgrade = checkExistingModule(module,version); } catch(err) { diff --git a/packages/node_modules/@node-red/runtime/locales/en-US/runtime.json b/packages/node_modules/@node-red/runtime/locales/en-US/runtime.json index fea4d5591..3047295de 100644 --- a/packages/node_modules/@node-red/runtime/locales/en-US/runtime.json +++ b/packages/node_modules/@node-red/runtime/locales/en-US/runtime.json @@ -32,6 +32,8 @@ "install-failed": "Install failed", "install-failed-long": "Installation of module __name__ failed:", "install-failed-not-found": "$t(server.install.install-failed-long) module not found", + "install-failed-name": "$t(server.install.install-failed-long) invalid module name: __name__", + "install-failed-url": "$t(server.install.install-failed-long) invalid url: __url__", "upgrading": "Upgrading module: __name__ to version: __version__", "upgraded": "Upgraded module: __name__. Restart Node-RED to use the new version", "upgrade-failed-not-found": "$t(server.install.install-failed-long) version not found", diff --git a/test/unit/@node-red/registry/lib/installer_spec.js b/test/unit/@node-red/registry/lib/installer_spec.js index 045a81e0d..1eb79d723 100644 --- a/test/unit/@node-red/registry/lib/installer_spec.js +++ b/test/unit/@node-red/registry/lib/installer_spec.js @@ -77,6 +77,30 @@ describe('nodes/registry/installer', function() { }); describe("installs module", function() { + it("rejects module name that includes version", function(done) { + installer.installModule("module@version",null,null).catch(function(err) { + err.code.should.be.eql('invalid_module_name'); + done(); + }).catch(done); + }); + it("rejects missing module name", function(done) { + installer.installModule("",null,null).catch(function(err) { + err.code.should.be.eql('invalid_module_name'); + done(); + }).catch(done); + }); + it("rejects null module name", function(done) { + installer.installModule(null,null,null).catch(function(err) { + err.code.should.be.eql('invalid_module_name'); + done(); + }).catch(done); + }); + it("rejects invalid url", function(done) { + installer.installModule("module",null,"abc").catch(function(err) { + err.code.should.be.eql('invalid_module_url'); + done(); + }); + }); it("rejects when npm returns a 404", function(done) { var res = { code: 1, @@ -89,7 +113,7 @@ describe('nodes/registry/installer', function() { installer.installModule("this_wont_exist").catch(function(err) { err.should.have.property("code",404); done(); - }); + }).catch(done); }); it("rejects when npm does not find specified version", function(done) { var res = { @@ -108,7 +132,7 @@ describe('nodes/registry/installer', function() { installer.installModule("this_wont_exist","0.1.2").catch(function(err) { err.code.should.be.eql(404); done(); - }); + }).catch(done); }); it("rejects when update requested to existing version", function(done) { sinon.stub(typeRegistry,"getModuleInfo", function() { @@ -119,7 +143,7 @@ describe('nodes/registry/installer', function() { installer.installModule("this_wont_exist","0.1.1").catch(function(err) { err.code.should.be.eql('module_already_loaded'); done(); - }); + }).catch(done); }); it("rejects when update requested to existing version and url", function(done) { sinon.stub(typeRegistry,"getModuleInfo", function() { @@ -130,7 +154,7 @@ describe('nodes/registry/installer', function() { installer.installModule("this_wont_exist","0.1.1","https://example/foo-0.1.1.tgz").catch(function(err) { err.code.should.be.eql('module_already_loaded'); done(); - }); + }).catch(done); }); it("rejects with generic error", function(done) { var res = { @@ -143,8 +167,9 @@ describe('nodes/registry/installer', function() { initInstaller(p) installer.installModule("this_wont_exist").then(function() { done(new Error("Unexpected success")); - }).catch(function(err) { - done(); + }).catch(err => { + // Expected result + done() }); }); it("succeeds when module is found", function(done) { @@ -169,9 +194,7 @@ describe('nodes/registry/installer', function() { // commsMessages[0].topic.should.equal("node/added"); // commsMessages[0].msg.should.eql(nodeInfo.nodes); done(); - }).catch(function(err) { - done(err); - }); + }).catch(done); }); it("rejects when non-existant path is provided", function(done) { this.timeout(20000); @@ -208,9 +231,7 @@ describe('nodes/registry/installer', function() { installer.installModule(resourcesDir).then(function(info) { info.should.eql(nodeInfo); done(); - }).catch(function(err) { - done(err); - }); + }).catch(done); }); it("succeeds when url is valid node-red module", function(done) { var nodeInfo = {nodes:{module:"foo",types:["a"]}}; @@ -231,9 +252,7 @@ describe('nodes/registry/installer', function() { installer.installModule("this_wont_exist",null,"https://example/foo-0.1.1.tgz").then(function(info) { info.should.eql(nodeInfo); done(); - }).catch(function(err) { - done(err); - }); + }).catch(done); }); }); @@ -265,8 +284,9 @@ describe('nodes/registry/installer', function() { installer.uninstallModule("this_wont_exist").then(function() { done(new Error("Unexpected success")); - }).catch(function(err) { - done(); + }).catch(err => { + // Expected result + done() }); }); it("succeeds when module is found", function(done) { @@ -294,9 +314,7 @@ describe('nodes/registry/installer', function() { // commsMessages[0].topic.should.equal("node/removed"); // commsMessages[0].msg.should.eql(nodeInfo); done(); - }).catch(function(err) { - done(err); - }); + }).catch(done); }); }); });