Skip to content

Commit ccb24fd

Browse files
fix: harden split execution contracts (vllm-project#235)
## Summary - harden the split execution endpoints added in vllm-project#216 with validated secret types, separate hydrate, context, response, and persist size budgets, stable 401, 409, and 413 error envelopes, and propagated shutdown errors - make duplicate persistence atomic by mapping the database uniqueness violation to a stable `409 response_already_stored` response - strictly validate caller-relayed JSON and SSE response lifecycles, required fields, item identity and type, terminal status, and permanent output ID/index uniqueness before persistence - treat `response.output_item.done` as authoritative so missing deltas cannot silently truncate stored message content, and reject stream item types that cannot be represented safely - document the workload-token contract, deployment trust boundary, limits, and retry behavior The workload token authenticates a trusted coordinator, not a tenant. Per-response tenant ownership remains tracked in vllm-project#107, so these endpoints should stay on an encrypted, policy-restricted service network. ## Test Plan - `cargo fmt --all -- --check` - `CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo clippy -j 2 --workspace --all-targets -- -D warnings` - `CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test --workspace -j 2` - `uv run --with-requirements docs/requirements.txt mkdocs build --strict` The standard PostgreSQL tests that require `TEST_POSTGRES_URL` remained ignored. --------- Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
1 parent fb7a370 commit ccb24fd

21 files changed

Lines changed: 1224 additions & 138 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ All notable changes to Agentic API are documented here.
1818

1919
### Fixed
2020

21+
- Hardened split execution with atomic duplicate persistence, strict relayed-response validation, independent secret
22+
validation, bounded hydrate and persist payloads, stable error envelopes, and graceful shutdown error propagation.
2123
- Forwarded Responses `text` generation settings, including structured output
2224
formats and verbosity, through typed HTTP, WebSocket, and gateway-tool paths.
2325
- Replaced `WebSearchActionSearch::new` and `WebSearchCall::new` with fallible

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ serde_json = { version = "1", features = ["raw_value"] }
3737
thiserror = "2"
3838
tokio = { version = "1", features = ["full"] }
3939
tokio-util = "0.7"
40+
tower = { version = "0.5", features = ["util"] }
4041
tower-http = { version = "0.6", features = ["cors"] }
4142
tracing = "0.1"
4243
tracing-subscriber = { version = "0.3", features = ["env-filter"] }

crates/agentic-llm-d/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,4 @@ agentic-core = { workspace = true, features = [] }
2828
reqwest = { workspace = true, features = ["json"] }
2929
serde_json.workspace = true
3030
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
31+
tower.workspace = true

crates/agentic-llm-d/src/context.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ use agentic_core::types::io::{ResponsesInput, ToolChoice};
1414
use agentic_core::types::request_response::RequestPayload;
1515
use agentic_core::types::tools::ResponsesTool;
1616

17+
use crate::SigningKey;
18+
1719
/// What `hydrate` returns. Raw JSON: the caller forwards it uninterpreted.
1820
#[derive(Debug, Clone, Serialize, Deserialize)]
1921
pub struct Hydration {
@@ -95,7 +97,7 @@ struct SealedClaims {
9597
///
9698
/// # Errors
9799
/// [`ExecutorError::InvalidRequest`] if the token cannot be produced.
98-
pub fn seal(context: SplitContext, key: &[u8]) -> ExecutorResult<String> {
100+
pub fn seal(context: SplitContext, key: &SigningKey) -> ExecutorResult<String> {
99101
let expires = SystemTime::now()
100102
.checked_add(CONTEXT_TTL)
101103
.and_then(|at| at.duration_since(UNIX_EPOCH).ok())
@@ -105,20 +107,24 @@ pub fn seal(context: SplitContext, key: &[u8]) -> ExecutorResult<String> {
105107
aud: AUDIENCE.to_owned(),
106108
ctx: context,
107109
};
108-
encode(&Header::new(Algorithm::HS256), &claims, &EncodingKey::from_secret(key))
109-
.map_err(|error| ExecutorError::InvalidRequest(format!("cannot seal context: {error}")))
110+
encode(
111+
&Header::new(Algorithm::HS256),
112+
&claims,
113+
&EncodingKey::from_secret(key.as_bytes()),
114+
)
115+
.map_err(|error| ExecutorError::InvalidRequest(format!("cannot seal context: {error}")))
110116
}
111117

112118
/// Opens a sealed context, rejecting one that was tampered with or has expired.
113119
///
114120
/// # Errors
115121
/// [`ExecutorError::InvalidRequest`] for a bad signature, a wrong audience, or
116122
/// a context past its expiry.
117-
pub fn unseal(token: &str, key: &[u8]) -> ExecutorResult<SplitContext> {
123+
pub fn unseal(token: &str, key: &SigningKey) -> ExecutorResult<SplitContext> {
118124
let mut validation = Validation::new(Algorithm::HS256);
119125
validation.set_audience(&[AUDIENCE]);
120126
validation.set_required_spec_claims(&["exp", "aud"]);
121-
decode::<SealedClaims>(token, &DecodingKey::from_secret(key), &validation)
127+
decode::<SealedClaims>(token, &DecodingKey::from_secret(key.as_bytes()), &validation)
122128
.map(|data| data.claims.ctx)
123129
.map_err(|error| ExecutorError::InvalidRequest(format!("context rejected: {error}")))
124130
}

crates/agentic-llm-d/src/handler.rs

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ use axum::extract::{Request, State};
99
use axum::http::StatusCode;
1010
use axum::middleware::Next;
1111
use axum::response::{IntoResponse, Response};
12-
use serde::Deserialize;
1312
use serde::de::DeserializeOwned;
13+
use serde::{Deserialize, Serialize};
1414
use serde_json::value::RawValue;
1515
use tracing::warn;
1616

@@ -24,7 +24,10 @@ use agentic_core::types::request_response::RequestPayload;
2424
use crate::BackendState;
2525
use crate::context::{Hydration, ensure_splittable, seal, unseal};
2626

27-
const MAX_BODY_SIZE: usize = 10 * 1024 * 1024;
27+
const MAX_HYDRATE_BODY_SIZE: usize = 2 * 1024 * 1024;
28+
const MAX_PERSIST_BODY_SIZE: usize = 16 * 1024 * 1024;
29+
const MAX_CONTEXT_SIZE: usize = 6 * 1024 * 1024;
30+
const MAX_UPSTREAM_BODY_SIZE: usize = 4 * 1024 * 1024;
2831
/// The calling workload's shared secret.
2932
pub const WORKLOAD_TOKEN_HEADER: &str = "x-agentic-workload-token";
3033
/// Readiness means storage answers - llm-d owns the model fleet.
@@ -47,10 +50,12 @@ pub async fn require_token(State(state): State<BackendState>, request: Request,
4750
.get(WORKLOAD_TOKEN_HEADER)
4851
.and_then(|value| value.to_str().ok());
4952
match presented {
50-
Some(token) if token_matches(token, &state.api_token) => next.run(request).await,
51-
_ => json(
53+
Some(token) if token_matches(token, state.secrets.workload_token()) => next.run(request).await,
54+
_ => api_error(
5255
StatusCode::UNAUTHORIZED,
53-
br#"{"error":{"type":"invalid_request_error","message":"missing or invalid bearer token"}}"#.to_vec(),
56+
"authentication_error",
57+
"invalid_workload_token",
58+
"missing or invalid workload token",
5459
),
5560
}
5661
}
@@ -78,7 +83,7 @@ pub async fn ready(State(state): State<BackendState>) -> StatusCode {
7883
}
7984

8085
pub async fn hydrate(State(state): State<BackendState>, req: Request) -> Response {
81-
let payload: RequestPayload = match read_json(req.into_body()).await {
86+
let payload: RequestPayload = match read_json(req.into_body(), MAX_HYDRATE_BODY_SIZE).await {
8287
Ok(payload) => payload,
8388
Err(response) => return response,
8489
};
@@ -101,12 +106,17 @@ async fn build_hydration(
101106
ensure_splittable(&ctx.enriched_request)?;
102107
let stream = ctx.original_request.stream;
103108
let request = RawValue::from_string(upstream_request(&ctx, stream)?).map_err(ExecutorError::JsonError)?;
104-
let context = seal(ctx.into(), &state.signing_key)?;
109+
let context = seal(ctx.into(), state.secrets.signing_key())?;
110+
if context.len() > MAX_CONTEXT_SIZE {
111+
return Err(ExecutorError::PayloadTooLarge(
112+
"hydrated context exceeds the split-execution size budget".to_owned(),
113+
));
114+
}
105115
Ok(Hydration { request, context })
106116
}
107117

108118
pub async fn persist(State(state): State<BackendState>, req: Request) -> Response {
109-
let PersistRequest { context, response, sse } = match read_json(req.into_body()).await {
119+
let PersistRequest { context, response, sse } = match read_json(req.into_body(), MAX_PERSIST_BODY_SIZE).await {
110120
Ok(request) => request,
111121
Err(response) => return response,
112122
};
@@ -119,7 +129,20 @@ pub async fn persist(State(state): State<BackendState>, req: Request) -> Respons
119129
return error_response(ExecutorError::InvalidRequest(message));
120130
}
121131
};
122-
let context = match unseal(&context, &state.signing_key) {
132+
if context.len() > MAX_CONTEXT_SIZE {
133+
return error_response(ExecutorError::PayloadTooLarge(
134+
"sealed context exceeds the split-execution size budget".to_owned(),
135+
));
136+
}
137+
let upstream_size = match upstream {
138+
UpstreamBody::Json(body) | UpstreamBody::Sse(body) => body.len(),
139+
};
140+
if upstream_size > MAX_UPSTREAM_BODY_SIZE {
141+
return error_response(ExecutorError::PayloadTooLarge(
142+
"upstream response exceeds the split-execution size budget".to_owned(),
143+
));
144+
}
145+
let context = match unseal(&context, state.secrets.signing_key()) {
123146
Ok(context) => context,
124147
Err(error) => return error_response(error),
125148
};
@@ -142,14 +165,38 @@ fn error_response(error: ExecutorError) -> Response {
142165
}
143166

144167
#[allow(clippy::result_large_err)] // an axum `Response` is the idiomatic error here
145-
async fn read_json<T: DeserializeOwned>(body: Body) -> Result<T, Response> {
146-
let too_large = br#"{"error":{"type":"invalid_request_error","message":"request body too large"}}"#;
147-
let bytes = axum::body::to_bytes(body, MAX_BODY_SIZE)
168+
async fn read_json<T: DeserializeOwned>(body: Body, limit: usize) -> Result<T, Response> {
169+
let bytes = axum::body::to_bytes(body, limit)
148170
.await
149-
.map_err(|_| json(StatusCode::PAYLOAD_TOO_LARGE, too_large.to_vec()))?;
171+
.map_err(|_| error_response(ExecutorError::PayloadTooLarge("request body too large".to_owned())))?;
150172
serde_json::from_slice(&bytes).map_err(|error| error_response(ExecutorError::from(error)))
151173
}
152174

175+
#[derive(Serialize)]
176+
struct ApiErrorEnvelope<'a> {
177+
error: ApiErrorBody<'a>,
178+
}
179+
180+
#[derive(Serialize)]
181+
struct ApiErrorBody<'a> {
182+
message: &'a str,
183+
#[serde(rename = "type")]
184+
error_type: &'a str,
185+
code: &'a str,
186+
}
187+
188+
fn api_error(status: StatusCode, error_type: &str, code: &str, message: &str) -> Response {
189+
let body = serde_json::to_vec(&ApiErrorEnvelope {
190+
error: ApiErrorBody {
191+
message,
192+
error_type,
193+
code,
194+
},
195+
})
196+
.expect("static API error serializes");
197+
json(status, body)
198+
}
199+
153200
fn json(status: StatusCode, body: Vec<u8>) -> Response {
154201
Response::builder()
155202
.status(status)

crates/agentic-llm-d/src/lib.rs

Lines changed: 110 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,120 @@ use axum::{Router, middleware};
1212

1313
use agentic_core::executor::ExecutionContext;
1414

15+
const MIN_SECRET_LEN: usize = 32;
16+
17+
/// Invalid backend authentication or signing material.
18+
#[derive(Debug, thiserror::Error)]
19+
pub enum SecretError {
20+
#[error("{name} must be at least {MIN_SECRET_LEN} bytes of independently generated randomness")]
21+
TooShort { name: &'static str },
22+
#[error("the signing key and workload token must be generated independently")]
23+
Reused,
24+
}
25+
26+
/// Validated secrets shared by the backend handlers.
27+
#[derive(Clone)]
28+
pub struct BackendSecrets {
29+
signing_key: SigningKey,
30+
workload_token: Arc<str>,
31+
}
32+
33+
/// Validated key used to sign and verify split-execution contexts.
34+
#[derive(Clone)]
35+
pub struct SigningKey(Arc<[u8]>);
36+
37+
impl SigningKey {
38+
/// Validates a context-signing key.
39+
///
40+
/// # Errors
41+
/// Returns [`SecretError`] when the key is shorter than 32 non-whitespace
42+
/// bytes.
43+
pub fn new(value: Vec<u8>) -> Result<Self, SecretError> {
44+
if non_whitespace_ascii_len(&value) < MIN_SECRET_LEN {
45+
return Err(SecretError::TooShort { name: "signing key" });
46+
}
47+
Ok(Self(value.into()))
48+
}
49+
50+
pub(crate) fn as_bytes(&self) -> &[u8] {
51+
&self.0
52+
}
53+
}
54+
55+
impl BackendSecrets {
56+
/// Validates and constructs the secrets required by the split routes.
57+
///
58+
/// # Errors
59+
/// Returns [`SecretError`] when either value is too short or both values are
60+
/// identical.
61+
pub fn new(signing_key: Vec<u8>, workload_token: String) -> Result<Self, SecretError> {
62+
let signing_key = SigningKey::new(signing_key)?;
63+
if non_whitespace_ascii_len(workload_token.as_bytes()) < MIN_SECRET_LEN {
64+
return Err(SecretError::TooShort { name: "workload token" });
65+
}
66+
if signing_key.as_bytes() == workload_token.as_bytes() {
67+
return Err(SecretError::Reused);
68+
}
69+
Ok(Self {
70+
signing_key,
71+
workload_token: workload_token.into(),
72+
})
73+
}
74+
75+
pub(crate) fn signing_key(&self) -> &SigningKey {
76+
&self.signing_key
77+
}
78+
79+
pub(crate) fn workload_token(&self) -> &str {
80+
&self.workload_token
81+
}
82+
}
83+
84+
fn non_whitespace_ascii_len(value: &[u8]) -> usize {
85+
value.iter().filter(|byte| !byte.is_ascii_whitespace()).count()
86+
}
87+
88+
#[cfg(test)]
89+
mod tests {
90+
use super::*;
91+
92+
#[test]
93+
fn signing_keys_and_workload_tokens_must_be_long_and_independent() {
94+
assert!(matches!(
95+
BackendSecrets::new(Vec::new(), "b".repeat(MIN_SECRET_LEN)),
96+
Err(SecretError::TooShort { name: "signing key" })
97+
));
98+
assert!(matches!(
99+
BackendSecrets::new(vec![b'a'; MIN_SECRET_LEN], String::new()),
100+
Err(SecretError::TooShort { name: "workload token" })
101+
));
102+
assert!(matches!(
103+
BackendSecrets::new(vec![b'a'; MIN_SECRET_LEN], "a".repeat(MIN_SECRET_LEN)),
104+
Err(SecretError::Reused)
105+
));
106+
let sparse = format!("a{}b", " ".repeat(MIN_SECRET_LEN - 2));
107+
assert!(matches!(
108+
SigningKey::new(sparse.as_bytes().to_vec()),
109+
Err(SecretError::TooShort { name: "signing key" })
110+
));
111+
assert!(matches!(
112+
BackendSecrets::new(vec![b'a'; MIN_SECRET_LEN], sparse),
113+
Err(SecretError::TooShort { name: "workload token" })
114+
));
115+
assert!(matches!(
116+
SigningKey::new(vec![b'a'; MIN_SECRET_LEN - 1]),
117+
Err(SecretError::TooShort { name: "signing key" })
118+
));
119+
BackendSecrets::new(vec![b'a'; MIN_SECRET_LEN], "b".repeat(MIN_SECRET_LEN)).expect("independent secrets");
120+
}
121+
}
122+
15123
/// All the endpoints need — far less than the gateway's `AppState`.
16124
#[derive(Clone)]
17125
pub struct BackendState {
18126
pub exec_ctx: Arc<ExecutionContext>,
19-
/// Seals the context between hydrate and persist.
20-
pub signing_key: Arc<Vec<u8>>,
21-
/// Shared secret every split-route caller must present.
22-
pub api_token: Arc<String>,
127+
/// Validated signing and workload-authentication material.
128+
pub secrets: BackendSecrets,
23129
}
24130

25131
/// The whole surface: two split-execution endpoints and two probes.

crates/agentic-llm-d/src/main.rs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@ struct Cli {
3131
api_token: String,
3232
}
3333

34-
/// Long enough that a guessed value is not worth trying.
35-
const MIN_SECRET_LEN: usize = 32;
36-
3734
#[tokio::main]
3835
async fn main() -> Result<(), runner::Error> {
3936
tracing_subscriber::fmt::init();
@@ -52,25 +49,6 @@ async fn main() -> Result<(), runner::Error> {
5249
tools: ToolRuntimeConfig::default(),
5350
};
5451

55-
// Checked before binding: a short secret is worse than none, it looks configured.
56-
for (name, value) in [
57-
("AGENTIC_LLM_D_SIGNING_KEY", &cli.signing_key),
58-
("AGENTIC_LLM_D_API_TOKEN", &cli.api_token),
59-
] {
60-
if value.trim().len() < MIN_SECRET_LEN {
61-
eprintln!("{name} must be at least {MIN_SECRET_LEN} characters of independently generated randomness");
62-
std::process::exit(2);
63-
}
64-
}
65-
66-
// Independence is the property we can actually check. The token travels in a
67-
// header on every request, so reusing it as the signing key would let anyone
68-
// who sees one forge the other.
69-
if cli.signing_key == cli.api_token {
70-
eprintln!("AGENTIC_LLM_D_SIGNING_KEY and AGENTIC_LLM_D_API_TOKEN must be generated independently");
71-
std::process::exit(2);
72-
}
73-
7452
let shutdown = CancellationToken::new();
7553
let on_signal = shutdown.clone();
7654
tokio::spawn(async move {

0 commit comments

Comments
 (0)