feat(hub): integrate exec approval manager with Hub and Gateway
- ExecApprovalManager: tracks pending approvals, sends to clients via Gateway, resolves on RPC response, auto-denies on timeout (fail-closed) - RPC handler: resolveExecApproval for client decision delivery - Hub integration: creates approval callback per agent, injects into AsyncAgent, registers RPC handler, cancels pending on agent close - Reads/writes exec approval config and allowlist from agent profile - Test coverage for manager: request/resolve, timeout, cancel, errors Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
yushen
parent
89089ef866
commit
d742e668d7
4 changed files with 496 additions and 1 deletions
217
src/hub/exec-approval-manager.test.ts
Normal file
217
src/hub/exec-approval-manager.test.ts
Normal file
|
|
@ -0,0 +1,217 @@
|
|||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { ExecApprovalManager } from "./exec-approval-manager.js";
|
||||
|
||||
describe("ExecApprovalManager", () => {
|
||||
let manager: ExecApprovalManager;
|
||||
let sendToClient: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
sendToClient = vi.fn();
|
||||
manager = new ExecApprovalManager(sendToClient, 5000); // 5s timeout for tests
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("sends approval request to client and resolves on decision", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "rm -rf /tmp/test",
|
||||
cwd: "/workspace",
|
||||
riskLevel: "dangerous",
|
||||
riskReasons: ["Recursive delete"],
|
||||
});
|
||||
|
||||
// Verify sendToClient was called
|
||||
expect(sendToClient).toHaveBeenCalledTimes(1);
|
||||
const [agentId, request] = sendToClient.mock.calls[0]!;
|
||||
expect(agentId).toBe("agent-1");
|
||||
expect(request.command).toBe("rm -rf /tmp/test");
|
||||
expect(request.approvalId).toBeTruthy();
|
||||
|
||||
// Resolve the approval
|
||||
const resolved = manager.resolveApproval(request.approvalId, "allow-once");
|
||||
expect(resolved).toBe(true);
|
||||
|
||||
const result = await promise;
|
||||
expect(result.approved).toBe(true);
|
||||
expect(result.decision).toBe("allow-once");
|
||||
});
|
||||
|
||||
it("resolves with deny when decision is deny", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "sudo reboot",
|
||||
riskLevel: "dangerous",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
const request = sendToClient.mock.calls[0]![1];
|
||||
manager.resolveApproval(request.approvalId, "deny");
|
||||
|
||||
const result = await promise;
|
||||
expect(result.approved).toBe(false);
|
||||
expect(result.decision).toBe("deny");
|
||||
});
|
||||
|
||||
it("resolves with allow-always", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "git push",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
const request = sendToClient.mock.calls[0]![1];
|
||||
manager.resolveApproval(request.approvalId, "allow-always");
|
||||
|
||||
const result = await promise;
|
||||
expect(result.approved).toBe(true);
|
||||
expect(result.decision).toBe("allow-always");
|
||||
});
|
||||
|
||||
it("auto-denies on timeout (fail-closed)", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "dangerous-command",
|
||||
riskLevel: "dangerous",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
// Fast-forward past timeout
|
||||
vi.advanceTimersByTime(6000);
|
||||
|
||||
const result = await promise;
|
||||
expect(result.approved).toBe(false);
|
||||
expect(result.decision).toBe("deny");
|
||||
});
|
||||
|
||||
it("returns false when resolving unknown approval", () => {
|
||||
const resolved = manager.resolveApproval("unknown-id", "allow-once");
|
||||
expect(resolved).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false when resolving already-resolved approval", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
const request = sendToClient.mock.calls[0]![1];
|
||||
|
||||
// First resolve succeeds
|
||||
expect(manager.resolveApproval(request.approvalId, "allow-once")).toBe(true);
|
||||
// Second resolve fails
|
||||
expect(manager.resolveApproval(request.approvalId, "deny")).toBe(false);
|
||||
|
||||
await promise;
|
||||
});
|
||||
|
||||
it("cancels all pending approvals for an agent", async () => {
|
||||
const promise1 = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd1",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
const promise2 = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd2",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
const promise3 = manager.requestApproval({
|
||||
agentId: "agent-2",
|
||||
command: "cmd3",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
// Cancel agent-1's approvals
|
||||
manager.cancelPending("agent-1");
|
||||
|
||||
const result1 = await promise1;
|
||||
const result2 = await promise2;
|
||||
|
||||
expect(result1.approved).toBe(false);
|
||||
expect(result1.decision).toBe("deny");
|
||||
expect(result2.approved).toBe(false);
|
||||
expect(result2.decision).toBe("deny");
|
||||
|
||||
// agent-2's approval should still be pending
|
||||
expect(manager.pendingCount).toBe(1);
|
||||
|
||||
// Resolve agent-2's approval
|
||||
const request3 = sendToClient.mock.calls[2]![1];
|
||||
manager.resolveApproval(request3.approvalId, "allow-once");
|
||||
const result3 = await promise3;
|
||||
expect(result3.approved).toBe(true);
|
||||
});
|
||||
|
||||
it("auto-denies when sendToClient throws", async () => {
|
||||
const failingSender = vi.fn().mockImplementation(() => {
|
||||
throw new Error("Connection lost");
|
||||
});
|
||||
const failManager = new ExecApprovalManager(failingSender, 5000);
|
||||
|
||||
const result = await failManager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
expect(result.approved).toBe(false);
|
||||
expect(result.decision).toBe("deny");
|
||||
});
|
||||
|
||||
it("getSnapshot returns request details", () => {
|
||||
manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "ls",
|
||||
riskLevel: "safe",
|
||||
riskReasons: [],
|
||||
});
|
||||
|
||||
const request = sendToClient.mock.calls[0]![1];
|
||||
const snapshot = manager.getSnapshot(request.approvalId);
|
||||
|
||||
expect(snapshot).toBeTruthy();
|
||||
expect(snapshot!.command).toBe("ls");
|
||||
expect(snapshot!.agentId).toBe("agent-1");
|
||||
});
|
||||
|
||||
it("getSnapshot returns null for unknown id", () => {
|
||||
expect(manager.getSnapshot("unknown")).toBeNull();
|
||||
});
|
||||
|
||||
it("tracks pendingCount correctly", () => {
|
||||
expect(manager.pendingCount).toBe(0);
|
||||
|
||||
manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd1",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
expect(manager.pendingCount).toBe(1);
|
||||
|
||||
manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd2",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
});
|
||||
expect(manager.pendingCount).toBe(2);
|
||||
|
||||
const request = sendToClient.mock.calls[0]![1];
|
||||
manager.resolveApproval(request.approvalId, "deny");
|
||||
expect(manager.pendingCount).toBe(1);
|
||||
});
|
||||
});
|
||||
136
src/hub/exec-approval-manager.ts
Normal file
136
src/hub/exec-approval-manager.ts
Normal file
|
|
@ -0,0 +1,136 @@
|
|||
/**
|
||||
* Exec Approval Manager — Hub-side approval tracking
|
||||
*
|
||||
* Manages pending approval requests, sends them to connected clients,
|
||||
* and resolves them when clients respond via RPC.
|
||||
*/
|
||||
|
||||
import { v7 as uuidv7 } from "uuid";
|
||||
import type {
|
||||
ExecApprovalRequest,
|
||||
ApprovalDecision,
|
||||
ApprovalResult,
|
||||
} from "../agent/tools/exec-approval-types.js";
|
||||
import { DEFAULT_APPROVAL_TIMEOUT_MS } from "../agent/tools/exec-approval-types.js";
|
||||
|
||||
interface PendingEntry {
|
||||
resolve: (result: ApprovalResult) => void;
|
||||
timer: NodeJS.Timeout;
|
||||
request: ExecApprovalRequest;
|
||||
}
|
||||
|
||||
/**
|
||||
* Callback type for sending approval requests to clients.
|
||||
* The Hub wires this to Gateway message sending.
|
||||
*/
|
||||
export type SendApprovalToClient = (
|
||||
agentId: string,
|
||||
payload: ExecApprovalRequest,
|
||||
) => void;
|
||||
|
||||
export class ExecApprovalManager {
|
||||
private readonly pending = new Map<string, PendingEntry>();
|
||||
|
||||
constructor(
|
||||
private readonly sendToClient: SendApprovalToClient,
|
||||
private readonly defaultTimeoutMs: number = DEFAULT_APPROVAL_TIMEOUT_MS,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Create an approval request and send it to the client.
|
||||
* Returns a Promise that resolves when the client responds or times out.
|
||||
*/
|
||||
requestApproval(params: {
|
||||
agentId: string;
|
||||
command: string;
|
||||
cwd?: string;
|
||||
riskLevel: "safe" | "needs-review" | "dangerous";
|
||||
riskReasons: string[];
|
||||
timeoutMs?: number;
|
||||
}): Promise<ApprovalResult> {
|
||||
const approvalId = uuidv7();
|
||||
const timeoutMs = params.timeoutMs ?? this.defaultTimeoutMs;
|
||||
const expiresAtMs = Date.now() + timeoutMs;
|
||||
|
||||
const request: ExecApprovalRequest = {
|
||||
approvalId,
|
||||
agentId: params.agentId,
|
||||
command: params.command,
|
||||
cwd: params.cwd,
|
||||
riskLevel: params.riskLevel,
|
||||
riskReasons: params.riskReasons,
|
||||
expiresAtMs,
|
||||
};
|
||||
|
||||
return new Promise<ApprovalResult>((resolve) => {
|
||||
// Timeout: auto-deny (fail-closed)
|
||||
const timer = setTimeout(() => {
|
||||
if (this.pending.has(approvalId)) {
|
||||
this.pending.delete(approvalId);
|
||||
resolve({ approved: false, decision: "deny" });
|
||||
}
|
||||
}, timeoutMs);
|
||||
|
||||
this.pending.set(approvalId, { resolve, timer, request });
|
||||
|
||||
// Send to client via Gateway
|
||||
try {
|
||||
this.sendToClient(params.agentId, request);
|
||||
} catch (err) {
|
||||
// If sending fails, auto-deny (fail-closed)
|
||||
clearTimeout(timer);
|
||||
this.pending.delete(approvalId);
|
||||
console.error(`[ExecApprovalManager] Failed to send approval request: ${err}`);
|
||||
resolve({ approved: false, decision: "deny" });
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a pending approval with a client decision.
|
||||
* Returns true if the approval was found and resolved, false otherwise.
|
||||
*/
|
||||
resolveApproval(approvalId: string, decision: ApprovalDecision): boolean {
|
||||
const entry = this.pending.get(approvalId);
|
||||
if (!entry) return false;
|
||||
|
||||
clearTimeout(entry.timer);
|
||||
this.pending.delete(approvalId);
|
||||
|
||||
entry.resolve({
|
||||
approved: decision !== "deny",
|
||||
decision,
|
||||
});
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancel all pending approvals for an agent (e.g., on agent close).
|
||||
* All pending requests are resolved as denied.
|
||||
*/
|
||||
cancelPending(agentId: string): void {
|
||||
for (const [id, entry] of this.pending) {
|
||||
if (entry.request.agentId === agentId) {
|
||||
clearTimeout(entry.timer);
|
||||
this.pending.delete(id);
|
||||
entry.resolve({ approved: false, decision: "deny" });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a snapshot of a pending approval request (for debugging).
|
||||
*/
|
||||
getSnapshot(approvalId: string): ExecApprovalRequest | null {
|
||||
const entry = this.pending.get(approvalId);
|
||||
return entry ? { ...entry.request } : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get count of pending approvals (for monitoring).
|
||||
*/
|
||||
get pendingCount(): number {
|
||||
return this.pending.size;
|
||||
}
|
||||
}
|
||||
110
src/hub/hub.ts
110
src/hub/hub.ts
|
|
@ -23,6 +23,12 @@ import { createDeleteAgentHandler } from "./rpc/handlers/delete-agent.js";
|
|||
import { createUpdateGatewayHandler } from "./rpc/handlers/update-gateway.js";
|
||||
import { DeviceStore, type DeviceMeta } from "./device-store.js";
|
||||
import { createVerifyHandler } from "./rpc/handlers/verify.js";
|
||||
import { ExecApprovalManager } from "./exec-approval-manager.js";
|
||||
import { createResolveExecApprovalHandler } from "./rpc/handlers/resolve-exec-approval.js";
|
||||
import { evaluateCommandSafety, requiresApproval } from "../agent/tools/exec-safety.js";
|
||||
import { addAllowlistEntry, recordAllowlistUse, matchAllowlist } from "../agent/tools/exec-allowlist.js";
|
||||
import type { ExecApprovalCallback, ExecApprovalConfig, ApprovalResult } from "../agent/tools/exec-approval-types.js";
|
||||
import { readProfileConfig, writeProfileConfig } from "../agent/profile/storage.js";
|
||||
|
||||
export class Hub {
|
||||
private readonly agents = new Map<string, AsyncAgent>();
|
||||
|
|
@ -30,6 +36,7 @@ export class Hub {
|
|||
private readonly agentStreamIds = new Map<string, string>();
|
||||
private readonly agentStreamCounters = new Map<string, number>();
|
||||
private readonly rpc: RpcDispatcher;
|
||||
private readonly approvalManager: ExecApprovalManager;
|
||||
private client: GatewayClient;
|
||||
readonly deviceStore: DeviceStore;
|
||||
private _onConfirmDevice: ((deviceId: string, agentId: string, meta?: DeviceMeta) => Promise<boolean>) | null = null;
|
||||
|
|
@ -67,6 +74,16 @@ export class Hub {
|
|||
this.rpc.register("deleteAgent", createDeleteAgentHandler(this));
|
||||
this.rpc.register("updateGateway", createUpdateGatewayHandler(this));
|
||||
|
||||
// Initialize exec approval manager
|
||||
this.approvalManager = new ExecApprovalManager((agentId, payload) => {
|
||||
const targetDeviceId = this.agentSenders.get(agentId);
|
||||
if (!targetDeviceId) {
|
||||
throw new Error(`No client device found for agent ${agentId}`);
|
||||
}
|
||||
this.client.send(targetDeviceId, "exec-approval-request", payload);
|
||||
});
|
||||
this.rpc.register("resolveExecApproval", createResolveExecApprovalHandler(this.approvalManager));
|
||||
|
||||
// Register as global singleton for cross-module access (subagent tools, announce flow)
|
||||
setHub(this);
|
||||
|
||||
|
|
@ -198,7 +215,9 @@ export class Hub {
|
|||
}
|
||||
}
|
||||
|
||||
const agent = new AsyncAgent({ sessionId: id, profileId: options?.profileId ?? "default" });
|
||||
const profileId = options?.profileId ?? "default";
|
||||
const onExecApprovalNeeded = this.createExecApprovalCallback(profileId);
|
||||
const agent = new AsyncAgent({ sessionId: id, profileId, onExecApprovalNeeded });
|
||||
this.agents.set(agent.sessionId, agent);
|
||||
|
||||
// Persist to agent store (skip during restore to avoid duplicates)
|
||||
|
|
@ -324,6 +343,94 @@ export class Hub {
|
|||
return agent;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create an exec approval callback for an agent.
|
||||
* This wires the safety evaluation + Hub approval manager together.
|
||||
*/
|
||||
private createExecApprovalCallback(profileId: string): ExecApprovalCallback {
|
||||
return async (command: string, cwd: string | undefined): Promise<ApprovalResult> => {
|
||||
// Load exec approval config from profile
|
||||
let config: ExecApprovalConfig = {};
|
||||
try {
|
||||
const profileConfig = readProfileConfig(profileId);
|
||||
config = profileConfig?.execApproval ?? {};
|
||||
} catch {
|
||||
// No profile config, use defaults
|
||||
}
|
||||
|
||||
const security = config.security ?? "allowlist";
|
||||
const ask = config.ask ?? "on-miss";
|
||||
|
||||
// Security: deny blocks everything
|
||||
if (security === "deny") {
|
||||
return { approved: false, decision: "deny" };
|
||||
}
|
||||
|
||||
// Security: full allows everything
|
||||
if (security === "full") {
|
||||
return { approved: true, decision: "allow-once" };
|
||||
}
|
||||
|
||||
// Evaluate safety
|
||||
const evaluation = evaluateCommandSafety(command, config);
|
||||
|
||||
// Check if approval is needed
|
||||
const needsApproval = requiresApproval({
|
||||
ask,
|
||||
security,
|
||||
analysisOk: evaluation.analysisOk,
|
||||
allowlistSatisfied: evaluation.allowlistSatisfied,
|
||||
});
|
||||
|
||||
if (!needsApproval) {
|
||||
// Record allowlist usage
|
||||
if (evaluation.allowlistSatisfied) {
|
||||
const match = matchAllowlist(config.allowlist ?? [], command);
|
||||
if (match) {
|
||||
try {
|
||||
const profileConfig = readProfileConfig(profileId) ?? {};
|
||||
const updated = recordAllowlistUse(profileConfig.execApproval?.allowlist ?? [], match, command);
|
||||
writeProfileConfig(profileId, { ...profileConfig, execApproval: { ...config, allowlist: updated } });
|
||||
} catch {
|
||||
// Non-critical: don't fail command for usage recording
|
||||
}
|
||||
}
|
||||
}
|
||||
return { approved: true, decision: "allow-once" };
|
||||
}
|
||||
|
||||
// Request approval via Hub → Gateway → Client
|
||||
const result = await this.approvalManager.requestApproval({
|
||||
agentId: profileId,
|
||||
command,
|
||||
cwd,
|
||||
riskLevel: evaluation.riskLevel,
|
||||
riskReasons: evaluation.reasons,
|
||||
timeoutMs: config.timeoutMs,
|
||||
});
|
||||
|
||||
// Handle allow-always: persist to profile allowlist
|
||||
if (result.decision === "allow-always") {
|
||||
try {
|
||||
const profileConfig = readProfileConfig(profileId) ?? {};
|
||||
const currentAllowlist = profileConfig.execApproval?.allowlist ?? [];
|
||||
// Extract binary pattern for allowlist
|
||||
const binary = command.trim().split(/\s+/)[0];
|
||||
const pattern = binary ? `${binary} **` : command;
|
||||
const updated = addAllowlistEntry(currentAllowlist, pattern);
|
||||
writeProfileConfig(profileId, {
|
||||
...profileConfig,
|
||||
execApproval: { ...config, allowlist: updated },
|
||||
});
|
||||
} catch {
|
||||
// Non-critical: command still allowed even if persistence fails
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
};
|
||||
}
|
||||
|
||||
getAgent(id: string): AsyncAgent | undefined {
|
||||
return this.agents.get(id);
|
||||
}
|
||||
|
|
@ -338,6 +445,7 @@ export class Hub {
|
|||
const agent = this.agents.get(id);
|
||||
if (!agent) return false;
|
||||
agent.close();
|
||||
this.approvalManager.cancelPending(id);
|
||||
this.agents.delete(id);
|
||||
this.agentSenders.delete(id);
|
||||
this.agentStreamIds.delete(id);
|
||||
|
|
|
|||
34
src/hub/rpc/handlers/resolve-exec-approval.ts
Normal file
34
src/hub/rpc/handlers/resolve-exec-approval.ts
Normal file
|
|
@ -0,0 +1,34 @@
|
|||
import type { RpcHandler } from "../dispatcher.js";
|
||||
import { RpcError } from "../dispatcher.js";
|
||||
import type { ExecApprovalManager } from "../../exec-approval-manager.js";
|
||||
import type { ApprovalDecision } from "../../../agent/tools/exec-approval-types.js";
|
||||
|
||||
interface ResolveExecApprovalParams {
|
||||
approvalId: string;
|
||||
decision: ApprovalDecision;
|
||||
}
|
||||
|
||||
const VALID_DECISIONS = new Set<ApprovalDecision>(["allow-once", "allow-always", "deny"]);
|
||||
|
||||
export function createResolveExecApprovalHandler(
|
||||
approvalManager: ExecApprovalManager,
|
||||
): RpcHandler {
|
||||
return async (params: unknown) => {
|
||||
const { approvalId, decision } = (params ?? {}) as ResolveExecApprovalParams;
|
||||
|
||||
if (!approvalId || typeof approvalId !== "string") {
|
||||
throw new RpcError("INVALID_PARAMS", "approvalId is required");
|
||||
}
|
||||
|
||||
if (!decision || !VALID_DECISIONS.has(decision)) {
|
||||
throw new RpcError("INVALID_PARAMS", `Invalid decision: ${decision}. Must be allow-once, allow-always, or deny`);
|
||||
}
|
||||
|
||||
const resolved = approvalManager.resolveApproval(approvalId, decision);
|
||||
if (!resolved) {
|
||||
throw new RpcError("NOT_FOUND", "Approval request not found or already resolved");
|
||||
}
|
||||
|
||||
return { ok: true };
|
||||
};
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue