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().
This commit is contained in:
@@ -809,7 +809,8 @@ def _get_bridge_dir() -> Path:
|
|||||||
return user_bridge
|
return user_bridge
|
||||||
|
|
||||||
# Check for npm
|
# 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]")
|
console.print("[red]npm not found. Please install Node.js >= 18.[/red]")
|
||||||
raise typer.Exit(1)
|
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"))
|
shutil.copytree(source, user_bridge, ignore=shutil.ignore_patterns("node_modules", "dist"))
|
||||||
|
|
||||||
# Install and build
|
# Install and build
|
||||||
is_win = sys.platform == "win32"
|
|
||||||
try:
|
try:
|
||||||
console.print(" Installing dependencies...")
|
console.print(" Installing dependencies...")
|
||||||
if is_win:
|
subprocess.run([npm_path, "install"], cwd=user_bridge, check=True, capture_output=True)
|
||||||
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)
|
|
||||||
|
|
||||||
console.print(" Building...")
|
console.print(" Building...")
|
||||||
if is_win:
|
subprocess.run([npm_path, "run", "build"], cwd=user_bridge, check=True, capture_output=True)
|
||||||
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)
|
|
||||||
|
|
||||||
console.print("[green]✓[/green] Bridge ready\n")
|
console.print("[green]✓[/green] Bridge ready\n")
|
||||||
except subprocess.CalledProcessError as e:
|
except subprocess.CalledProcessError as e:
|
||||||
@@ -864,6 +858,7 @@ def _get_bridge_dir() -> Path:
|
|||||||
@channels_app.command("login")
|
@channels_app.command("login")
|
||||||
def channels_login():
|
def channels_login():
|
||||||
"""Link device via QR code."""
|
"""Link device via QR code."""
|
||||||
|
import shutil
|
||||||
import subprocess
|
import subprocess
|
||||||
|
|
||||||
from nanobot.config.loader import load_config
|
from nanobot.config.loader import load_config
|
||||||
@@ -882,15 +877,15 @@ def channels_login():
|
|||||||
env["BRIDGE_TOKEN"] = bridge_token
|
env["BRIDGE_TOKEN"] = bridge_token
|
||||||
env["AUTH_DIR"] = str(get_runtime_subdir("whatsapp-auth"))
|
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:
|
try:
|
||||||
if sys.platform == "win32":
|
subprocess.run([npm_path, "start"], cwd=bridge_dir, check=True, env=env)
|
||||||
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)
|
|
||||||
except subprocess.CalledProcessError as e:
|
except subprocess.CalledProcessError as e:
|
||||||
console.print(f"[red]Bridge failed: {e}[/red]")
|
console.print(f"[red]Bridge failed: {e}[/red]")
|
||||||
except FileNotFoundError:
|
|
||||||
console.print("[red]npm not found. Please install Node.js.[/red]")
|
|
||||||
|
|
||||||
|
|
||||||
# ============================================================================
|
# ============================================================================
|
||||||
|
|||||||
Reference in New Issue
Block a user