fix(guardrails): run input guardrails on /v1/responses and /v1/embeddings (#719) - #541
Conversation
…ings (#719) A configured input guardrail that blocks on /v1/chat/completions was silently skipped on /v1/responses and /v1/embeddings: the same input that 422s on the chat surface returned 200 (responses echoed it back; embeddings embedded it). A customer-configured content/DLP block was bypassable simply by calling a non-chat surface. - responses.rs: synthesize a role-preserving ChatFormat from the Responses `input` (string | array of message items) + optional `instructions`, resolve the per-request guardrail chain, and run check_input before dispatch; Block short-circuits to 422 content_filter. Mirrors the /v1/messages pattern. - embeddings.rs: synthesize a ChatFormat of user messages from `input` (string | array of strings) and run check_input before the upstream call; Block short-circuits to 422. Embeddings responses are vectors, so there is no output hook to run. - Only Block is enforced; Rewrite/Bypass are not applied to the outgoing non-chat body (matching /v1/messages). Matched-pattern detail stays in ops logs only (#153) — the wire envelope stays generic so callers can't enumerate the blocklist via error probing. /v1/messages already runs input + output guardrails (unchanged here). Output guardrails on /v1/responses (non-streaming + streaming) are the remaining #719 scope item and are tracked as a follow-up. Tests: 6 wiremock unit tests across both surfaces — block on string and array input returns 422 content_filter with the upstream never contacted (expect(0)); benign input still forwards (expect(1)) and returns 200; the blocked literal must not leak into the wire message. cargo fmt + clippy clean; 385 aisix-proxy lib tests pass.
|
Warning Review limit reached
More reviews will be available in 6 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends input content filtering to embeddings and responses endpoints. Both handlers now convert their request inputs into chat format, execute configured guardrail chains, and block requests with 422 content-filter errors before reaching upstream when guardrails trigger. Tests verify blocking and pass-through behavior for both string and array input forms. ChangesInput Guardrails for Embeddings and Responses
Sequence DiagramssequenceDiagram
participant Client
participant EmbeddingsHandler
participant GuardrailChain
participant UpstreamEmbedding
Client->>EmbeddingsHandler: POST /v1/embeddings with input
EmbeddingsHandler->>EmbeddingsHandler: Convert input to ChatFormat
EmbeddingsHandler->>GuardrailChain: Resolve guardrail chain
alt Guardrail Blocks
GuardrailChain-->>EmbeddingsHandler: GuardrailVerdict::Block
EmbeddingsHandler-->>Client: 422 content_filter error
else Guardrail Passes
GuardrailChain-->>EmbeddingsHandler: GuardrailVerdict::Pass
EmbeddingsHandler->>UpstreamEmbedding: Forward embedding request
UpstreamEmbedding-->>Client: Embedding response
end
sequenceDiagram
participant Client
participant ResponsesHandler
participant GuardrailChain
participant UpstreamResponse
Client->>ResponsesHandler: POST /v1/responses with instructions and input
ResponsesHandler->>ResponsesHandler: Convert instructions and input to ChatFormat
ResponsesHandler->>GuardrailChain: Resolve guardrail chain
alt Guardrail Blocks
GuardrailChain-->>ResponsesHandler: GuardrailVerdict::Block
ResponsesHandler-->>Client: 422 content_filter error
else Guardrail Passes
GuardrailChain-->>ResponsesHandler: GuardrailVerdict::Pass
ResponsesHandler->>UpstreamResponse: Forward response request
UpstreamResponse-->>Client: Response completion
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
…sponses (#719) Independent-audit MEDIUM-1 on #541: a Responses `input` array may contain `function_call_output` items whose caller-supplied tool-result text lives under `output`, not `content`. That text reaches the model, so leaving it unscanned re-opened the #719 surface-switch bypass on the tool-result channel (the /v1/chat/completions equivalent — a role:tool message — is scanned by all-roles guardrails). `responses_item_text` now reads both `content` and `output` (each a string or an array of typed text parts). A `function_call_output` item carries no `role`, so it maps to a user message and is scanned by every guardrail kind (including text-moderation's user-only default). Tests: function_call_output with a blocked literal in `output` -> 422 content_filter (upstream never contacted); top-level `instructions` with a blocked literal -> 422. fmt + clippy clean; all 10 input_guardrail tests pass.
…ses (#719) Re-audit LOW-1 on #541: mcp_approval_response items carry caller-supplied justification text under `reason`, which reaches the model. responses_item_text now reads content/output/reason so no input-bearing channel on the Responses `input` union is silently skipped. Reading a key absent on other item types is a no-op. Test added; fmt + clippy clean, 11 input_guardrail tests pass.
What
Closes the input half of #719: a configured input guardrail that blocks on
/v1/chat/completionswas silently skipped on/v1/responsesand/v1/embeddings. The same input that returns422 content_filteron the chat surface returned200on these surfaces —/v1/responsesechoed the content back,/v1/embeddingsembedded it. A customer-configured content/DLP block was therefore bypassable just by switching surface.This PR makes both non-chat input-bearing surfaces run the resolved input guardrail chain before dispatch, exactly as
/v1/chat/completionsand/v1/messagesalready do.Why this happened
Guardrail invocation was inlined per-surface.
chat.rsandmessages.rseach resolve the chain and callcheck_input;responses.rshad zero guardrail calls andembeddings.rsexplicitly documented "embeddings bypass guardrails today". So the enforcement gap was per-surface, not in the chain itself.Changes
crates/aisix-proxy/src/responses.rsRequestContext { model_id, api_key_id, team_id }), and if non-empty runcheck_input.Block→ProxyError::ContentFiltered(422 content_filter).responses_input_to_chat()synthesizes a role-preservingChatFormatfrom the Responses-API body:inputas a bare string, or an array of message items whosecontentis a string or typed parts (input_text/output_text/text); optional top-levelinstructions→ system message. Roles are preserved so the guardrail's user-vs-all message selection behaves identically to chat. The synthesized request is for scanning only — the original body is still forwarded verbatim.crates/aisix-proxy/src/embeddings.rsembeddings_input_to_chat()turns theinput(string / array of strings) into user messages. Embeddings responses are vectors → no output hook.Shared semantics (both surfaces), matching
/v1/messages:Blockis enforced.Rewrite/Bypassare not applied to the outgoing non-chat body (same deliberate limitation/v1/messagesdocuments).reasonis logged for ops but the wire message stays generic ("request blocked by content policy") so callers can't enumerate the blocklist by probing error responses.Reference / spec
inputshape (string | array of input items; messagecontentis a string or typed parts): https://platform.openai.com/docs/api-reference/responses/create. Embeddingsinput(string | array of strings): https://platform.openai.com/docs/api-reference/embeddings/create.messages.rsinput check (resolve chain →is_empty()guard → translate body →check_input→ContentFilteredonBlock).#719 scope status
/v1/responsesruns input guardrails — this PR/v1/embeddingsruns input guardrails (carries scannable user content) — this PR/v1/messagesruns input + output guardrails — already present (messages.rsinput:398, output non-streaming:1353, streaming:1529); verified, no change/v1/responsesnon-streaming + streaming) — deliberately deferred to a follow-up PRWhy output is a separate PR (not in this one)
Output coverage on
/v1/responsesmust be all-or-nothing: adding it to the non-streaming path only would makestream:truea new asymmetric bypass of the exact class #719 is about. Streaming output requires parsing the Responses-API SSE event protocol (response.output_text.delta/response.completed) — which the gateway deliberately does not do today (SSE is passed through verbatim; streamingusageextraction was itself deferred under #404). That is a focused, protocol-specific change with its own regression surface, so it is isolated for independent review rather than bundled with this clean, high-value input fix. Follow-up will use buffer-then-scan (BufferFull) semantics, matching the chat surface's secure default.Test plan
6 new wiremock unit tests (3 per surface):
422+error.type == content_filter+ upstream never contacted (expect(0))422 content_filterexpect(1)) →200(guardrail must not block clean traffic)cargo fmt+cargo clippy --all-targetsclean; 385aisix-proxylib tests pass.The real-Azure
M1acceptance test (held back in AISIX-Cloud#718) will be re-added to AISIX-Cloud once this merges, per the source-blind discipline.Independent audit (CLAUDE.md §8) — findings + resolution
A fresh agent reviewed this PR cold. It verified the core fix is solid: synthesized text is actually scannable (
message_scan_text), the input check runs before the streaming split (covers both modes), #153 redaction holds (no blocklist leak),422 content_filterconfirmed, the forwarded upstream body is byte-identical, the no-guardrail path is a pure no-op, and the 6 tests genuinely assert the contract (wiremockexpect(0)/expect(1)).function_call_outputtool-result text not scanned (narrowed feat(bedrock): honor ANONYMIZE — mask-and-continue instead of block #719 bypass on the tool-result channel): FIXED in code (commit3fba23d).responses_item_textnow scans bothcontentandoutput; such items map to a user message and are scanned by every guardrail kind. New tests cover it (function_call_outputblock +instructionsblock)./v1/messagesordering (intentionally mirrored), so it is consistent shipped behavior, not a regression. Aligning all passthrough endpoints withchat.rs(which checks before reservation) is tracked in Guardrail input check runs after rate-limit reservation on /v1/messages, /v1/responses, /v1/embeddings (burns a quota slot on a content block) #542.applied_guardrails/guardrail_blockedthrough the non-chat emit paths is tracked in Attribute guardrail blocks in telemetry for /v1/responses and /v1/embeddings (Blocked tab under-counts) #543.instructions-only block;function_call_outputblock). TheBypass/fail-open path is correct by construction (theif let Blockmatch forwards every non-Block verdict) and exercised by the benign-forwards test.A second, focused audit of the MEDIUM-1 fix returned MERGE: the
function_call_outputchannel is fully closed (string + array-of-parts forms; no over-scan of normal items;#153intact), and it confirmed the genericoutputread also coverscustom_tool_call_output/*_call_output. It found one further LOW residual —mcp_approval_response.reason(a niche MCP-approval justification that also reaches the model) was unscanned. Fixed (commit appended):responses_item_textnow also readsreason, with a test. Net: every input-bearing channel on theinputunion is scanned.Merge gate: both MEDIUMs fixed-or-justified; both LOW residuals fixed; LOW telemetry tracked in #543. No HIGH findings; both audits recommend merge.
Refs #719, #379, #542, #543.