[CORRUPTED] Synthetic Benchmark PR #36132 - FIX: silent failure when watched words contains invalid regex

[tomerqodo]

User description

Benchmark PR discourse#36132

Type: Corrupted (contains bugs)

Original PR Title: FIX: silent failure when watched words contains invalid regex
Original PR Description: If a watched words list contains any invalid regex, the test modal won't work because the "compiled" regex (that contains all the watched words) will be invalid.

This PR introduces a fallback to matching individual watched words when the compiled regex is invalid as well as displaying the error for each invalid regex.

Screen.Recording.2025-11-20.at.10.34.05.mov

Internal ref - t/168206
Original PR URL: discourse#36132

---

PR Type

Bug fix, Enhancement, Tests

---

Description

• Handle invalid regex in watched words gracefully with fallback matching

• Display individual regex errors instead of silent failures

• Improve test modal to work with mixed valid/invalid regex patterns

• Add comprehensive test coverage for invalid regex scenarios

---

Diagram Walkthrough

flowchart LR
  A["Invalid Regex Detection"] --> B["Log Warning & Skip Invalid"]
  B --> C["Fallback to Individual Words"]
  C --> D["Display Error Details"]
  E["Test Modal"] --> F["Try Compiled Expression"]
  F --> G{Compilation Failed?}
  G -->|Yes| C
  G -->|No| H["Show Matches"]

Loading

File Walkthrough

	Relevant files
Bug fix	word_watcher.rb Add error handling and fallback for invalid regex                app/services/word_watcher.rb Added error handling for individual regex validation with logging Fixed case sensitivity flag logic in compiled regex creation Added try-catch for compiled regex creation to prevent silent failures Invalid regexes are now skipped instead of breaking the entire compilation +16/-3
Tests	word_watcher_spec.rb Add tests for invalid regex handling                                          spec/services/word_watcher_spec.rb Added test context for invalid regex causing compilation failure Tests verify no exception is raised and valid words still compile Tests confirm match detection works for valid words with invalid regex present Tests validate serialized regexps still work with invalid regex +36/-0    admin-watched-words-test.js Add acceptance tests for invalid regex scenarios                  frontend/discourse/tests/acceptance/admin-watched-words-test.js Added test suite for mixed valid and invalid regex patterns Added test suite for block action with invalid compiled expression Added test suite for unicode flag validation Tests verify test modal works with fallback matching and error display +204/-0
Enhancement	action.js Refactor to handle multiple regex errors                                  frontend/discourse/admin/controllers/admin-watched-words/action.js Changed from single regexpError to regexpErrors array for multiple errors Added @cached decorator for performance optimization Improved error message parsing to extract clean error descriptions Added deduplication logic to prevent duplicate error messages +27/-6    staff_logs.scss Add styling for regex error display                                            app/assets/stylesheets/admin/staff_logs.scss Added new .watched-word-regex-errors class for error list styling Configured max-height with scrolling for long error lists Applied disc list style and padding for better readability +7/-0      watched-word-testing.gjs Add error handling and fallback in test modal                        frontend/discourse/admin/components/modal/watched-word-testing.gjs Refactored matches getter to matchesAndErrors returning both matches and errors Added error handling for individual word regex compilation in test modal Implemented fallback to individual word matching when compiled expression fails Added error message cleaning utility function +97/-42  action.gjs Update template to display multiple errors                              frontend/discourse/admin/templates/admin-watched-words/action.gjs Changed from single error message to error list display Added strong header with i18n key for multiple errors Iterates through error array showing word and error message Applied new .watched-word-regex-errors styling class +12/-2
Documentation	client.en.yml Add i18n key for multiple errors                                                  config/locales/client.en.yml Added new i18n key invalid_regex_multiple for multiple error header Supports displaying multiple regex errors in a list format +1/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Error message exposure Description: The regex error message shown to admins is derived from e.message and displayed in the UI after only splitting on ": ", which may still expose full engine error text and potentially sensitive patterns; sanitize or map errors to predefined messages to avoid leaking internal details. action.js [53-61] Referred Code new RegExp(regexp, "gui"); } catch (e) { const key = `${id}:${word}:${e.message}`; if (!seen.has(key)) { seen.add(key); const parts = e.message.split(": "); const cleanError = parts[parts.length - 1]; errors.push({ word, error: cleanError }); }
	Error message exposure Description: The modal surfaces raw e.message from the JS RegExp engine (only trimmed) back to the admin user, which could leak engine-specific details or unexpected strings from user-supplied patterns; prefer whitelisting known error types or generic messages. watched-word-testing.gjs [32-35] Referred Code const addError = (word, message) => { errors.push({ word, error: this.cleanErrorMessage(message) }); };
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Logging Context: Newly added error handling logs regex compilation issues but may lack user/context identifiers needed for comprehensive audit trails. Referred Code Rails.logger.warn( "Watched word '#{word}' has invalid regex '#{r}' for #{action}: #{e.message}", ) nil end end .select { |r| r.present? } .join("|") next if regexp.blank? # Add word boundaries to the regexp for regular watched words regexp = match_word_regexp( regexp, engine: engine, ) if !SiteSetting.watched_words_regular_expressions? # Add case insensitive flag if needed begin Regexp.new(regexp, group_key == :case_sensitive ? Regexp::IGNORECASE : nil) ... (clipped 6 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error Detail Exposure: User-facing admin UI surfaces raw regex error messages which may reveal engine internals; confirm this is acceptable for admin scope and not exposed to regular users. Referred Code const key = `${id}:${word}:${e.message}`; if (!seen.has(key)) { seen.add(key); const parts = e.message.split(": "); const cleanError = parts[parts.length - 1]; errors.push({ word, error: cleanError }); } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix inverted case-sensitivity regex logic Invert the ternary condition for applying Regexp::IGNORECASE to correctly handle case-sensitive and case-insensitive watched words. app/services/word_watcher.rb [104-112] # Add case insensitive flag if needed -- Regexp.new(regexp, group_key == :case_sensitive ? nil : Regexp::IGNORECASE) -+ begin -+ Regexp.new(regexp, group_key == :case_sensitive ? Regexp::IGNORECASE : nil) -+ rescue RegexpError => e -+ raise if raise_errors -+ Rails.logger.warn( -+ "Watched word compilation failed for #{action} (#{group_key}): #{e.message}. Regexp: #{regexp}", -+ ) -+ nil -+ end + begin + Regexp.new(regexp, group_key == :case_sensitive ? nil : Regexp::IGNORECASE) + rescue RegexpError => e + raise if raise_errors + Rails.logger.warn( + "Watched word compilation failed for #{action} (#{group_key}): #{e.message}. Regexp: #{regexp}", + ) + nil + end [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a logic inversion bug introduced in the PR that makes case-sensitive rules case-insensitive and vice-versa, which is a critical flaw.	High
More