mirror of
https://github.com/moltbot/moltbot.git
synced 2026-03-08 06:54:24 +00:00
refactor(security): unify exec approval request matching
This commit is contained in:
@@ -1,21 +1,13 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import type { ExecApprovalDecision } from "../infra/exec-approvals.js";
|
||||
import type {
|
||||
ExecApprovalDecision,
|
||||
ExecApprovalRequestPayload as InfraExecApprovalRequestPayload,
|
||||
} from "../infra/exec-approvals.js";
|
||||
|
||||
// Grace period to keep resolved entries for late awaitDecision calls
|
||||
const RESOLVED_ENTRY_GRACE_MS = 15_000;
|
||||
|
||||
export type ExecApprovalRequestPayload = {
|
||||
command: string;
|
||||
commandArgv?: string[] | null;
|
||||
cwd?: string | null;
|
||||
nodeId?: string | null;
|
||||
host?: string | null;
|
||||
security?: string | null;
|
||||
ask?: string | null;
|
||||
agentId?: string | null;
|
||||
resolvedPath?: string | null;
|
||||
sessionKey?: string | null;
|
||||
};
|
||||
export type ExecApprovalRequestPayload = InfraExecApprovalRequestPayload;
|
||||
|
||||
export type ExecApprovalRecord = {
|
||||
id: string;
|
||||
|
||||
94
src/gateway/node-invoke-system-run-approval-match.test.ts
Normal file
94
src/gateway/node-invoke-system-run-approval-match.test.ts
Normal file
@@ -0,0 +1,94 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
import { approvalMatchesSystemRunRequest } from "./node-invoke-system-run-approval-match.js";
|
||||
|
||||
describe("approvalMatchesSystemRunRequest", () => {
|
||||
test("matches legacy command text when binding fields match", () => {
|
||||
const result = approvalMatchesSystemRunRequest({
|
||||
cmdText: "echo SAFE",
|
||||
argv: ["echo", "SAFE"],
|
||||
request: {
|
||||
host: "node",
|
||||
command: "echo SAFE",
|
||||
cwd: "/tmp",
|
||||
agentId: "agent-1",
|
||||
sessionKey: "session-1",
|
||||
},
|
||||
binding: {
|
||||
cwd: "/tmp",
|
||||
agentId: "agent-1",
|
||||
sessionKey: "session-1",
|
||||
},
|
||||
});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("rejects legacy command mismatch", () => {
|
||||
const result = approvalMatchesSystemRunRequest({
|
||||
cmdText: "echo PWNED",
|
||||
argv: ["echo", "PWNED"],
|
||||
request: {
|
||||
host: "node",
|
||||
command: "echo SAFE",
|
||||
},
|
||||
binding: {
|
||||
cwd: null,
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test("enforces exact argv binding when commandArgv is set", () => {
|
||||
const result = approvalMatchesSystemRunRequest({
|
||||
cmdText: "echo SAFE",
|
||||
argv: ["echo", "SAFE"],
|
||||
request: {
|
||||
host: "node",
|
||||
command: "echo SAFE",
|
||||
commandArgv: ["echo", "SAFE"],
|
||||
},
|
||||
binding: {
|
||||
cwd: null,
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
});
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test("rejects argv mismatch even when command text matches", () => {
|
||||
const result = approvalMatchesSystemRunRequest({
|
||||
cmdText: "echo SAFE",
|
||||
argv: ["echo", "SAFE"],
|
||||
request: {
|
||||
host: "node",
|
||||
command: "echo SAFE",
|
||||
commandArgv: ["echo SAFE"],
|
||||
},
|
||||
binding: {
|
||||
cwd: null,
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test("rejects non-node host requests", () => {
|
||||
const result = approvalMatchesSystemRunRequest({
|
||||
cmdText: "echo SAFE",
|
||||
argv: ["echo", "SAFE"],
|
||||
request: {
|
||||
host: "gateway",
|
||||
command: "echo SAFE",
|
||||
},
|
||||
binding: {
|
||||
cwd: null,
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
},
|
||||
});
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
51
src/gateway/node-invoke-system-run-approval-match.ts
Normal file
51
src/gateway/node-invoke-system-run-approval-match.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
import type { ExecApprovalRequestPayload } from "../infra/exec-approvals.js";
|
||||
|
||||
export type SystemRunApprovalBinding = {
|
||||
cwd: string | null;
|
||||
agentId: string | null;
|
||||
sessionKey: string | null;
|
||||
};
|
||||
|
||||
function argvMatchesRequest(requestedArgv: string[], argv: string[]): boolean {
|
||||
if (requestedArgv.length === 0 || requestedArgv.length !== argv.length) {
|
||||
return false;
|
||||
}
|
||||
for (let i = 0; i < requestedArgv.length; i += 1) {
|
||||
if (requestedArgv[i] !== argv[i]) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export function approvalMatchesSystemRunRequest(params: {
|
||||
cmdText: string;
|
||||
argv: string[];
|
||||
request: ExecApprovalRequestPayload;
|
||||
binding: SystemRunApprovalBinding;
|
||||
}): boolean {
|
||||
if (params.request.host !== "node") {
|
||||
return false;
|
||||
}
|
||||
|
||||
const requestedArgv = params.request.commandArgv;
|
||||
if (Array.isArray(requestedArgv)) {
|
||||
if (!argvMatchesRequest(requestedArgv, params.argv)) {
|
||||
return false;
|
||||
}
|
||||
} else if (!params.cmdText || params.request.command !== params.cmdText) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ((params.request.cwd ?? null) !== params.binding.cwd) {
|
||||
return false;
|
||||
}
|
||||
if ((params.request.agentId ?? null) !== params.binding.agentId) {
|
||||
return false;
|
||||
}
|
||||
if ((params.request.sessionKey ?? null) !== params.binding.sessionKey) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
@@ -13,14 +13,14 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
},
|
||||
};
|
||||
|
||||
function makeRecord(command: string, commandArgv?: string[] | null): ExecApprovalRecord {
|
||||
function makeRecord(command: string, commandArgv?: string[]): ExecApprovalRecord {
|
||||
return {
|
||||
id: "approval-1",
|
||||
request: {
|
||||
host: "node",
|
||||
nodeId: "node-1",
|
||||
command,
|
||||
commandArgv: commandArgv ?? null,
|
||||
commandArgv,
|
||||
cwd: null,
|
||||
agentId: null,
|
||||
sessionKey: null,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { resolveSystemRunCommand } from "../infra/system-run-command.js";
|
||||
import type { ExecApprovalRecord } from "./exec-approval-manager.js";
|
||||
import { approvalMatchesSystemRunRequest } from "./node-invoke-system-run-approval-match.js";
|
||||
|
||||
type SystemRunParamsLike = {
|
||||
command?: unknown;
|
||||
@@ -53,53 +54,6 @@ function clientHasApprovals(client: ApprovalClient | null): boolean {
|
||||
return scopes.includes("operator.admin") || scopes.includes("operator.approvals");
|
||||
}
|
||||
|
||||
function approvalMatchesRequest(
|
||||
cmdText: string,
|
||||
argv: string[],
|
||||
params: SystemRunParamsLike,
|
||||
record: ExecApprovalRecord,
|
||||
): boolean {
|
||||
if (record.request.host !== "node") {
|
||||
return false;
|
||||
}
|
||||
|
||||
const requestedArgv = Array.isArray(record.request.commandArgv)
|
||||
? record.request.commandArgv
|
||||
: null;
|
||||
if (requestedArgv) {
|
||||
if (requestedArgv.length === 0 || requestedArgv.length !== argv.length) {
|
||||
return false;
|
||||
}
|
||||
for (let i = 0; i < requestedArgv.length; i += 1) {
|
||||
if (requestedArgv[i] !== argv[i]) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
} else if (!cmdText || record.request.command !== cmdText) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const reqCwd = record.request.cwd ?? null;
|
||||
const runCwd = normalizeString(params.cwd) ?? null;
|
||||
if (reqCwd !== runCwd) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const reqAgentId = record.request.agentId ?? null;
|
||||
const runAgentId = normalizeString(params.agentId) ?? null;
|
||||
if (reqAgentId !== runAgentId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const reqSessionKey = record.request.sessionKey ?? null;
|
||||
const runSessionKey = normalizeString(params.sessionKey) ?? null;
|
||||
if (reqSessionKey !== runSessionKey) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
function pickSystemRunParams(raw: Record<string, unknown>): Record<string, unknown> {
|
||||
// Defensive allowlist: only forward fields that the node-host `system.run` handler understands.
|
||||
// This prevents future internal control fields from being smuggled through the gateway.
|
||||
@@ -250,7 +204,18 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
|
||||
};
|
||||
}
|
||||
|
||||
if (!approvalMatchesRequest(cmdText, cmdTextResolution.argv, p, snapshot)) {
|
||||
if (
|
||||
!approvalMatchesSystemRunRequest({
|
||||
cmdText,
|
||||
argv: cmdTextResolution.argv,
|
||||
request: snapshot.request,
|
||||
binding: {
|
||||
cwd: normalizeString(p.cwd) ?? null,
|
||||
agentId: normalizeString(p.agentId) ?? null,
|
||||
sessionKey: normalizeString(p.sessionKey) ?? null,
|
||||
},
|
||||
})
|
||||
) {
|
||||
return {
|
||||
ok: false,
|
||||
message: "approval id does not match request",
|
||||
|
||||
@@ -89,7 +89,7 @@ export const ExecApprovalRequestParamsSchema = Type.Object(
|
||||
{
|
||||
id: Type.Optional(NonEmptyString),
|
||||
command: NonEmptyString,
|
||||
commandArgv: Type.Optional(Type.Union([Type.Array(Type.String()), Type.Null()])),
|
||||
commandArgv: Type.Optional(Type.Array(Type.String())),
|
||||
cwd: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
nodeId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
host: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
|
||||
@@ -43,7 +43,7 @@ export function createExecApprovalHandlers(
|
||||
const p = params as {
|
||||
id?: string;
|
||||
command: string;
|
||||
commandArgv?: string[] | null;
|
||||
commandArgv?: string[];
|
||||
cwd?: string;
|
||||
nodeId?: string;
|
||||
host?: string;
|
||||
@@ -63,7 +63,7 @@ export function createExecApprovalHandlers(
|
||||
const nodeId = typeof p.nodeId === "string" ? p.nodeId.trim() : "";
|
||||
const commandArgv = Array.isArray(p.commandArgv)
|
||||
? p.commandArgv.map((entry) => String(entry))
|
||||
: null;
|
||||
: undefined;
|
||||
if (host === "node" && !nodeId) {
|
||||
respond(
|
||||
false,
|
||||
|
||||
@@ -11,20 +11,22 @@ export type ExecHost = "sandbox" | "gateway" | "node";
|
||||
export type ExecSecurity = "deny" | "allowlist" | "full";
|
||||
export type ExecAsk = "off" | "on-miss" | "always";
|
||||
|
||||
export type ExecApprovalRequestPayload = {
|
||||
command: string;
|
||||
commandArgv?: string[];
|
||||
cwd?: string | null;
|
||||
nodeId?: string | null;
|
||||
host?: string | null;
|
||||
security?: string | null;
|
||||
ask?: string | null;
|
||||
agentId?: string | null;
|
||||
resolvedPath?: string | null;
|
||||
sessionKey?: string | null;
|
||||
};
|
||||
|
||||
export type ExecApprovalRequest = {
|
||||
id: string;
|
||||
request: {
|
||||
command: string;
|
||||
commandArgv?: string[] | null;
|
||||
cwd?: string | null;
|
||||
nodeId?: string | null;
|
||||
host?: string | null;
|
||||
security?: string | null;
|
||||
ask?: string | null;
|
||||
agentId?: string | null;
|
||||
resolvedPath?: string | null;
|
||||
sessionKey?: string | null;
|
||||
};
|
||||
request: ExecApprovalRequestPayload;
|
||||
createdAtMs: number;
|
||||
expiresAtMs: number;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user