diff --git a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py index 57f33aad2f..d712487da8 100644 --- a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py +++ b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py @@ -79,13 +79,15 @@ # Python traceback anatomy. The orchestrator's generic request handler # (``server.py`` ``except Exception: traceback.print_exc(); _send_error(500, # "internal_error", ...)``) prints one traceback per unhandled exception, so the -# exception *type* and the innermost ``contextual_orchestrator`` frame are the -# only evidence of what escaped. Frame lines are indented; the terminal line -# (``Type: message``) starts at column 0. Only the bounded type identifier and -# the package-relative frame are re-emitted -- never the message, which can +# exception *type* and the innermost allowlisted orchestrator or review-bootstrap +# frame are the only evidence of what escaped. Frame lines are indented; the +# terminal line (``Type: message``) starts at column 0. Only the bounded type +# identifier and relative frame are re-emitted -- never the message, which can # carry provider bodies or credentials. _TRACEBACK_FRAME = re.compile( - r'^\s+File ".*?[/\\]contextual_orchestrator[/\\](?P[A-Za-z0-9_][A-Za-z0-9_/\\]*\.py)", ' + r'^\s+File ".*?[/\\](?Pcontextual_orchestrator[/\\][A-Za-z0-9_][A-Za-z0-9_/\\]*\.py|' + r'scripts[/\\]ci[/\\]contextual_orchestrator_review_(?:policy|launcher)\.py|' + r'launch_sidecar\.py)", ' r"line (?P\d+), in (?P[A-Za-z0-9_<>]{1,80})$" ) _TRACEBACK_TERMINAL = re.compile( @@ -148,16 +150,19 @@ def sanitize_line(line: str) -> str | None: stripped = line.strip() if "\n" in stripped or "\r" in stripped: return None - request_failed = _REQUEST_FAILED.match(stripped) - if request_failed is not None: - summary = ( - f"request_failed status={request_failed.group('status')} " - f"code={request_failed.group('code')}" - ) - request_id = request_failed.group("request_id") - if request_id is not None: - summary += f" request_id={request_id}" - return summary + # Cheap substring guards avoid regex evaluation for unrelated lines. Both + # substring search and regex matching remain linear in the input length. + if "request_failed" in stripped: + request_failed = _REQUEST_FAILED.match(stripped) + if request_failed is not None: + summary = ( + f"request_failed status={request_failed.group('status')} " + f"code={request_failed.group('code')}" + ) + request_id = request_failed.group("request_id") + if request_id is not None: + summary += f" request_id={request_id}" + return summary http_request = _HTTP_REQUEST.match(stripped) if http_request is not None: return " ".join( @@ -171,23 +176,25 @@ def sanitize_line(line: str) -> str | None: f"request_id={http_request.group('request_id')}", ) ) - provider_discovery_failed = _PROVIDER_DISCOVERY_FAILED.search(stripped) - if provider_discovery_failed is not None: - return ( - f"provider_discovery_failed provider={provider_discovery_failed.group('provider')} " - f"code={provider_discovery_failed.group('code')}" - ) - preflight_route_rejected = _PREFLIGHT_ROUTE_REJECTED.search(stripped) - if preflight_route_rejected is not None: - summary = ( - f"preflight_route_{preflight_route_rejected.group('event')} " - f"provider={preflight_route_rejected.group('provider')} " - f"error_type={preflight_route_rejected.group('error_type')}" - ) - http_status = preflight_route_rejected.group("http_status") - if http_status is not None: - summary += f" http_status={http_status}" - return summary + if "provider_discovery_failed" in stripped: + provider_discovery_failed = _PROVIDER_DISCOVERY_FAILED.search(stripped) + if provider_discovery_failed is not None: + return ( + f"provider_discovery_failed provider={provider_discovery_failed.group('provider')} " + f"code={provider_discovery_failed.group('code')}" + ) + if "preflight_route_" in stripped: + preflight_route_rejected = _PREFLIGHT_ROUTE_REJECTED.search(stripped) + if preflight_route_rejected is not None: + summary = ( + f"preflight_route_{preflight_route_rejected.group('event')} " + f"provider={preflight_route_rejected.group('provider')} " + f"error_type={preflight_route_rejected.group('error_type')}" + ) + http_status = preflight_route_rejected.group("http_status") + if http_status is not None: + summary += f" http_status={http_status}" + return summary orchestrator_event = _sanitize_orchestrator_event(stripped) if orchestrator_event is not None: return orchestrator_event @@ -214,8 +221,9 @@ def main() -> int: terminal ``Type: message`` line (emitting ``unexpected_exception type=... frame=...``), at the next header or allowlisted line, or at end of stream (``type=unknown``). Indented lines inside it are frames and source echoes: - consumed, not counted as omitted, and only a ``contextual_orchestrator`` - frame's package path, line and function are retained. Any other column-0 + consumed, not counted as omitted, and only an allowlisted orchestrator or + review-bootstrap frame's relative path, line and function are retained. Any + other column-0 line closes the traceback and is classified like every other line. """ omitted = 0 @@ -235,7 +243,7 @@ def main() -> int: frame_match = _TRACEBACK_FRAME.match(line.rstrip("\n")) if frame_match is not None: module = frame_match.group("module").replace("\\", "/") - frame = f"contextual_orchestrator/{module}:{frame_match.group('line')}:{frame_match.group('function')}" + frame = f"{module}:{frame_match.group('line')}:{frame_match.group('function')}" continue if line[:1].isspace(): continue diff --git a/tests/test_contextual_orchestrator_review_runtime_preflight.py b/tests/test_contextual_orchestrator_review_runtime_preflight.py index 0b3e38cb6c..ed97b7c8f9 100644 --- a/tests/test_contextual_orchestrator_review_runtime_preflight.py +++ b/tests/test_contextual_orchestrator_review_runtime_preflight.py @@ -1733,6 +1733,24 @@ def test_sidecar_stream_sanitizer_allowlists_only_bounded_diagnostics() -> None: assert sanitize_line("provider response sk-secret") is None +def test_sidecar_stream_sanitizer_fast_path_guards_preserve_regex_contracts() -> None: + """Substring guards skip unrelated lines without admitting partial diagnostics.""" + sanitize_line = _load_sanitizer()["sanitize_line"] + + assert sanitize_line("request_failed but invalid") is None + assert sanitize_line("provider_discovery_failed but invalid") is None + assert sanitize_line("preflight_route_ but invalid") is None + + assert sanitize_line("request_failed status=500 code=error") is not None + assert ( + sanitize_line("provider_discovery_failed provider=test code=error") is not None + ) + assert ( + sanitize_line("preflight_route_rejected provider=test error_type=error") + is not None + ) + + def test_sidecar_stream_sanitizer_preserves_bounded_http_request_identity() -> None: """Review endpoints keep safe success correlation without arbitrary URL data.""" sanitize_line = _load_sanitizer()["sanitize_line"] @@ -2146,6 +2164,64 @@ def test_sidecar_stream_sanitizer_closes_a_truncated_traceback_at_end_of_stream( ] +@pytest.mark.parametrize( + ("frame_line", "expected_frame"), + [ + ( + ' File "/work/.github/scripts/ci/contextual_orchestrator_review_policy.py", ' + "line 412, in validate_policy", + "scripts/ci/contextual_orchestrator_review_policy.py:412:validate_policy", + ), + ( + ' File "C:\\a\\.github\\.github\\scripts\\ci\\contextual_orchestrator_review_launcher.py", ' + "line 91, in ", + "scripts/ci/contextual_orchestrator_review_launcher.py:91:", + ), + ( + ' File "/tmp/contextual-orchestrator-sidecar/launch_sidecar.py", ' + "line 37, in ", + "launch_sidecar.py:37:", + ), + ], +) +def test_sidecar_stream_sanitizer_keeps_allowlisted_review_bootstrap_frame( + monkeypatch: pytest.MonkeyPatch, + frame_line: str, + expected_frame: str, +) -> None: + """Keep a trusted bootstrap frame without exposing the exception message.""" + secret = "sk-secret-must-not-enter-artifact" + lines = _sanitize_stream( + monkeypatch, + "Traceback (most recent call last):\n" + f"{frame_line}\n" + f" raise PolicyError('{secret}')\n" + f"scripts.ci.contextual_orchestrator_review_policy.PolicyError: {secret}\n", + ) + assert lines == [ + "unexpected_exception " + "type=scripts.ci.contextual_orchestrator_review_policy.PolicyError " + f"frame={expected_frame}", + ] + assert secret not in "\n".join(lines) + + +def test_sidecar_stream_sanitizer_rejects_neighboring_review_script_frame( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A neighboring script is not trusted merely because it shares the directory.""" + secret = "sk-secret-must-not-enter-artifact" + lines = _sanitize_stream( + monkeypatch, + "Traceback (most recent call last):\n" + ' File "/work/.github/scripts/ci/untrusted.py", line 7, in leak\n' + f" raise RuntimeError('{secret}')\n" + f"RuntimeError: {secret}\n", + ) + assert lines == ["unexpected_exception type=RuntimeError frame=unknown"] + assert secret not in "\n".join(lines) + + def test_sidecar_stream_sanitizer_does_not_treat_free_text_as_an_exception( monkeypatch: pytest.MonkeyPatch, ) -> None: