Skip to content

Pass regex capture groups to known-error help text and fixes#343

Merged
rubberduck203 merged 1 commit into
mainfrom
worktree-pass-regex-match-to-fix
Jul 23, 2026
Merged

Pass regex capture groups to known-error help text and fixes#343
rubberduck203 merged 1 commit into
mainfrom
worktree-pass-regex-match-to-fix

Conversation

@rubberduck203

Copy link
Copy Markdown
Contributor

Summary

  • ScopeKnownError patterns can now capture text from the matched error line and reuse it in help text and in fix's commands/prompt, via the existing minijinja {{ }} templating: positional groups as {{ captures[1] }} (index 0 is the whole match) and named groups like (?<file>.*) as {{ file }}. Closes Feature: Allow passing regex matches to fixes #196.
  • KnownError stores Vec<Regex> instead of a RegexSet (needed to pull capture groups out of a match) and the raw DoctorFixSpec instead of a pre-built DoctorFix — fix-building moves to match time, once captures are known, via new _with_captures sibling functions (DoctorCommand::try_new_with_captures, DoctorCommands::from_commands_with_captures, DoctorFix::from_spec_with_captures). The originals are unchanged thin wrappers, so ScopeDoctorGroup fixes are untouched.
  • The fix is still eagerly validated at config-load time (rendered once with empty captures) so a broken template fails fast instead of only surfacing when a line actually matches.
  • A capture group named working_dir or captures is rejected at load time, since it would otherwise silently shadow the reserved template variable of the same name.
  • help_text rendering falls back to the raw string on a template error, since it predates this feature and may contain literal brace-like text that was never meant to be a template.
  • Docs/schema/examples updated: doc comments on KnownErrorPattern/KnownErrorSpec, a new examples/v1alpha/KnownErrorWithCapture.yaml, and the generated schema/*.json (the docs site symlinks to these).

Test plan

  • cargo build, cargo test (177 unit + all integration suites pass)
  • cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings clean
  • New integration fixture tests/test-cases/intercept-known-error-capture/ (test_intercept_fix_uses_captured_regex_group) — captures a filename from a real error message and uses it, unmodified, in help text and the fix command
  • New integration fixture tests/test-cases/intercept-known-error-multi-command-fix/ (test_intercept_fix_stops_after_first_command_fails) — confirms a multi-command fix stops after the first failing command; manually verified this test fails if that short-circuit is broken
  • Unit tests for capture extraction, reserved-name rejection, eager fix validation at load time, and template substitution (positional/named/missing/coexistence with working_dir)
  • Ran an independent code review + coverage + mutation testing pass; fixed everything that surfaced (reserved-name collision, help-text template crash, deferred fix validation, fmt drift, a dead .expect(), and one real mutation-testing gap in the fix short-circuit logic)

ScopeKnownError patterns can now capture text from the matched error line and
reuse it in help text and fix commands/prompts, via minijinja's existing
{{ }} templating: positional groups as {{ captures[1] }} (index 0 is the
whole match) and named groups like (?<file>.*) as {{ file }}.

Fix-building moves from config-load time to match time so captures are
available when needed, but the fix is still eagerly validated at load time
(with empty captures) so a broken template fails fast rather than only
surfacing mid-run. Capture group names "working_dir" and "captures" are
rejected at load time since they'd otherwise shadow the reserved template
variables.

Closes #196.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rubberduck203
rubberduck203 marked this pull request as ready for review July 23, 2026 17:09
@dryrunsecurity

Copy link
Copy Markdown

DryRun Security

This pull request introduces a critical command injection vulnerability in src/shared/models/internal/command.rs where unsanitized regex capture groups are directly substituted into executed command strings. Although the issue is not currently blocking, it poses a severe security risk if an attacker can control the log output matched by the KnownError regex.

🔴 Command Injection in src/shared/models/internal/command.rs (drs_8506a502)
Vulnerability Command Injection
Description The application substitutes regex capture groups directly into command strings, which are then executed. While the DoctorCommand structure is used, the command string is ultimately passed to an execution provider. If an attacker can control the log output that matches the KnownError regex, they can inject shell metacharacters into the capture groups, which are then rendered into the command string and executed. The current implementation does not appear to sanitize these captures for shell safety before execution.

let cmd = substitute_templates_with_captures(working_dir, command, captures)?;
let cmd = extract_command_path(containing_dir, &cmd);
let cmd = Self::expand_command(&cmd);
Ok(DoctorCommand::from_str(&cmd))


Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.

@rubberduck203
rubberduck203 merged commit de047b6 into main Jul 23, 2026
12 of 13 checks passed
@rubberduck203
rubberduck203 deleted the worktree-pass-regex-match-to-fix branch July 23, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Allow passing regex matches to fixes

1 participant