From a347ccc2a2be29b0fa4c016b5b153ad9e61b79dc Mon Sep 17 00:00:00 2001 From: willamhou Date: Wed, 24 Jun 2026 09:49:35 +0800 Subject: [PATCH] fix(security): redact + status-map server errors; extract core::redact (H5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-D (+ PR-0) of the security-hardening plan. - PR-0: move the generic credential/secret redaction (Redactor, secret_values_of, value/key/scalar masking) out of the audit feature into an always-compiled `core::redact`; `audit::redact` re-exports it and keeps the audit-only AuditMeta. - H5: /v1/exec no longer returns raw upstream error text. error_response maps each AdapterError to a proper HTTP status (Auth→401, RateLimited→429 +Retry-After, NotFound/SiteNotFound→404, Unsupported→400, AuditUnavailable→503, Upstream→502, Other→500) and returns a structured {error:{kind,message}} body with the message redacted using this request's credential values. The unknown-site path uses the same structured shape. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/core/src/audit/redact.rs | 306 +------------------------------- crates/core/src/lib.rs | 1 + crates/core/src/redact.rs | 301 +++++++++++++++++++++++++++++++ crates/server/src/handlers.rs | 150 ++++++++++++---- 4 files changed, 426 insertions(+), 332 deletions(-) create mode 100644 crates/core/src/redact.rs diff --git a/crates/core/src/audit/redact.rs b/crates/core/src/audit/redact.rs index 1162d97..808b245 100644 --- a/crates/core/src/audit/redact.rs +++ b/crates/core/src/audit/redact.rs @@ -1,34 +1,12 @@ -//! Redaction of request/response payloads before they enter an audit receipt. +//! Audit-specific redaction metadata. //! -//! Two complementary defences (C2): -//! 1. **Key-name redaction** — values under sensitive keys (`token`, `password`, -//! `*_token`, …) are masked regardless of content. -//! 2. **Secret-value redaction** — the actual credential strings (pulled from -//! [`crate::types::Credentials`]) are masked wherever they appear, under *any* -//! key or as a substring of a larger string. This is what makes the leak guard -//! hold even when an upstream echoes a token back in its response. +//! The generic value/key/secret redaction now lives in the always-compiled +//! [`crate::redact`] module (so non-audit code can use it too); this module +//! re-exports it and adds the audit-only `AuditMeta`. -use serde_json::{Map, Value}; +use serde_json::Value; -use crate::types::{CredentialData, Credentials}; - -/// The placeholder written in place of redacted content. -pub const REDACTED: &str = "[REDACTED]"; - -/// Keys whose values are always masked (compared case-insensitively). -const DEFAULT_DENY_EXACT: &[&str] = &[ - "token", - "password", - "secret", - "authorization", - "api_key", - "acs_token", - "cookie", - "cookies", -]; - -/// Key suffixes whose values are always masked (e.g. `access_token`, `refresh_token`). -const DEFAULT_DENY_SUFFIX: &[&str] = &["_token"]; +pub use crate::redact::{secret_values_of, Redactor, REDACTED}; /// Non-secret descriptor + opaque credential reference attached to the signed /// request envelope under `_relais_audit`. `t0`/`t1` are the **true** request @@ -54,275 +32,3 @@ impl AuditMeta { }) } } - -/// Redacts JSON values by key name and by secret value. -#[derive(Debug, Clone)] -pub struct Redactor { - deny_exact: Vec, - deny_suffix: Vec, -} - -impl Default for Redactor { - fn default() -> Self { - Self { - deny_exact: DEFAULT_DENY_EXACT.iter().map(|s| s.to_string()).collect(), - deny_suffix: DEFAULT_DENY_SUFFIX.iter().map(|s| s.to_string()).collect(), - } - } -} - -impl Redactor { - pub fn new() -> Self { - Self::default() - } - - fn key_is_sensitive(&self, key: &str) -> bool { - let k = key.to_ascii_lowercase(); - self.deny_exact.iter().any(|d| d == &k) || self.deny_suffix.iter().any(|s| k.ends_with(s)) - } - - /// Returns `v` with sensitive keys masked and any occurrence of a `secret` - /// (non-empty) masked, recursively — in object **keys** and values, in string - /// leaves (substring), and in numeric/boolean leaves whose textual form is - /// exactly a secret. The result is what gets hashed and stored, so the proof is - /// honest about what was recorded. - pub fn redact_value(&self, v: &Value, secrets: &[String]) -> Value { - self.redact_inner(v, &sorted_secrets(secrets)) - } - - fn redact_inner(&self, v: &Value, secrets: &[&str]) -> Value { - match v { - Value::Object(map) => { - let mut out = Map::with_capacity(map.len()); - for (k, val) in map { - // mask a secret used AS a key name too (keys were skipped before). - // Disambiguate if masking collapses two keys to the same string, so - // a field is never silently dropped (data-integrity, RQ1). - let key = unique_key(&out, mask_secrets(k, secrets)); - if self.key_is_sensitive(k) { - out.insert(key, Value::String(REDACTED.to_string())); - } else { - out.insert(key, self.redact_inner(val, secrets)); - } - } - Value::Object(out) - } - Value::Array(items) => Value::Array( - items - .iter() - .map(|i| self.redact_inner(i, secrets)) - .collect(), - ), - Value::String(s) => Value::String(mask_secrets(s, secrets)), - Value::Number(_) | Value::Bool(_) => { - // a secret like "123" or "true" present as a JSON scalar (HIGH) - let text = v.to_string(); - if secrets.iter().any(|sec| text == *sec) { - Value::String(REDACTED.to_string()) - } else { - v.clone() - } - } - Value::Null => Value::Null, - } - } - - /// Mask secrets in a plain string (e.g. an error message). Used by the response - /// envelope so error text never leaks a credential. - /// - /// **Residual risk (accepted for v1, RQ3):** this is exact-substring masking. A - /// secret that a nested error (`reqwest`/`anyhow`) emits in a *transformed* form - /// — URL-encoded, base64, truncated — can survive, because v1 redaction is - /// denylist + raw-value matching, not format-aware. Tracked in the design's - /// best-effort redaction stance; format-aware redaction is out of scope for v1. - pub fn redact_str(&self, s: &str, secrets: &[String]) -> String { - mask_secrets(s, &sorted_secrets(secrets)) - } -} - -/// Return `key`, or a `key#N` variant that does not yet exist in `out`, so masking -/// two distinct keys to the same string never silently drops a field (RQ1). -fn unique_key(out: &Map, key: String) -> String { - if !out.contains_key(&key) { - return key; - } - let mut n = 2; - loop { - let candidate = format!("{key}#{n}"); - if !out.contains_key(&candidate) { - return candidate; - } - n += 1; - } -} - -/// Non-empty secrets, longest first, so an overlapping shorter secret can't leave -/// suffix material from a longer one (MEDIUM). -fn sorted_secrets(secrets: &[String]) -> Vec<&str> { - let mut v: Vec<&str> = secrets - .iter() - .map(|s| s.as_str()) - .filter(|s| !s.is_empty()) - .collect(); - v.sort_by_key(|b| std::cmp::Reverse(b.len())); - v -} - -/// Replace every occurrence of each secret substring with [`REDACTED`]. -fn mask_secrets(s: &str, secrets: &[&str]) -> String { - let mut out = s.to_string(); - for secret in secrets { - if out.contains(secret) { - out = out.replace(secret, REDACTED); - } - } - out -} - -/// The actual secret strings carried by `credentials` — matched directly against -/// **all** `CredentialData` variants (not via `bearer_token()`, which omits the -/// refresh token). -pub fn secret_values_of(creds: &Option) -> Vec { - let mut out = Vec::new(); - if let Some(c) = creds { - match &c.data { - CredentialData::ApiKey { token } => out.push(token.clone()), - CredentialData::OAuth { - access_token, - refresh_token, - .. - } => { - out.push(access_token.clone()); - if let Some(rt) = refresh_token { - out.push(rt.clone()); - } - } - CredentialData::Cookie { cookies, .. } => { - out.extend(cookies.values().cloned()); - } - } - } - out.retain(|s| !s.is_empty()); - out -} - -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - #[test] - fn masks_sensitive_keys_case_insensitive_and_suffix() { - let r = Redactor::new(); - let v = json!({ - "Authorization": "Bearer abc", - "API_KEY": "k", - "access_token": "t", - "customer_id": "42", - "nested": { "password": "p", "ok": "keep" } - }); - let out = r.redact_value(&v, &[]); - assert_eq!(out["Authorization"], json!(REDACTED)); - assert_eq!(out["API_KEY"], json!(REDACTED)); - assert_eq!(out["access_token"], json!(REDACTED)); - assert_eq!(out["customer_id"], json!("42")); - assert_eq!(out["nested"]["password"], json!(REDACTED)); - assert_eq!(out["nested"]["ok"], json!("keep")); - } - - #[test] - fn masks_secret_values_under_any_key_and_as_substring() { - let r = Redactor::new(); - let secrets = vec!["SUPERSECRET".to_string()]; - let v = json!({ - "echoed": "SUPERSECRET", - "in_text": "prefix SUPERSECRET suffix", - "arr": ["x", "SUPERSECRET"] - }); - let out = r.redact_value(&v, &secrets); - assert_eq!(out["echoed"], json!(REDACTED)); - assert_eq!(out["in_text"], json!(format!("prefix {REDACTED} suffix"))); - assert_eq!(out["arr"][1], json!(REDACTED)); - } - - #[test] - fn empty_secret_does_not_mask_everything() { - let r = Redactor::new(); - let out = r.redact_value(&json!({"a": "b"}), &["".to_string()]); - assert_eq!(out["a"], json!("b")); - } - - #[test] - fn masks_secret_used_as_object_key() { - let r = Redactor::new(); - let secrets = vec!["SECRETKEY".to_string()]; - let out = r.redact_value(&json!({ "SECRETKEY": "v", "ok": 1 }), &secrets); - let s = serde_json::to_string(&out).unwrap(); - assert!(!s.contains("SECRETKEY"), "secret survived as a key: {s}"); - assert_eq!(out["ok"], json!(1)); - } - - #[test] - fn masks_numeric_scalar_equal_to_secret() { - let r = Redactor::new(); - let secrets = vec!["123456".to_string()]; - let out = r.redact_value(&json!({ "pin": 123456, "qty": 2 }), &secrets); - assert_eq!(out["pin"], json!(REDACTED)); - assert_eq!(out["qty"], json!(2)); - } - - #[test] - fn longest_secret_masked_first() { - let r = Redactor::new(); - // "abc" is a prefix of "abcdef"; masking the longer first avoids leaving "def" - let secrets = vec!["abc".to_string(), "abcdef".to_string()]; - let out = r.redact_value(&json!({ "v": "abcdef" }), &secrets); - assert_eq!(out["v"], json!(REDACTED)); - } - - #[test] - fn masked_key_collision_is_disambiguated_not_dropped() { - let r = Redactor::new(); - // both keys contain the secret → both mask to the same base; keep both. - let secrets = vec!["S".to_string()]; - let out = r.redact_value(&json!({ "Sa": 1, "Sb": 2 }), &secrets); - let obj = out.as_object().unwrap(); - assert_eq!(obj.len(), 2, "a field was silently dropped: {out}"); - } - - #[test] - fn redact_str_masks_error_text() { - let r = Redactor::new(); - let masked = r.redact_str("auth failed for TOKENXYZ", &["TOKENXYZ".to_string()]); - assert!(!masked.contains("TOKENXYZ")); - } - - #[test] - fn secret_values_cover_all_variants() { - use crate::types::AuthType; - use std::collections::HashMap; - - let api = Credentials::api_key("apitok"); - assert_eq!(secret_values_of(&Some(api)), vec!["apitok".to_string()]); - - let oauth = Credentials::oauth("acc", Some("ref".to_string()), None); - let got = secret_values_of(&Some(oauth)); - assert!(got.contains(&"acc".to_string()) && got.contains(&"ref".to_string())); - - let mut cookies = HashMap::new(); - cookies.insert("session".to_string(), "cookieval".to_string()); - let cookie = Credentials { - credential_type: AuthType::Cookie, - data: CredentialData::Cookie { - cookies, - domain: "x".into(), - captured_at: chrono::Utc::now(), - expires_at: None, - }, - }; - assert_eq!( - secret_values_of(&Some(cookie)), - vec!["cookieval".to_string()] - ); - } -} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 448c109..cbdc97c 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -3,6 +3,7 @@ pub mod adapter; pub mod audit; pub mod error; pub mod oauth; +pub mod redact; pub mod router; pub mod token_refresh; pub mod types; diff --git a/crates/core/src/redact.rs b/crates/core/src/redact.rs new file mode 100644 index 0000000..dc79b07 --- /dev/null +++ b/crates/core/src/redact.rs @@ -0,0 +1,301 @@ +//! Generic credential/secret redaction — **always compiled** (not feature-gated), so +//! non-audit code (e.g. the server error path, PR-D) can sanitise output too. +//! +//! Two complementary defences: +//! 1. **Key-name redaction** — values under sensitive keys (`token`, `password`, +//! `*_token`, …) are masked regardless of content. +//! 2. **Secret-value redaction** — the actual credential strings (pulled from +//! [`crate::types::Credentials`]) are masked wherever they appear, under *any* key +//! or as a substring of a larger string. +//! +//! Audit-specific metadata (`AuditMeta`) lives in [`crate::audit::redact`], which +//! re-exports this module. + +use serde_json::{Map, Value}; + +use crate::types::{CredentialData, Credentials}; + +/// The placeholder written in place of redacted content. +pub const REDACTED: &str = "[REDACTED]"; + +/// Keys whose values are always masked (compared case-insensitively). +const DEFAULT_DENY_EXACT: &[&str] = &[ + "token", + "password", + "secret", + "authorization", + "api_key", + "acs_token", + "cookie", + "cookies", +]; + +/// Key suffixes whose values are always masked (e.g. `access_token`, `refresh_token`). +const DEFAULT_DENY_SUFFIX: &[&str] = &["_token"]; + +/// Redacts JSON values by key name and by secret value. +#[derive(Debug, Clone)] +pub struct Redactor { + deny_exact: Vec, + deny_suffix: Vec, +} + +impl Default for Redactor { + fn default() -> Self { + Self { + deny_exact: DEFAULT_DENY_EXACT.iter().map(|s| s.to_string()).collect(), + deny_suffix: DEFAULT_DENY_SUFFIX.iter().map(|s| s.to_string()).collect(), + } + } +} + +impl Redactor { + pub fn new() -> Self { + Self::default() + } + + fn key_is_sensitive(&self, key: &str) -> bool { + let k = key.to_ascii_lowercase(); + self.deny_exact.iter().any(|d| d == &k) || self.deny_suffix.iter().any(|s| k.ends_with(s)) + } + + /// Returns `v` with sensitive keys masked and any occurrence of a `secret` + /// (non-empty) masked, recursively — in object **keys** and values, in string + /// leaves (substring), and in numeric/boolean leaves whose textual form is + /// exactly a secret. + pub fn redact_value(&self, v: &Value, secrets: &[String]) -> Value { + self.redact_inner(v, &sorted_secrets(secrets)) + } + + fn redact_inner(&self, v: &Value, secrets: &[&str]) -> Value { + match v { + Value::Object(map) => { + let mut out = Map::with_capacity(map.len()); + for (k, val) in map { + // mask a secret used AS a key name too (keys were skipped before). + // Disambiguate if masking collapses two keys to the same string, so + // a field is never silently dropped (data-integrity, RQ1). + let key = unique_key(&out, mask_secrets(k, secrets)); + if self.key_is_sensitive(k) { + out.insert(key, Value::String(REDACTED.to_string())); + } else { + out.insert(key, self.redact_inner(val, secrets)); + } + } + Value::Object(out) + } + Value::Array(items) => Value::Array( + items + .iter() + .map(|i| self.redact_inner(i, secrets)) + .collect(), + ), + Value::String(s) => Value::String(mask_secrets(s, secrets)), + Value::Number(_) | Value::Bool(_) => { + // a secret like "123" or "true" present as a JSON scalar (HIGH) + let text = v.to_string(); + if secrets.iter().any(|sec| text == *sec) { + Value::String(REDACTED.to_string()) + } else { + v.clone() + } + } + Value::Null => Value::Null, + } + } + + /// Mask secrets in a plain string (e.g. an error message). + /// + /// **Residual risk (accepted for v1, RQ3):** this is exact-substring masking. A + /// secret a nested error emits in a *transformed* form (URL-encoded, base64, + /// truncated) can survive — v1 redaction is denylist + raw-value matching, not + /// format-aware. + pub fn redact_str(&self, s: &str, secrets: &[String]) -> String { + mask_secrets(s, &sorted_secrets(secrets)) + } +} + +/// Return `key`, or a `key#N` variant not yet in `out`, so masking two distinct keys +/// to the same string never silently drops a field (RQ1). +fn unique_key(out: &Map, key: String) -> String { + if !out.contains_key(&key) { + return key; + } + let mut n = 2; + loop { + let candidate = format!("{key}#{n}"); + if !out.contains_key(&candidate) { + return candidate; + } + n += 1; + } +} + +/// Non-empty secrets, longest first, so an overlapping shorter secret can't leave +/// suffix material from a longer one. +fn sorted_secrets(secrets: &[String]) -> Vec<&str> { + let mut v: Vec<&str> = secrets + .iter() + .map(|s| s.as_str()) + .filter(|s| !s.is_empty()) + .collect(); + v.sort_by_key(|b| std::cmp::Reverse(b.len())); + v +} + +/// Replace every occurrence of each secret substring with [`REDACTED`]. +fn mask_secrets(s: &str, secrets: &[&str]) -> String { + let mut out = s.to_string(); + for secret in secrets { + if out.contains(secret) { + out = out.replace(secret, REDACTED); + } + } + out +} + +/// The actual secret strings carried by `credentials` — matched directly against +/// **all** `CredentialData` variants (not via `bearer_token()`, which omits the +/// refresh token). +pub fn secret_values_of(creds: &Option) -> Vec { + let mut out = Vec::new(); + if let Some(c) = creds { + match &c.data { + CredentialData::ApiKey { token } => out.push(token.clone()), + CredentialData::OAuth { + access_token, + refresh_token, + .. + } => { + out.push(access_token.clone()); + if let Some(rt) = refresh_token { + out.push(rt.clone()); + } + } + CredentialData::Cookie { cookies, .. } => { + out.extend(cookies.values().cloned()); + } + } + } + out.retain(|s| !s.is_empty()); + out +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn masks_sensitive_keys_case_insensitive_and_suffix() { + let r = Redactor::new(); + let v = json!({ + "Authorization": "Bearer abc", + "API_KEY": "k", + "access_token": "t", + "customer_id": "42", + "nested": { "password": "p", "ok": "keep" } + }); + let out = r.redact_value(&v, &[]); + assert_eq!(out["Authorization"], json!(REDACTED)); + assert_eq!(out["API_KEY"], json!(REDACTED)); + assert_eq!(out["access_token"], json!(REDACTED)); + assert_eq!(out["customer_id"], json!("42")); + assert_eq!(out["nested"]["password"], json!(REDACTED)); + assert_eq!(out["nested"]["ok"], json!("keep")); + } + + #[test] + fn masks_secret_values_under_any_key_and_as_substring() { + let r = Redactor::new(); + let secrets = vec!["SUPERSECRET".to_string()]; + let v = json!({ + "echoed": "SUPERSECRET", + "in_text": "prefix SUPERSECRET suffix", + "arr": ["x", "SUPERSECRET"] + }); + let out = r.redact_value(&v, &secrets); + assert_eq!(out["echoed"], json!(REDACTED)); + assert_eq!(out["in_text"], json!(format!("prefix {REDACTED} suffix"))); + assert_eq!(out["arr"][1], json!(REDACTED)); + } + + #[test] + fn empty_secret_does_not_mask_everything() { + let r = Redactor::new(); + let out = r.redact_value(&json!({"a": "b"}), &["".to_string()]); + assert_eq!(out["a"], json!("b")); + } + + #[test] + fn masks_secret_used_as_object_key() { + let r = Redactor::new(); + let secrets = vec!["SECRETKEY".to_string()]; + let out = r.redact_value(&json!({ "SECRETKEY": "v", "ok": 1 }), &secrets); + let s = serde_json::to_string(&out).unwrap(); + assert!(!s.contains("SECRETKEY"), "secret survived as a key: {s}"); + assert_eq!(out["ok"], json!(1)); + } + + #[test] + fn masks_numeric_scalar_equal_to_secret() { + let r = Redactor::new(); + let secrets = vec!["123456".to_string()]; + let out = r.redact_value(&json!({ "pin": 123456, "qty": 2 }), &secrets); + assert_eq!(out["pin"], json!(REDACTED)); + assert_eq!(out["qty"], json!(2)); + } + + #[test] + fn longest_secret_masked_first() { + let r = Redactor::new(); + let secrets = vec!["abc".to_string(), "abcdef".to_string()]; + let out = r.redact_value(&json!({ "v": "abcdef" }), &secrets); + assert_eq!(out["v"], json!(REDACTED)); + } + + #[test] + fn masked_key_collision_is_disambiguated_not_dropped() { + let r = Redactor::new(); + let secrets = vec!["S".to_string()]; + let out = r.redact_value(&json!({ "Sa": 1, "Sb": 2 }), &secrets); + let obj = out.as_object().unwrap(); + assert_eq!(obj.len(), 2, "a field was silently dropped: {out}"); + } + + #[test] + fn redact_str_masks_error_text() { + let r = Redactor::new(); + let masked = r.redact_str("auth failed for TOKENXYZ", &["TOKENXYZ".to_string()]); + assert!(!masked.contains("TOKENXYZ")); + } + + #[test] + fn secret_values_cover_all_variants() { + use crate::types::AuthType; + use std::collections::HashMap; + + let api = Credentials::api_key("apitok"); + assert_eq!(secret_values_of(&Some(api)), vec!["apitok".to_string()]); + + let oauth = Credentials::oauth("acc", Some("ref".to_string()), None); + let got = secret_values_of(&Some(oauth)); + assert!(got.contains(&"acc".to_string()) && got.contains(&"ref".to_string())); + + let mut cookies = HashMap::new(); + cookies.insert("session".to_string(), "cookieval".to_string()); + let cookie = Credentials { + credential_type: AuthType::Cookie, + data: CredentialData::Cookie { + cookies, + domain: "x".into(), + captured_at: chrono::Utc::now(), + expires_at: None, + }, + }; + assert_eq!( + secret_values_of(&Some(cookie)), + vec!["cookieval".to_string()] + ); + } +} diff --git a/crates/server/src/handlers.rs b/crates/server/src/handlers.rs index 6c88f93..0d3ab5e 100644 --- a/crates/server/src/handlers.rs +++ b/crates/server/src/handlers.rs @@ -1,10 +1,12 @@ use axum::{ extract::{Path, State}, - http::StatusCode, - response::IntoResponse, + http::{header, HeaderValue, StatusCode}, + response::{IntoResponse, Response}, Json, }; +use relais_core::redact::{secret_values_of, Redactor}; use relais_core::types::{Credentials, ExecContext}; +use relais_core::AdapterError; use serde::Deserialize; use serde_json::{json, Value}; @@ -26,10 +28,7 @@ pub async fn list_apis( State(state): State, Path(site): Path, ) -> Result { - let adapter = state - .router - .get(&site) - .ok_or(StatusCode::NOT_FOUND)?; + let adapter = state.router.get(&site).ok_or(StatusCode::NOT_FOUND)?; Ok(Json(adapter.resources())) } @@ -48,14 +47,10 @@ pub async fn get_spec( let (site_id, resource_id, action_id) = (parts[0], parts[1], parts[2]); - let adapter = state - .router - .get(site_id) - .ok_or(StatusCode::NOT_FOUND)?; + let adapter = state.router.get(site_id).ok_or(StatusCode::NOT_FOUND)?; let resources = adapter.resources(); - let action = find_action(&resources, resource_id, action_id) - .ok_or(StatusCode::NOT_FOUND)?; + let action = find_action(&resources, resource_id, action_id).ok_or(StatusCode::NOT_FOUND)?; Ok(Json(action)) } @@ -93,17 +88,11 @@ pub struct ExecRequest { } /// POST /v1/exec — execute an action via the router. -pub async fn exec_action( - State(state): State, - Json(body): Json, -) -> Result)> { +pub async fn exec_action(State(state): State, Json(body): Json) -> Response { // Existence check for a clean 404 (router.exec would otherwise map a missing - // site to a 500). The actual execution goes through the router choke point. + // site to a 500). Use the same structured error shape as every other error. if state.router.get(&body.site).is_none() { - return Err(( - StatusCode::NOT_FOUND, - Json(json!({"error": format!("site '{}' not found", body.site)})), - )); + return error_response(&AdapterError::SiteNotFound(body.site.clone()), &None); } // Look up credentials from vault for this site. @@ -122,12 +111,8 @@ pub async fn exec_action( // If the token is expired, try to refresh it automatically. let credentials = if let Some(cred) = credentials { if cred.is_expired() { - match relais_core::token_refresh::maybe_refresh( - &cred, - &body.site, - state.vault.as_ref(), - ) - .await + match relais_core::token_refresh::maybe_refresh(&cred, &body.site, state.vault.as_ref()) + .await { Ok(refreshed) => Some(refreshed), Err(e) => { @@ -162,10 +147,111 @@ pub async fn exec_action( }; match state.router.exec(&ctx).await { - Ok(response) => Ok(Json(response)), - Err(err) => Err(( - StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({"error": err.to_string()})), - )), + Ok(response) => Json(response).into_response(), + Err(err) => error_response(&err, &ctx.credentials), + } +} + +/// Map an adapter error to (HTTP status, stable `kind`). +fn classify(err: &AdapterError) -> (StatusCode, &'static str) { + match err { + AdapterError::Auth(_) => (StatusCode::UNAUTHORIZED, "auth"), + AdapterError::RateLimited { .. } => (StatusCode::TOO_MANY_REQUESTS, "rate_limited"), + AdapterError::NotFound(_) => (StatusCode::NOT_FOUND, "not_found"), + AdapterError::Unsupported(_) => (StatusCode::BAD_REQUEST, "unsupported"), + AdapterError::SiteNotFound(_) => (StatusCode::NOT_FOUND, "site_not_found"), + AdapterError::AuditUnavailable(_) => (StatusCode::SERVICE_UNAVAILABLE, "audit_unavailable"), + AdapterError::Upstream(_) => (StatusCode::BAD_GATEWAY, "upstream"), + AdapterError::Other(_) => (StatusCode::INTERNAL_SERVER_ERROR, "internal"), + } +} + +/// The error text with this request's credential values masked. Upstream error text +/// can echo a request credential, so it must not be returned to the caller verbatim +/// (the audit redaction boundary must not be bypassed on the error path). +fn redacted_message(err: &AdapterError, credentials: &Option) -> String { + Redactor::new().redact_str(&err.to_string(), &secret_values_of(credentials)) +} + +/// Build a safe HTTP response for an adapter error: proper status, a structured +/// redacted body, and a `Retry-After` header for rate limits. +fn error_response(err: &AdapterError, credentials: &Option) -> Response { + let (status, kind) = classify(err); + let message = redacted_message(err, credentials); + let body = Json(json!({ "error": { "kind": kind, "message": message } })); + let mut resp = (status, body).into_response(); + if let AdapterError::RateLimited { retry_after_secs } = err { + if let Ok(v) = HeaderValue::from_str(&retry_after_secs.to_string()) { + resp.headers_mut().insert(header::RETRY_AFTER, v); + } + } + resp +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn redacts_request_credential_in_error_message() { + // An error whose text echoes the request's API token must be masked. + let creds = Some(Credentials::api_key("SECRET_TOKEN_abc")); + let err = AdapterError::Auth("upstream said: token SECRET_TOKEN_abc bad".into()); + let msg = redacted_message(&err, &creds); + assert!(!msg.contains("SECRET_TOKEN_abc"), "token leaked: {msg}"); + } + + #[test] + fn maps_known_variants_to_statuses() { + let cases = [ + ( + AdapterError::Auth("x".into()), + StatusCode::UNAUTHORIZED, + "auth", + ), + ( + AdapterError::NotFound("x".into()), + StatusCode::NOT_FOUND, + "not_found", + ), + ( + AdapterError::Unsupported("x".into()), + StatusCode::BAD_REQUEST, + "unsupported", + ), + ( + AdapterError::RateLimited { + retry_after_secs: 1, + }, + StatusCode::TOO_MANY_REQUESTS, + "rate_limited", + ), + ( + AdapterError::SiteNotFound("x".into()), + StatusCode::NOT_FOUND, + "site_not_found", + ), + ]; + for (err, status, kind) in cases { + let (got_status, got_kind) = classify(&err); + assert_eq!(got_status, status, "status for {err:?}"); + assert_eq!(got_kind, kind, "kind for {err:?}"); + } + } + + #[test] + fn rate_limited_sets_retry_after_header() { + let resp = error_response( + &AdapterError::RateLimited { + retry_after_secs: 42, + }, + &None, + ); + assert_eq!(resp.status(), StatusCode::TOO_MANY_REQUESTS); + assert_eq!( + resp.headers().get(header::RETRY_AFTER).unwrap(), + "42", + "Retry-After header" + ); } }