Codebase audit: bug fixes and robustness improvements for the Remote SSH MCP server (#1)

* Fix remote_browse_dir schema: path is optional (defaults to /home)

The input schema declared 'path' as required while the handler defaulted
missing values to /home. The two were contradictory; the handler default
matches the documented behavior and is friendlier for the folder picker.
Drop the bogus required entry so a missing path no longer trips
client-side validation.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

* Allow remote_select_workspace to persist env-only hosts

REMOTE_SSH_HOSTS is documented as a valid host source, but
selectWorkspaceInConfig threw 'does not exist' whenever the host was
loaded only from the environment (or any source other than the writable
config file). Pass the resolved host profile as a seed so the writable
config can be populated transparently when the alias is missing on disk.

The pre-existing 'does not exist' behavior is preserved when no seed is
supplied, which keeps the existing test contract intact.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

* Harden remote_write_file overwrite handling

* Remove the dead 'operator' ternary that always evaluated to '>'.
* Replace the two-stage 'test ! -e PATH && >' guard with POSIX noclobber
  ('set -C'), which makes the no-overwrite case atomic with respect to
  the redirection rather than racing between the existence probe and the
  write.
* Reject non-string content with a clear error instead of letting
  Buffer.from(undefined) bubble up as 'first argument must be ...'.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

* Make audit() resilient to filesystem failures

Audit logging is a side-channel for operations teams; a missing parent
directory or unwritable path should not break the primary SSH tool call.
Create the parent directory before writing, and swallow append failures
to stderr instead of throwing into the JSON-RPC handler.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

* Return proper JSON-RPC errors instead of silently succeeding

* Unknown methods now produce a -32601 'Method not found' error rather
  than an empty success response, so clients can tell when a tool was
  mistyped or removed.
* JSON parse failures emit a -32700 error with id: null per JSON-RPC
  2.0, instead of being dropped on the floor when the original line
  could not be parsed.
* Plumb error.code through writeError so handlers can attach meaningful
  codes (parse, method-not-found, etc.) while preserving the existing
  -32000 default for application errors.
* Treat any notifications/* method as a no-op so future MCP
  notifications do not regress into 'Method not found'.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

* Harden remote_search_text query handling

* Reject empty/non-string queries up front instead of running an
  effectively-wildcard 'rg ""' that scans the whole tree.
* Pass the search pattern via -e and add a '--' separator before the
  positional target, so queries that begin with '-' (e.g. searching for
  a flag-like token) are no longer interpreted as ripgrep/grep options.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

* Tighten input validation for remove_host / blocked patterns + tests

* remote_remove_host now rejects empty/blank names before consulting the
  config, instead of producing 'Connection  does not exist'.
* assertCommandAllowed now ignores malformed blockedCommandPatterns
  entries rather than letting a bad user-supplied regex crash command
  execution.
* Extend tests to cover:
  - selectWorkspaceInConfig accepting a seed profile when the alias is
    not yet in the writable config (the env-var path);
  - remote_browse_dir input schema no longer marking 'path' as required;
  - notifications/* still resolving to an empty result;
  - unknown JSON-RPC methods rejecting with code -32601.

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

---------

Co-authored-by: MD. Mehedi Hossain <MehediHossain95@users.noreply.github.com>

Author: MehediHossain95

plugins/remote-ssh/scripts/remote-ssh-mcp.js

@@ -189,7 +189,14 @@ function audit(event) {
     version: SERVER_VERSION,
     ...event,
   };
-  fs.appendFileSync(expandHome(auditPath), `${JSON.stringify(record)}\n`);
+
+  const resolved = expandHome(auditPath);
+  try {
+    fs.mkdirSync(path.dirname(resolved), { recursive: true });
+    fs.appendFileSync(resolved, `${JSON.stringify(record)}\n`);
+  } catch (error) {
+    process.stderr.write(`remote-ssh: audit log write failed (${error.message})\n`);
+  }
 }
 
 function shellQuote(value) {
@@ -261,7 +268,11 @@ function assertCommandAllowed(config, command) {
   }
 
   const blocked = (config.blockedCommandPatterns || []).find((pattern) => {
-    return new RegExp(pattern, "i").test(command);
+    try {
+      return new RegExp(pattern, "i").test(command);
+    } catch {
+      return false;
+    }
   });
   if (blocked) {
     throw new Error(`Command denied by policy for host alias ${config.alias}: ${blocked}`);
@@ -403,18 +414,20 @@ function buildBrowseDirCommand(remotePath, limit) {
   ].join(" && ");
 }
 
-function selectWorkspaceInConfig(config, alias, workspacePath) {
-  if (!config.hosts[alias]) {
+function selectWorkspaceInConfig(config, alias, workspacePath, seedProfile) {
+  const normalized = normalizeRemotePath(workspacePath);
+  const existing = config.hosts[alias];
+  if (!existing && !seedProfile) {
     throw new Error(`Connection ${alias} does not exist.`);
   }
-  const normalized = normalizeRemotePath(workspacePath);
-  const host = { ...config.hosts[alias] };
-  const allowedPaths = Array.isArray(host.allowedPaths) ? [...host.allowedPaths] : [];
+  const base = existing ? { ...existing } : { ...seedProfile };
+  delete base.alias;
+  const allowedPaths = Array.isArray(base.allowedPaths) ? [...base.allowedPaths] : [];
   if (!allowedPaths.includes(normalized)) {
     allowedPaths.push(normalized);
   }
   config.hosts[alias] = {
-    ...host,
+    ...base,
     workspaceRoot: normalized,
     allowedPaths,
   };
@@ -696,7 +709,7 @@ const tools = [
         path: { type: "string", description: "Absolute remote directory path.", default: "/home" },
         limit: { type: "integer", minimum: 1, maximum: 1000, default: 200 },
       },
-      required: ["host", "path"],
+      required: ["host"],
       additionalProperties: false,
     },
   },
@@ -911,6 +924,9 @@ async function callTool(name, args) {
 
   if (name === "remote_remove_host") {
     const alias = String(args.name || "").trim();
+    if (!alias) {
+      throw new Error("remote_remove_host requires a non-empty name.");
+    }
     const { file, config } = readWritableConfig();
     if (!config.hosts[alias]) {
       throw new Error(`Connection ${alias} does not exist in ${file}.`);
@@ -981,7 +997,7 @@ async function callTool(name, args) {
       });
     }
     const { file, config: writableConfig } = readWritableConfig();
-    const profile = selectWorkspaceInConfig(writableConfig, config.alias, remotePath);
+    const profile = selectWorkspaceInConfig(writableConfig, config.alias, remotePath, config);
     writeWritableConfig(file, writableConfig);
     return textResult({
       saved: true,
@@ -1016,15 +1032,20 @@ async function callTool(name, args) {
     return textResult(await runSsh(config, command, name));
   }
   if (name === "remote_search_text") {
+    if (typeof args.query !== "string" || args.query.length === 0) {
+      throw new Error("remote_search_text requires a non-empty query string.");
+    }
     const target = resolveRemoteWorkspacePath(config, args.root, args.path || ".", "search");
     const limit = Math.max(1, Math.min(Number(args.limit || 200), 1000));
-    const fixed = args.regex ? "" : "-F";
+    const rgMode = args.regex ? "" : "-F ";
     const grepMode = args.regex ? "-E" : "-F";
+    const quotedQuery = shellQuote(args.query);
+    const quotedTarget = shellQuote(target);
     const command = [
       "if command -v rg >/dev/null 2>&1; then",
-      `rg --line-number --hidden --glob '!.git' ${fixed} ${shellQuote(args.query)} ${shellQuote(target)} | head -n ${limit};`,
+      `rg --line-number --hidden --glob '!.git' ${rgMode}-e ${quotedQuery} -- ${quotedTarget} | head -n ${limit};`,
       "else",
-      `grep -RIn ${grepMode} --exclude-dir=.git ${shellQuote(args.query)} ${shellQuote(target)} | head -n ${limit};`,
+      `grep -RIn ${grepMode} --exclude-dir=.git -e ${quotedQuery} -- ${quotedTarget} | head -n ${limit};`,
       "fi",
     ].join(" ");
     return textResult(await runSsh(config, command, name));
@@ -1076,11 +1097,15 @@ async function callTool(name, args) {
     if (!config.allowWrites) {
       throw new Error(`Writes are disabled for host alias ${config.alias}. Set allowWrites=true to enable.`);
     }
+    if (typeof args.content !== "string") {
+      throw new Error("remote_write_file requires content as a UTF-8 string.");
+    }
     const remotePath = assertPathAllowed(config, args.path, "write");
     const encoded = Buffer.from(args.content, "utf8").toString("base64");
-    const operator = args.overwrite ? ">" : ">";
-    const guard = args.overwrite ? "" : `test ! -e ${shellQuote(remotePath)} && `;
-    const writeCommand = `${guard}printf %s ${shellQuote(encoded)} | base64 -d ${operator} ${shellQuote(remotePath)}`;
+    const quotedPath = shellQuote(remotePath);
+    const writeCommand = args.overwrite
+      ? `printf %s ${shellQuote(encoded)} | base64 -d > ${quotedPath}`
+      : `set -C; printf %s ${shellQuote(encoded)} | base64 -d > ${quotedPath}`;
     return textResult(await runSsh(config, writeCommand, name));
   }
 
@@ -1127,10 +1152,12 @@ async function handle(request) {
   if (request.method === "tools/call") {
     return callTool(request.params.name, request.params.arguments || {});
   }
-  if (request.method === "notifications/initialized") {
+  if (request.method && request.method.startsWith("notifications/")) {
     return {};
   }
-  return {};
+  const err = new Error(`Method not found: ${request.method}`);
+  err.code = -32601;
+  throw err;
 }
 
 function writeResponse(id, result) {
@@ -1139,12 +1166,13 @@ function writeResponse(id, result) {
 }
 
 function writeError(id, error) {
-  if (id === undefined || id === null) return;
+  if (id === undefined) return;
+  const code = Number.isInteger(error && error.code) ? error.code : -32000;
   process.stdout.write(
     JSON.stringify({
       jsonrpc: "2.0",
-      id,
-      error: { code: -32000, message: error.message },
+      id: id === undefined ? null : id,
+      error: { code, message: error.message },
     }) + "\n",
   );
 }
@@ -1157,10 +1185,17 @@ function startServer() {
     let request;
     try {
       request = JSON.parse(line);
+    } catch (error) {
+      const parseError = new Error(`Parse error: ${error.message}`);
+      parseError.code = -32700;
+      writeError(null, parseError);
+      return;
+    }
+    try {
       const result = await handle(request);
       writeResponse(request.id, result);
     } catch (error) {
-      writeError(request && request.id, error);
+      writeError(request.id, error);
     }
   });
 

plugins/remote-ssh/test/remote-ssh-mcp.test.js

@@ -141,10 +141,42 @@ assert.deepEqual(selected.allowedPaths, ["/home/mehedi", "/home/mehedi/projects/
 assert.equal(selected.identityFile, "~/.ssh/id_ed25519");
 assert.throws(() => selectWorkspaceInConfig(writableConfig, "missing", "/home/mehedi"), /does not exist/);
 
+const writableConfigEmpty = { hosts: {} };
+const seedProfile = {
+  alias: "envbox",
+  user: "envuser",
+  host: "10.0.0.5",
+  identityFile: "~/.ssh/id_ed25519",
+  allowedPaths: ["/srv"],
+  allowWrites: false,
+};
+const seededSelection = selectWorkspaceInConfig(writableConfigEmpty, "envbox", "/srv/app", seedProfile);
+assert.equal(seededSelection.workspaceRoot, "/srv/app");
+assert.deepEqual(seededSelection.allowedPaths, ["/srv", "/srv/app"]);
+assert.equal(seededSelection.user, "envuser");
+assert.equal(seededSelection.host, "10.0.0.5");
+assert.equal(seededSelection.alias, undefined, "alias field should not leak into the saved profile");
+
+const browseDir = tools.find((tool) => tool.name === "remote_browse_dir");
+assert.deepEqual(browseDir.inputSchema.required, ["host"], "remote_browse_dir should only require 'host'");
+
 handle({ jsonrpc: "2.0", id: 1, method: "tools/list" }).then((response) => {
   assert.ok(response.tools.length >= 20);
   return handle({ jsonrpc: "2.0", id: 2, method: "resources/read", params: { uri: "ui://remote-ssh/folder-picker.html" } });
 }).then((response) => {
   assert.match(response.contents[0].text, /Remote SSH Folder Picker/);
+  return handle({ jsonrpc: "2.0", id: 3, method: "notifications/initialized" });
+}).then((response) => {
+  assert.deepEqual(response, {}, "notifications/* should return empty result");
+  return handle({ jsonrpc: "2.0", id: 4, method: "totally/unknown" }).then(
+    () => {
+      throw new Error("Unknown methods should throw with -32601");
+    },
+    (error) => {
+      assert.equal(error.code, -32601, "unknown method should carry JSON-RPC -32601 code");
+      assert.match(error.message, /Method not found/);
+    },
+  );
+}).then(() => {
   console.log("remote-ssh-mcp tests passed");
 });