diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index f3d9d9514..a9107f39e 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -2,8 +2,6 @@ ## Summary -Signed-off-by: Your Name - ## Related Issue @@ -42,3 +40,7 @@ Signed-off-by: Your Name - [ ] Follows the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md). Try running the `update-docs` agent skill to draft changes while complying with the style guide. For example, prompt your agent with "`/update-docs` catch up the docs for the new changes I made in this PR." - [ ] New pages include SPDX license header and frontmatter, if creating a new page. - [ ] Cross-references and links verified. + +--- + +Signed-off-by: Your Name diff --git a/bin/nemoclaw.js b/bin/nemoclaw.js index 77118bdd4..f587e88d6 100755 --- a/bin/nemoclaw.js +++ b/bin/nemoclaw.js @@ -502,16 +502,31 @@ function uninstall(args) { exitWithSpawnResult(result); } + // Download to file before execution — prevents partial-download execution. + // Upstream URL is a rolling release so SHA-256 pinning isn't practical. console.log(` Local uninstall script not found; falling back to ${REMOTE_UNINSTALL_URL}`); - const forwardedArgs = args.map(shellQuote).join(" "); - const command = forwardedArgs.length > 0 - ? `curl -fsSL ${shellQuote(REMOTE_UNINSTALL_URL)} | bash -s -- ${forwardedArgs}` - : `curl -fsSL ${shellQuote(REMOTE_UNINSTALL_URL)} | bash`; - const result = spawnSync("bash", ["-c", command], { - stdio: "inherit", - cwd: ROOT, - env: process.env, - }); + const uninstallDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-uninstall-")); + const uninstallScript = path.join(uninstallDir, "uninstall.sh"); + let result; + let downloadFailed = false; + try { + try { + execFileSync("curl", ["-fsSL", REMOTE_UNINSTALL_URL, "-o", uninstallScript], { stdio: "inherit" }); + } catch { + console.error(` Failed to download uninstall script from ${REMOTE_UNINSTALL_URL}`); + downloadFailed = true; + } + if (!downloadFailed) { + result = spawnSync("bash", [uninstallScript, ...args], { + stdio: "inherit", + cwd: ROOT, + env: process.env, + }); + } + } finally { + fs.rmSync(uninstallDir, { recursive: true, force: true }); + } + if (downloadFailed) process.exit(1); exitWithSpawnResult(result); } diff --git a/install.sh b/install.sh index 70948ad2b..2eae8ca34 100755 --- a/install.sh +++ b/install.sh @@ -485,14 +485,28 @@ install_or_upgrade_ollama() { info "Ollama v${current} meets minimum requirement (>= v${OLLAMA_MIN_VERSION})" else info "Ollama v${current:-unknown} is below v${OLLAMA_MIN_VERSION} — upgrading…" - curl -fsSL https://ollama.com/install.sh | sh + # Upstream URL is a rolling release so SHA-256 pinning isn't practical, + # but download-then-execute allows inspection and prevents partial-download execution. + ( + tmpdir="$(mktemp -d)" + trap 'rm -rf "$tmpdir"' EXIT + curl -fsSL https://ollama.com/install.sh -o "$tmpdir/install_ollama.sh" + sh "$tmpdir/install_ollama.sh" + ) info "Ollama upgraded to $(get_ollama_version)" fi else # No ollama — only install if a GPU is present if detect_gpu; then info "GPU detected — installing Ollama…" - curl -fsSL https://ollama.com/install.sh | sh + # Upstream URL is a rolling release so SHA-256 pinning isn't practical, + # but download-then-execute allows inspection and prevents partial-download execution. + ( + tmpdir="$(mktemp -d)" + trap 'rm -rf "$tmpdir"' EXIT + curl -fsSL https://ollama.com/install.sh -o "$tmpdir/install_ollama.sh" + sh "$tmpdir/install_ollama.sh" + ) info "Ollama installed: v$(get_ollama_version)" else warn "No GPU detected — skipping Ollama installation." diff --git a/scripts/brev-setup.sh b/scripts/brev-setup.sh index 1d1367086..e31fa0c46 100755 --- a/scripts/brev-setup.sh +++ b/scripts/brev-setup.sh @@ -40,7 +40,14 @@ export DEBIAN_FRONTEND=noninteractive # --- 0. Node.js (needed for services) --- if ! command -v node >/dev/null 2>&1; then info "Installing Node.js..." - curl -fsSL https://deb.nodesource.com/setup_22.x | sudo -E bash - >/dev/null 2>&1 + # Upstream URL is a rolling release so SHA-256 pinning isn't practical, + # but download-then-execute allows inspection and prevents partial-download execution. + ( + tmpdir="$(mktemp -d)" + trap 'rm -rf "$tmpdir"' EXIT + curl -fsSL https://deb.nodesource.com/setup_22.x -o "$tmpdir/setup_node.sh" + sudo -E bash "$tmpdir/setup_node.sh" >/dev/null 2>&1 + ) sudo apt-get install -y -qq nodejs >/dev/null 2>&1 info "Node.js $(node --version) installed" else diff --git a/test/runner.test.js b/test/runner.test.js index 12b50cc2e..f9222f0c4 100644 --- a/test/runner.test.js +++ b/test/runner.test.js @@ -300,4 +300,61 @@ describe("regression guards", () => { expect(src).toContain("shasum -a 256 -c"); }); }); + + describe("curl-pipe-to-shell guards (#574, #583)", () => { + // Strip comment lines, then join line continuations so multiline + // curl ... |\n bash patterns are caught by the single-line regex. + const stripComments = (src, commentPrefix) => + src.split("\n").filter((l) => !l.trim().startsWith(commentPrefix)).join("\n"); + + const joinContinuations = (src) => + src.replace(/\\\n\s*/g, " "); + + const collapseMultilinePipes = (src) => + src.replace(/\|\s*\n\s*/g, "| "); + + const normalize = (src, commentPrefix) => + collapseMultilinePipes(joinContinuations(stripComments(src, commentPrefix))); + + const shellViolationRe = /curl\s[^|]*\|\s*(sh|bash|sudo\s+(-\S+\s+)*(sh|bash))\b/; + const jsViolationRe = /curl.*\|\s*(sh|bash|sudo\s+(-\S+\s+)*(sh|bash))\b/; + + const findShellViolations = (src) => { + const normalized = normalize(src, "#"); + return normalized.split("\n").filter((line) => { + const t = line.trim(); + if (t.startsWith("printf") || t.startsWith("echo")) return false; + return shellViolationRe.test(t); + }); + }; + + const findJsViolations = (src) => { + const normalized = normalize(src, "//"); + return normalized.split("\n").filter((line) => { + const t = line.trim(); + if (t.startsWith("*")) return false; + return jsViolationRe.test(t); + }); + }; + + it("install.sh does not pipe curl to shell", () => { + const src = fs.readFileSync(path.join(import.meta.dirname, "..", "install.sh"), "utf-8"); + expect(findShellViolations(src)).toEqual([]); + }); + + it("scripts/install.sh does not pipe curl to shell", () => { + const src = fs.readFileSync(path.join(import.meta.dirname, "..", "scripts", "install.sh"), "utf-8"); + expect(findShellViolations(src)).toEqual([]); + }); + + it("scripts/brev-setup.sh does not pipe curl to shell", () => { + const src = fs.readFileSync(path.join(import.meta.dirname, "..", "scripts", "brev-setup.sh"), "utf-8"); + expect(findShellViolations(src)).toEqual([]); + }); + + it("bin/nemoclaw.js does not pipe curl to shell", () => { + const src = fs.readFileSync(path.join(import.meta.dirname, "..", "bin", "nemoclaw.js"), "utf-8"); + expect(findJsViolations(src)).toEqual([]); + }); + }); });