Add some proper validation of module/url properties in install api

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)) { 
+                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) {

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",

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) {
+            }).catch(err => {
-                done();
+                // 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) {
+            }).catch(done);
-                done(err);
-            });
         });
         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) {
+            }).catch(done);
-                done(err);
-            });
         });
         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) {
+            }).catch(done);
-                done(err);
-            });
         });
 
     });
@@ -265,8 +284,9 @@ describe('nodes/registry/installer', function() {
 
             installer.uninstallModule("this_wont_exist").then(function() {
                 done(new Error("Unexpected success"));
-            }).catch(function(err) {
+            }).catch(err => {
-                done();
+                // 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) {
+            }).catch(done);
-                done(err);
-            });
         });
     });
 });