From 345c860e36fb8e3b6e24171c120a5e2614b40cdf Mon Sep 17 00:00:00 2001 From: Sprite Date: Mon, 10 Aug 2026 19:48:30 +0000 Subject: [PATCH 1/3] fix(policy): enforce command-form approval rules --- src/deployment/deployment-layer.ts | 6 ++++-- test/deployment-layer-load.test.ts | 25 ++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/deployment/deployment-layer.ts b/src/deployment/deployment-layer.ts index 9ee0c27e..a7c01331 100644 --- a/src/deployment/deployment-layer.ts +++ 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 { @@ -163,7 +165,7 @@ export function parseToolDescriptor(raw: string, sourcePath: string): ToolDescri for (const [i, approval] of (out.approvals ?? []).entries()) { const compiled = compileApproval(binary, approval); try { - new RegExp(compiled.pattern, "i"); + compileSafeRegex(compiled.pattern, "i"); } catch (e) { throw new Error(`${sourcePath}: approvals[${i}] is not a valid regex: ${(e as Error).message}`, { cause: e }); } @@ -612,7 +614,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( diff --git a/test/deployment-layer-load.test.ts b/test/deployment-layer-load.test.ts index f7360657..bc62bc81 100644 --- a/test/deployment-layer-load.test.ts +++ b/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", () => { From 9460b8de37375e5d6633d79eabf67bbe2c95d7f0 Mon Sep 17 00:00:00 2001 From: Sprite Date: Mon, 10 Aug 2026 20:49:54 +0000 Subject: [PATCH 2/3] fix(cli): keep approval validation in sync --- cli/src/safe-regex.ts | 67 ++++++++++++++++++++++++++++++ cli/src/sandbox-layer.ts | 8 +++- cli/test/tool-descriptor.test.ts | 6 +-- src/deployment/deployment-layer.ts | 7 +++- 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 cli/src/safe-regex.ts diff --git a/cli/src/safe-regex.ts b/cli/src/safe-regex.ts new file mode 100644 index 00000000..fb1600b3 --- /dev/null +++ b/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); +} diff --git a/cli/src/sandbox-layer.ts b/cli/src/sandbox-layer.ts index 45390ee9..22a471fd 100644 --- a/cli/src/sandbox-layer.ts +++ b/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 }; } diff --git a/cli/test/tool-descriptor.test.ts b/cli/test/tool-descriptor.test.ts index 90506e52..c5098c25 100644 --- a/cli/test/tool-descriptor.test.ts +++ b/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" }), { diff --git a/src/deployment/deployment-layer.ts b/src/deployment/deployment-layer.ts index a7c01331..6d9f5d13 100644 --- a/src/deployment/deployment-layer.ts +++ b/src/deployment/deployment-layer.ts @@ -165,7 +165,7 @@ export function parseToolDescriptor(raw: string, sourcePath: string): ToolDescri for (const [i, approval] of (out.approvals ?? []).entries()) { const compiled = compileApproval(binary, approval); try { - compileSafeRegex(compiled.pattern, "i"); + new RegExp(compiled.pattern, "i"); } catch (e) { throw new Error(`${sourcePath}: approvals[${i}] is not a valid regex: ${(e as Error).message}`, { cause: e }); } @@ -177,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`, From 8a22b907c06077c6a3af8a77536186a17c2589f9 Mon Sep 17 00:00:00 2001 From: Sprite Date: Mon, 10 Aug 2026 20:51:11 +0000 Subject: [PATCH 3/3] chore(cli): bump package version --- cli/package-lock.json | 4 ++-- cli/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/package-lock.json b/cli/package-lock.json index 6a122921..8b469f6a 100644 --- a/cli/package-lock.json +++ b/cli/package-lock.json @@ -1,12 +1,12 @@ { "name": "@yc-software/qm", - "version": "0.1.5", + "version": "0.1.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@yc-software/qm", - "version": "0.1.5", + "version": "0.1.6", "license": "MIT", "bin": { "qm": "dist/bin/qm.js" diff --git a/cli/package.json b/cli/package.json index d7349677..4fbc357d 100644 --- a/cli/package.json +++ b/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",