Greptile Summary

This PR fixes a regression introduced by the GHSA-h3f9-mjwj-w476 security fix, where rawCommand was not updated after hardenApprovedExecutionPaths pinned a bare executable name (e.g. poccmd) to its absolute realpath (e.g. /usr/bin/echo). Because the consistency validator compares rawCommand against formatExecCommand(argv), having a stale rawCommand (still containing the bare name) caused every direct-command invocation via nodes.run to fail with RAW_COMMAND_MISMATCH.

Key changes:

• buildSystemRunApprovalPlan now checks whether hardening changed the argv array via reference equality (hardening.argv === command.argv); if so, it regenerates rawCommand using formatExecCommand(hardening.argv) so validation will pass downstream.
• The display-facing cmdText field returned to callers is intentionally left as the pre-hardening user-readable command text (unaffected by the fix).
• A new build-plan mode test case installs a poccmd → /bin/echo symlink into PATH and asserts plan.rawCommand === formatExecCommand(plan.argv) after the plan is built, directly covering the fixed code path.

Confidence Score: 4/5

• Safe to merge — the fix is minimal, targeted, and correctly restores nodes.run direct-command functionality broken by the prior security patch.
• The reference-equality check (hardening.argv === command.argv) is valid and well-grounded in the implementation of hardenApprovedExecutionPaths, which always returns the original params.argv reference when no pinning occurs and allocates a new array only when it does. The regenerated rawCommand via formatExecCommand is exactly what the downstream consistency validator computes, so the fix closes the mismatch without introducing new divergence. The test case verifies the invariant directly. One point off because the test does not explicitly assert the pre-fix broken state (i.e., it would also pass if rawCommand were never set at all), and the end-to-end path through the approval socket is not unit-tested here.
• No files require special attention.

_{Last reviewed commit: b25ad3b}

Merged via squash.

• Prepared head SHA: a798790
• Merge commit: c8ebd48

Thanks @Sid-Qin!

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟡 Control-character/log injection risk via rawCommand regeneration using formatExecCommand

Description

buildSystemRunApprovalPlan(...) now regenerates plan.rawCommand from the hardened argv when PATH pinning rewrites argv[0].

Because formatExecCommand(argv) only quotes whitespace / " and does not escape control characters (e.g. \n, \r, \t, ANSI \x1b), an attacker who can influence argv tokens or the resolved executable path (e.g. by placing a weirdly-named executable in a writable PATH directory) can cause rawCommand to contain embedded newlines/control characters.

Security impact (grounded in current flows):

• plan.rawCommand is used as human-facing approval text (gateway uses plan.rawCommand ?? formatExecCommand(plan.argv) as the ExecApprovalRequestPayload.command).
• cmdText/command strings are also propagated into exec-related system events (exec.denied/exec.started) and may be surfaced in operator UIs or logs.
• Control characters in those strings can enable log/UI injection (multi-line spoofing, confusing approvals, terminal escape sequence injection in non-Discord sinks), and can also create ambiguity if any downstream component ever re-parses rawCommand as a shell command.

Vulnerable code (new behavior):

const rawCommand = hardening.argvChanged
  ? formatExecCommand(hardening.argv) || null
  : command.cmdText.trim() || null;

Relevant formatter behavior:

const needsQuotes = /\s|"/.test(arg);
return `"${arg.replace(/"/g, "\\\"")}"`;
​// (no escaping/sanitization for \r, \n, \t, \x1b, etc.)

Recommendation

Treat rawCommand as display-only and ensure it is safe and unambiguous for UIs/logs.

Options (in increasing robustness):

1. Escape control characters for display (at minimum \r, \n, \t, \x1b, and other C0 controls) before embedding into approval prompts/system events.

2. Prefer a reversible/collision-resistant encoding for display, e.g. render argv as JSON so control characters are unambiguously escaped:

export function formatExecCommandForDisplay(argv: string[]): string {
  ​// JSON escaping distinguishes newline vs literal "\\n".
  return argv.map((a) => JSON.stringify(a)).join(" ");
}

3. Keep storing/approving against structured argv (already done) and avoid any downstream code re-parsing rawCommand as an execution string.

If you must keep a shell-like rendering, implement a dedicated escaping layer that:

• encodes control chars (\n -> \\n, \r -> \\r, \x1b -> \\x1b, etc.)
• is deterministic and (ideally) reversible
• is applied consistently anywhere rawCommand/cmdText is displayed or logged.

Analyzed PR: #33137 at commit a798790

_{Last updated on: 2026-03-04T17:23:36Z}