From 58fc34d3f42b483236d3819fe03d000aa5e2536a Mon Sep 17 00:00:00 2001
From: Peixian Gong <peixiangong@microsoft.com>
Date: Wed, 4 Mar 2026 13:43:30 +0800
Subject: [PATCH] refactor: use shutil.which() instead of shell=True for npm
 calls

Replace platform-specific shell=True logic with shutil.which('npm') to
resolve the full path to the npm executable. This is cleaner because:

- No shell=True needed (safer, no shell injection risk)
- No platform-specific branching (sys.platform checks removed)
- Works identically on Windows, macOS, and Linux
- shutil.which() resolves npm.cmd on Windows automatically

The npm path check that already existed in _get_bridge_dir() is now
reused as the resolved path for subprocess calls. The same pattern is
applied to channels_login().
---
 nanobot/cli/commands.py | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py
index 065eb71..e538688 100644
--- a/nanobot/cli/commands.py
+++ b/nanobot/cli/commands.py
@@ -809,7 +809,8 @@ def _get_bridge_dir() -> Path:
         return user_bridge
 
     # Check for npm
-    if not shutil.which("npm"):
+    npm_path = shutil.which("npm")
+    if not npm_path:
         console.print("[red]npm not found. Please install Node.js >= 18.[/red]")
         raise typer.Exit(1)
 
@@ -837,19 +838,12 @@ def _get_bridge_dir() -> Path:
     shutil.copytree(source, user_bridge, ignore=shutil.ignore_patterns("node_modules", "dist"))
 
     # Install and build
-    is_win = sys.platform == "win32"
     try:
         console.print("  Installing dependencies...")
-        if is_win:
-            subprocess.run("npm install", cwd=user_bridge, check=True, capture_output=True, shell=True)
-        else:
-            subprocess.run(["npm", "install"], cwd=user_bridge, check=True, capture_output=True)
+        subprocess.run([npm_path, "install"], cwd=user_bridge, check=True, capture_output=True)
 
         console.print("  Building...")
-        if is_win:
-            subprocess.run("npm run build", cwd=user_bridge, check=True, capture_output=True, shell=True)
-        else:
-            subprocess.run(["npm", "run", "build"], cwd=user_bridge, check=True, capture_output=True)
+        subprocess.run([npm_path, "run", "build"], cwd=user_bridge, check=True, capture_output=True)
 
         console.print("[green]✓[/green] Bridge ready\n")
     except subprocess.CalledProcessError as e:
@@ -864,6 +858,7 @@ def _get_bridge_dir() -> Path:
 @channels_app.command("login")
 def channels_login():
     """Link device via QR code."""
+    import shutil
     import subprocess
 
     from nanobot.config.loader import load_config
@@ -882,15 +877,15 @@ def channels_login():
         env["BRIDGE_TOKEN"] = bridge_token
     env["AUTH_DIR"] = str(get_runtime_subdir("whatsapp-auth"))
 
+    npm_path = shutil.which("npm")
+    if not npm_path:
+        console.print("[red]npm not found. Please install Node.js.[/red]")
+        raise typer.Exit(1)
+
     try:
-        if sys.platform == "win32":
-            subprocess.run("npm start", cwd=bridge_dir, check=True, env=env, shell=True)
-        else:
-            subprocess.run(["npm", "start"], cwd=bridge_dir, check=True, env=env)
+        subprocess.run([npm_path, "start"], cwd=bridge_dir, check=True, env=env)
     except subprocess.CalledProcessError as e:
         console.print(f"[red]Bridge failed: {e}[/red]")
-    except FileNotFoundError:
-        console.print("[red]npm not found. Please install Node.js.[/red]")
 
 
 # ============================================================================