fix(core): keep escaped whitespace at pipe-list entry edges (#11860)

A hook matcher that is a pipe-separated list is rebuilt from its trimmed
entries. Trimming removed whitespace the author had escaped and left its
backslash dangling, so `\.env\ |\.pem` was rebuilt as `\.env\|\.pem`,
where the pipe is literal: the matcher silently stopped matching
`/x/secrets.pem`, and `write_file\ |note.*` stopped matching
`notebook_edit`. The rebuilt expression still compiled, so nothing was
logged.

Trailing whitespace at an entry edge is now kept when an odd run of
backslashes precedes it. After an even run the backslashes escape each
other, so the whitespace is still padding and is trimmed. Leading
whitespace is always padding.

Part of #11610.
This commit is contained in:
qqqys authored and GitHub committed 2026-09-14 15:51:49 +00:00
1 parent 6c04a19927
commit f7e48d5c4b
3 files changed
+40 -2

No files matched your search

+1 -1
View File
@@ -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`.
@@ -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);
+25 -1
View File
@@ -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(