fix(security): block shell-wrapper line-continuation allowlist bypass
This commit is contained in:
@@ -22,9 +22,18 @@ describe("formatSystemRunAllowlistMissMessage", () => {
|
||||
expect(formatSystemRunAllowlistMissMessage()).toBe("SYSTEM_RUN_DENIED: allowlist miss");
|
||||
});
|
||||
|
||||
it("adds shell-wrapper guidance when wrappers are blocked", () => {
|
||||
expect(
|
||||
formatSystemRunAllowlistMissMessage({
|
||||
shellWrapperBlocked: true,
|
||||
}),
|
||||
).toContain("shell wrappers like sh/bash/zsh -c require approval");
|
||||
});
|
||||
|
||||
it("adds Windows shell-wrapper guidance when blocked by cmd.exe policy", () => {
|
||||
expect(
|
||||
formatSystemRunAllowlistMissMessage({
|
||||
shellWrapperBlocked: true,
|
||||
windowsShellWrapperBlocked: true,
|
||||
}),
|
||||
).toContain("Windows shell wrappers like cmd.exe /c require approval");
|
||||
@@ -42,6 +51,7 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
approved: false,
|
||||
isWindows: false,
|
||||
cmdInvocation: false,
|
||||
shellWrapperInvocation: false,
|
||||
});
|
||||
expect(decision.allowed).toBe(false);
|
||||
if (decision.allowed) {
|
||||
@@ -61,6 +71,7 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
approved: false,
|
||||
isWindows: false,
|
||||
cmdInvocation: false,
|
||||
shellWrapperInvocation: false,
|
||||
});
|
||||
expect(decision.allowed).toBe(false);
|
||||
if (decision.allowed) {
|
||||
@@ -80,6 +91,7 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
approved: false,
|
||||
isWindows: false,
|
||||
cmdInvocation: false,
|
||||
shellWrapperInvocation: false,
|
||||
});
|
||||
expect(decision.allowed).toBe(true);
|
||||
if (!decision.allowed) {
|
||||
@@ -98,6 +110,7 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
approved: false,
|
||||
isWindows: false,
|
||||
cmdInvocation: false,
|
||||
shellWrapperInvocation: false,
|
||||
});
|
||||
expect(decision.allowed).toBe(false);
|
||||
if (decision.allowed) {
|
||||
@@ -107,7 +120,27 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
expect(decision.errorMessage).toBe("SYSTEM_RUN_DENIED: allowlist miss");
|
||||
});
|
||||
|
||||
it("treats Windows cmd.exe wrappers as allowlist misses", () => {
|
||||
it("treats shell wrappers as allowlist misses", () => {
|
||||
const decision = evaluateSystemRunPolicy({
|
||||
security: "allowlist",
|
||||
ask: "off",
|
||||
analysisOk: true,
|
||||
allowlistSatisfied: true,
|
||||
approvalDecision: null,
|
||||
approved: false,
|
||||
isWindows: false,
|
||||
cmdInvocation: false,
|
||||
shellWrapperInvocation: true,
|
||||
});
|
||||
expect(decision.allowed).toBe(false);
|
||||
if (decision.allowed) {
|
||||
throw new Error("expected denied decision");
|
||||
}
|
||||
expect(decision.shellWrapperBlocked).toBe(true);
|
||||
expect(decision.errorMessage).toContain("shell wrappers like sh/bash/zsh -c");
|
||||
});
|
||||
|
||||
it("keeps Windows-specific guidance for cmd.exe wrappers", () => {
|
||||
const decision = evaluateSystemRunPolicy({
|
||||
security: "allowlist",
|
||||
ask: "off",
|
||||
@@ -117,11 +150,13 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
approved: false,
|
||||
isWindows: true,
|
||||
cmdInvocation: true,
|
||||
shellWrapperInvocation: true,
|
||||
});
|
||||
expect(decision.allowed).toBe(false);
|
||||
if (decision.allowed) {
|
||||
throw new Error("expected denied decision");
|
||||
}
|
||||
expect(decision.shellWrapperBlocked).toBe(true);
|
||||
expect(decision.windowsShellWrapperBlocked).toBe(true);
|
||||
expect(decision.errorMessage).toContain("Windows shell wrappers like cmd.exe /c");
|
||||
});
|
||||
@@ -136,6 +171,7 @@ describe("evaluateSystemRunPolicy", () => {
|
||||
approved: false,
|
||||
isWindows: false,
|
||||
cmdInvocation: false,
|
||||
shellWrapperInvocation: false,
|
||||
});
|
||||
expect(decision.allowed).toBe(true);
|
||||
if (!decision.allowed) {
|
||||
|
||||
@@ -5,6 +5,7 @@ export type ExecApprovalDecision = "allow-once" | "allow-always" | null;
|
||||
export type SystemRunPolicyDecision = {
|
||||
analysisOk: boolean;
|
||||
allowlistSatisfied: boolean;
|
||||
shellWrapperBlocked: boolean;
|
||||
windowsShellWrapperBlocked: boolean;
|
||||
requiresAsk: boolean;
|
||||
approvalDecision: ExecApprovalDecision;
|
||||
@@ -28,6 +29,7 @@ export function resolveExecApprovalDecision(value: unknown): ExecApprovalDecisio
|
||||
}
|
||||
|
||||
export function formatSystemRunAllowlistMissMessage(params?: {
|
||||
shellWrapperBlocked?: boolean;
|
||||
windowsShellWrapperBlocked?: boolean;
|
||||
}): string {
|
||||
if (params?.windowsShellWrapperBlocked) {
|
||||
@@ -37,6 +39,13 @@ export function formatSystemRunAllowlistMissMessage(params?: {
|
||||
"approve once/always or run with --ask on-miss|always)"
|
||||
);
|
||||
}
|
||||
if (params?.shellWrapperBlocked) {
|
||||
return (
|
||||
"SYSTEM_RUN_DENIED: allowlist miss " +
|
||||
"(shell wrappers like sh/bash/zsh -c require approval; " +
|
||||
"approve once/always or run with --ask on-miss|always)"
|
||||
);
|
||||
}
|
||||
return "SYSTEM_RUN_DENIED: allowlist miss";
|
||||
}
|
||||
|
||||
@@ -49,11 +58,13 @@ export function evaluateSystemRunPolicy(params: {
|
||||
approved?: boolean;
|
||||
isWindows: boolean;
|
||||
cmdInvocation: boolean;
|
||||
shellWrapperInvocation: boolean;
|
||||
}): SystemRunPolicyDecision {
|
||||
const shellWrapperBlocked = params.security === "allowlist" && params.shellWrapperInvocation;
|
||||
const windowsShellWrapperBlocked =
|
||||
params.security === "allowlist" && params.isWindows && params.cmdInvocation;
|
||||
const analysisOk = windowsShellWrapperBlocked ? false : params.analysisOk;
|
||||
const allowlistSatisfied = windowsShellWrapperBlocked ? false : params.allowlistSatisfied;
|
||||
shellWrapperBlocked && params.isWindows && params.cmdInvocation;
|
||||
const analysisOk = shellWrapperBlocked ? false : params.analysisOk;
|
||||
const allowlistSatisfied = shellWrapperBlocked ? false : params.allowlistSatisfied;
|
||||
const approvedByAsk = params.approvalDecision !== null || params.approved === true;
|
||||
|
||||
if (params.security === "deny") {
|
||||
@@ -63,6 +74,7 @@ export function evaluateSystemRunPolicy(params: {
|
||||
errorMessage: "SYSTEM_RUN_DISABLED: security=deny",
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
shellWrapperBlocked,
|
||||
windowsShellWrapperBlocked,
|
||||
requiresAsk: false,
|
||||
approvalDecision: params.approvalDecision,
|
||||
@@ -83,6 +95,7 @@ export function evaluateSystemRunPolicy(params: {
|
||||
errorMessage: "SYSTEM_RUN_DENIED: approval required",
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
shellWrapperBlocked,
|
||||
windowsShellWrapperBlocked,
|
||||
requiresAsk,
|
||||
approvalDecision: params.approvalDecision,
|
||||
@@ -94,9 +107,13 @@ export function evaluateSystemRunPolicy(params: {
|
||||
return {
|
||||
allowed: false,
|
||||
eventReason: "allowlist-miss",
|
||||
errorMessage: formatSystemRunAllowlistMissMessage({ windowsShellWrapperBlocked }),
|
||||
errorMessage: formatSystemRunAllowlistMissMessage({
|
||||
shellWrapperBlocked,
|
||||
windowsShellWrapperBlocked,
|
||||
}),
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
shellWrapperBlocked,
|
||||
windowsShellWrapperBlocked,
|
||||
requiresAsk,
|
||||
approvalDecision: params.approvalDecision,
|
||||
@@ -108,6 +125,7 @@ export function evaluateSystemRunPolicy(params: {
|
||||
allowed: true,
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
shellWrapperBlocked,
|
||||
windowsShellWrapperBlocked,
|
||||
requiresAsk,
|
||||
approvalDecision: params.approvalDecision,
|
||||
|
||||
@@ -166,6 +166,37 @@ export async function handleSystemRunInvoke(opts: {
|
||||
const cmdInvocation = shellCommand
|
||||
? opts.isCmdExeInvocation(segments[0]?.argv ?? [])
|
||||
: opts.isCmdExeInvocation(argv);
|
||||
const policy = evaluateSystemRunPolicy({
|
||||
security,
|
||||
ask,
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
approvalDecision,
|
||||
approved: opts.params.approved === true,
|
||||
isWindows,
|
||||
cmdInvocation,
|
||||
shellWrapperInvocation: shellCommand !== null,
|
||||
});
|
||||
analysisOk = policy.analysisOk;
|
||||
allowlistSatisfied = policy.allowlistSatisfied;
|
||||
if (!policy.allowed) {
|
||||
await opts.sendNodeEvent(
|
||||
opts.client,
|
||||
"exec.denied",
|
||||
opts.buildExecEventPayload({
|
||||
sessionKey,
|
||||
runId,
|
||||
host: "node",
|
||||
command: cmdText,
|
||||
reason: policy.eventReason,
|
||||
}),
|
||||
);
|
||||
await opts.sendInvokeResult({
|
||||
ok: false,
|
||||
error: { code: "UNAVAILABLE", message: policy.errorMessage },
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const useMacAppExec = opts.preferMacAppExecHost;
|
||||
if (useMacAppExec) {
|
||||
@@ -232,37 +263,6 @@ export async function handleSystemRunInvoke(opts: {
|
||||
}
|
||||
}
|
||||
|
||||
const policy = evaluateSystemRunPolicy({
|
||||
security,
|
||||
ask,
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
approvalDecision,
|
||||
approved: opts.params.approved === true,
|
||||
isWindows,
|
||||
cmdInvocation,
|
||||
});
|
||||
analysisOk = policy.analysisOk;
|
||||
allowlistSatisfied = policy.allowlistSatisfied;
|
||||
if (!policy.allowed) {
|
||||
await opts.sendNodeEvent(
|
||||
opts.client,
|
||||
"exec.denied",
|
||||
opts.buildExecEventPayload({
|
||||
sessionKey,
|
||||
runId,
|
||||
host: "node",
|
||||
command: cmdText,
|
||||
reason: policy.eventReason,
|
||||
}),
|
||||
);
|
||||
await opts.sendInvokeResult({
|
||||
ok: false,
|
||||
error: { code: "UNAVAILABLE", message: policy.errorMessage },
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (policy.approvalDecision === "allow-always" && security === "allowlist") {
|
||||
if (policy.analysisOk) {
|
||||
const patterns = resolveAllowAlwaysPatterns({
|
||||
|
||||
Reference in New Issue
Block a user