From bc782ca4d4f408953e6704da17e59cbcae3b22e5 Mon Sep 17 00:00:00 2001 From: Rishi Vhavle Date: Fri, 6 Feb 2026 12:48:28 +0530 Subject: [PATCH] fix: escape regex special chars in pattern matcher Fixes #1521. When hook matcher patterns contained regex special characters like parentheses, the pattern-matcher would throw 'SyntaxError: Invalid regular expression: unmatched parentheses' because these characters were not escaped before constructing the RegExp. The fix escapes all regex special characters (.+?^${}()|[\]\) EXCEPT the asterisk (*) which is intentionally converted to .* for glob-style matching. Add comprehensive test suite for pattern-matcher covering: - Exact matching (case-insensitive) - Wildcard matching (glob-style *) - Pipe-separated patterns - All regex special characters (parentheses, brackets, etc.) - Edge cases (empty matcher, complex patterns) --- src/shared/pattern-matcher.test.ts | 177 +++++++++++++++++++++++++++++ src/shared/pattern-matcher.ts | 13 ++- 2 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 src/shared/pattern-matcher.test.ts diff --git a/src/shared/pattern-matcher.test.ts b/src/shared/pattern-matcher.test.ts new file mode 100644 index 00000000..e95c9a8f --- /dev/null +++ b/src/shared/pattern-matcher.test.ts @@ -0,0 +1,177 @@ +import { describe, test, expect } from "bun:test" +import { matchesToolMatcher, findMatchingHooks } from "./pattern-matcher" +import type { ClaudeHooksConfig } from "../hooks/claude-code-hooks/types" + +describe("matchesToolMatcher", () => { + describe("exact matching", () => { + //#given a pattern without wildcards + //#when matching against a tool name + //#then it should match case-insensitively + + test("matches exact tool name", () => { + expect(matchesToolMatcher("bash", "bash")).toBe(true) + }) + + test("matches case-insensitively", () => { + expect(matchesToolMatcher("Bash", "bash")).toBe(true) + expect(matchesToolMatcher("bash", "BASH")).toBe(true) + }) + + test("does not match different tool names", () => { + expect(matchesToolMatcher("bash", "edit")).toBe(false) + }) + }) + + describe("wildcard matching", () => { + //#given a pattern with asterisk wildcard + //#when matching against tool names + //#then it should treat * as glob-style wildcard + + test("matches prefix wildcard", () => { + expect(matchesToolMatcher("lsp_goto_definition", "lsp_*")).toBe(true) + expect(matchesToolMatcher("lsp_find_references", "lsp_*")).toBe(true) + }) + + test("matches suffix wildcard", () => { + expect(matchesToolMatcher("file_read", "*_read")).toBe(true) + }) + + test("matches middle wildcard", () => { + expect(matchesToolMatcher("get_user_info", "get_*_info")).toBe(true) + }) + + test("matches multiple wildcards", () => { + expect(matchesToolMatcher("get_user_data", "*_user_*")).toBe(true) + }) + + test("single asterisk matches any tool", () => { + expect(matchesToolMatcher("anything", "*")).toBe(true) + }) + }) + + describe("pipe-separated patterns", () => { + //#given multiple patterns separated by pipes + //#when matching against tool names + //#then it should match if any pattern matches + + test("matches first pattern", () => { + expect(matchesToolMatcher("bash", "bash | edit | write")).toBe(true) + }) + + test("matches middle pattern", () => { + expect(matchesToolMatcher("edit", "bash | edit | write")).toBe(true) + }) + + test("matches last pattern", () => { + expect(matchesToolMatcher("write", "bash | edit | write")).toBe(true) + }) + + test("does not match if none match", () => { + expect(matchesToolMatcher("read", "bash | edit | write")).toBe(false) + }) + }) + + describe("regex special character escaping (issue #1521)", () => { + //#given a pattern containing regex special characters + //#when matching against tool names + //#then it should NOT throw SyntaxError and should handle them as literals + + test("handles parentheses in pattern without throwing", () => { + expect(() => matchesToolMatcher("bash", "bash(*)")).not.toThrow() + expect(matchesToolMatcher("bash(test)", "bash(*)")).toBe(true) + }) + + test("handles unmatched opening parenthesis", () => { + expect(() => matchesToolMatcher("test", "test(*")).not.toThrow() + }) + + test("handles unmatched closing parenthesis", () => { + expect(() => matchesToolMatcher("test", "test*)")).not.toThrow() + }) + + test("handles square brackets", () => { + expect(() => matchesToolMatcher("test", "test[*]")).not.toThrow() + expect(matchesToolMatcher("test[1]", "test[*]")).toBe(true) + }) + + test("handles plus sign", () => { + expect(() => matchesToolMatcher("test", "test+*")).not.toThrow() + }) + + test("handles question mark", () => { + expect(() => matchesToolMatcher("test", "test?*")).not.toThrow() + }) + + test("handles caret", () => { + expect(() => matchesToolMatcher("test", "^test*")).not.toThrow() + }) + + test("handles dollar sign", () => { + expect(() => matchesToolMatcher("test", "test$*")).not.toThrow() + }) + + test("handles curly braces", () => { + expect(() => matchesToolMatcher("test", "test{*}")).not.toThrow() + }) + + test("handles pipe as pattern separator", () => { + expect(() => matchesToolMatcher("test", "test|value")).not.toThrow() + expect(matchesToolMatcher("test", "test|value")).toBe(true) + expect(matchesToolMatcher("value", "test|value")).toBe(true) + }) + + test("handles backslash", () => { + expect(() => matchesToolMatcher("test\\path", "test\\*")).not.toThrow() + }) + + test("handles dot", () => { + expect(() => matchesToolMatcher("test.ts", "test.*")).not.toThrow() + expect(matchesToolMatcher("test.ts", "test.*")).toBe(true) + }) + + test("complex pattern with multiple special chars", () => { + expect(() => matchesToolMatcher("func(arg)", "func(*)")).not.toThrow() + expect(matchesToolMatcher("func(arg)", "func(*)")).toBe(true) + }) + }) + + describe("empty matcher", () => { + //#given an empty or undefined matcher + //#when matching + //#then it should match everything + + test("empty string matches everything", () => { + expect(matchesToolMatcher("anything", "")).toBe(true) + }) + }) +}) + +describe("findMatchingHooks", () => { + const mockHooks: ClaudeHooksConfig = { + PreToolUse: [ + { matcher: "bash", hooks: [{ type: "command", command: "/test/hook1" }] }, + { matcher: "edit*", hooks: [{ type: "command", command: "/test/hook2" }] }, + { matcher: "*", hooks: [{ type: "command", command: "/test/hook3" }] }, + ], + } + + test("finds hooks matching exact tool name", () => { + const result = findMatchingHooks(mockHooks, "PreToolUse", "bash") + expect(result.length).toBe(2) // "bash" and "*" + }) + + test("finds hooks matching wildcard pattern", () => { + const result = findMatchingHooks(mockHooks, "PreToolUse", "edit_file") + expect(result.length).toBe(2) // "edit*" and "*" + }) + + test("returns all hooks when no toolName provided", () => { + const result = findMatchingHooks(mockHooks, "PreToolUse") + expect(result.length).toBe(3) + }) + + test("returns empty array for non-existent event", () => { + const result = findMatchingHooks(mockHooks, "PostToolUse", "bash") + expect(result.length).toBe(0) + }) +}) diff --git a/src/shared/pattern-matcher.ts b/src/shared/pattern-matcher.ts index e2367362..f28568a6 100644 --- a/src/shared/pattern-matcher.ts +++ b/src/shared/pattern-matcher.ts @@ -1,5 +1,14 @@ import type { ClaudeHooksConfig, HookMatcher } from "../hooks/claude-code-hooks/types" +/** + * Escape all regex special characters EXCEPT asterisk (*). + * Asterisk is preserved for glob-to-regex conversion. + */ +function escapeRegexExceptAsterisk(str: string): string { + // Escape all regex special chars except * (which we convert to .* for glob matching) + return str.replace(/[.+?^${}()|[\]\\]/g, "\\$&") +} + export function matchesToolMatcher(toolName: string, matcher: string): boolean { if (!matcher) { return true @@ -7,7 +16,9 @@ export function matchesToolMatcher(toolName: string, matcher: string): boolean { const patterns = matcher.split("|").map((p) => p.trim()) return patterns.some((p) => { if (p.includes("*")) { - const regex = new RegExp(`^${p.replace(/\*/g, ".*")}$`, "i") + // First escape regex special chars (except *), then convert * to .* + const escaped = escapeRegexExceptAsterisk(p) + const regex = new RegExp(`^${escaped.replace(/\*/g, ".*")}$`, "i") return regex.test(toolName) } return p.toLowerCase() === toolName.toLowerCase()