From eca404a7e82fc876cc3b61573ff8d0a94874c7c8 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:51:50 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20command=20injection=20risk=20in=20reconcile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: connerkup <72580335+connerkup@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ packages/core/src/loop.test.ts | 14 +++++++------- packages/core/src/reconcile.ts | 10 ++++++++-- 3 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..119d9f4 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-24 - [Command Injection] Replace execSync with spawnSync +**Vulnerability:** Found `execSync` being used with string concatenation in `reconcile.ts` and `loop.test.ts`. This poses a command injection risk, especially if variables like `tempFile` contain spaces or malicious shell characters. +**Learning:** Avoid using shell execution functions like `execSync` with dynamically constructed strings. Even with careful escaping, it's error-prone. +**Prevention:** Always use `spawnSync` (or `spawn`) with an array of arguments, which bypasses the shell and prevents command injection. Also prefer using `process.execPath` instead of relying on `"node"` being in the `PATH`. diff --git a/packages/core/src/loop.test.ts b/packages/core/src/loop.test.ts index f9d430f..379f772 100644 --- a/packages/core/src/loop.test.ts +++ b/packages/core/src/loop.test.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -173,12 +173,12 @@ describe("ledger benchmark e2e", () => { it("reconcile passes validator for correct ledger implementation", () => { const dir = setupWorkspace(); const mergeTarget = path.join(dir, "auth_service.js"); - execSync(`node "${fixturePath("mock-dev-ledger.mjs")}" "${path.join(dir, "developer_output.json")}"`, { - stdio: "pipe", - }); - execSync(`node "${fixturePath("mock-audit-pass.mjs")}" "${path.join(dir, "auditor_output.json")}"`, { - stdio: "pipe", - }); + + let res = spawnSync(process.execPath, [fixturePath("mock-dev-ledger.mjs"), path.join(dir, "developer_output.json")], { stdio: "pipe" }); + if (res.status !== 0) throw new Error("Mock dev ledger failed"); + + res = spawnSync(process.execPath, [fixturePath("mock-audit-pass.mjs"), path.join(dir, "auditor_output.json")], { stdio: "pipe" }); + if (res.status !== 0) throw new Error("Mock audit pass failed"); const result = reconcile({ workspace: dir, diff --git a/packages/core/src/reconcile.ts b/packages/core/src/reconcile.ts index b0285bc..2a95f89 100644 --- a/packages/core/src/reconcile.ts +++ b/packages/core/src/reconcile.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import crypto from "node:crypto"; import fs from "node:fs"; import path from "node:path"; @@ -139,7 +139,13 @@ export function reconcile(options: ReconcileOptions): ReconcileResult { fs.writeFileSync(tempFile, extractedCode, "utf8"); try { - execSync(`node --check "${tempFile}"`, { stdio: "pipe" }); + const result = spawnSync(process.execPath, ["--check", tempFile], { stdio: "pipe" }); + if (result.error) throw result.error; + if (result.status !== 0) { + const err = new Error("Command failed"); + (err as any).stderr = result.stderr; + throw err; + } log.audit("Syntax check passed"); } catch (err) { const stderr =