diff --git a/AGENTS.md b/AGENTS.md index 49149dfa81..0972af51c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,14 @@ see [`docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`](docs/adr/0003 2026-08-30 amendment and its 2026-08-31 correction, which retracts an earlier false claim of explicit owner direction and records the resulting availability risk as open and unreviewed, not accepted. +Sidecar diagnostics may retain only a server-generated `request_id` matching +exactly 32 lowercase hexadecimal characters, plus the producer's explicit `-` +or `` marker where that event contract permits it. Keep free-form +provider errors omitted; malformed, uppercase, short, long, or otherwise +unbounded identifiers must not pass the sanitizer. +HTTP success summaries are narrower still: preserve correlation only for the +review sidecar's fixed health, chat-completions, and responses paths. Never +allowlist arbitrary request paths merely because the producer stripped queries. The materialization contract is also covered by [`docs/doctoring/exact-artifact-sbom-attestation.md`](docs/doctoring/exact-artifact-sbom-attestation.md). ## Actions queue and protected-merge procedure diff --git a/CLAUDE.md b/CLAUDE.md index 30db1fc23b..7dde78f5d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -146,6 +146,13 @@ repeatable compile command. `contextual-orchestrator/orchestrator/free`). Keep the ZDR-first policy and the exact-head/vendoring pins in `scripts/ci/zdr_policy.py` and `scripts/ci/contextual_orchestrator_review_sidecar.sh` in sync with their contract tests. + Sanitized route diagnostics preserve only server request IDs that are exactly + 32 lowercase hexadecimal characters, plus an event contract's explicit `-` or + `` marker; never widen that field to arbitrary text or re-emit provider + error messages. + HTTP success correlation is limited to the review sidecar's fixed health, + chat-completions, and responses paths; query stripping alone does not make an + arbitrary request path safe for CI artifacts. - **`pull_request_target` trust boundary.** The required review workflows run the *base branch's* trusted scripts. A PR that edits the trusted review workflows can fail its own checks until the base branch catches up; a same-head manual `workflow_dispatch` Strix run may supply review evidence diff --git a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py index 9df99e1b70..57f33aad2f 100644 --- a/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py +++ b/scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py @@ -7,14 +7,27 @@ import sys +_REQUEST_ID = r"[0-9a-f]{32}" +_PROVIDER_REQUEST_ID = rf"(?:{_REQUEST_ID}|-)" +_NUMBER = r"\d+(?:\.\d+)?" _REQUEST_FAILED = re.compile( r"^(?:(?:[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2},[0-9]{3} )?" r"(?:DEBUG|INFO|WARNING|ERROR)[: ]contextual_orchestrator\.server[: ])?" r"request_failed status=(?P[1-5][0-9]{2}) " - r"code=(?P[A-Za-z0-9_.-]{1,64})" - r"(?= |$)(?: request_id=(?P[0-9a-f]{32}|)(?= |$))?" + r"code=(?P[A-Za-z0-9_.-]{1,64})(?= |$)" + rf"(?: request_id=(?P{_REQUEST_ID}|)(?=$|\s))?" r"(?! request_id=)" ) +_HTTP_REQUEST = re.compile( + r"^(?:(?:[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2},[0-9]{3} )?" + r"(?:DEBUG|INFO|WARNING|ERROR)[: ]contextual_orchestrator\.server[: ])?" + r"http_request method=(?PGET|POST) " + r"path=(?P/healthz|/v1/chat/completions|/v1/responses|-) " + r"status=(?P[1-5][0-9]{2}|-) " + rf"latency_ms=(?P{_NUMBER}) " + r"session_id_hash=(?P[0-9a-f]{64}|-) " + rf"request_id=(?P{_REQUEST_ID})$" +) _PROVIDER_DISCOVERY_FAILED = re.compile( r"provider_discovery_failed provider=(?P[a-z][a-z0-9_]{0,63}) " r"code=(?P[A-Za-z0-9_.-]{1,64})" @@ -31,7 +44,6 @@ _AGENT_ID = r"[a-z][a-z0-9_]*" _MODEL_ID = r"[A-Za-z0-9_./:-]+" _ERROR_TYPE = r"[A-Za-z_][A-Za-z0-9_.]*" -_NUMBER = r"\d+(?:\.\d+)?" # contextual_orchestrator/orchestrator.py templates at the vendored pin. Every # field is a bounded identifier or number; ``error_message`` is free text and is # deliberately excluded from the match so it can never be re-emitted. @@ -40,17 +52,24 @@ _ORCHESTRATOR_EVENTS = tuple( re.compile(pattern) for pattern in ( - rf"^provider_attempt agent_id={_AGENT_ID} model={_MODEL_ID} attempt=\d+/\d+$", + rf"^provider_attempt agent_id={_AGENT_ID} model={_MODEL_ID} attempt=\d+/\d+" + rf"(?: request_id={_PROVIDER_REQUEST_ID})?$", rf"^provider_attempt_failed agent_id={_AGENT_ID} model={_MODEL_ID} attempt=\d+ " rf"error_type={_ERROR_TYPE} transient=(?:True|False)" - rf"(?: provider_status=(?:[1-5][0-9]{{2}}|None))?(?= error_message=)", - rf"^provider_backoff agent_id={_AGENT_ID} attempt=\d+ delay_seconds={_NUMBER}$", + rf"(?: provider_status=(?:[1-5][0-9]{{2}}|None))?" + rf"(?: request_id={_PROVIDER_REQUEST_ID})?(?= error_message=)", + rf"^provider_backoff agent_id={_AGENT_ID} attempt=\d+ delay_seconds={_NUMBER}" + rf"(?: request_id={_PROVIDER_REQUEST_ID})?$", rf"^provider_exhausted agent_id={_AGENT_ID} model={_MODEL_ID} attempts=\d+ " - rf"final_error_type={_ERROR_TYPE}$", + rf"final_error_type={_ERROR_TYPE}(?: request_id={_PROVIDER_REQUEST_ID})?$", rf"^provider_rejected_permanent agent_id={_AGENT_ID} model={_MODEL_ID} attempts=\d+ " - rf"final_error_type={_ERROR_TYPE}$", + rf"final_error_type={_ERROR_TYPE}(?: request_id={_PROVIDER_REQUEST_ID})?$", rf"^provider_no_retry_budget agent_id={_AGENT_ID} model={_MODEL_ID} attempts=\d+ " - rf"final_error_type={_ERROR_TYPE} transient=(?:True|False)$", + rf"final_error_type={_ERROR_TYPE} transient=(?:True|False)" + rf"(?: request_id={_PROVIDER_REQUEST_ID})?$", + rf"^provider_one_shot_call_failed agent_id={_AGENT_ID} model={_MODEL_ID} attempts=\d+ " + rf"final_error_type={_ERROR_TYPE} transient=(?:True|False)" + rf"(?: request_id={_PROVIDER_REQUEST_ID})?$", rf"^circuit_failure agent_id={_AGENT_ID} failures={_NUMBER} threshold=\d+$", rf"^circuit_opened agent_id={_AGENT_ID} failures={_NUMBER} threshold=\d+ reset_seconds={_NUMBER}$", rf"^circuit_reset agent_id={_AGENT_ID}$", @@ -139,6 +158,19 @@ def sanitize_line(line: str) -> str | None: 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( + ( + "http_request", + f"method={http_request.group('method')}", + f"path={http_request.group('path')}", + f"status={http_request.group('status')}", + f"latency_ms={http_request.group('latency')}", + f"session_id_hash={http_request.group('session_id_hash')}", + f"request_id={http_request.group('request_id')}", + ) + ) provider_discovery_failed = _PROVIDER_DISCOVERY_FAILED.search(stripped) if provider_discovery_failed is not None: return ( diff --git a/tests/test_contextual_orchestrator_review_runtime_preflight.py b/tests/test_contextual_orchestrator_review_runtime_preflight.py index 73899f10c9..0b3e38cb6c 100644 --- a/tests/test_contextual_orchestrator_review_runtime_preflight.py +++ b/tests/test_contextual_orchestrator_review_runtime_preflight.py @@ -1733,6 +1733,33 @@ def test_sidecar_stream_sanitizer_allowlists_only_bounded_diagnostics() -> None: assert sanitize_line("provider response sk-secret") is 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"] + request_id = "0123456789abcdef0123456789abcdef" + session_hash = "ab" * 32 + event = ( + "http_request method=POST path=/v1/chat/completions status=200 " + f"latency_ms=125.2 session_id_hash={session_hash} request_id={request_id}" + ) + assert sanitize_line(event) == event + assert sanitize_line(f"INFO:contextual_orchestrator.server:{event}") == event + assert sanitize_line( + "http_request method=GET path=/healthz status=200 latency_ms=0.4 " + f"session_id_hash=- request_id={request_id}" + ) == ( + "http_request method=GET path=/healthz status=200 latency_ms=0.4 " + f"session_id_hash=- request_id={request_id}" + ) + for unsafe_event in ( + event.replace("/v1/chat/completions", "/v1/files/private-name"), + event.replace(request_id, "A" * 32), + event.replace(session_hash, "ab" * 31), + event + " token=sk-secret", + ): + assert sanitize_line(unsafe_event) is None + + def test_sidecar_stream_sanitizer_admits_orchestrator_route_events() -> None: """Per-route attempt, retry-budget, and circuit events survive with bounded fields only. @@ -1769,6 +1796,62 @@ def test_sidecar_stream_sanitizer_admits_orchestrator_route_events() -> None: assert sanitize_line( "provider_backoff agent_id=nvidia_nim_x attempt=1 delay_seconds=0.500" ) == "provider_backoff agent_id=nvidia_nim_x attempt=1 delay_seconds=0.500" + request_id = "0123456789abcdef0123456789abcdef" + assert sanitize_line( + "provider_attempt agent_id=nvidia_nim_x model=m/x attempt=1/3 " + f"request_id={request_id}" + ) == ( + "provider_attempt agent_id=nvidia_nim_x model=m/x attempt=1/3 " + f"request_id={request_id}" + ) + assert sanitize_line( + "provider_backoff agent_id=nvidia_nim_x attempt=1 delay_seconds=0.500 " + f"request_id={request_id}" + ) == ( + "provider_backoff agent_id=nvidia_nim_x attempt=1 delay_seconds=0.500 " + f"request_id={request_id}" + ) + for event in ( + "provider_exhausted agent_id=nvidia_nim_x model=m/x attempts=2 final_error_type=HTTPError", + "provider_rejected_permanent agent_id=nvidia_nim_x model=m/x attempts=1 final_error_type=ValueError", + "provider_no_retry_budget agent_id=nvidia_nim_x model=m/x attempts=1 final_error_type=HTTPError transient=False", + "provider_one_shot_call_failed agent_id=nvidia_nim_x model=m/x attempts=1 final_error_type=HTTPError transient=False", + ): + correlated_event = f"{event} request_id={request_id}" + assert sanitize_line(correlated_event) == correlated_event + absent_event = f"{event} request_id=-" + assert sanitize_line(absent_event) == absent_event + request_failed = sanitize_line( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True " + f"request_id={request_id} error_message=Bearer sk-secret" + ) + assert request_failed == ( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True " + f"request_id={request_id} error_message=" + ) + assert "sk-secret" not in request_failed + provider_status_failed = sanitize_line( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True provider_status=429 " + f"request_id={request_id} error_message=Bearer sk-secret" + ) + assert provider_status_failed == ( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True provider_status=429 " + f"request_id={request_id} error_message=" + ) + assert sanitize_line( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True request_id=- error_message=unbound" + ) == ( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True request_id=- error_message=" + ) + assert sanitize_line( + f"request_failed status=502 code=provider_error request_id={request_id}" + ) == f"request_failed status=502 code=provider_error request_id={request_id}" assert sanitize_line( "INFO:contextual_orchestrator.orchestrator:provider_no_retry_budget agent_id=bytez_a " "model=m/x attempts=1 final_error_type=InvalidChatResponse transient=False" @@ -1798,6 +1881,24 @@ def test_sidecar_stream_sanitizer_admits_orchestrator_route_events() -> None: "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 error_type=E transient=False" ) is None assert sanitize_line(f"DEBUG:contextual_orchestrator.orchestrator:{secret}") is None + for invalid_request_id in ( + "0123456789abcdef0123456789abcde", + "0123456789abcdef0123456789abcdef0", + "0123456789ABCDEF0123456789ABCDEF", + "not-a-request-id", + ): + assert sanitize_line( + "provider_attempt agent_id=nvidia_nim_x model=m/x attempt=1/3 " + f"request_id={invalid_request_id}" + ) is None + assert sanitize_line( + "provider_attempt_failed agent_id=nvidia_nim_x model=m/x attempt=1 " + "error_type=HTTPError transient=True " + f"request_id={invalid_request_id} error_message=raw-error" + ) is None + assert sanitize_line( + f"request_failed status=502 code=provider_error request_id={invalid_request_id}" + ) is None @pytest.mark.parametrize("status", [None, "100", "429", "599", "None"])