From a597938be85e54cf667710a8cb1b22ddc1b2a111 Mon Sep 17 00:00:00 2001
From: wangchunyue <80630709+openperf@users.noreply.github.com>
Date: Thu, 2 Apr 2026 16:40:10 +0800
Subject: [PATCH] fix(exec): strip invalid approval policy enums during config
 normalization (#59112)

* fix(exec): strip invalid security/ask enum values during config normalization

* fix(exec): narrow invalid approvals config cleanup

---------

Co-authored-by: scoootscooob <zhentongfan@gmail.com>
---
 src/infra/exec-approvals-config.test.ts | 104 ++++++++++++++++++++++++
 src/infra/exec-approvals.ts             |  38 +++++++--
 2 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/src/infra/exec-approvals-config.test.ts b/src/infra/exec-approvals-config.test.ts
index 7d5a39bf8f6..19ea2414d2c 100644
--- a/src/infra/exec-approvals-config.test.ts
+++ b/src/infra/exec-approvals-config.test.ts
@@ -354,3 +354,107 @@ describe("normalizeExecApprovals handles string allowlist entries (#9790)", () =
     }
   });
 });
+
+describe("normalizeExecApprovals strips invalid security/ask enum values (#59006)", () => {
+  it("drops invalid defaults.security values like 'none'", () => {
+    const file = {
+      version: 1,
+      defaults: { security: "none" },
+      agents: {},
+    } as unknown as ExecApprovalsFile;
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.defaults?.security).toBeUndefined();
+  });
+
+  it("drops invalid defaults.ask values like 'never'", () => {
+    const file = {
+      version: 1,
+      defaults: { ask: "never" },
+      agents: {},
+    } as unknown as ExecApprovalsFile;
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.defaults?.ask).toBeUndefined();
+  });
+
+  it("drops invalid defaults.askFallback values", () => {
+    const file = {
+      version: 1,
+      defaults: { askFallback: "none" },
+      agents: {},
+    } as unknown as ExecApprovalsFile;
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.defaults?.askFallback).toBeUndefined();
+  });
+
+  it("preserves valid defaults.security and defaults.ask values", () => {
+    const file: ExecApprovalsFile = {
+      version: 1,
+      defaults: { security: "full", ask: "off", askFallback: "deny" },
+      agents: {},
+    };
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.defaults?.security).toBe("full");
+    expect(normalized.defaults?.ask).toBe("off");
+    expect(normalized.defaults?.askFallback).toBe("deny");
+  });
+
+  it("drops invalid agent-level security/ask values", () => {
+    const file = {
+      version: 1,
+      agents: {
+        main: { security: "none", ask: "never", askFallback: "open" },
+      },
+    } as unknown as ExecApprovalsFile;
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.agents?.main?.security).toBeUndefined();
+    expect(normalized.agents?.main?.ask).toBeUndefined();
+    expect(normalized.agents?.main?.askFallback).toBeUndefined();
+  });
+
+  it("drops invalid wildcard agent security/ask values", () => {
+    const file = {
+      version: 1,
+      agents: {
+        "*": { security: "none", ask: "off" },
+      },
+    } as unknown as ExecApprovalsFile;
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.agents?.["*"]?.security).toBeUndefined();
+    expect(normalized.agents?.["*"]?.ask).toBe("off");
+  });
+
+  it("resolves to built-in defaults when invalid values are stripped", () => {
+    const file = {
+      version: 1,
+      defaults: { security: "none", ask: "never" },
+      agents: {
+        "*": { security: "none", ask: "off" },
+      },
+    } as unknown as ExecApprovalsFile;
+    const resolved = resolveExecApprovalsFromFile({ file });
+    // Invalid "none" in defaults is stripped, so fallback to DEFAULT_SECURITY ("deny")
+    expect(resolved.defaults.security).toBe("deny");
+    // Invalid "never" in defaults is stripped, so fallback to DEFAULT_ASK ("on-miss")
+    expect(resolved.defaults.ask).toBe("on-miss");
+    // Wildcard agent "none" is stripped, so agent inherits resolved defaults
+    expect(resolved.agent.security).toBe("deny");
+    // Wildcard agent ask="off" is valid and preserved
+    expect(resolved.agent.ask).toBe("off");
+  });
+
+  it("strips non-string policy values (e.g. numbers, booleans) without throwing", () => {
+    const file = {
+      version: 1,
+      defaults: { security: 1, ask: true, askFallback: ["deny"] },
+      agents: {
+        main: { security: 42, ask: false },
+      },
+    } as unknown as ExecApprovalsFile;
+    const normalized = normalizeExecApprovals(file);
+    expect(normalized.defaults?.security).toBeUndefined();
+    expect(normalized.defaults?.ask).toBeUndefined();
+    expect(normalized.defaults?.askFallback).toBeUndefined();
+    expect(normalized.agents?.main?.security).toBeUndefined();
+    expect(normalized.agents?.main?.ask).toBeUndefined();
+  });
+});
diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts
index 3603228af95..7f815da49e8 100644
--- a/src/infra/exec-approvals.ts
+++ b/src/infra/exec-approvals.ts
@@ -28,6 +28,11 @@ export function normalizeExecTarget(value?: string | null): ExecTarget | null {
   return normalizeExecHost(normalized);
 }
 
+/** Coerce a raw JSON field to string, returning undefined for non-string types. */
+function toStringOrUndefined(value: unknown): string | undefined {
+  return typeof value === "string" ? value : undefined;
+}
+
 export function normalizeExecSecurity(value?: string | null): ExecSecurity | null {
   const normalized = value?.trim().toLowerCase();
   if (normalized === "deny" || normalized === "allowlist" || normalized === "full") {
@@ -291,6 +296,17 @@ function stripAllowlistCommandText(
   return changed ? next : allowlist;
 }
 
+function sanitizeExecApprovalPolicy(
+  policy: ExecApprovalsDefaults | ExecApprovalsAgent | undefined,
+): ExecApprovalsDefaults {
+  return {
+    security: normalizeExecSecurity(toStringOrUndefined(policy?.security)) ?? undefined,
+    ask: normalizeExecAsk(toStringOrUndefined(policy?.ask)) ?? undefined,
+    askFallback: normalizeExecSecurity(toStringOrUndefined(policy?.askFallback)) ?? undefined,
+    autoAllowSkills: policy?.autoAllowSkills,
+  };
+}
+
 export function normalizeExecApprovals(file: ExecApprovalsFile): ExecApprovalsFile {
   const socketPath = file.socket?.path?.trim();
   const token = file.socket?.token?.trim();
@@ -305,10 +321,23 @@ export function normalizeExecApprovals(file: ExecApprovalsFile): ExecApprovalsFi
     const coerced = coerceAllowlistEntries(agent.allowlist);
     const withIds = ensureAllowlistIds(coerced);
     const allowlist = stripAllowlistCommandText(withIds);
-    if (allowlist !== agent.allowlist) {
-      agents[key] = { ...agent, allowlist };
+    const sanitizedPolicy = sanitizeExecApprovalPolicy(agent);
+    const agentChanged =
+      allowlist !== agent.allowlist ||
+      sanitizedPolicy.security !== agent.security ||
+      sanitizedPolicy.ask !== agent.ask ||
+      sanitizedPolicy.askFallback !== agent.askFallback;
+    if (agentChanged) {
+      agents[key] = {
+        ...agent,
+        allowlist,
+        security: sanitizedPolicy.security,
+        ask: sanitizedPolicy.ask,
+        askFallback: sanitizedPolicy.askFallback,
+      };
     }
   }
+  const sanitizedDefaults = sanitizeExecApprovalPolicy(file.defaults);
   const normalized: ExecApprovalsFile = {
     version: 1,
     socket: {
@@ -316,10 +345,7 @@ export function normalizeExecApprovals(file: ExecApprovalsFile): ExecApprovalsFi
       token: token && token.length > 0 ? token : undefined,
     },
     defaults: {
-      security: file.defaults?.security,
-      ask: file.defaults?.ask,
-      askFallback: file.defaults?.askFallback,
-      autoAllowSkills: file.defaults?.autoAllowSkills,
+      ...sanitizedDefaults,
     },
     agents,
   };