mirror of
https://github.com/moltbot/moltbot.git
synced 2026-04-28 08:52:45 +00:00
fix: harden exec allowlist parsing
This commit is contained in:
@@ -128,6 +128,8 @@ Shell chaining and redirections are not auto-allowed in allowlist mode.
|
|||||||
|
|
||||||
Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfies the allowlist
|
Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfies the allowlist
|
||||||
(including safe bins or skill auto-allow). Redirections remain unsupported in allowlist mode.
|
(including safe bins or skill auto-allow). Redirections remain unsupported in allowlist mode.
|
||||||
|
Command substitution (`$()` / backticks) is rejected during allowlist parsing, including inside
|
||||||
|
double quotes; use single quotes if you need literal `$()` text.
|
||||||
|
|
||||||
Default safe bins: `jq`, `grep`, `cut`, `sort`, `uniq`, `head`, `tail`, `tr`, `wc`.
|
Default safe bins: `jq`, `grep`, `cut`, `sort`, `uniq`, `head`, `tail`, `tr`, `wc`.
|
||||||
|
|
||||||
|
|||||||
@@ -131,6 +131,36 @@ describe("exec approvals shell parsing", () => {
|
|||||||
expect(res.ok).toBe(true);
|
expect(res.ok).toBe(true);
|
||||||
expect(res.segments[0]?.argv).toEqual(["/bin/echo", "ok"]);
|
expect(res.segments[0]?.argv).toEqual(["/bin/echo", "ok"]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("rejects command substitution inside double quotes", () => {
|
||||||
|
const res = analyzeShellCommand({ command: 'echo "output: $(whoami)"' });
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
expect(res.reason).toBe("unsupported shell token: $()");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects backticks inside double quotes", () => {
|
||||||
|
const res = analyzeShellCommand({ command: 'echo "output: `id`"' });
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
expect(res.reason).toBe("unsupported shell token: `");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects command substitution outside quotes", () => {
|
||||||
|
const res = analyzeShellCommand({ command: "echo $(whoami)" });
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
expect(res.reason).toBe("unsupported shell token: $()");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows escaped command substitution inside double quotes", () => {
|
||||||
|
const res = analyzeShellCommand({ command: 'echo "output: \\$(whoami)"' });
|
||||||
|
expect(res.ok).toBe(true);
|
||||||
|
expect(res.segments[0]?.argv[0]).toBe("echo");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows command substitution syntax inside single quotes", () => {
|
||||||
|
const res = analyzeShellCommand({ command: "echo 'output: $(whoami)'" });
|
||||||
|
expect(res.ok).toBe(true);
|
||||||
|
expect(res.segments[0]?.argv[0]).toBe("echo");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("exec approvals shell allowlist (chained commands)", () => {
|
describe("exec approvals shell allowlist (chained commands)", () => {
|
||||||
@@ -185,6 +215,18 @@ describe("exec approvals shell allowlist (chained commands)", () => {
|
|||||||
expect(result.analysisOk).toBe(true);
|
expect(result.analysisOk).toBe(true);
|
||||||
expect(result.allowlistSatisfied).toBe(true);
|
expect(result.allowlistSatisfied).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("respects escaped quotes when splitting chains", () => {
|
||||||
|
const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }];
|
||||||
|
const result = evaluateShellAllowlist({
|
||||||
|
command: '/usr/bin/echo "foo\\" && bar"',
|
||||||
|
allowlist,
|
||||||
|
safeBins: new Set(),
|
||||||
|
cwd: "/tmp",
|
||||||
|
});
|
||||||
|
expect(result.analysisOk).toBe(true);
|
||||||
|
expect(result.allowlistSatisfied).toBe(true);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("exec approvals safe bins", () => {
|
describe("exec approvals safe bins", () => {
|
||||||
|
|||||||
@@ -585,6 +585,11 @@ export type ExecCommandAnalysis = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);
|
const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);
|
||||||
|
const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);
|
||||||
|
|
||||||
|
function isDoubleQuoteEscape(next: string | undefined): next is string {
|
||||||
|
return Boolean(next && DOUBLE_QUOTE_ESCAPES.has(next));
|
||||||
|
}
|
||||||
|
|
||||||
type IteratorAction = "split" | "skip" | "include" | { reject: string };
|
type IteratorAction = "split" | "skip" | "include" | { reject: string };
|
||||||
|
|
||||||
@@ -637,6 +642,21 @@ function iterateQuoteAware(
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (inDouble) {
|
if (inDouble) {
|
||||||
|
if (ch === "\\" && isDoubleQuoteEscape(next)) {
|
||||||
|
buf += ch;
|
||||||
|
buf += next;
|
||||||
|
i += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (ch === "$" && next === "(") {
|
||||||
|
return { ok: false, reason: "unsupported shell token: $()" };
|
||||||
|
}
|
||||||
|
if (ch === "`") {
|
||||||
|
return { ok: false, reason: "unsupported shell token: `" };
|
||||||
|
}
|
||||||
|
if (ch === "\n" || ch === "\r") {
|
||||||
|
return { ok: false, reason: "unsupported shell token: newline" };
|
||||||
|
}
|
||||||
if (ch === '"') {
|
if (ch === '"') {
|
||||||
inDouble = false;
|
inDouble = false;
|
||||||
}
|
}
|
||||||
@@ -749,6 +769,12 @@ function tokenizeShellSegment(segment: string): string[] | null {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (inDouble) {
|
if (inDouble) {
|
||||||
|
const next = segment[i + 1];
|
||||||
|
if (ch === "\\" && isDoubleQuoteEscape(next)) {
|
||||||
|
buf += next;
|
||||||
|
i += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if (ch === '"') {
|
if (ch === '"') {
|
||||||
inDouble = false;
|
inDouble = false;
|
||||||
} else {
|
} else {
|
||||||
@@ -1067,6 +1093,7 @@ function splitCommandChain(command: string): string[] | null {
|
|||||||
|
|
||||||
for (let i = 0; i < command.length; i += 1) {
|
for (let i = 0; i < command.length; i += 1) {
|
||||||
const ch = command[i];
|
const ch = command[i];
|
||||||
|
const next = command[i + 1];
|
||||||
if (escaped) {
|
if (escaped) {
|
||||||
buf += ch;
|
buf += ch;
|
||||||
escaped = false;
|
escaped = false;
|
||||||
@@ -1085,6 +1112,12 @@ function splitCommandChain(command: string): string[] | null {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (inDouble) {
|
if (inDouble) {
|
||||||
|
if (ch === "\\" && isDoubleQuoteEscape(next)) {
|
||||||
|
buf += ch;
|
||||||
|
buf += next;
|
||||||
|
i += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if (ch === '"') {
|
if (ch === '"') {
|
||||||
inDouble = false;
|
inDouble = false;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user