From abb70efe988a04305a8baa18ce07052c36fcc566 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 8 May 2026 18:11:14 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20command=20injection=20in=20pm2=20API=20route?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced `exec` with `execFile` passing child process arguments as arrays, and validated that `appName` does not start with a hyphen to prevent flag injection, closing a shell command injection vector. Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com> --- .jules/sentinel.md | 5 +++++ src/pages/api/pm2.ts | 51 ++++++++++++++++++++++---------------------- 2 files changed, 31 insertions(+), 25 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..84531f81 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,5 @@ + +## 2024-05-18 - PM2 API Command Injection Prevention +**Vulnerability:** Found a shell injection vulnerability in `src/pages/api/pm2.ts` where user inputs (appName, scriptPath, env properties) were directly string concatenated into an `exec` command without sanitization. +**Learning:** Concatenating paths and user-provided environment values into a shell execution via `exec` allows an attacker to trivially bypass constraints by appending arbitrary shell commands. This affects local endpoints wrapping complex CLI utilities (like `pm2`). +**Prevention:** Instead of using shell interpolation via `exec`, use `execFile` or `spawn` to run the underlying executable (like `npx` or `npm`) by passing all arguments as an array (`['pm2', 'start', ...]`). Also, pass directory constraints via the `cwd` option, safely pass environment variables through the `env` option (extending `process.env`), and validate user parameters (such as `appName`) to ensure they don't start with a hyphen (`-`) to avoid flag injection. diff --git a/src/pages/api/pm2.ts b/src/pages/api/pm2.ts index 47f46113..d7cb8e87 100644 --- a/src/pages/api/pm2.ts +++ b/src/pages/api/pm2.ts @@ -1,9 +1,9 @@ import type { APIRoute } from 'astro'; -import { exec } from 'node:child_process'; +import { execFile } from 'node:child_process'; import util from 'node:util'; import fs from 'node:fs'; -const execPromise = util.promisify(exec); +const execFilePromise = util.promisify(execFile); export const POST: APIRoute = async ({ request }) => { try { @@ -13,17 +13,16 @@ export const POST: APIRoute = async ({ request }) => { return new Response(JSON.stringify({ error: 'action and appName are required' }), { status: 400 }); } - let command = ''; + // 🛡️ Sentinel: Input validation to prevent flag injection in PM2 CLI + if (appName.startsWith('-')) { + return new Response(JSON.stringify({ error: 'appName cannot start with a hyphen' }), { status: 400 }); + } if (action === 'start' || action === 'start_prod') { if (!scriptPath) return new Response(JSON.stringify({ error: 'scriptPath required to start' }), { status: 400 }); - // Build env variables string - let envString = ''; - if (env) { - for (const [key, value] of Object.entries(env)) { - envString += `${key}="${value}" `; - } - } + + // Prepare secure environment dictionary instead of string concatenation + const childEnv = { ...process.env, ...(env || {}) }; let npmScript = action === 'start_prod' ? 'start' : 'dev'; @@ -60,24 +59,26 @@ export const POST: APIRoute = async ({ request }) => { } catch(e) {} if (hasBuild) { - command = `cd ${scriptPath} && npm run build && ${envString} npx -y pm2 start npm --name "${appName}" -- run ${npmScript}`; - } else { - command = `cd ${scriptPath} && ${envString} npx -y pm2 start npm --name "${appName}" -- run ${npmScript}`; + // 🛡️ Sentinel: Using execFile with array of args to prevent shell injection + await execFilePromise('npm', ['run', 'build'], { cwd: scriptPath, env: childEnv }); } - } else { - command = `cd ${scriptPath} && ${envString} npx -y pm2 start npm --name "${appName}" -- run ${npmScript}`; } - } else if (action === 'stop') { - command = `npx -y pm2 stop "${appName}"`; - } else if (action === 'delete') { - command = `npx -y pm2 delete "${appName}"`; - } else if (action === 'restart') { - command = `npx -y pm2 restart "${appName}"`; - } else { - return new Response(JSON.stringify({ error: 'Unknown action' }), { status: 400 }); + + const { stdout, stderr } = await execFilePromise('npx', ['-y', 'pm2', 'start', 'npm', '--name', appName, '--', 'run', npmScript], { cwd: scriptPath, env: childEnv }); + return new Response(JSON.stringify({ status: 'ok', stdout, stderr }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); } - const { stdout, stderr } = await execPromise(command); + let pm2Action = ''; + if (action === 'stop') pm2Action = 'stop'; + else if (action === 'delete') pm2Action = 'delete'; + else if (action === 'restart') pm2Action = 'restart'; + else return new Response(JSON.stringify({ error: 'Unknown action' }), { status: 400 }); + + // 🛡️ Sentinel: Using execFile with array of args to prevent shell injection + const { stdout, stderr } = await execFilePromise('npx', ['-y', 'pm2', pm2Action, appName]); return new Response(JSON.stringify({ status: 'ok', stdout, stderr }), { status: 200, @@ -93,7 +94,7 @@ export const POST: APIRoute = async ({ request }) => { export const GET: APIRoute = async () => { try { - const { stdout } = await execPromise('npx -y pm2 jlist'); + const { stdout } = await execFilePromise('npx', ['-y', 'pm2', 'jlist']); const list = JSON.parse(stdout); return new Response(JSON.stringify(list), { status: 200,