fix(shared/redaction): add exception handling to redact_value() for pattern.sub() failures

Add a try-except block around pattern.sub() calls in redact_value() so that
malformed or adversarial regex patterns cannot crash the logging pipeline.
When a pattern raises an exception during substitution, a warning is logged
via the module-level structlog logger and processing continues with the next
pattern. Partial redactions from patterns that succeeded before the failure
are preserved in the returned value.

Also adds a module-level structlog logger (_log) for internal diagnostics,
BDD unit tests (tdd_redact_value_pattern_exception_10567.feature) covering
all exception-handling scenarios, and Robot Framework integration tests
verifying the fix is present in the source.

ISSUES CLOSED: #10567

features/steps/tdd_redact_value_pattern_exception_10567_steps.py

@@ -0,0 +1,244 @@
+"""Step definitions for TDD test: redact_value() pattern.sub() exception handling.
+
+Issue: #10567 — redact_value() doesn't handle pattern.sub() exceptions
+Branch: fix/redaction-pattern-exception-handling
+
+This test captures the bug: when a registered regex pattern raises an exception
+during pattern.sub() (e.g. due to a malformed pattern, ReDoS, or memory error),
+redact_value() propagates the exception to the caller instead of logging a warning
+and continuing with the next pattern.
+
+The fix adds a try-except block around pattern.sub() calls so that:
+- Exceptions are caught and logged as warnings
+- Processing continues with the next pattern
+- The function never crashes due to a problematic pattern
+"""
+
+from __future__ import annotations
+
+import re
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+from behave import given, then, when
+from behave.runner import Context
+
+from cleveragents.shared import redaction as redaction_module
+from cleveragents.shared.redaction import (
+    _SECRET_PATTERNS,
+    _patterns_lock,
+    redact_value,
+    secrets_masking_processor,
+)
+
+# ── Helpers ───────────────────────────────────────────────────────
+
+
+class _ExplodingPattern:
+    """A fake compiled-pattern object whose .sub() always raises re.error."""
+
+    def sub(self, repl: str, string: str) -> str:
+        raise re.error("simulated pattern.sub() failure")
+
+
+def _snapshot_patterns() -> list[Any]:
+    """Return a copy of the current _SECRET_PATTERNS list."""
+    with _patterns_lock:
+        return list(_SECRET_PATTERNS)
+
+
+def _restore_patterns(snapshot: list[Any]) -> None:
+    """Restore _SECRET_PATTERNS to a previous snapshot."""
+    with _patterns_lock:
+        _SECRET_PATTERNS.clear()
+        _SECRET_PATTERNS.extend(snapshot)
+
+
+# ── Background ────────────────────────────────────────────────────
+
+
+@given("the redaction module is available for exception handling tests")
+def step_redaction_module_available(context: Context) -> None:
+    context.pattern_snapshot = _snapshot_patterns()
+    if not hasattr(context, "_cleanup_handlers"):
+        context._cleanup_handlers = []
+    context._cleanup_handlers.append(lambda: _restore_patterns(context.pattern_snapshot))
+
+
+# ── Given steps ──────────────────────────────────────────────────
+
+
+@given("a malformed regex pattern is registered for exception testing")
+def step_register_malformed_pattern(context: Context) -> None:
+    with _patterns_lock:
+        _SECRET_PATTERNS.append(_ExplodingPattern())  # type: ignore[arg-type]
+
+
+@given("a pattern that raises an exception during substitution is registered")
+def step_register_exploding_pattern(context: Context) -> None:
+    with _patterns_lock:
+        _SECRET_PATTERNS.append(_ExplodingPattern())  # type: ignore[arg-type]
+
+
+@given("a failing pattern is registered before a valid secret pattern")
+def step_register_failing_before_valid(context: Context) -> None:
+    # Insert the exploding pattern at position 0 so it runs before the real patterns
+    with _patterns_lock:
+        _SECRET_PATTERNS.insert(0, _ExplodingPattern())  # type: ignore[arg-type]
+
+
+@given("a failing pattern is registered between two valid patterns")
+def step_register_failing_between_valid(context: Context) -> None:
+    # Insert the exploding pattern between existing patterns
+    with _patterns_lock:
+        if len(_SECRET_PATTERNS) >= 2:
+            _SECRET_PATTERNS.insert(1, _ExplodingPattern())  # type: ignore[arg-type]
+        else:
+            _SECRET_PATTERNS.append(_ExplodingPattern())  # type: ignore[arg-type]
+
+
+# ── When steps ───────────────────────────────────────────────────
+
+
+@when("I call redact_value with a plain string")
+def step_call_redact_value_plain(context: Context) -> None:
+    context.redact_exception: Exception | None = None
+    context.redact_result: str | None = None
+    try:
+        context.redact_result = redact_value("hello world")
+    except Exception as exc:
+        context.redact_exception = exc
+
+
+@when("I call redact_value with a plain string and capture warnings")
+def step_call_redact_value_capture_warnings(context: Context) -> None:
+    context.redact_exception = None
+    context.redact_result = None
+    context.warning_calls: list[Any] = []
+
+    logger_mock = MagicMock()
+
+    def capture_warning(*args: Any, **kwargs: Any) -> None:
+        context.warning_calls.append((args, kwargs))
+
+    logger_mock.warning = capture_warning
+
+    with patch.object(redaction_module, "_log", logger_mock):
+        try:
+            context.redact_result = redact_value("hello world")
+        except Exception as exc:
+            context.redact_exception = exc
+
+
+@when("I call redact_value with a string containing a known secret")
+def step_call_redact_value_with_secret(context: Context) -> None:
+    context.redact_exception = None
+    context.redact_result = None
+    try:
+        # Use a known OpenAI key pattern that should be redacted
+        context.redact_result = redact_value("key=sk-proj-abcdefghijklmnopqrstuvwxyz1234")
+    except Exception as exc:
+        context.redact_exception = exc
+
+
+@when("I call redact_value with a string containing two different secrets")
+def step_call_redact_value_with_two_secrets(context: Context) -> None:
+    context.redact_exception = None
+    context.redact_result = None
+    try:
+        # Two different secret patterns: OpenAI key and Anthropic key
+        context.redact_result = redact_value(
+            "openai=sk-proj-abcdefghijklmnopqrstuvwxyz1234 "
+            "anthropic=sk-ant-api03-abcdefghijklmnopqrstuvwxyz"
+        )
+    except Exception as exc:
+        context.redact_exception = exc
+
+
+@when("I apply the masking processor to an event dict with a plain string value")
+def step_apply_masking_processor_with_bad_pattern(context: Context) -> None:
+    context.processor_exception: Exception | None = None
+    context.processor_result: Any = None
+    event_dict = {"event": "hello world", "key": "some value"}
+    try:
+        context.processor_result = secrets_masking_processor(
+            logger=None,
+            method_name="info",
+            event_dict=dict(event_dict),
+        )
+    except Exception as exc:
+        context.processor_exception = exc
+
+
+# ── Then steps ───────────────────────────────────────────────────
+
+
+@then("redact_value should return a string without raising an exception")
+def step_redact_value_no_exception(context: Context) -> None:
+    assert context.redact_exception is None, (
+        f"redact_value() raised an exception: {context.redact_exception!r}. "
+        f"Bug #10567: pattern.sub() exceptions must be caught and handled gracefully."
+    )
+    assert isinstance(context.redact_result, str), (
+        f"Expected a string result but got {type(context.redact_result)!r}"
+    )
+
+
+@then("a warning should have been logged about the pattern failure")
+def step_warning_logged(context: Context) -> None:
+    assert context.redact_exception is None, (
+        f"redact_value() raised an exception: {context.redact_exception!r}"
+    )
+    assert len(context.warning_calls) > 0, (
+        "Expected at least one warning to be logged when a pattern raises an exception, "
+        "but no warnings were captured. "
+        "Bug #10567: redact_value() must log a warning when pattern.sub() fails."
+    )
+
+
+@then("the known secret should still be redacted in the result")
+def step_known_secret_redacted(context: Context) -> None:
+    assert context.redact_exception is None, (
+        f"redact_value() raised an exception: {context.redact_exception!r}"
+    )
+    assert context.redact_result is not None
+    assert "***REDACTED***" in context.redact_result, (
+        f"Expected '***REDACTED***' in result '{context.redact_result}'. "
+        f"Bug #10567: redact_value() must continue processing remaining patterns "
+        f"after a pattern failure."
+    )
+    assert "sk-proj-abcdefghijklmnopqrstuvwxyz1234" not in context.redact_result, (
+        f"Secret was not redacted in result '{context.redact_result}'"
+    )
+
+
+@then("both secrets should be redacted in the result")
+def step_both_secrets_redacted(context: Context) -> None:
+    assert context.redact_exception is None, (
+        f"redact_value() raised an exception: {context.redact_exception!r}"
+    )
+    assert context.redact_result is not None
+    assert "sk-proj-abcdefghijklmnopqrstuvwxyz1234" not in context.redact_result, (
+        f"First secret was not redacted in result '{context.redact_result}'"
+    )
+    assert "sk-ant-api03-abcdefghijklmnopqrstuvwxyz" not in context.redact_result, (
+        f"Second secret was not redacted in result '{context.redact_result}'"
+    )
+
+
+@then("the masking processor should return without raising an exception")
+def step_processor_no_exception(context: Context) -> None:
+    assert context.processor_exception is None, (
+        f"secrets_masking_processor() raised an exception: {context.processor_exception!r}. "
+        f"Bug #10567: the structlog pipeline must remain stable even when a pattern fails."
+    )
+
+
+@then("the event dict should be returned with the value present")
+def step_event_dict_has_value(context: Context) -> None:
+    assert context.processor_result is not None, (
+        "Expected a result from secrets_masking_processor but got None"
+    )
+    assert "event" in context.processor_result, (
+        f"Expected 'event' key in processor result: {context.processor_result!r}"
+    )

features/tdd_redact_value_pattern_exception_10567.feature

@@ -0,0 +1,39 @@
+@tdd_issue @tdd_issue_10567
+Feature: TDD — redact_value() handles pattern.sub() exceptions gracefully
+  As a developer relying on the structlog pipeline
+  I want redact_value() to never crash when a registered pattern raises an exception
+  So that a malformed or adversarial regex cannot bring down the logging system
+
+  Background:
+    Given the redaction module is available for exception handling tests
+
+  @tdd_issue @tdd_issue_10567
+  Scenario: redact_value does not crash when a pattern raises re.error
+    Given a malformed regex pattern is registered for exception testing
+    When I call redact_value with a plain string
+    Then redact_value should return a string without raising an exception
+
+  @tdd_issue @tdd_issue_10567
+  Scenario: redact_value logs a warning when a pattern substitution fails
+    Given a pattern that raises an exception during substitution is registered
+    When I call redact_value with a plain string and capture warnings
+    Then a warning should have been logged about the pattern failure
+
+  @tdd_issue @tdd_issue_10567
+  Scenario: redact_value continues processing remaining patterns after a failure
+    Given a failing pattern is registered before a valid secret pattern
+    When I call redact_value with a string containing a known secret
+    Then the known secret should still be redacted in the result
+
+  @tdd_issue @tdd_issue_10567
+  Scenario: redact_value returns partial result when some patterns succeed and one fails
+    Given a failing pattern is registered between two valid patterns
+    When I call redact_value with a string containing two different secrets
+    Then both secrets should be redacted in the result
+
+  @tdd_issue @tdd_issue_10567
+  Scenario: secrets_masking_processor remains stable when redact_value encounters a bad pattern
+    Given a pattern that raises an exception during substitution is registered
+    When I apply the masking processor to an event dict with a plain string value
+    Then the masking processor should return without raising an exception
+    And the event dict should be returned with the value present

robot/security_secrets.robot

@@ -72,3 +72,16 @@ Logging Config Integrates Masking Processor
     ${content}=    Get File    ${SRC_DIR}/config/logging.py
     Should Contain    ${content}    secrets_masking_processor
     Should Contain    ${content}    configure_structlog
+
+Redaction Module Has Exception Handling In redact_value
+    [Documentation]    Verify redact_value() wraps pattern.sub() in try-except (issue #10567)
+    ${content}=    Get File    ${SRC_DIR}/shared/redaction.py
+    Should Contain    ${content}    try:
+    Should Contain    ${content}    except Exception
+    Should Contain    ${content}    pattern substitution failed
+
+Redaction Module Has Module Logger
+    [Documentation]    Verify redaction.py declares a module-level structlog logger (issue #10567)
+    ${content}=    Get File    ${SRC_DIR}/shared/redaction.py
+    Should Contain    ${content}    _log
+    Should Contain    ${content}    structlog.get_logger

src/cleveragents/shared/redaction.py

@@ -20,8 +20,13 @@ import threading
 from collections.abc import MutableMapping
 from typing import Any
 
+import structlog
+
 REDACTED = "***REDACTED***"
 
+# Module-level logger for internal diagnostics (pattern failure warnings).
+_log: structlog.stdlib.BoundLogger = structlog.get_logger(__name__)
+
 # ── Sensitive key-name substrings ──────────────────────────────
 
 _SENSITIVE_SUBSTRINGS: set[str] = {
@@ -129,12 +134,19 @@ def is_sensitive_key(key: str) -> bool:
 def redact_value(value: str) -> str:
     """Scan a string for secret patterns and replace matches.
 
+    Each registered pattern is applied in sequence.  If a pattern raises
+    an exception during substitution (e.g. due to a malformed regex,
+    ReDoS, or memory exhaustion), a warning is logged and processing
+    continues with the next pattern.  The function never propagates
+    pattern-level exceptions to the caller.
+
     Args:
         value: The string to scan and redact.
 
     Returns:
         The string with any detected secrets replaced by
-        ``***REDACTED***``.
+        ``***REDACTED***``.  Partial redactions from patterns that
+        succeeded before a failure are preserved in the returned value.
     """
     if not value:
         return value
@@ -142,7 +154,14 @@ def redact_value(value: str) -> str:
     with _patterns_lock:
         patterns = list(_SECRET_PATTERNS)
     for pattern in patterns:
-        result = pattern.sub(REDACTED, result)
+        try:
+            result = pattern.sub(REDACTED, result)
+        except Exception as exc:
+            _log.warning(
+                "redact_value: pattern substitution failed; skipping pattern",
+                pattern=repr(pattern),
+                exc_info=exc,
+            )
     return result