lightningd: convert plugin->cmd to absolute path, fail plugin early when non-existent

Otherwise different relative paths (e.g. /dir/plugin and /dir/../dir/plugin) to same plugin
executable would not be recognized as the same plugin

lightningd/options.c

@@ -458,11 +458,14 @@ static char *opt_add_proxy_addr(const char *arg, struct lightningd *ld)
 
 static char *opt_add_plugin(const char *arg, struct lightningd *ld)
 {
+	struct plugin *p;
 	if (plugin_blacklisted(ld->plugins, arg)) {
 		log_info(ld->log, "%s: disabled via disable-plugin", arg);
 		return NULL;
 	}
-	plugin_register(ld->plugins, arg, NULL, false, NULL, NULL);
+	p = plugin_register(ld->plugins, arg, NULL, false, NULL, NULL);
+	if (!p)
+		return tal_fmt(NULL, "Failed to register %s: %s", arg, strerror(errno));
 	return NULL;
 }
 
@@ -485,11 +488,14 @@ static char *opt_clear_plugins(struct lightningd *ld)
 
 static char *opt_important_plugin(const char *arg, struct lightningd *ld)
 {
+	struct plugin *p;
 	if (plugin_blacklisted(ld->plugins, arg)) {
 		log_info(ld->log, "%s: disabled via disable-plugin", arg);
 		return NULL;
 	}
-	plugin_register(ld->plugins, arg, NULL, true, NULL, NULL);
+	p = plugin_register(ld->plugins, arg, NULL, true, NULL, NULL);
+	if (!p)
+		return tal_fmt(NULL, "Failed to register %s: %s", arg, strerror(errno));
 	return NULL;
 }
 

lightningd/plugin.c

@@ -253,11 +253,16 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES,
 			       const jsmntok_t *params STEALS)
 {
 	struct plugin *p, *p_temp;
+	char* abspath;
 	u32 chksum;
 
+	abspath = path_canon(tmpctx, path);
+	if (!abspath) {
+		return NULL;
+	}
 	/* Don't register an already registered plugin */
 	list_for_each(&plugins->plugins, p_temp, list) {
-		if (streq(path, p_temp->cmd)) {
+		if (streq(abspath, p_temp->cmd)) {
 			/* If added as "important", upgrade to "important".  */
 			if (important)
 				p_temp->important = true;
@@ -276,7 +281,7 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES,
 
 	p = tal(plugins, struct plugin);
 	p->plugins = plugins;
-	p->cmd = tal_strdup(p, path);
+	p->cmd = tal_steal(p, abspath);
 	p->checksum = file_checksum(plugins->ld, p->cmd);
 	p->shortname = path_basename(p, p->cmd);
 	p->start_cmd = start_cmd;
@@ -912,6 +917,11 @@ static const char *plugin_opt_add(struct plugin *plugin, const char *buffer,
 
 	popt->name = tal_fmt(popt, "--%.*s", nametok->end - nametok->start,
 			     buffer + nametok->start);
+
+	/* an "|" alias could circumvent the unique-option-name check */
+	if (strchr(popt->name, '|'))
+		return tal_fmt(plugin, "Option \"name\" may not contain '|'");
+
 	popt->description = NULL;
 	if (deptok) {
 		if (!json_to_bool(buffer, deptok, &popt->deprecated))

lightningd/plugin_control.c

@@ -71,8 +71,8 @@ plugin_dynamic_start(struct plugin_command *pcmd, const char *plugin_path,
 
 	if (!p)
 		return command_fail(pcmd->cmd, JSONRPC2_INVALID_PARAMS,
-				    "%s: already registered",
+				    "%s: %s", plugin_path,
-				    plugin_path);
+					    errno ? strerror(errno) : "already registered");
 
 	/* This will come back via plugin_cmd_killed or plugin_cmd_succeeded */
 	err = plugin_send_getmanifest(p);

tests/test_plugin.py

@@ -2074,10 +2074,11 @@ def test_important_plugin(node_factory):
     n = node_factory.get_node(options={"important-plugin": os.path.join(pluginsdir, "nonexistent")},
                               may_fail=True, expect_fail=True,
                               allow_broken_log=True, start=False)
+
     n.daemon.start(wait_for_initialized=False)
     # Will exit with failure code.
     assert n.daemon.wait() == 1
-    assert n.daemon.is_in_stderr(r"error starting plugin '.*nonexistent'")
+    assert n.daemon.is_in_stderr(r"Failed to register .*nonexistent: No such file or directory")
 
     # Check we exit if the important plugin dies.
     n.daemon.opts['important-plugin'] = os.path.join(pluginsdir, "fail_by_itself.py")