Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ This repo is developed end-to-end by agents — no human reviewer needs small re
- "Documented follow-up" without an issue is how gaps rot: it lives in one PR description and no one ever comes back.
- **An emit function on `Metrics` with no caller is invisible to every check we run.** Its methods are `pub`, so dead-code analysis never fires; unit tests call it directly and pass; the only symptom is a series that never appears in a scrape, which is indistinguishable from "no traffic yet". A metric family is shipped when an **e2e asserts it in `GET /metrics`** after driving real traffic — not when `Metrics` can emit it. (Twice now: `record_proxy_request` until #888, then `record_deployment_request` + `record_routing_fallback` until #972.)
- Test coverage must include each wired endpoint, not just chat: an e2e that only drives `/v1/chat/completions` will stay green forever while Anthropic-SDK (`/v1/messages`) and Codex (`/v1/responses`) traffic silently misbehaves.
- **Never gate the guardrail chain on having found text to scan.** "Nothing to scan" is not "nothing to decide": a `kind: custom` row's verdict can be independent of the text, so a call site that skips the chain when its collect walk came back empty converts an operator's block rule into a silent allow. The call site always consults the chain; only a guardrail kind may decide it needs text, and every remote kind already self-guards, so consulting costs no provider round-trip. `crates/aisix-proxy/src/guardrail_coverage.rs` enforces this: it parses the routing table out of `build_router`'s own source, so a new route fails the census until it declares a posture, and every surface declared `Enforced` is driven against an unconditional-block guardrail.
- Prefer hoisting the shared logic into one chokepoint (e.g. `resolve_attempt_models`) so the family can't drift again.

(Two recurrences of the same lesson: #471 — a Model-Group dispatch fix landed only on `/v1/messages` while `/v1/responses` and `count_tokens` had the identical gap; then #715 — `least_busy`'s in-flight counter shipped fed by chat.rs only (#684 left messages/responses as an un-filed "follow-up"), so the strategy silently degraded to declaration order for Claude Code / Codex traffic until #716. The EWMA for `least_latency` (#682) wired all three endpoints at once and never had this problem — that's the standard.)
Expand Down
86 changes: 80 additions & 6 deletions crates/aisix-guardrails/src/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,9 +610,14 @@ impl Guardrail for CustomGuardrail {
})
.filter(|m| !m.text.is_empty())
.collect();
if messages.is_empty() {
return GuardrailVerdict::Allow;
}
// No early return when there is nothing to scan. A script is a
// policy over the CALL, not a text matcher: `checkInput` may block
// on `ctx.model`, on a secret-backed lookup, or unconditionally,
// and an operator who wrote such a rule expects it to decide a
// request that happens to carry no text (an argument-less MCP tool
// call, a transcription with no `prompt`). Running a local sandbox
// on an empty context is cheap; the remote kinds keep their own
// empty-text guards, so this costs no provider round-trip.
let segments: Vec<String> = messages.iter().map(|m| m.text.clone()).collect();
let text = segments.join("\n");
let ctx = ScriptContext {
Expand All @@ -635,9 +640,12 @@ impl Guardrail for CustomGuardrail {
return GuardrailVerdict::Allow;
}
let text = resp.guardrail_output_text();
if text.is_empty() {
return GuardrailVerdict::Allow;
}
// Same rule as `check_input`: an empty response still gets a
// verdict. This hook is reached once per response by the call
// sites that use the plain `check_output` fold (jobs, passthrough,
// realtime frames) — the LLM and streaming families route a
// segment moderator through `moderate_output_segments` instead —
// so it adds at most one sandbox run to a response with no text.
let messages = [ScriptMessage {
role: aisix_gateway::Role::Assistant,
text: text.clone(),
Expand Down Expand Up @@ -1318,6 +1326,72 @@ mod tests {
);
}

/// A script's verdict is a decision about the CALL, not a text match,
/// so it must be consulted even when the request carries nothing to
/// scan. This is the bug that let an MCP `tools/call` with
/// `"arguments": {}` — and an audio upload with no `prompt` — execute
/// under an unconditional block rule.
#[tokio::test]
async fn empty_input_still_reaches_the_script() {
let cfg = config("export function checkInput() { return { action: 'block' }; }");
let g = guardrail(&cfg, false);
for req in [
request(""),
ChatFormat::new("gpt-4o", vec![]),
ChatFormat::new("gpt-4o", vec![ChatMessage::user(""), ChatMessage::user("")]),
] {
let verdict = g.check_input(&req).await;
assert!(
matches!(verdict, GuardrailVerdict::Block { .. }),
"an unconditional block must fire with no text to scan: {verdict:?}",
);
}
}

/// The segment hook — the path the MCP and LLM families take for a
/// segment-moderating member — has the same obligation with zero slots.
#[tokio::test]
async fn empty_segment_list_still_reaches_the_script() {
let cfg = config("export function checkInput() { return { action: 'block' }; }");
let outcome = guardrail(&cfg, false).moderate_input_segments(&[]).await;
assert!(
matches!(outcome.verdict, GuardrailVerdict::Block { .. }),
"{:?}",
outcome.verdict,
);
}

/// The other half of the same rule: a script that DOES look at the text
/// must still see an empty context as a clean one, so removing the
/// short-circuit cannot turn "no text" into a spurious block.
#[tokio::test]
async fn empty_input_allows_a_text_matching_script() {
let cfg = config(
"export function checkInput(ctx) {
return ctx.text.includes('bomb') ? { action: 'block' } : { action: 'none' };
}",
);
let verdict = guardrail(&cfg, false).check_input(&request("")).await;
assert!(matches!(verdict, GuardrailVerdict::Allow), "{verdict:?}");
}

#[tokio::test]
async fn empty_output_still_reaches_the_script() {
let cfg = config("export function checkOutput() { return { action: 'block' }; }");
let resp = aisix_gateway::ChatResponse {
id: String::new(),
model: "gpt-4o".into(),
message: aisix_gateway::ChatMessage::assistant(String::new()),
finish_reason: aisix_gateway::FinishReason::Stop,
usage: aisix_gateway::UsageStats::default(),
};
let verdict = guardrail(&cfg, false).check_output(&resp).await;
assert!(
matches!(verdict, GuardrailVerdict::Block { .. }),
"{verdict:?}",
);
}

#[tokio::test]
async fn a_hook_the_module_does_not_export_allows() {
let cfg = config("export function checkOutput() { return { action: 'block' }; }");
Expand Down
108 changes: 107 additions & 1 deletion crates/aisix-proxy/src/a2a.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@
//! control (the key's `allowed_agents`), rate-limit + budget (`quota::enforce`),
//! and a usage event into the shared sink. The upstream credential is held
//! gateway-side and never reaches the caller. Guardrails over A2A message
//! content are a later step.
//! content run on the INPUT hook: the caller's message text is screened
//! before the agent is contacted, on the same env / api-key / team scopes
//! an LLM request resolves. `/a2a` carries no model or MCP-server id, so a
//! guardrail attached to one of those scopes does not apply here (the
//! attachment schema says as much) — an env-wide DLP rule does. The OUTPUT
//! hook is not wired: an A2A answer arrives as artifacts and status updates
//! across a stream that may run for hours, and moderating it needs the
//! streamed-output machinery the LLM surfaces have, not a one-shot check.
//!
//! The request body is forwarded verbatim to the upstream agent, so the caller
//! speaks whichever A2A wire version the agent is pinned to; the gateway does
Expand Down Expand Up @@ -272,6 +279,27 @@ async fn dispatch(
// records which task the caller was asking about.
call.facts.observe_request(&value);

// Input guardrails. Before the reservation, like every other surface
// (#542): a content-policy refusal must not burn an RPM slot. This
// endpoint used to run no chain at all, so an operator's env-wide
// policy was a no-op the moment a caller switched from `/v1/*` to
// `/a2a/*` with the same key.
if let Some(response) = guardrail_block_response(
state,
&snapshot,
&auth,
request_id,
agent,
&call,
&value,
rpc_id.clone(),
trace.as_ref(),
)
.await
{
return response;
}

// Reuse the LLM path's rate-limit + budget gate. The reservation is held
// for the call and released without committing tokens: the counts this
// endpoint reports are the gateway's own reading of the words, not an
Expand Down Expand Up @@ -679,6 +707,84 @@ fn a2a_error_status(err: &A2aError) -> StatusCode {
}
}

/// Screen the caller's A2A message text on the input hook. `Some(response)`
/// = blocked, and the blocked call is recorded as an undispatched usage
/// event the way a quota refusal is.
///
/// The scanned text is `params.message` — the only caller-authored content
/// the protocol carries — extracted with the same walker telemetry uses, but
/// uncapped: the telemetry cap exists to bound a metric, and scanning a
/// prefix would leave the tail of a long message unscreened. The body is
/// already bounded by `request_body_limit_bytes`.
///
/// Operations that carry no message (`tasks/get`, `tasks/cancel`) still run
/// the chain on empty text. That is the point: a guardrail that decides
/// about the CALL rather than about its words must get to decide.
#[allow(clippy::too_many_arguments)]
async fn guardrail_block_response(
state: &ProxyState,
snapshot: &aisix_core::AisixSnapshot,
auth: &AuthenticatedKey,
request_id: &str,
agent: &str,
call: &A2aCall,
value: &serde_json::Value,
rpc_id: Option<serde_json::Value>,
trace: Option<&std::sync::Arc<aisix_obs::RequestTraceBundle>>,
) -> Option<Response> {
let chain = state
.guardrail_index
.resolve(&aisix_guardrails::RequestContext {
passthrough_route_id: "",
model_id: "",
mcp_server_id: "",
api_key_id: &auth.entry.id,
team_id: auth.key().team_id.as_deref(),
});
if chain.is_empty() {
return None;
}
let text = request_text(value, |buf, s| buf.push_str(s));
let chat = aisix_gateway::ChatFormat::new(
A2A_MODEL_LABEL,
vec![aisix_gateway::ChatMessage::user(text)],
);
let (verdict, _hits) = aisix_guardrails::Guardrail::check_input_observed(&chain, &chat).await;
let aisix_guardrails::GuardrailVerdict::Block {
reason,
guardrail_name,
unavailable,
} = verdict
else {
return None;
};
tracing::warn!(
guardrail_hook = "input",
agent = %agent,
reason = %reason,
"guardrail blocked A2A request",
);
let message = crate::error::guardrail_block_message(
"request",
guardrail_name.as_deref(),
unavailable.as_deref(),
);
let response = a2a_error_response(rpc_id, StatusCode::UNPROCESSABLE_ENTITY, &message);
emit_a2a_usage(
state,
snapshot,
auth,
request_id,
agent,
call,
response.status().as_u16(),
Duration::ZERO,
trace,
/* dispatched */ false,
);
Some(response)
}

/// Build a JSON-RPC error envelope for a gateway-side failure, echoing the
/// request id. A2A clients expect a JSON-RPC body, so the failure surfaces as
/// an error object they can handle rather than a bare HTTP error.
Expand Down
9 changes: 8 additions & 1 deletion crates/aisix-proxy/src/audio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,14 @@ async fn multipart_dispatch(
.filter(|s| !s.is_empty())
.map(|s| ChatMessage::user(s.to_string()))
.collect();
if !prompt_messages.is_empty() {
// The chain runs whether or not a `prompt` part was supplied.
// Gating on "we found text" made the check a text matcher's
// privilege: a guardrail that decides about the CALL — a policy
// script, an unconditional block scoped to this model — never
// fired on the ordinary shape of this endpoint (an upload with no `prompt`),
// so an operator's rule silently allowed exactly the requests
// that carry nothing to match.
{
let chat = aisix_gateway::ChatFormat::new(&model_name, prompt_messages);
let (verdict, hits) =
aisix_guardrails::Guardrail::check_input_observed(&resolved_chain, &chat).await;
Expand Down
Loading