Skip to content

fix(discover): exclude_commands bypass for env-prefix, sub cmd + regex - #1398

Merged
aeppling merged 3 commits into
developfrom
fix/exclude-commands-bypass
Apr 19, 2026
Merged

aeppling merged 3 commits into
developfrom
fix/exclude-commands-bypass

Conversation

@aeppling

Copy link
Copy Markdown
Contributor

Summary

Changes

Single file: src/discover/registry.rs

  • ExcludePattern enum , compiled regex or prefix fallback, precompiled once per rewrite_command call
  • compile_exclude_patterns() , anchoring logic + eprintln! on invalid regex
  • is_excluded() , matches against env-stripped command using precompiled patterns
  • Modified rewrite_segment_inner() to strip env prefix before exclude check

Test plan

  • test_exclude_env_prefixed_command , PGPASSWORD=x psql excluded by ["psql"]
  • test_exclude_subcommand_pattern , git push origin main excluded by ["git push"]
  • test_exclude_regex_pattern , curl http://... excluded by ["^curl"]
  • test_exclude_invalid_regex_fallback , invalid regex doesn't crash, falls back to prefix
  • test_exclude_does_not_substring_match , "go" doesn't exclude golangci-lint
  • All 4 pre-existing exclude tests still pass
  • Full suite: 1597 passed, 0 failed

Strip env prefix (sudo, VAR=val) before exclude check so PGPASSWORD=x psql matches ["psql"]. Match against full command instead of first token so ["git push"] matches "git push origin main".

Compile exclude entries as regex so ["^curl"] works as pattern.

Invalid regex falls back to prefix match with eprintln warning.
Non-regex patterns auto-anchor with ^ and \b to prevent substring matches (e.g. "go" won't exclude "golangci-lint").
@pszymkowiak pszymkowiak added bug Something isn't working effort-small Quelques heures, 1 fichier labels Apr 19, 2026
@pszymkowiak

Copy link
Copy Markdown
Collaborator

[w] wshm · Automated triage by AI

📊 Automated PR Analysis

🐛 Type bug-fix
🟡 Risk medium

Summary

Fixes the exclude_commands matching logic in the discover registry to handle env-prefixed commands (e.g., PGPASSWORD=x psql), multi-token subcommand patterns (e.g., "git push"), and regex patterns (e.g., "^curl"). Non-regex patterns are auto-anchored with ^ and \b to prevent substring false positives. Invalid regex patterns gracefully fall back to prefix matching with a warning.

Review Checklist

  • Tests present
  • Breaking change
  • Docs updated

Linked issues: #917, #243, #1335


Analyzed automatically by wshm · This is an automated analysis, not a human review.

Replace \b with ($|\s) to only match end-of-string or whitespace.

Empty string and bare "^" patterns produce regexes that match all commands, silently disabling all rewrites on a config typo. Skip
  trivial patterns with an eprintln warning.

@pszymkowiak pszymkowiak left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean fix for 3 real user-visible regressions (#917, #243, #1335). ExcludePattern enum with Regex/Prefix fallback is a good design, regex::escape + auto-anchoring ^{esc}($|\s) correctly prevents substring false-positives ("go" no longer excludes golangci-lint). Env-prefix stripping via existing ENV_PREFIX regex is consistent with classify_command.

Verified locally:

  • cargo test discover::registry → 232/232
  • Rewrite still wraps env correctly: FOO=bar git status → FOO=bar rtk git status
  • Docs updated in configuration.md + hooks/README.md

Non-blocking follow-ups:

  1. test_exclude_invalid_regex_fallback passes for the wrong reason. "curl[" is not ^-prefixed, so it goes through regex::escape → ^curl\[($|\s) which is a valid regex matching literal curl[. The Prefix fallback path is never exercised. To actually test the fallback, use an anchored invalid pattern like "^curl[" (no escape → regex compile fails → ExcludePattern::Prefix branch).
  2. compile_exclude_patterns runs on every rewrite_command call, so an invalid pattern in config.toml will eprintln! on every Bash command the hook intercepts. Consider log-once or compile-at-config-load for a follow-up.
  3. Minor perf: regex re-compiled per call (~10-50µs for 3-5 patterns, negligible vs 10ms startup, but cache at Config level would be cleaner).

@aeppling
aeppling merged commit ca4c59c into develop Apr 19, 2026
10 checks passed
thehoff pushed a commit to thehoff/contextcrawler that referenced this pull request May 14, 2026
fix(discover): exclude_commands bypass for env-prefix, sub cmd + regex
thehoff pushed a commit to thehoff/contextcrawler that referenced this pull request May 14, 2026
fix(discover): exclude_commands bypass for env-prefix, sub cmd + regex
@aeppling
aeppling deleted the fix/exclude-commands-bypass branch June 30, 2026 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working effort-small Quelques heures, 1 fichier

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants