feat(exec-approval): default to full/off security and support no-timeout
- Change default security from "allowlist" to "full" (allow all commands) - Change default ask from "on-miss" to "off" (never prompt) - Change DEFAULT_APPROVAL_TIMEOUT_MS from 60s to -1 (no timeout) - Support timeoutMs=-1 to wait indefinitely for user decision - Update CLI and Hub approval flows to skip setTimeout when timeout<0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
91aa433f34
commit
a36cbac3fd
4 changed files with 38 additions and 34 deletions
|
|
@ -50,8 +50,8 @@ export function createCliApprovalCallback(
|
|||
const runtimeConfig = { ...config, allowlist: [...(config.allowlist ?? [])] };
|
||||
|
||||
return async (command: string, cwd: string | undefined): Promise<ApprovalResult> => {
|
||||
const security = runtimeConfig.security ?? "allowlist";
|
||||
const ask = runtimeConfig.ask ?? "on-miss";
|
||||
const security = runtimeConfig.security ?? "full";
|
||||
const ask = runtimeConfig.ask ?? "off";
|
||||
const timeoutMs = runtimeConfig.timeoutMs ?? DEFAULT_APPROVAL_TIMEOUT_MS;
|
||||
|
||||
// Security: deny blocks everything
|
||||
|
|
@ -137,13 +137,15 @@ function promptTerminal(
|
|||
rl.close();
|
||||
};
|
||||
|
||||
// Timeout: auto-deny
|
||||
const timer = setTimeout(() => {
|
||||
if (resolved) return;
|
||||
process.stderr.write(dim(`\n Approval timed out (${timeoutMs / 1000}s). Denying.\n\n`));
|
||||
cleanup();
|
||||
resolve("deny");
|
||||
}, timeoutMs);
|
||||
// Timeout: auto-deny (skip if timeoutMs is -1 for no timeout)
|
||||
const timer = timeoutMs >= 0
|
||||
? setTimeout(() => {
|
||||
if (resolved) return;
|
||||
process.stderr.write(dim(`\n Approval timed out (${timeoutMs / 1000}s). Denying.\n\n`));
|
||||
cleanup();
|
||||
resolve("deny");
|
||||
}, timeoutMs)
|
||||
: null;
|
||||
|
||||
// Display approval prompt
|
||||
process.stderr.write("\n");
|
||||
|
|
@ -161,7 +163,7 @@ function promptTerminal(
|
|||
rl.question(
|
||||
` ${bold("[a]")}llow once / ${bold("[A]")}llow always / ${bold("[d]")}eny (default: deny): `,
|
||||
(answer) => {
|
||||
clearTimeout(timer);
|
||||
if (timer) clearTimeout(timer);
|
||||
cleanup();
|
||||
|
||||
const trimmed = answer.trim();
|
||||
|
|
@ -177,7 +179,7 @@ function promptTerminal(
|
|||
|
||||
// Handle Ctrl+C gracefully
|
||||
rl.on("close", () => {
|
||||
clearTimeout(timer);
|
||||
if (timer) clearTimeout(timer);
|
||||
if (!resolved) {
|
||||
resolved = true;
|
||||
resolve("deny");
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ export interface ExecApprovalConfig {
|
|||
security?: ExecSecurity;
|
||||
/** Ask mode: "off" never asks, "on-miss" asks when allowlist misses, "always" always asks */
|
||||
ask?: ExecAsk;
|
||||
/** Timeout before auto-deny in milliseconds (default: 60_000) */
|
||||
/** Timeout before auto-deny in milliseconds (default: 60_000). Set to -1 for no timeout. */
|
||||
timeoutMs?: number;
|
||||
/** Fallback security level on timeout (default: "deny" — fail-closed) */
|
||||
askFallback?: ExecSecurity;
|
||||
|
|
@ -58,8 +58,8 @@ export interface ExecApprovalConfig {
|
|||
allowlist?: ExecAllowlistEntry[];
|
||||
}
|
||||
|
||||
/** Default timeout for approval requests (60 seconds) */
|
||||
export const DEFAULT_APPROVAL_TIMEOUT_MS = 60_000;
|
||||
/** Default timeout for approval requests (-1 = no timeout, wait indefinitely) */
|
||||
export const DEFAULT_APPROVAL_TIMEOUT_MS = -1;
|
||||
|
||||
// ============ Allowlist ============
|
||||
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ import { DEFAULT_APPROVAL_TIMEOUT_MS } from "../agent/tools/exec-approval-types.
|
|||
|
||||
interface PendingEntry {
|
||||
resolve: (result: ApprovalResult) => void;
|
||||
timer: NodeJS.Timeout;
|
||||
timer: NodeJS.Timeout | null;
|
||||
request: ExecApprovalRequest;
|
||||
}
|
||||
|
||||
|
|
@ -52,7 +52,7 @@ export class ExecApprovalManager {
|
|||
}): Promise<ApprovalResult> {
|
||||
const approvalId = uuidv7();
|
||||
const timeoutMs = params.timeoutMs ?? this.defaultTimeoutMs;
|
||||
const expiresAtMs = Date.now() + timeoutMs;
|
||||
const expiresAtMs = timeoutMs >= 0 ? Date.now() + timeoutMs : -1;
|
||||
|
||||
const request: ExecApprovalRequest = {
|
||||
approvalId,
|
||||
|
|
@ -65,19 +65,21 @@ export class ExecApprovalManager {
|
|||
};
|
||||
|
||||
return new Promise<ApprovalResult>((resolve) => {
|
||||
// Timeout: follow askFallback (default: fail-closed)
|
||||
const timer = setTimeout(() => {
|
||||
if (this.pending.has(approvalId)) {
|
||||
this.pending.delete(approvalId);
|
||||
const fallback = params.askFallback ?? "deny";
|
||||
const decision =
|
||||
fallback === "full" ||
|
||||
(fallback === "allowlist" && params.allowlistSatisfied)
|
||||
? "allow-once"
|
||||
: "deny";
|
||||
resolve({ approved: decision !== "deny", decision });
|
||||
}
|
||||
}, timeoutMs);
|
||||
// Timeout: follow askFallback (default: fail-closed). Skip if timeoutMs is -1 (no timeout).
|
||||
const timer = timeoutMs >= 0
|
||||
? setTimeout(() => {
|
||||
if (this.pending.has(approvalId)) {
|
||||
this.pending.delete(approvalId);
|
||||
const fallback = params.askFallback ?? "deny";
|
||||
const decision =
|
||||
fallback === "full" ||
|
||||
(fallback === "allowlist" && params.allowlistSatisfied)
|
||||
? "allow-once"
|
||||
: "deny";
|
||||
resolve({ approved: decision !== "deny", decision });
|
||||
}
|
||||
}, timeoutMs)
|
||||
: null;
|
||||
|
||||
this.pending.set(approvalId, { resolve, timer, request });
|
||||
|
||||
|
|
@ -86,7 +88,7 @@ export class ExecApprovalManager {
|
|||
this.sendToClient(params.agentId, request);
|
||||
} catch (err) {
|
||||
// If sending fails, auto-deny (fail-closed)
|
||||
clearTimeout(timer);
|
||||
if (timer) clearTimeout(timer);
|
||||
this.pending.delete(approvalId);
|
||||
console.error(`[ExecApprovalManager] Failed to send approval request: ${err}`);
|
||||
resolve({ approved: false, decision: "deny" });
|
||||
|
|
@ -102,7 +104,7 @@ export class ExecApprovalManager {
|
|||
const entry = this.pending.get(approvalId);
|
||||
if (!entry) return false;
|
||||
|
||||
clearTimeout(entry.timer);
|
||||
if (entry.timer) clearTimeout(entry.timer);
|
||||
this.pending.delete(approvalId);
|
||||
|
||||
entry.resolve({
|
||||
|
|
@ -120,7 +122,7 @@ export class ExecApprovalManager {
|
|||
cancelPending(agentId: string): void {
|
||||
for (const [id, entry] of this.pending) {
|
||||
if (entry.request.agentId === agentId) {
|
||||
clearTimeout(entry.timer);
|
||||
if (entry.timer) clearTimeout(entry.timer);
|
||||
this.pending.delete(id);
|
||||
entry.resolve({ approved: false, decision: "deny" });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -422,8 +422,8 @@ export class Hub {
|
|||
// No profile config, use defaults
|
||||
}
|
||||
|
||||
const security = config.security ?? "allowlist";
|
||||
const ask = config.ask ?? "on-miss";
|
||||
const security = config.security ?? "full";
|
||||
const ask = config.ask ?? "off";
|
||||
|
||||
// Security: deny blocks everything
|
||||
if (security === "deny") {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue