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)
This commit is contained in:
177
src/shared/pattern-matcher.test.ts
Normal file
177
src/shared/pattern-matcher.test.ts
Normal file
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user