From 9785e4f96541b597b2ebd2e5e7f04ca75ab05767 Mon Sep 17 00:00:00 2001 From: Sai Sharan Dugini Date: Thu, 26 Mar 2026 15:17:25 +0530 Subject: [PATCH] fix: validate openshell binary to prevent npm package shadowing - Add isOpenshellCLI() check to verify the resolved binary is the real OpenShell CLI (via --version output), not the npm gateway package - Surface clear error message with manual install steps when install-openshell.sh fails Fixes #967 --- bin/lib/onboard.js | 8 ++++++++ bin/lib/resolve-openshell.js | 32 +++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index e74d49921..119601ca2 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -1111,6 +1111,14 @@ function installOpenshell() { if (output) { console.error(output); } + console.error(""); + console.error(" OpenShell CLI binary failed to install."); + console.error(" You can install it manually:"); + console.error(""); + console.error(' curl -fsSL "https://github.com/NVIDIA/OpenShell/releases/latest/download/openshell-$(uname -m)-unknown-linux-musl.tar.gz" -o /tmp/openshell.tar.gz'); + console.error(" tar xzf /tmp/openshell.tar.gz -C /tmp"); + console.error(" install -m 755 /tmp/openshell /usr/local/bin/openshell"); + console.error(""); return { installed: false, localBin: null, futureShellPathHint: null }; } const localBin = process.env.XDG_BIN_HOME || path.join(process.env.HOME || "", ".local", "bin"); diff --git a/bin/lib/resolve-openshell.js b/bin/lib/resolve-openshell.js index 345e218e4..c8e5d9bfe 100644 --- a/bin/lib/resolve-openshell.js +++ b/bin/lib/resolve-openshell.js @@ -4,26 +4,52 @@ const { execSync } = require("child_process"); const fs = require("fs"); +/** + * Verify that the binary at `binPath` is the OpenShell CLI and not another + * package that happens to share the same name (e.g. the npm `openshell` + * gateway package installed as a transitive dependency of `openclaw`). + * + * The OpenShell CLI prints a version string starting with "openshell " + * when invoked with `--version`. + */ +function isOpenshellCLI(binPath) { + try { + const out = execSync(`"${binPath}" --version`, { + encoding: "utf-8", + timeout: 5000, + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + return /^openshell\s+\d+/.test(out); + } catch { + return false; + } +} + /** * Resolve the openshell binary path. * * Checks `command -v` first (must return an absolute path to prevent alias * injection), then falls back to common installation directories. * + * Every candidate is verified with `isOpenshellCLI()` to ensure the resolved + * binary is the real OpenShell CLI and not a same-named npm package. + * * @param {object} [opts] DI overrides for testing * @param {string|null} [opts.commandVResult] Mock result (undefined = run real command) * @param {function} [opts.checkExecutable] (path) => boolean + * @param {function} [opts.checkCLI] (path) => boolean — override for `isOpenshellCLI` * @param {string} [opts.home] HOME override * @returns {string|null} Absolute path to openshell, or null if not found */ function resolveOpenshell(opts = {}) { const home = opts.home ?? process.env.HOME; + const checkCLI = opts.checkCLI || isOpenshellCLI; // Step 1: command -v if (opts.commandVResult === undefined) { try { const found = execSync("command -v openshell", { encoding: "utf-8" }).trim(); - if (found.startsWith("/")) return found; + if (found.startsWith("/") && checkCLI(found)) return found; } catch { /* ignored */ } } else if (opts.commandVResult && opts.commandVResult.startsWith("/")) { return opts.commandVResult; @@ -40,10 +66,10 @@ function resolveOpenshell(opts = {}) { "/usr/bin/openshell", ]; for (const p of candidates) { - if (checkExecutable(p)) return p; + if (checkExecutable(p) && checkCLI(p)) return p; } return null; } -module.exports = { resolveOpenshell }; +module.exports = { resolveOpenshell, isOpenshellCLI };