From 8626adac2caf2b8fa22752494308f59f3ceb09fc Mon Sep 17 00:00:00 2001 From: Patrick Lee Scott Date: Wed, 10 Jun 2026 23:50:06 -0500 Subject: [PATCH] fix: trust gRPC metadata over payload session vars; mask internal errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gRPC transport let the request payload `session_variables` override transport metadata when building the `Session`. Behind a trusted gateway that injects authenticated identity headers as gRPC metadata, a client could spoof identity by putting `x-hasura-user-id` / `x-hasura-role` in the request body (e.g. claim role `admin` or impersonate another user) — the payload silently won. This is an identity-spoofing hole. Trust model (now documented loudly on `Session`, the HTTP/gRPC entry points, and the README "Security / Trust Boundary" section): the framework does NOT authenticate. A trusted proxy must strip client-supplied `x-hasura-*` headers and inject only authenticated ones. Transport metadata/headers are trusted; the request payload is not. Changes: - gRPC `build_session`: apply payload vars first, then let trusted metadata overwrite colliding keys. Metadata now wins. Payload-only keys still pass through (preserves the Hasura-action path where verified claims arrive in the payload with no metadata injected). - gRPC errors: route the response body through a shared `HandlerError::client_facing_message()`, masking internal (5xx) detail to "Internal server error" and logging the original server-side. Previously gRPC returned raw `e.to_string()`, which could leak SQL/driver detail — HTTP already masked. The masking policy now lives in one place (error.rs) and both transports reuse it (no duplicated logic). - Docs: rustdoc trust-boundary notes on `Session`, `session_from_headers`, and `build_session`; README HTTP/gRPC transport notes plus a dedicated "Security / Trust Boundary" section. - Tests: gRPC metadata-wins-over-payload (anti-spoof) and payload-applies-when-metadata-absent; HTTP client-supplied identity header trusted verbatim (documents why the proxy is required). Implements [[tasks/grpc-session-metadata-precedence]] Also covers the gRPC error-masking item from [[tasks/transport-ingress-security-hardening]] Co-Authored-By: Claude Fable 5 --- README.md | 33 ++++++++++++++++-- src/microsvc/error.rs | 18 ++++++++++ src/microsvc/grpc.rs | 50 ++++++++++++++++++++------- src/microsvc/http.rs | 28 ++++++--------- src/microsvc/session.rs | 33 ++++++++++++++++++ tests/microsvc/transport_grpc.rs | 59 ++++++++++++++++++++++++++++++++ tests/microsvc/transport_http.rs | 26 ++++++++++++++ 7 files changed, 215 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 3f75f5c..7a398c4 100644 --- a/README.md +++ b/README.md @@ -1001,7 +1001,7 @@ Event projection handlers use `EVENT` / `EVENTS` and `event handlers::...` in th ### HTTP Transport (requires `http` feature) -The `http` feature adds an axum-based HTTP transport. Every registered command becomes a `POST /:command` endpoint. Request headers flow into the `Session`: +The `http` feature adds an axum-based HTTP transport. Every registered command becomes a `POST /:command` endpoint. Request headers flow into the `Session` verbatim — including `x-hasura-*` identity headers, which the framework does **not** authenticate. Deploy behind a trusted proxy that strips client-supplied identity headers and injects authenticated ones (see [Security / Trust Boundary](#security--trust-boundary)). ```rust use std::sync::Arc; @@ -1047,7 +1047,7 @@ microsvc::serve_grpc(service, "[::1]:50051").await?; | `Dispatch` | `GrpcRequest` | `GrpcResponse` | Dispatch a command. `input` = JSON string, `session_variables` = metadata map. | | `Health` | `HealthRequest` | `HealthResponse` | Health check. | -Session handling mirrors HTTP — gRPC metadata headers are merged with payload `session_variables` (payload takes precedence). Errors are returned inside `GrpcResponse.status` (HTTP-style status codes), keeping client behavior identical across transports. +Session handling mirrors HTTP — gRPC metadata headers are merged with payload `session_variables`. **Transport metadata (trusted, proxy-injected) takes precedence over the client-controlled payload**, so a client cannot spoof identity via the request body. See [Security / Trust Boundary](#security--trust-boundary) below. Errors are returned inside `GrpcResponse.status` (HTTP-style status codes) with internal (5xx) error detail masked to a generic message, keeping client behavior identical across transports. ### Bus Transport @@ -1074,6 +1074,35 @@ methods directly. See [Service Bus](#service-bus) above. | `Repository` | 500 | | `Other` | 500 | +Internal (5xx) errors are **masked** before being returned to clients — the +response body carries a generic `"Internal server error"` so SQL text, driver +detail, or internal paths never leak. The original error is logged +server-side. Client-fault (4xx) errors keep their descriptive message. This +applies identically to the HTTP and gRPC transports. + +### Security / Trust Boundary + +**This framework does NOT authenticate requests.** The `Session` is built from +whatever the transport provides — HTTP request headers, gRPC metadata, and (for +gRPC) the request payload's `session_variables`. Identity values such as +`x-hasura-user-id` and `x-hasura-role` are trusted at face value by handlers. + +You **must** deploy `microsvc` behind a **trusted proxy / API gateway** (e.g. +Hasura, or an authenticating ingress) that: + +- **Strips** any client-supplied `x-hasura-*` headers/metadata on the way in, and +- **Injects** only identity claims it has authenticated. + +Without that proxy, any caller can set `x-hasura-user-id` / `x-hasura-role` and +assume any identity or role. + +**Source precedence:** when identity arrives in more than one place, the trusted +transport channel wins over the client-controlled payload. For gRPC, transport +**metadata overrides** payload `session_variables` — a client cannot override a +proxy-injected `x-hasura-user-id` via the request body. For HTTP, request headers +populate the session and the proxy is responsible for ensuring they are +authenticated. Never trust the request body for identity. + ## Read Models Read models are query-optimized relational projections derived from aggregates, event records, or published messages. They are written as declared relational rows using table metadata from `#[derive(ReadModel)]`. Use JSON/JSONB columns for whole-view or semistructured fields. diff --git a/src/microsvc/error.rs b/src/microsvc/error.rs index 410a418..9599e92 100644 --- a/src/microsvc/error.rs +++ b/src/microsvc/error.rs @@ -93,6 +93,24 @@ impl HandlerError { } } + /// Message safe to return to an untrusted client across a transport. + /// + /// Server-internal failures (status 5xx — repository/driver/other errors) + /// are masked to a generic string so SQL text, driver detail, or internal + /// paths never leak to callers. Client-fault errors (4xx — unknown command, + /// decode, rejection, auth, guard) keep their descriptive message because + /// the caller caused them and the detail helps them correct the request. + /// + /// Both the HTTP and gRPC transports route error bodies through this so the + /// masking policy lives in exactly one place. + pub fn client_facing_message(&self) -> String { + if self.status_code() >= 500 { + "Internal server error".to_string() + } else { + self.to_string() + } + } + /// Classify this error for transport retry purposes (retryable vs permanent). /// /// Transient failures (repository errors, not-found, otherwise-unclassified) diff --git a/src/microsvc/grpc.rs b/src/microsvc/grpc.rs index 957c0e0..44a65ab 100644 --- a/src/microsvc/grpc.rs +++ b/src/microsvc/grpc.rs @@ -160,7 +160,10 @@ impl CommandService for GrpcHandler { } }; - // Build session: start with metadata headers, then overlay payload values + // Build session: payload values first, then transport metadata wins. + // Transport metadata is trusted (injected by a trusted proxy); the + // payload is client-controlled and MUST NOT override it. See + // [`build_session`] and the `Session` trust-boundary docs. let session = build_session(&metadata, req.session_variables); match self.service.dispatch(&req.command, input, session).await { @@ -168,10 +171,19 @@ impl CommandService for GrpcHandler { status: 200, body: value.to_string(), })), - Err(e) => Ok(Response::new(GrpcResponse { - status: e.status_code() as u32, - body: json!({ "error": e.to_string() }).to_string(), - })), + Err(e) => { + // Mirror the HTTP transport: never leak internal error detail + // (SQL/driver text) to clients. Server-internal failures are + // masked; client-fault errors keep their descriptive message. + let status = e.status_code(); + if status >= 500 { + eprintln!("microsvc command `{}` failed: {e}", req.command); + } + Ok(Response::new(GrpcResponse { + status: status as u32, + body: json!({ "error": e.client_facing_message() }).to_string(), + })) + } } } @@ -196,15 +208,32 @@ impl CommandService for GrpcHandler { /// Build a session from gRPC metadata and payload session variables. /// -/// 1. Start with gRPC metadata headers (lowercased key → value) -/// 2. Overlay payload `session_variables` (payload takes precedence) +/// **Trust boundary (security-critical):** transport metadata is TRUSTED — +/// it is injected by a trusted proxy/gateway that authenticates the caller +/// and strips any client-supplied `x-hasura-*` headers. The request payload +/// `session_variables` are CLIENT-CONTROLLED and therefore UNTRUSTED. +/// +/// Precedence: **metadata wins.** Payload values are applied first, then +/// metadata overwrites any colliding key. This prevents a client from +/// spoofing identity (e.g. `x-hasura-user-id` / `x-hasura-role`) via the +/// request body when behind a trusted gateway. See the [`Session`] docs for +/// the framework-wide trust model. +/// +/// Payload-only keys (not present in metadata) still pass through, preserving +/// the Hasura action use case where Hasura forwards verified claims in the +/// payload and no transport metadata is injected. fn build_session( metadata: &tonic::metadata::MetadataMap, payload_vars: HashMap, ) -> Session { let mut vars = HashMap::new(); - // Metadata headers + // Untrusted payload first. + for (k, v) in payload_vars { + vars.insert(k, v); + } + + // Trusted metadata wins — overwrites any payload-supplied key. for kv in metadata.iter() { if let tonic::metadata::KeyAndValueRef::Ascii(key, value) = kv { if let Ok(v) = value.to_str() { @@ -213,11 +242,6 @@ fn build_session( } } - // Payload overrides - for (k, v) in payload_vars { - vars.insert(k, v); - } - Session::from_map(vars) } diff --git a/src/microsvc/http.rs b/src/microsvc/http.rs index ed8778b..d62e420 100644 --- a/src/microsvc/http.rs +++ b/src/microsvc/http.rs @@ -80,7 +80,7 @@ async fn command_handler( if status.is_server_error() { eprintln!("microsvc command `{command}` failed: {err}"); } - let body = json!({ "error": error_message_for_response(&err) }); + let body = json!({ "error": err.client_facing_message() }); (status, Json(body)).into_response() } } @@ -96,17 +96,14 @@ fn status_for_error(error: &HandlerError) -> StatusCode { } } -fn error_message_for_response(error: &HandlerError) -> String { - if status_for_error(error).is_server_error() { - "Internal server error".to_string() - } else { - error.to_string() - } -} - /// Extract session variables from HTTP headers. /// -/// All headers are lowercased and included as session variables. +/// **Trust boundary (security-critical):** every request header is copied +/// verbatim into the [`Session`] — including `x-hasura-user-id` and +/// `x-hasura-role`. The framework does NOT authenticate. A trusted proxy in +/// front of this service MUST strip any client-supplied `x-hasura-*` headers +/// and inject only authenticated ones. Without that proxy, any client can set +/// these headers and assume any identity/role. See the [`Session`] docs. fn session_from_headers(headers: &HeaderMap) -> Session { let mut vars = std::collections::HashMap::new(); for (name, value) in headers.iter() { @@ -168,24 +165,21 @@ mod tests { } #[test] - fn error_message_for_response_preserves_client_errors() { + fn client_facing_message_preserves_client_errors() { let error = HandlerError::Rejected("invalid command".into()); - assert_eq!( - error_message_for_response(&error), - "rejected: invalid command" - ); + assert_eq!(error.client_facing_message(), "rejected: invalid command"); } #[test] - fn error_message_for_response_hides_server_errors() { + fn client_facing_message_hides_server_errors() { let errors = [ HandlerError::Repository(RepositoryError::Model("store failed".into())), HandlerError::Other(Box::new(std::io::Error::other("handler failed"))), ]; for error in errors { - assert_eq!(error_message_for_response(&error), "Internal server error"); + assert_eq!(error.client_facing_message(), "Internal server error"); } } } diff --git a/src/microsvc/session.rs b/src/microsvc/session.rs index 771a776..48d55c1 100644 --- a/src/microsvc/session.rs +++ b/src/microsvc/session.rs @@ -13,6 +13,39 @@ use std::collections::HashMap; /// "x-hasura-role": "customer" /// } /// ``` +/// +/// # Trust boundary (security-critical) +/// +/// **This framework does NOT authenticate.** A `Session` is built from +/// whatever the transport hands it — HTTP request headers, gRPC metadata, or +/// the request payload's `session_variables`. None of that is verified here. +/// Identity values such as `x-hasura-user-id` and `x-hasura-role` are +/// trusted at face value by [`Session::user_id`], [`Session::role`], and any +/// handler that reads them. +/// +/// You MUST deploy this behind a **trusted proxy / API gateway** (e.g. +/// Hasura, or an authenticating ingress) that: +/// +/// - **Strips** any client-supplied `x-hasura-*` headers/metadata on inbound +/// requests, and +/// - **Injects** only authenticated identity claims it has verified. +/// +/// Without that proxy, any caller can set `x-hasura-user-id` / +/// `x-hasura-role` and assume any identity or role. +/// +/// ## Source precedence +/// +/// When a request carries identity in more than one place, the **trusted +/// transport channel wins over the client-controlled payload**: +/// +/// - **gRPC:** transport metadata (trusted, proxy-injected) overrides payload +/// `session_variables` (client-controlled). See `grpc::build_session`. +/// - **HTTP:** request headers populate the session; the proxy is responsible +/// for ensuring those headers are authenticated. See +/// `http::session_from_headers`. +/// +/// Treat metadata/headers as trusted only insofar as your proxy guarantees +/// it; treat the request body as never trustworthy for identity. #[derive(Debug, Clone, Default)] pub struct Session { variables: HashMap, diff --git a/tests/microsvc/transport_grpc.rs b/tests/microsvc/transport_grpc.rs index 986ee2f..04719ee 100644 --- a/tests/microsvc/transport_grpc.rs +++ b/tests/microsvc/transport_grpc.rs @@ -178,6 +178,65 @@ async fn metadata_flows_to_session() { assert_eq!(body, json!({ "user_id": "user-42" })); } +/// Security: a client must not be able to spoof identity via the payload. +/// Transport metadata is injected by a trusted proxy and MUST win over the +/// client-controlled `session_variables`. Here the payload claims to be +/// `attacker`, but the proxy-injected metadata says `trusted-user` — the +/// handler must see `trusted-user`. +#[tokio::test] +async fn grpc_payload_session_variables_do_not_override_metadata() { + let service = counter_service(); + let mut client = start_server(service).await; + + let mut payload_vars = std::collections::HashMap::new(); + payload_vars.insert("x-hasura-user-id".to_string(), "attacker".to_string()); + + let mut request = tonic::Request::new(GrpcRequest { + command: "session.identify".into(), + input: json!({}).to_string(), + session_variables: payload_vars, + }); + request + .metadata_mut() + .insert("x-hasura-user-id", "trusted-user".parse().unwrap()); + + let resp = client.dispatch(request).await.unwrap().into_inner(); + + assert_eq!(resp.status, 200); + let body: serde_json::Value = serde_json::from_str(&resp.body).unwrap(); + // Trusted metadata wins; the payload-supplied identity is ignored. + assert_eq!(body, json!({ "user_id": "trusted-user" })); +} + +/// Payload-only session variables (no colliding metadata) still flow through, +/// preserving the Hasura-action use case where verified claims arrive in the +/// payload and no transport metadata is injected. +#[tokio::test] +async fn grpc_payload_session_variables_apply_when_metadata_absent() { + let service = counter_service(); + let mut client = start_server(service).await; + + let mut payload_vars = std::collections::HashMap::new(); + payload_vars.insert( + "x-hasura-user-id".to_string(), + "user-from-payload".to_string(), + ); + + let resp = client + .dispatch(GrpcRequest { + command: "session.identify".into(), + input: json!({}).to_string(), + session_variables: payload_vars, + }) + .await + .unwrap() + .into_inner(); + + assert_eq!(resp.status, 200); + let body: serde_json::Value = serde_json::from_str(&resp.body).unwrap(); + assert_eq!(body, json!({ "user_id": "user-from-payload" })); +} + #[tokio::test] async fn missing_session_returns_401_status() { let service = counter_service(); diff --git a/tests/microsvc/transport_http.rs b/tests/microsvc/transport_http.rs index 9cedfac..f5e1bd2 100644 --- a/tests/microsvc/transport_http.rs +++ b/tests/microsvc/transport_http.rs @@ -156,6 +156,32 @@ async fn headers_flow_to_session() { assert_eq!(body, json!({ "user_id": "user-42" })); } +/// Documents the HTTP trust boundary: request headers are copied into the +/// `Session` verbatim and trusted at face value — the framework does NOT +/// authenticate. A client-supplied `x-hasura-user-id` is reflected straight +/// through, which is precisely why a trusted proxy must strip/inject these +/// headers in production. See `session_from_headers` and the `Session` docs. +#[tokio::test] +async fn client_supplied_identity_header_is_trusted_verbatim() { + let service = counter_service(); + let base = start_server(service).await; + let client = reqwest::Client::new(); + + let resp = client + .post(format!("{base}/session.identify")) + // No proxy in front: the client sets its own identity. The framework + // trusts it as-is. In production a trusted proxy must overwrite this. + .header("x-hasura-user-id", "client-claimed-id") + .json(&json!({})) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + + let body: serde_json::Value = resp.json().await.unwrap(); + assert_eq!(body, json!({ "user_id": "client-claimed-id" })); +} + #[tokio::test] async fn missing_session_returns_401() { let service = counter_service();