Fix command-form approval rules

cli/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@yc-software/qm",
-  "version": "0.1.5",
+  "version": "0.1.6",
   "license": "MIT",
   "description": "Control-plane CLI for portable QM deployments on Docker, Fly, and AWS.",
   "type": "module",

cli/src/safe-regex.ts

@@ -0,0 +1,67 @@
+const MAX_PATTERN_CHARS = 256;
+
+export function compileSafeRegex(pattern: string, flags = ""): RegExp {
+  if (!pattern || pattern.length > MAX_PATTERN_CHARS)
+    throw new Error(`pattern must be 1-${MAX_PATTERN_CHARS} characters`);
+  if (/\\[1-9]|\\k<|\(\?[=!<]/.test(pattern)) throw new Error("backreferences and lookarounds are not supported");
+
+  const groups: Array<{ quantified: boolean; alternation: boolean }> = [];
+  let escaped = false;
+  let inClass = false;
+  let previousQuantifier = false;
+  let closed: { quantified: boolean; alternation: boolean } | null = null;
+  for (let i = 0; i < pattern.length; i++) {
+    const ch = pattern[i]!;
+    if (escaped) {
+      escaped = false;
+      previousQuantifier = false;
+      closed = null;
+      continue;
+    }
+    if (ch === "\\") {
+      escaped = true;
+      continue;
+    }
+    if (ch === "[") {
+      inClass = true;
+      previousQuantifier = false;
+      closed = null;
+      continue;
+    }
+    if (ch === "]" && inClass) {
+      inClass = false;
+      continue;
+    }
+    if (inClass) continue;
+    if (ch === "(") {
+      groups.push({ quantified: false, alternation: false });
+      previousQuantifier = false;
+      closed = null;
+      continue;
+    }
+    if (ch === "|") {
+      if (groups.length) groups[groups.length - 1]!.alternation = true;
+      previousQuantifier = false;
+      closed = null;
+      continue;
+    }
+    if (ch === ")") {
+      closed = groups.pop() ?? { quantified: false, alternation: false };
+      previousQuantifier = false;
+      continue;
+    }
+    const quantifier = ch === "*" || ch === "+" || (ch === "?" && pattern[i - 1] !== "(") || ch === "{";
+    if (quantifier) {
+      if (previousQuantifier || (closed && (closed.quantified || closed.alternation))) {
+        throw new Error("nested or ambiguous repetition is not supported");
+      }
+      if (groups.length) groups[groups.length - 1]!.quantified = true;
+      previousQuantifier = true;
+      closed = null;
+      continue;
+    }
+    previousQuantifier = false;
+    closed = null;
+  }
+  return new RegExp(pattern, flags);
+}

cli/src/sandbox-layer.ts

@@ -2,6 +2,7 @@ import { existsSync, readFileSync, readdirSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { JUNK_FILE, deploymentLayerBundle } from "./deployment-layer.ts";
 import { errMessage } from "./log.ts";
+import { compileSafeRegex } from "./safe-regex.ts";
 
 export type ApprovalDecision = "require_approval" | "deny";
 
@@ -184,6 +185,11 @@ export function parseToolDescriptor(raw: string, sourcePath: string): ToolDescri
         `${sourcePath}: approvals[${i}] pattern is too slow to evaluate — it may cause catastrophic backtracking`,
       );
     }
+    try {
+      compileSafeRegex(compiled.pattern, "i");
+    } catch (e) {
+      throw new Error(`${sourcePath}: approvals[${i}] is not a valid regex: ${errMessage(e)}`, { cause: e });
+    }
     if (approval.pattern !== undefined && !rawApprovalTargetsTool(binary, approval.pattern)) {
       throw new Error(
         `${sourcePath}: approvals[${i}].pattern must refer to its own tool binary by starting with \\b${binary}\\b and may not use a top-level alternative`,
@@ -623,7 +629,7 @@ export function compileApproval(binary: string, a: ToolApproval): { pattern: str
   const decision: ApprovalDecision = a.decision ?? "require_approval";
   if (a.pattern !== undefined) return { pattern: a.pattern, decision };
   const words = (a.command ?? "").trim().split(/\s+/).filter(Boolean).map(escapeRegex);
-  const pattern = `\\b${[escapeRegex(binary), ...words].join("\\s+")}(?:\\b|(?=\\s|$))`;
+  const pattern = `\\b${[escapeRegex(binary), ...words].join("\\s+")}(?:\\b|\\s|$)`;
   return { pattern, decision };
 }
 

cli/test/tool-descriptor.test.ts

@@ -235,15 +235,15 @@ test("approvals: command|pattern (exactly one), decision enum, reason optional",
 
 test("compileApproval anchors a command to the binary and builds the regex; pattern is verbatim", () => {
   assert.deepEqual(compileApproval("my-tool", { command: "deploy" }), {
-    pattern: "\\bmy-tool\\s+deploy(?:\\b|(?=\\s|$))",
+    pattern: "\\bmy-tool\\s+deploy(?:\\b|\\s|$)",
     decision: "require_approval",
   });
   assert.deepEqual(compileApproval("my-tool", { command: "secrets set" }), {
-    pattern: "\\bmy-tool\\s+secrets\\s+set(?:\\b|(?=\\s|$))",
+    pattern: "\\bmy-tool\\s+secrets\\s+set(?:\\b|\\s|$)",
     decision: "require_approval",
   });
   assert.deepEqual(compileApproval("my-tool", { command: "delete", decision: "deny" }), {
-    pattern: "\\bmy-tool\\s+delete(?:\\b|(?=\\s|$))",
+    pattern: "\\bmy-tool\\s+delete(?:\\b|\\s|$)",
     decision: "deny",
   });
   assert.deepEqual(compileApproval("my-tool", { pattern: "\\bmy-tool\\b\\s+--force\\b" }), {

src/deployment/deployment-layer.ts

@@ -1,3 +1,5 @@
+import { compileSafeRegex } from "../util/safe-regex.ts";
+
 export type ApprovalDecision = "require_approval" | "deny";
 
 export interface ToolApproval {
@@ -175,6 +177,11 @@ export function parseToolDescriptor(raw: string, sourcePath: string): ToolDescri
         `${sourcePath}: approvals[${i}] pattern is too slow to evaluate — it may cause catastrophic backtracking`,
       );
     }
+    try {
+      compileSafeRegex(compiled.pattern, "i");
+    } catch (e) {
+      throw new Error(`${sourcePath}: approvals[${i}] is not a valid regex: ${(e as Error).message}`, { cause: e });
+    }
     if (approval.pattern !== undefined && !rawApprovalTargetsTool(binary, approval.pattern)) {
       throw new Error(
         `${sourcePath}: approvals[${i}].pattern must refer to its own tool binary by starting with \\b${binary}\\b and may not use a top-level alternative`,
@@ -612,7 +619,7 @@ export function compileApproval(binary: string, a: ToolApproval): { pattern: str
   const decision: ApprovalDecision = a.decision ?? "require_approval";
   if (a.pattern !== undefined) return { pattern: a.pattern, decision };
   const words = (a.command ?? "").trim().split(/\s+/).filter(Boolean).map(escapeRegex);
-  return { pattern: `\\b${[escapeRegex(binary), ...words].join("\\s+")}(?:\\b|(?=\\s|$))`, decision };
+  return { pattern: `\\b${[escapeRegex(binary), ...words].join("\\s+")}(?:\\b|\\s|$)`, decision };
 }
 
 export function interpolateSplitEnv(

test/deployment-layer-load.test.ts

@@ -8,6 +8,7 @@ import { buildApp } from "../src/wiring.ts";
 import { testConfig } from "./support/test-config.ts";
 import { parseToolDescriptor } from "../src/deployment/deployment-layer.ts";
 import { BASE_EPHEMERAL_CRED_LINKS, BASE_RESIDENT_AUTH_PATHS } from "../src/credentials/resident-paths.ts";
+import { evaluateCommandWithLayer } from "../src/policy/command-policy.ts";
 
 const credentialFile = (path: string) => ({ path, kind: "file" as const });
 const credentialDirectory = (path: string) => ({ path, kind: "directory" as const });
@@ -121,13 +122,31 @@ test("loadDeploymentLayer: an authless tool contributes no connector or paths",
   assert.deepEqual(layer.advertisedTools, ["helper tool"]);
 });
 
-test("approval rules target install.binary rather than the descriptor id", () => {
+test("command-form approval rules target install.binary and survive safe-regex evaluation", () => {
   const layer = loadDeploymentLayer(
     layerDir({
-      acme: { id: "acme", install: { binary: "acmectl" }, approvals: [{ command: "delete", decision: "deny" }] },
+      acme: {
+        id: "acme",
+        install: { binary: "acmectl" },
+        approvals: [{ command: "delete", decision: "require_approval" }],
+      },
     }),
   );
-  assert.equal(layer.commandRules[0]?.pattern, "\\bacmectl\\s+delete(?:\\b|(?=\\s|$))");
+  assert.equal(layer.commandRules[0]?.pattern, "\\bacmectl\\s+delete(?:\\b|\\s|$)");
+  const policy = { mode: "denylist" as const, rules: [] };
+  assert.equal(
+    evaluateCommandWithLayer("acmectl delete project", policy, layer.commandRules).decision,
+    "require_approval",
+  );
+  assert.equal(evaluateCommandWithLayer("acmectl delete", policy, layer.commandRules).decision, "require_approval");
+  assert.equal(evaluateCommandWithLayer("acmectl deleteall", policy, layer.commandRules).decision, "allow");
+});
+
+test("loadDeploymentLayer rejects approval patterns the policy engine cannot evaluate", () => {
+  assert.throws(
+    () => loadDeploymentLayer(layerDir({ acme: { id: "acme", approvals: [{ pattern: "\\bacme\\b(?=\\s|$)" }] } })),
+    /lookarounds are not supported/,
+  );
 });
 
 test("loadDeploymentLayer: a dir with no tools/ is a valid, empty layer", () => {