From 2f67e51bcde8680a6014f47f60fcd792710dd999 Mon Sep 17 00:00:00 2001 From: Flint Date: Tue, 21 Jul 2026 09:25:34 -0700 Subject: [PATCH 1/2] harden(mail): validate match patterns + escape prefix interpolation for regex safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two regex-safety improvements on the mail-handling path: - Match/routing patterns from tps.yaml are now safety-validated at schema time, not just checked for compilation. RegexStringSchema additionally runs safe-regex, so a well-formed-but-unsafe pattern fails validation and the manifest is dropped before the pattern can reach the runtime match sites (mail-handler routing + matchesFilter body match). Corrected the comments that implied "validated" already meant "safe" — it previously only meant "parses". - task-result-mail escapes the interpolated `prefix` before building its strip pattern, so regex metacharacters in a prefix (e.g. the parentheses in "Task complete (via Flair)") are matched literally instead of being interpreted as regex syntax. No behavior change for metacharacter-free prefixes; the parenthesized caller now de-duplicates its prefix as intended. Adds safe-regex (+ @types/safe-regex) to the cli package. Tests cover schema-time rejection of unsafe patterns (bounded, fast) and literal prefix matching. Co-Authored-By: Claude Fable 5 --- bun.lock | 8 ++++ packages/cli/package.json | 2 + packages/cli/src/schema/manifest.ts | 32 +++++++++---- packages/cli/src/utils/mail-handler.ts | 2 +- packages/cli/src/utils/manifest.ts | 2 +- packages/cli/src/utils/task-result-mail.ts | 11 ++++- packages/cli/test/manifest-schema.test.ts | 52 ++++++++++++++++++++++ packages/cli/test/task-result-mail.test.ts | 22 +++++++++ 8 files changed, 120 insertions(+), 11 deletions(-) diff --git a/bun.lock b/bun.lock index 17365a8..ff35b1d 100644 --- a/bun.lock +++ b/bun.lock @@ -53,6 +53,7 @@ "msgpackr": "^1.11.8", "noise-handshake": "^4.2.0", "react": "^18.3.1", + "safe-regex": "^2.1.1", "snooplogg": "^6.1.1", "ws": "^8.19.0", "zod": "^3.24.0", @@ -63,6 +64,7 @@ "@types/js-yaml": "^4.0.9", "@types/node": "^22.0.0", "@types/react": "^18.3.0", + "@types/safe-regex": "^1.1.6", "fast-check": "^4.5.3", "typescript": "^5.7.0", }, @@ -207,6 +209,8 @@ "@types/react": ["@types/react@18.3.28", "", { "dependencies": { "@types/prop-types": "*", "csstype": "^3.2.2" } }, "sha512-z9VXpC7MWrhfWipitjNdgCauoMLRdIILQsAEV+ZesIzBq/oUlxk0m3ApZuMFCXdnS4U7KrI+l3WRUEGQ8K1QKw=="], + "@types/safe-regex": ["@types/safe-regex@1.1.6", "", {}, "sha512-CQ/uPB9fLOPKwDsrTeVbNIkwfUthTWOx0l6uIGwVFjZxv7e68pCW5gtTYFzdJi3EBJp8h8zYhJbTasAbX7gEMQ=="], + "@types/ws": ["@types/ws@8.18.1", "", { "dependencies": { "@types/node": "*" } }, "sha512-ThVF6DCVhA8kUGy+aazFQ4kXQ7E1Ty7A3ypFOe0IcJV8O/M511G99AW24irKrW56Wt44yG9+ij8FaqoBGkuBXg=="], "ansi-escapes": ["ansi-escapes@7.3.0", "", { "dependencies": { "environment": "^1.0.0" } }, "sha512-BvU8nYgGQBxcmMuEeUEmNTvrMVjJNSH7RgW24vXexN4Ven6qCvy4TntnvlnwnMLTVlcRQQdbRY8NKnaIoeWDNg=="], @@ -341,10 +345,14 @@ "react-reconciler": ["react-reconciler@0.29.2", "", { "dependencies": { "loose-envify": "^1.1.0", "scheduler": "^0.23.2" }, "peerDependencies": { "react": "^18.3.1" } }, "sha512-zZQqIiYgDCTP/f1N/mAR10nJGrPD2ZR+jDSEsKWJHYC7Cm2wodlwbR3upZRdC3cjIjSlTLNVyO7Iu0Yy7t2AYg=="], + "regexp-tree": ["regexp-tree@0.1.27", "", { "bin": { "regexp-tree": "bin/regexp-tree" } }, "sha512-iETxpjK6YoRWJG5o6hXLwvjYAoW+FEZn9os0PD/b6AP6xQwsa/Y7lCVgIixBbUPMfhu+i2LtdeAqVTgGlQarfA=="], + "require-addon": ["require-addon@1.2.0", "", { "dependencies": { "bare-addon-resolve": "^1.3.0" } }, "sha512-VNPDZlYgIYQwWp9jMTzljx+k0ZtatKlcvOhktZ/anNPI3dQ9NXk7cq2U4iJ1wd9IrytRnYhyEocFWbkdPb+MYA=="], "restore-cursor": ["restore-cursor@4.0.0", "", { "dependencies": { "onetime": "^5.1.0", "signal-exit": "^3.0.2" } }, "sha512-I9fPXU9geO9bHOt9pHHOhOkYerIMsmVaWB0rA2AI9ERh/+x/i7MV5HKBNrg+ljO5eoPVgCcnFuRjJ9uH6I/3eg=="], + "safe-regex": ["safe-regex@2.1.1", "", { "dependencies": { "regexp-tree": "~0.1.1" } }, "sha512-rx+x8AMzKb5Q5lQ95Zoi6ZbJqwCLkqi3XuJXp5P3rT8OEc6sZCJG5AE5dU3lsgRr/F4Bs31jSlVN+j5KrsGu9A=="], + "scheduler": ["scheduler@0.23.2", "", { "dependencies": { "loose-envify": "^1.1.0" } }, "sha512-UOShsPwz7NrMUqhR6t0hWjFduvOzbtv7toDH1/hIrfRNIDBnnBWd0CwJTGvTpngVlmwGCdP9/Zl/tVrDqcuYzQ=="], "shebang-command": ["shebang-command@2.0.0", "", { "dependencies": { "shebang-regex": "^3.0.0" } }, "sha512-kHxr2zZpYtdmrN1qDjrrX/Z1rR1kG8Dx+gkpK1G4eXmvXswmcE1hTWBWYUzlraYw1/yZp6YuDY77YtvbN0dmDA=="], diff --git a/packages/cli/package.json b/packages/cli/package.json index c94f3ee..eb6a9a7 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -54,6 +54,7 @@ "msgpackr": "^1.11.8", "noise-handshake": "^4.2.0", "react": "^18.3.1", + "safe-regex": "^2.1.1", "snooplogg": "^6.1.1", "ws": "^8.19.0", "zod": "^3.24.0" @@ -63,6 +64,7 @@ "@types/js-yaml": "^4.0.9", "@types/node": "^22.0.0", "@types/react": "^18.3.0", + "@types/safe-regex": "^1.1.6", "fast-check": "^4.5.3", "typescript": "^5.7.0", "@tpsdev-ai/agent": "workspace:*" diff --git a/packages/cli/src/schema/manifest.ts b/packages/cli/src/schema/manifest.ts index 4fdf167..671bfee 100644 --- a/packages/cli/src/schema/manifest.ts +++ b/packages/cli/src/schema/manifest.ts @@ -1,13 +1,29 @@ import { z } from "zod"; +import safeRegex from "safe-regex"; -const RegexStringSchema = z.string().refine((val) => { - try { - new RegExp(val); // nosemgrep: detect-non-literal-regexp — validating user-provided pattern from config - return true; - } catch { - return false; - } -}, { message: "Invalid regular expression" }); +// Patterns come from tps.yaml manifests and are evaluated against inbound mail +// bodies. Compiling a pattern only proves it is well-formed — it does not prove +// it is safe to evaluate: a well-formed pattern can still have super-linear +// match cost on short input. We therefore validate on two axes here so that +// "validated" means "safe to run", not merely "parses": +// 1. it compiles to a RegExp, and +// 2. safeRegex accepts it (rejects nested-quantifier / ambiguous-repetition +// patterns whose evaluation cost is not bounded by the input length). +// Manifests carrying a rejected pattern fail schema validation and are dropped, +// so an unsafe pattern never reaches the runtime match sites. +const RegexStringSchema = z + .string() + .refine((val) => { + try { + new RegExp(val); // nosemgrep: detect-non-literal-regexp — validating a config-provided pattern for well-formedness + return true; + } catch { + return false; + } + }, { message: "Invalid regular expression" }) + .refine((val) => safeRegex(val), { + message: "Regular expression pattern rejected by safety validation", + }); export const MailHandlerMatchSchema = z.object({ from: z.array(z.string()).optional(), diff --git a/packages/cli/src/utils/mail-handler.ts b/packages/cli/src/utils/mail-handler.ts index 73f6683..a5daebc 100644 --- a/packages/cli/src/utils/mail-handler.ts +++ b/packages/cli/src/utils/mail-handler.ts @@ -44,7 +44,7 @@ export async function runHandlerPipeline( if (manifest.routing) { for (const rule of manifest.routing) { try { - const re = new RegExp(rule.pattern); // nosemgrep: detect-non-literal-regexp — pattern from validated tps.yaml config + const re = new RegExp(rule.pattern); // nosemgrep: detect-non-literal-regexp — pattern safety-validated at schema load (RegexStringSchema) if (re.test(bodyToTest)) { return { type: "forward", to: rule.to, body: msg.body }; } diff --git a/packages/cli/src/utils/manifest.ts b/packages/cli/src/utils/manifest.ts index 477fc6f..4c44a30 100644 --- a/packages/cli/src/utils/manifest.ts +++ b/packages/cli/src/utils/manifest.ts @@ -120,7 +120,7 @@ export function matchesFilter( } if (filter.bodyPattern) { - const re = new RegExp(filter.bodyPattern); // nosemgrep: detect-non-literal-regexp — pattern from validated tps.yaml config + const re = new RegExp(filter.bodyPattern); // nosemgrep: detect-non-literal-regexp — pattern safety-validated at schema load (RegexStringSchema) const bodyToTest = msg.body.trim().slice(0, 1024); // Capped at 1024 (S15) if (!re.test(bodyToTest)) return false; } diff --git a/packages/cli/src/utils/task-result-mail.ts b/packages/cli/src/utils/task-result-mail.ts index 6f2e115..de171e4 100644 --- a/packages/cli/src/utils/task-result-mail.ts +++ b/packages/cli/src/utils/task-result-mail.ts @@ -1,6 +1,15 @@ +// Escape regex metacharacters so an interpolated string is matched literally. +function escapeRegex(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + export function formatTaskCompleteMailBody(summary: string, prefix = "Task complete"): string { const trimmedSummary = summary.trimStart(); - const prefixPattern = new RegExp(`^${prefix}:?(?:\\r?\\n\\s*|\\s+)`, "i"); + // `prefix` is interpolated into a pattern, so it must be escaped — otherwise + // metacharacters in the prefix (e.g. parentheses in "Task complete (via …)") + // are interpreted as regex syntax and the leading-prefix strip silently + // misbehaves. Escaping keeps the match strictly literal. + const prefixPattern = new RegExp(`^${escapeRegex(prefix)}:?(?:\\r?\\n\\s*|\\s+)`, "i"); // nosemgrep: detect-non-literal-regexp — prefix escaped above const normalizedSummary = trimmedSummary.replace(prefixPattern, ""); return `${prefix}:\n\n${normalizedSummary}`; } diff --git a/packages/cli/test/manifest-schema.test.ts b/packages/cli/test/manifest-schema.test.ts index a7aa264..3e3b431 100644 --- a/packages/cli/test/manifest-schema.test.ts +++ b/packages/cli/test/manifest-schema.test.ts @@ -49,4 +49,56 @@ describe("TpsYamlSchema", () => { }); expect(res.success).toBe(true); }); + + test("accepts ordinary match and routing patterns", () => { + const res = TpsYamlSchema.safeParse({ + name: "agent-1", + capabilities: { + mail_handler: { match: { bodyPattern: "^deploy\\b.*(prod|staging)" } } + }, + routing: [ + { pattern: "urgent|priority", to: "other-agent" }, + { pattern: "a{1,50}", to: "third-agent" } + ] + }); + expect(res.success).toBe(true); + }); + + test("rejects an unsafe routing pattern at schema validation", () => { + // A pattern that compiles fine but whose match cost is not bounded by input + // length must be refused before it can ever be handed to new RegExp().test(). + const res = TpsYamlSchema.safeParse({ + name: "agent-1", + routing: [ + { pattern: "(a+)+$", to: "other-agent" } + ] + }); + expect(res.success).toBe(false); + }); + + test("rejects an unsafe body match pattern at schema validation", () => { + const res = TpsYamlSchema.safeParse({ + name: "agent-1", + capabilities: { + mail_handler: { match: { bodyPattern: "([a-zA-Z]+)*$" } } + } + }); + expect(res.success).toBe(false); + }); + + test("pattern validation is bounded — settles quickly", () => { + // Before schema-time validation, such a pattern would be accepted and only + // exercised much later during matching. Validation must settle well under a + // small time bound rather than getting stuck evaluating the pattern. + const start = Date.now(); + const res = TpsYamlSchema.safeParse({ + name: "agent-1", + routing: [ + { pattern: "(.*a){25}", to: "other-agent" } + ] + }); + const elapsedMs = Date.now() - start; + expect(res.success).toBe(false); + expect(elapsedMs).toBeLessThan(500); + }); }); diff --git a/packages/cli/test/task-result-mail.test.ts b/packages/cli/test/task-result-mail.test.ts index ae80874..b38323d 100644 --- a/packages/cli/test/task-result-mail.test.ts +++ b/packages/cli/test/task-result-mail.test.ts @@ -19,4 +19,26 @@ describe("formatTaskCompleteMailBody", () => { "Task complete:\n\nShipped the fix", ); }); + + test("treats regex metacharacters in the prefix literally", () => { + // With prefix "a.c" the leading-prefix strip must only fire on a literal + // "a.c", never on "abc" (which it would if "." were treated as regex "any"). + expect(formatTaskCompleteMailBody("abc: shipped", "a.c")).toBe( + "a.c:\n\nabc: shipped", + ); + expect(formatTaskCompleteMailBody("a.c: shipped", "a.c")).toBe( + "a.c:\n\nshipped", + ); + }); + + test("strips a metacharacter-bearing prefix used by a real caller", () => { + // The "(via Flair)" caller carries parentheses; the strip must match them + // literally and de-duplicate the leading prefix as intended. + expect( + formatTaskCompleteMailBody( + "Task complete (via Flair): done", + "Task complete (via Flair)", + ), + ).toBe("Task complete (via Flair):\n\ndone"); + }); }); From 15d3790dee9f06cea3e7b8309fa1f25d9bb7dd91 Mon Sep 17 00:00:00 2001 From: Flint Date: Tue, 21 Jul 2026 09:28:22 -0700 Subject: [PATCH 2/2] docs: neutral phrasing in pattern-validation comment Co-Authored-By: Claude Fable 5 --- packages/cli/src/schema/manifest.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/schema/manifest.ts b/packages/cli/src/schema/manifest.ts index 671bfee..7def944 100644 --- a/packages/cli/src/schema/manifest.ts +++ b/packages/cli/src/schema/manifest.ts @@ -2,15 +2,13 @@ import { z } from "zod"; import safeRegex from "safe-regex"; // Patterns come from tps.yaml manifests and are evaluated against inbound mail -// bodies. Compiling a pattern only proves it is well-formed — it does not prove -// it is safe to evaluate: a well-formed pattern can still have super-linear -// match cost on short input. We therefore validate on two axes here so that -// "validated" means "safe to run", not merely "parses": +// bodies. Compiling a pattern only proves it is well-formed — not that its +// evaluation cost is bounded by input length. We therefore validate on two +// axes so "validated" means "safe to run", not merely "parses": // 1. it compiles to a RegExp, and -// 2. safeRegex accepts it (rejects nested-quantifier / ambiguous-repetition -// patterns whose evaluation cost is not bounded by the input length). +// 2. safeRegex accepts it (keeps match cost bounded relative to input). // Manifests carrying a rejected pattern fail schema validation and are dropped, -// so an unsafe pattern never reaches the runtime match sites. +// so a rejected pattern never reaches the runtime match sites. const RegexStringSchema = z .string() .refine((val) => {