fix: add security guardrails and cleanup lint warnings

extensions/llm/src/factory.test.ts

@@ -1,24 +1,8 @@
 import { describe, it, expect } from "vitest";
-import type { CompletionRequest } from "@step-cli/protocol";
 import { createChatCompletionClient } from "./factory.js";
 import { AnthropicMessagesClient } from "./anthropic-client.js";
 import { OpenAICompatibleClient } from "./openai-client.js";
 
-// ---------------------------------------------------------------------------
-// Helpers
-// ---------------------------------------------------------------------------
-
-/** Build a minimal CompletionRequest for testing. */
-function baseRequest(
-  overrides: Partial<CompletionRequest> = {},
-): CompletionRequest {
-  return {
-    model: "test-model",
-    messages: [{ role: "user", content: "hello" }],
-    ...overrides,
-  };
-}
-
 /** Create a mock HttpTransport that returns a canned JSON response. */
 function mockTransport(
   body: unknown,

packages/core/src/policy/tool-policy.test.ts

@@ -145,52 +145,80 @@ describe("ToolPolicy", () => {
 
   // -- evaluate: dangerous command patterns --
 
-  it("denies dangerous command: rm -rf /", () => {
+  it("requires confirmation for dangerous command: rm -rf /", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
     });
     const spec = makeToolSpec({ name: "bash", risk: "execute" });
     const inspection: ToolCallInspection = { command: "rm -rf /" };
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
-    expect(decision.reason).toContain("dangerous command");
+    expect(decision.mode).toBe("confirm");
+    expect(decision.reason).toContain("requires confirmation");
+  });
+
+  it("forces confirmation for dangerous commands despite an allow override", () => {
+    const policy = new ToolPolicy({
+      mode: "auto",
+      nonInteractiveApproval: "allow",
+      overrides: { bash: "allow" },
+    });
+    const spec = makeToolSpec({ name: "bash", risk: "execute" });
+    const decision = policy.evaluate("bash", "{}", spec, {
+      command: "rm -rf /",
+    });
+
+    expect(decision.mode).toBe("confirm");
+  });
+
+  it("still honors allow overrides for benign commands", () => {
+    const policy = new ToolPolicy({
+      mode: "confirm",
+      nonInteractiveApproval: "deny",
+      overrides: { bash: "allow" },
+    });
+    const spec = makeToolSpec({ name: "bash", risk: "execute" });
+    const decision = policy.evaluate("bash", "{}", spec, {
+      command: "echo hello",
+    });
+
+    expect(decision.mode).toBe("allow");
   });
 
-  it("denies dangerous command: shutdown", () => {
+  it("requires confirmation for dangerous command: shutdown", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
     });
     const spec = makeToolSpec({ name: "bash", risk: "execute" });
     const inspection: ToolCallInspection = { command: "sudo shutdown now" };
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
+    expect(decision.mode).toBe("confirm");
   });
 
-  it("denies dangerous command: reboot", () => {
+  it("requires confirmation for dangerous command: reboot", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
     });
     const spec = makeToolSpec({ name: "bash", risk: "execute" });
     const inspection: ToolCallInspection = { command: "reboot" };
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
+    expect(decision.mode).toBe("confirm");
   });
 
-  it("denies dangerous command: mkfs", () => {
+  it("requires confirmation for dangerous command: mkfs", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
     });
     const spec = makeToolSpec({ name: "bash", risk: "execute" });
     const inspection: ToolCallInspection = { command: "mkfs.ext4 /dev/sda1" };
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
+    expect(decision.mode).toBe("confirm");
   });
 
-  it("denies dangerous command: dd if=", () => {
+  it("requires confirmation for dangerous command: dd if=", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -200,10 +228,10 @@ describe("ToolPolicy", () => {
       command: "dd if=/dev/zero of=/dev/sda",
     };
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
+    expect(decision.mode).toBe("confirm");
   });
 
-  it("denies encoded destructive shell commands", () => {
+  it("requires confirmation for encoded destructive shell commands", () => {
     const policy = new ToolPolicy({
       mode: "confirm",
       nonInteractiveApproval: "deny",
@@ -214,8 +242,8 @@ describe("ToolPolicy", () => {
     };
 
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
-    expect(decision.reason).toMatch(/dangerous command/i);
+    expect(decision.mode).toBe("confirm");
+    expect(decision.reason).toMatch(/requires confirmation/i);
   });
 
   it("allows benign encoded text", () => {
@@ -230,7 +258,7 @@ describe("ToolPolicy", () => {
     expect(decision.mode).toBe("confirm");
   });
 
-  it("denies destructive rm paths beyond filesystem root", () => {
+  it("requires confirmation for destructive rm paths beyond filesystem root", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -239,11 +267,11 @@ describe("ToolPolicy", () => {
     const inspection: ToolCallInspection = { command: "rm -rf /tmp/test" };
 
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
-    expect(decision.reason).toMatch(/dangerous command/i);
+    expect(decision.mode).toBe("confirm");
+    expect(decision.reason).toMatch(/requires confirmation/i);
   });
 
-  it("denies destructive rm variants with split force and recursive flags", () => {
+  it("requires confirmation for destructive rm variants with split force and recursive flags", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -257,12 +285,12 @@ describe("ToolPolicy", () => {
     ]) {
       const inspection: ToolCallInspection = { command };
       const decision = policy.evaluate("bash", "{}", spec, inspection);
-      expect(decision.mode).toBe("deny");
-      expect(decision.reason).toMatch(/dangerous command/i);
+      expect(decision.mode).toBe("confirm");
+      expect(decision.reason).toMatch(/requires confirmation/i);
     }
   });
 
-  it("denies destructive find delete variants", () => {
+  it("requires confirmation for destructive find delete variants", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -273,11 +301,11 @@ describe("ToolPolicy", () => {
     };
 
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
-    expect(decision.reason).toMatch(/dangerous command/i);
+    expect(decision.mode).toBe("confirm");
+    expect(decision.reason).toMatch(/requires confirmation/i);
   });
 
-  it("denies destructive workspace wipe variants", () => {
+  it("requires confirmation for destructive workspace wipe variants", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -288,11 +316,11 @@ describe("ToolPolicy", () => {
     };
 
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
-    expect(decision.reason).toMatch(/dangerous command/i);
+    expect(decision.mode).toBe("confirm");
+    expect(decision.reason).toMatch(/requires confirmation/i);
   });
 
-  it("denies git clean forced delete variants", () => {
+  it("requires confirmation for git clean forced delete variants", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -301,11 +329,11 @@ describe("ToolPolicy", () => {
     const inspection: ToolCallInspection = { command: "git clean -fdx" };
 
     const decision = policy.evaluate("bash", "{}", spec, inspection);
-    expect(decision.mode).toBe("deny");
-    expect(decision.reason).toMatch(/dangerous command/i);
+    expect(decision.mode).toBe("confirm");
+    expect(decision.reason).toMatch(/requires confirmation/i);
   });
 
-  it("denies git clean forced delete variants regardless of short flag order", () => {
+  it("requires confirmation for git clean forced delete variants regardless of short flag order", () => {
     const policy = new ToolPolicy({
       mode: "auto",
       nonInteractiveApproval: "allow",
@@ -315,8 +343,8 @@ describe("ToolPolicy", () => {
     for (const command of ["git clean -xdf", "git clean -x -d -f"]) {
       const inspection: ToolCallInspection = { command };
       const decision = policy.evaluate("bash", "{}", spec, inspection);
-      expect(decision.mode).toBe("deny");
-      expect(decision.reason).toMatch(/dangerous command/i);
+      expect(decision.mode).toBe("confirm");
+      expect(decision.reason).toMatch(/requires confirmation/i);
     }
   });
 

packages/core/src/policy/tool-policy.ts

@@ -69,9 +69,9 @@ export class ToolPolicy implements ToolPermissionPolicy {
     const command = inspection?.command?.trim();
     if (command && isDangerousCommand(command)) {
       return {
-        mode: "deny",
+        mode: "confirm",
         risk,
-        reason: `Blocked dangerous command pattern in ${toolName}: ${shorten(command, 120)}`,
+        reason: `Dangerous command pattern in ${toolName} requires confirmation: ${shorten(command, 120)}`,
       };
     }
 

packages/core/src/tools/native-impls/file-tools.ts

@@ -9,11 +9,13 @@ import type {
   ToolExecutionResult,
   ToolSpec,
 } from "@step-cli/protocol";
+import { resolveWorkspacePath } from "@step-cli/utils/path.js";
 import {
   asObject,
   optionalBoolean,
   optionalNumber,
   requireString,
+  safeParse,
   ToolArgError,
 } from "./parsers.js";
 
@@ -152,11 +154,6 @@ function parseEditArgs(rawArgs: string): EditArgs {
   };
 }
 
-function safeParse(rawArgs: string): unknown {
-  if (!rawArgs?.trim()) return {};
-  return JSON.parse(rawArgs);
-}
-
 async function readFileExecute(
   args: ReadArgs,
   ctx: ToolExecutionContext,
@@ -264,15 +261,6 @@ async function readSlice(
   return { ok: true, summary: collected.join("\n") };
 }
 
-function resolveWorkspacePath(
-  workspaceRoot: string,
-  candidate: string,
-): string {
-  return path.isAbsolute(candidate)
-    ? candidate
-    : path.resolve(workspaceRoot, candidate);
-}
-
 const WINDOWS_DRIVE_PATH = /^[A-Za-z]:[\\/]/;
 
 function rejectWindowsPathOnPosix(

packages/core/src/tools/native-impls/grep-regex.test.ts

@@ -0,0 +1,15 @@
+import { describe, expect, it } from "vitest";
+import { createSafeGrepRegex } from "./grep-regex.js";
+
+describe("createSafeGrepRegex", () => {
+  it("rejects nested quantifiers with catastrophic-backtracking risk", () => {
+    expect(createSafeGrepRegex("(a+)+b")).toBeNull();
+  });
+
+  it("accepts a normal grep pattern", () => {
+    const regex = createSafeGrepRegex("TODO|FIXME");
+
+    expect(regex?.test("// TODO: add coverage")).toBe(true);
+    expect(regex?.test("const value = 1")).toBe(false);
+  });
+});

packages/core/src/tools/native-impls/grep-regex.ts

@@ -0,0 +1,15 @@
+const REDOS_PATTERNS = [
+  /\([^)]*[+*][^)]*\)\s*(?:[+*]|\{\d+(?:,\d*)?\})/,
+  /\(\S*(?:\|.*){2,}\)\s*[+*]/,
+  /\((?:\w|\|){2,}\)\s*\+/,
+  /\(.*\)\s*\{\d+,\}/,
+];
+
+export function createSafeGrepRegex(pattern: string): RegExp | null {
+  try {
+    if (REDOS_PATTERNS.some((rule) => rule.test(pattern))) return null;
+    return new RegExp(pattern);
+  } catch {
+    return null;
+  }
+}

packages/core/src/tools/native-impls/parsers.ts

@@ -72,3 +72,8 @@ export function optionalBoolean(
   }
   return value;
 }
+
+export function safeParse(rawArgs: string): unknown {
+  if (!rawArgs?.trim()) return {};
+  return JSON.parse(rawArgs);
+}

packages/core/src/tools/native-impls/shell-tools.test.ts

@@ -0,0 +1,14 @@
+import { describe, expect, it } from "vitest";
+import { buildBashTool } from "./shell-tools.js";
+
+describe("Bash tool inspection", () => {
+  it("exposes the command to the permission policy", () => {
+    const tool = buildBashTool();
+    const inspection = tool.inspect?.({
+      args: { command: "rm -rf /" },
+      rawArgs: JSON.stringify({ command: "rm -rf /" }),
+    });
+
+    expect(inspection?.command).toBe("rm -rf /");
+  });
+});