diff --git a/docs/users/features/hooks.md b/docs/users/features/hooks.md index 2ba86a8d307..01c5af128f7 100644 --- a/docs/users/features/hooks.md +++ b/docs/users/features/hooks.md @@ -394,7 +394,7 @@ Hooks fire at specific points during a Qwen Code session. Different events suppo - Empty string `""`, `"*"` or `".*"` matches all events of that type - A matcher is first compared exactly. In a `|`-separated list such as `permission_prompt | idle_prompt`, the matcher matches when any entry, ignoring spaces around it, is `*`, `.*`, or exactly the value, unless the whole matcher starts with `^` or `(`, in which case it is only a regular expression. Only a `|` outside `[...]` and groups, and not escaped with a backslash, separates entries: the pipes in `notes\|todo\.md`, `foo[ |]bar` and `a(b | c)` are part of the regular expression, and the spaces around them are kept. List entries are never read as regular expressions on their own -- Otherwise the matcher is an unanchored regular expression (e.g., `^run_shell_command$`, `read_.*`, `(write_file|edit)`). For a list that does not start with `^` or `(`, the expression is built from the trimmed, non-empty entries, so a stray `|` as in `read_(file|edit)|` is ignored and never makes the matcher match everything, and a matcher of only `|` matches nothing. A matcher that starts with `^` or `(` is compiled exactly as written, so a trailing `|` there, as in `^write_file|`, does make it match everything. Because the expression is unanchored, `read` also matches `read_file` and `edit` also matches `notebook_edit`. Add `^` and `$` to match a whole value, and anchor exclusions as well: `^(?!write_file).*$` excludes `write_file`, while unanchored `(?!write_file).*` still matches it +- Otherwise the matcher is an unanchored regular expression (e.g., `^run_shell_command$`, `read_.*`, `(write_file|edit)`). For a list that does not start with `^` or `(`, the expression is built from the trimmed, non-empty entries (an escaped space at an entry's edge, as in `\.env\ |\.pem`, is kept), so a stray `|` as in `read_(file|edit)|` is ignored and never makes the matcher match everything, and a matcher of only `|` matches nothing. A matcher that starts with `^` or `(` is compiled exactly as written, so a trailing `|` there, as in `^write_file|`, does make it match everything. Because the expression is unanchored, `read` also matches `read_file` and `edit` also matches `notebook_edit`. Add `^` and `$` to match a whole value, and anchor exclusions as well: `^(?!write_file).*$` excludes `write_file`, while unanchored `(?!write_file).*` still matches it - The same rules apply to every event that supports a matcher, and to hooks registered by skills - Tool hooks receive the runtime tool id in `tool_name` (for example, `write_file`). Built-in display names such as `WriteFile` and `ReadFile` are also accepted as matcher aliases for compatibility, and so are the tool names permission rules accept, including Claude Code's `Bash`, `Read` and `Write`, so a matcher copied from a Claude Code config such as `"Bash"` or `"Write|Edit"` works unchanged. New configs should still prefer runtime ids. Aliases are only compared exactly, so anchor runtime ids (`^write_file$`), not display names. Each alias names exactly one tool: unlike a permission rule, a `Read` matcher does not also cover `grep_search` or `glob`, and a `Bash` matcher does not cover `monitor`. diff --git a/packages/core/src/hooks/hook-matcher.test.ts b/packages/core/src/hooks/hook-matcher.test.ts index 31ac2092ab0..a57c30cb7bb 100644 --- a/packages/core/src/hooks/hook-matcher.test.ts +++ b/packages/core/src/hooks/hook-matcher.test.ts @@ -90,6 +90,20 @@ describe('matchesHookPattern', () => { ); }); + it('keeps an escaped space at the edge of a list entry', () => { + expect(matchesHookPattern('\\.env\\ |\\.pem', '/x/secrets.pem')).toBe(true); + expect(matchesHookPattern('\\.env\\ |\\.pem', 'a.env ')).toBe(true); + expect(matchesHookPattern('\\.env\\ |\\.pem', 'a.env')).toBe(false); + expect(matchesHookPattern('write_file\\ |note.*', 'notebook_edit')).toBe( + true, + ); + expect(matchesHookPattern('read_.* | edit', 'read_file')).toBe(true); + }); + + it('trims a space after an escaped backslash at a list entry edge', () => { + expect(matchesHookPattern('C:\\\\temp\\\\ |x', 'C:\\temp\\')).toBe(true); + }); + it('never splits a list on an escaped pipe', () => { expect(matchesHookPattern('notes\\|', 'x/notes|y')).toBe(true); expect(matchesHookPattern('a\\||b', 'b')).toBe(true); diff --git a/packages/core/src/hooks/hook-matcher.ts b/packages/core/src/hooks/hook-matcher.ts index 78ecb11cebd..443fa3864e8 100644 --- a/packages/core/src/hooks/hook-matcher.ts +++ b/packages/core/src/hooks/hook-matcher.ts @@ -54,6 +54,29 @@ function splitListEntries(pattern: string): string[] { return entries; } +/** + * Removes the padding around a list entry. Trailing whitespace preceded by an + * odd run of backslashes is escaped, so it belongs to the expression and + * stays; after an even run the backslashes escape each other, so `C:\\ ` + * still trims to `C:\\`. Leading whitespace is always padding, because an + * escaped leading space starts with its backslash. + */ +function trimEntryEdges(entry: string): string { + const start = entry.length - entry.trimStart().length; + let end = entry.length; + while (end > start && /\s/.test(entry[end - 1])) { + let backslashes = 0; + while (entry[end - 2 - backslashes] === '\\') { + backslashes++; + } + if (backslashes % 2 === 1) { + break; + } + end--; + } + return entry.slice(start, end); +} + /** * Tests a hook `matcher` against the value an event is matched on, using the * same rules for every event and for both settings and session hooks: @@ -66,6 +89,7 @@ function splitListEntries(pattern: string): string[] { * and not escaped by a backslash separates entries: the pipes in * `notes\|todo\.md`, `foo[ |]bar` and `a(b | c)` belong to the expression, * spaces around them included. Entries are never compiled on their own. + * An escaped space at an entry's edge, as in `\.env\ |\.pem`, is kept. * - Otherwise the matcher is an unanchored regular expression tested against * the subject only; aliases are never matched through a regex. For a list * that does not start with `^` or `(`, the expression is rebuilt from the @@ -99,7 +123,7 @@ export function matchesHookPattern( // from the same trimmed entries, so `read_.* | edit` reads as // `read_.*|edit` while `foo[ |]bar` keeps its space. const alternatives = splitListEntries(pattern) - .map((entry) => entry.trim()) + .map((entry) => trimEntryEdges(entry)) .filter((entry) => entry !== ''); if ( alternatives.some(