git.iko.soy/crush
on main help sign in
refactor(hooks): move matcher compilation into the runner

The runner now compiles its own matchers, so a config round-trip can't silently strip them. Hooks with an invalid matcher are dropped with a warning instead of quietly matching everything.

commitc84ca2d92f42d52e3a6fdedc33c8df1b76e7a936 authorChristian Rocha <christian@rocha.is> dateTue 28 Apr 2026, 14:48 (6 months ago) parent38a846d
4 files changed+94−29
compare with main
unifiedsplit
internal/config/config.go+3−11 internal/config/load.go+11−10 internal/hooks/hooks_test.go+34 internal/hooks/runner.go+46−8
internal/config/config.go+3−11
@@ -9,7 +9,6 @@ import (
99 "maps"
1010 "net/http"
1111 "net/url"
12− "regexp"
1312 "slices"
1413 "strings"
1514 "time"
@@ -371,7 +370,9 @@ func (t ToolGrep) GetTimeout() time.Duration {
371370}
372371
373372// HookConfig defines a user-configured shell command that fires on a hook
374−// event (e.g. PreToolUse).
373+// event (e.g. PreToolUse). This is a pure-data struct: matcher compilation
374+// is owned by hooks.Runner so a JSON round-trip, merge, or reload can't
375+// silently drop compiled state.
375376type HookConfig struct {
376377 // Regex pattern tested against the tool name. Empty means match all.
377378 Matcher string `json:"matcher,omitempty" jsonschema:"description=Regex pattern tested against the tool name. Empty means match all tools."`
@@ -379,15 +380,6 @@ type HookConfig struct {
379380 Command string `json:"command" jsonschema:"required,description=Shell command to execute when the hook fires"`
380381 // Timeout in seconds. Default 30.
381382 Timeout int `json:"timeout,omitempty" jsonschema:"description=Timeout in seconds for the hook command,default=30"`
382−
383− // Compiled matcher regex. Not serialized.
384− matcherRegex *regexp.Regexp
385−}
386−
387−// MatcherRegex returns the compiled matcher regex, or nil if no matcher is
388−// set.
389−func (h *HookConfig) MatcherRegex() *regexp.Regexp {
390− return h.matcherRegex
391383}
392384
393385// TimeoutDuration returns the hook timeout as a time.Duration, defaulting
internal/config/load.go+11−10
@@ -905,8 +905,11 @@ func normalizeHookEvent(name string) string {
905905 }
906906}
907907
908−// ValidateHooks normalizes event names and compiles matcher regexes for all
909−// configured hooks. Returns an error if any regex is invalid.
908+// ValidateHooks normalizes event names and checks that every configured
909+// hook has a command and a syntactically valid matcher regex. Matcher
910+// compilation used for matching is owned by hooks.Runner; this function
911+// only validates up front so the user sees config errors at load time
912+// rather than on the first tool call.
910913func (c *Config) ValidateHooks() error {
911914 // Normalize event name keys.
912915 for event, eventHooks := range c.Hooks {
@@ -918,17 +921,15 @@ func (c *Config) ValidateHooks() error {
918921 }
919922
920923 for event, eventHooks := range c.Hooks {
921− for i := range eventHooks {
922− h := &c.Hooks[event][i]
924+ for i, h := range eventHooks {
923925 if h.Command == "" {
924926 return fmt.Errorf("hook %s[%d]: command is required", event, i)
925927 }
926− if h.Matcher != "" {
927− re, err := regexp.Compile(h.Matcher)
928− if err != nil {
929− return fmt.Errorf("hook %s[%d]: invalid matcher regex %q: %w", event, i, h.Matcher, err)
930− }
931− h.matcherRegex = re
928+ if h.Matcher == "" {
929+ continue
930+ }
931+ if _, err := regexp.Compile(h.Matcher); err != nil {
932+ return fmt.Errorf("hook %s[%d]: invalid matcher regex %q: %w", event, i, h.Matcher, err)
932933 }
933934 }
934935 }
internal/hooks/hooks_test.go+34
@@ -373,6 +373,40 @@ func TestRunnerMatcherFiltering(t *testing.T) {
373373 require.NoError(t, err)
374374 require.Equal(t, DecisionNone, result.Decision)
375375 })
376+
377+ // Runner must compile matchers itself; it cannot rely on
378+ // ValidateHooks having run first. This is the guarantee that prevents
379+ // the reload-drops-matcher class of bug.
380+ t.Run("runner compiles matcher without ValidateHooks", func(t *testing.T) {
381+ t.Parallel()
382+ raw := []config.HookConfig{
383+ {Command: `echo '{"decision":"deny","reason":"blocked"}'`, Matcher: "^bash$"},
384+ }
385+ r := NewRunner(raw, t.TempDir(), t.TempDir())
386+
387+ deny, err := r.Run(context.Background(), EventPreToolUse, "sess", "bash", `{}`)
388+ require.NoError(t, err)
389+ require.Equal(t, DecisionDeny, deny.Decision)
390+
391+ noop, err := r.Run(context.Background(), EventPreToolUse, "sess", "view", `{}`)
392+ require.NoError(t, err)
393+ require.Equal(t, DecisionNone, noop.Decision)
394+ })
395+
396+ // A matcher that fails to compile at Runner construction must not
397+ // degrade to match-everything; the hook is dropped instead.
398+ t.Run("runner skips hooks with invalid matcher", func(t *testing.T) {
399+ t.Parallel()
400+ raw := []config.HookConfig{
401+ {Command: `echo '{"decision":"deny","reason":"should not fire"}'`, Matcher: "[invalid"},
402+ }
403+ r := NewRunner(raw, t.TempDir(), t.TempDir())
404+
405+ result, err := r.Run(context.Background(), EventPreToolUse, "sess", "bash", `{}`)
406+ require.NoError(t, err)
407+ require.Equal(t, DecisionNone, result.Decision)
408+ require.Empty(t, r.Hooks())
409+ })
376410}
377411
378412func TestValidateHooksInvalidRegex(t *testing.T) {
internal/hooks/runner.go+46−8
@@ -5,6 +5,7 @@ import (
55 "context"
66 "log/slog"
77 "os/exec"
8+ "regexp"
89 "strings"
910 "sync"
1011 "time"
@@ -12,25 +13,63 @@ import (
1213 "github.com/charmbracelet/crush/internal/config"
1314)
1415
16+// compiledHook pairs a HookConfig with its compiled matcher regex. A nil
17+// matcher means "match every tool".
18+type compiledHook struct {
19+ cfg config.HookConfig
20+ matcher *regexp.Regexp
21+}
22+
1523// Runner executes hook commands and aggregates their results.
1624type Runner struct {
17− hooks []config.HookConfig
25+ hooks []compiledHook
1826 cwd string
1927 projectDir string
2028}
2129
22−// NewRunner creates a Runner from the given hook configs.
30+// NewRunner creates a Runner from the given hook configs. Each hook's
31+// Matcher is compiled here so the Runner is self-sufficient; callers do
32+// not have to pre-compile matchers on the config, and reloads or merges
33+// that rebuild HookConfig values can't silently strip compiled state.
34+//
35+// Hooks whose matcher fails to compile are skipped with a warning rather
36+// than treated as match-everything. ValidateHooks is expected to have
37+// caught syntax errors earlier, so this is defense in depth.
2338func NewRunner(hooks []config.HookConfig, cwd, projectDir string) *Runner {
39+ compiled := make([]compiledHook, 0, len(hooks))
40+ for _, h := range hooks {
41+ ch := compiledHook{cfg: h}
42+ if h.Matcher != "" {
43+ re, err := regexp.Compile(h.Matcher)
44+ if err != nil {
45+ slog.Warn("Hook matcher failed to compile; skipping hook",
46+ "matcher", h.Matcher,
47+ "command", h.Command,
48+ "error", err,
49+ )
50+ continue
51+ }
52+ ch.matcher = re
53+ }
54+ compiled = append(compiled, ch)
55+ }
2456 return &Runner{
25− hooks: hooks,
57+ hooks: compiled,
2658 cwd: cwd,
2759 projectDir: projectDir,
2860 }
2961}
3062
31−// Hooks returns the hook configs the runner was created with.
63+// Hooks returns the hook configs the runner was created with, in config
64+// order. Hooks whose matcher failed to compile at construction are
65+// omitted. Intended for diagnostics; callers should not rely on ordering
66+// or identity beyond that.
3267func (r *Runner) Hooks() []config.HookConfig {
33− return r.hooks
68+ out := make([]config.HookConfig, len(r.hooks))
69+ for i, h := range r.hooks {
70+ out[i] = h.cfg
71+ }
72+ return out
3473}
3574
3675// Run executes all matching hooks for the given event and tool, returning
@@ -93,9 +132,8 @@ func (r *Runner) Run(ctx context.Context, eventName, sessionID, toolName, toolIn
93132func (r *Runner) matchingHooks(toolName string) []config.HookConfig {
94133 var matched []config.HookConfig
95134 for _, h := range r.hooks {
96− re := h.MatcherRegex()
97− if re == nil || re.MatchString(toolName) {
98− matched = append(matched, h)
135+ if h.matcher == nil || h.matcher.MatchString(toolName) {
136+ matched = append(matched, h.cfg)
99137 }
100138 }
101139 return matched