Repository navigation
fix(rulebook): raise the hook's regex length bound from 400 to 2000 (v0.88.0, ENG-1170) #267
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1924,7 +1924,13 @@ def matcher_unsupported(m): | |
|
|
||
| _RX_KEYS = ("rx", "not_rx", "body_rx", "cmd_rx", "cmd_not_rx", "path_rx", "path_not_rx", | ||
| "content_rx", "content_not_rx", "exclude_rx", "converted_rx") | ||
| # Length is a bound on what one rule can cost to compile and store, NOT the | ||
| # backtracking guard — `_RX_NESTED` below is. A 60-character pattern can stall | ||
| # and a 1,500-character alternation of literal paths cannot, so the bound is | ||
| # wide enough for the second. It was 400 before 0.88.0: a hook older than that | ||
| # drops any rule carrying a longer pattern, which is why the server floors such | ||
| # a rule at `min_hook_version` 0.88.0. | ||
| _RX_MAX = 2000 | ||
|
There was a problem hiding this comment.
Raising this bound newly lets deeply nested patterns reach Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Confirmed on 3.9 and 3.12: the 1,001-char nested pattern raises RecursionError, not re.error. There was a problem hiding this comment.
With the new bound, a rule such as Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Real shape, but not one the length bound ever guarded: |
||
| # (a+)+, (\d+)+$, (a|a)+, (.*), .*.* — the classic backtracking shapes. A | ||
| # denylist, not a proof: stdlib `re` has no timeout, and a bounded matcher | ||
| # (worker + wall clock) is the Phase 2 answer named in §5.1. | ||
|
|
@@ -1933,14 +1939,17 @@ def matcher_unsupported(m): | |
|
|
||
| def rx_ok(pat): | ||
| """Load-time lint for a pattern that came off the wire (§5.1 fallback): | ||
| must compile, stay under `_RX_MAX`, and avoid the nested-quantifier shapes that | ||
| backtrack catastrophically. A rejected pattern drops the RULE, never the | ||
| hook — a server book can advise, it cannot stall a tool call.""" | ||
| if not isinstance(pat, str) or len(pat) > _RX_MAX or _RX_NESTED.search(pat): | ||
| return False | ||
| try: | ||
| re.compile(pat) | ||
| except Exception: | ||
| # Not only `re.error`: a deeply nested pattern raises RecursionError | ||
| # and `a{99999999999}` raises OverflowError. Whatever the parser | ||
| # objects to, the answer is the same — this pattern does not load. | ||
| return False | ||
| return True | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raising only the hook limit leaves the shipped authoring flow enforcing the old value:
plugins/memhub/skills/start-rulebook/scripts/starter_rulebook.py:51,96-103,694-696still truncates repo-derived alternations around 360 characters and reports every seeded pattern over 400 as a load failure. Consequently, repositories with enough generated/ignored/secret paths cannot benefit from the newly supported 401–2000-character patterns—the generator either omits paths or rejects an otherwise loadable rule. The create-rule guidance atplugins/memhub/skills/create-rule/SKILL.md:460-463also still tells authors that such patterns are silently dropped.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 8d6d768:
starter_rulebook.pyRX_MAXis 2000 and_fitcites it instead of a literal; create-rule SKILL.md LOAD line says 2000.