mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-05 06:52:56 +00:00
* fix(exec): parse nested approval metadata in followups
(cherry picked from commit 10ff9b318e77cda3d65f40d59bbab0f4a3f59da8)
* docs(changelog): note exec approval nested-paren parser fix
* fix(exec): sanitize denied-reason literals in (...)-delimited approval messages
The exec-approval followup wire format is `Exec denied (gateway id=..., <deniedReason>): cmd`. The producer at `src/agents/bash-tools.exec-host-gateway.ts:606` was emitting `approval-timeout (allowlist-miss)`, which embedded literal parens inside the metadata segment and broke the metadata/body boundary for naive parsers. Switch the literal to a colon-separated form (`approval-timeout: allowlist-miss`) so the surrounding `(...)` delimiter stays unambiguous.
The Gateway node-event surface at `src/gateway/server-node-events.ts:734` interpolates an untrusted `obj.reason` into the same `Exec denied (node=..., <reason>)` format. Strip parens from that field before interpolation so a buggy or hostile node payload cannot smuggle metadata into the body slot.
The robust nested-paren parser already in `src/agents/exec-approval-result.ts` stays as defense in depth. Extend `exec-approval-result.test.ts` to cover the canonical colon-separated `deniedReason` and confirm `formatExecDeniedUserMessage` still maps it to the timeout copy.
* fix(exec): require gateway/node metadata source to reject spoofed approval wrappers
The exec-approval result parser previously accepted any string starting with
"Exec denied (..." or "Exec finished (..." as a structured approval wrapper.
Generic command stdout that happened to start with these tokens would be
classified as kind: "denied" or "finished", letting a tool's output spoof a
resolved-approval event in pi-embedded-subscribe.handlers.tools.ts:1173.
Reported by Aisle as CWE-841 (Improper Enforcement of Behavioral Workflow),
medium severity. The fix validates that the parenthesized metadata starts with
either "gateway id=" or "node=" — both prefixes are emitted by the legitimate
approval generators (bash-tools.exec-host-gateway.ts, bash-tools.exec-host-node.ts,
gateway/server-node-events.ts) and are unlikely to appear in arbitrary command
output. Inputs that fail this check now return kind: "other", which all callers
already handle as a no-op.
* fix(exec): keep sandbox_blocked classification for raw exec-denied messages
After the spoof-guard tightening of parseExecApprovalResultText, inputs that
lack a gateway/node-sourced metadata prefix (such as the synthetic
"exec denied (allowlist-miss):" string used in classifier tests) no longer
return kind: "denied" and therefore no longer trigger formatExecDeniedUserMessage,
so isSandboxBlockedErrorMessage stopped recognising them.
Add a direct \bexec denied\s*\( alternative to SANDBOX_BLOCKED_RE so the
classifier still treats any raw "exec denied (" prefix as sandbox-blocked,
independent of whether the parser accepts the surrounding wrapper. This keeps
classifyProviderRuntimeFailureKind's existing behavior for unstructured exec-
denied messages.
161 lines
4.1 KiB
TypeScript
161 lines
4.1 KiB
TypeScript
import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js";
|
|
|
|
type ExecApprovalResult =
|
|
| {
|
|
kind: "denied";
|
|
raw: string;
|
|
metadata: string;
|
|
body: string;
|
|
}
|
|
| {
|
|
kind: "finished";
|
|
raw: string;
|
|
metadata: string;
|
|
body: string;
|
|
}
|
|
| {
|
|
kind: "completed";
|
|
raw: string;
|
|
body: string;
|
|
}
|
|
| {
|
|
kind: "other";
|
|
raw: string;
|
|
};
|
|
|
|
const EXEC_COMPLETED_RE = /^exec completed:\s*([\s\S]*)$/i;
|
|
|
|
// Approval-system-generated wrappers always start with either `gateway id=` or
|
|
// `node=` inside the parenthesized metadata (see bash-tools.exec-host-gateway.ts,
|
|
// bash-tools.exec-host-node.ts, and gateway/server-node-events.ts). Untrusted
|
|
// command stdout that happens to start with "Exec denied (...)" or
|
|
// "Exec finished (...)" should be rejected by the parser to prevent CWE-841
|
|
// spoofed approval events from arbitrary tool output.
|
|
const APPROVAL_METADATA_SOURCE_RE = /^(?:gateway\s+id=|node=)/i;
|
|
|
|
function parseExecApprovalResultWithMetadata(
|
|
raw: string,
|
|
prefix: string,
|
|
bodySeparator: ":" | "\n",
|
|
): { metadata: string; body: string } | null {
|
|
const normalizedRaw = normalizeLowercaseStringOrEmpty(raw);
|
|
const normalizedPrefix = normalizeLowercaseStringOrEmpty(prefix);
|
|
if (!normalizedRaw.startsWith(normalizedPrefix)) {
|
|
return null;
|
|
}
|
|
|
|
const metadataStart = prefix.length;
|
|
let depth = 1;
|
|
let metadataEnd = -1;
|
|
for (let index = metadataStart; index < raw.length; index += 1) {
|
|
const char = raw[index];
|
|
if (char === "(") {
|
|
depth += 1;
|
|
continue;
|
|
}
|
|
if (char === ")") {
|
|
depth -= 1;
|
|
if (depth === 0) {
|
|
metadataEnd = index;
|
|
break;
|
|
}
|
|
}
|
|
}
|
|
|
|
if (metadataEnd < 0) {
|
|
return null;
|
|
}
|
|
|
|
const metadata = raw.slice(metadataStart, metadataEnd).trim();
|
|
if (!APPROVAL_METADATA_SOURCE_RE.test(metadata)) {
|
|
return null;
|
|
}
|
|
|
|
const remainder = raw.slice(metadataEnd + 1);
|
|
if (bodySeparator === ":") {
|
|
if (!remainder.startsWith(":")) {
|
|
return null;
|
|
}
|
|
return {
|
|
metadata,
|
|
body: remainder.slice(1).trim(),
|
|
};
|
|
}
|
|
|
|
if (remainder && !remainder.startsWith("\n")) {
|
|
return null;
|
|
}
|
|
|
|
return {
|
|
metadata,
|
|
body: remainder.startsWith("\n") ? remainder.slice(1).trim() : "",
|
|
};
|
|
}
|
|
|
|
export function parseExecApprovalResultText(resultText: string): ExecApprovalResult {
|
|
const raw = resultText.trim();
|
|
if (!raw) {
|
|
return { kind: "other", raw };
|
|
}
|
|
|
|
const deniedResult = parseExecApprovalResultWithMetadata(raw, "Exec denied (", ":");
|
|
if (deniedResult) {
|
|
return {
|
|
kind: "denied",
|
|
raw,
|
|
metadata: deniedResult.metadata,
|
|
body: deniedResult.body,
|
|
};
|
|
}
|
|
|
|
const finishedResult = parseExecApprovalResultWithMetadata(raw, "Exec finished (", "\n");
|
|
if (finishedResult) {
|
|
return {
|
|
kind: "finished",
|
|
raw,
|
|
metadata: finishedResult.metadata,
|
|
body: finishedResult.body,
|
|
};
|
|
}
|
|
|
|
const completedMatch = EXEC_COMPLETED_RE.exec(raw);
|
|
if (completedMatch) {
|
|
return {
|
|
kind: "completed",
|
|
raw,
|
|
body: completedMatch[1]?.trim() ?? "",
|
|
};
|
|
}
|
|
|
|
return { kind: "other", raw };
|
|
}
|
|
|
|
export function isExecDeniedResultText(resultText: string): boolean {
|
|
return parseExecApprovalResultText(resultText).kind === "denied";
|
|
}
|
|
|
|
export function formatExecDeniedUserMessage(resultText: string): string | null {
|
|
const parsed = parseExecApprovalResultText(resultText);
|
|
if (parsed.kind !== "denied") {
|
|
return null;
|
|
}
|
|
|
|
const metadata = normalizeLowercaseStringOrEmpty(parsed.metadata);
|
|
if (metadata.includes("approval-timeout")) {
|
|
return "Command did not run: approval timed out.";
|
|
}
|
|
if (metadata.includes("user-denied")) {
|
|
return "Command did not run: approval was denied.";
|
|
}
|
|
if (metadata.includes("allowlist-miss")) {
|
|
return "Command did not run: approval is required.";
|
|
}
|
|
if (metadata.includes("approval-request-failed")) {
|
|
return "Command did not run: approval request failed.";
|
|
}
|
|
if (metadata.includes("spawn-failed") || metadata.includes("invoke-failed")) {
|
|
return "Command did not run.";
|
|
}
|
|
return "Command did not run.";
|
|
}
|