From 5debf4dd05652fc95b3e7f28c1beb9c388a4584c Mon Sep 17 00:00:00 2001 From: "a.tolmachev" Date: Sun, 19 Apr 2026 21:25:28 +0000 Subject: [PATCH] runtime: normalize structured secret errors --- TASKS.md | 10 +++---- apps/admin-api/src/service.rs | 12 ++++++++ apps/mcp-server/src/app.rs | 4 +++ crates/crank-runtime/src/auth.rs | 4 +-- crates/crank-runtime/src/error.rs | 19 ++++++------ crates/crank-runtime/src/executor.rs | 26 ++++++++++++++--- crates/crank-runtime/src/secret_crypto.rs | 35 +++++++++++++++++++---- 7 files changed, 85 insertions(+), 25 deletions(-) diff --git a/TASKS.md b/TASKS.md index 6420b69..366be29 100644 --- a/TASKS.md +++ b/TASKS.md @@ -2,18 +2,18 @@ ## Current -### `feat/typed-timestamps` +### `feat/error-structure-normalization` Status: in_progress DoD: -- domain and registry models use typed timestamps instead of `String` -- SQL row decoding uses timestamp types directly where possible -- API serialization stays RFC 3339 compatible +- runtime and API errors expose structured context where stable fields exist +- secret/runtime failure classes are mappable without parsing ad hoc message text +- opaque third-party details remain only where no stable structure exists ## Next -- `feat/error-structure-normalization` +- `feat/correlation-id-propagation` ## Backlog diff --git a/apps/admin-api/src/service.rs b/apps/admin-api/src/service.rs index 120c7be..ed26753 100644 --- a/apps/admin-api/src/service.rs +++ b/apps/admin-api/src/service.rs @@ -2169,6 +2169,7 @@ impl AdminService { .get_auth_profile(workspace_id, auth_profile_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load auth profile", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingAuthProfile { @@ -2207,6 +2208,7 @@ impl AdminService { streaming.max_session_lifetime_ms.unwrap_or(60_000) as i64, )) .ok_or_else(|| RuntimeError::SecretCrypto { + operation: "compute stream session expiration", details: "failed to compute stream session expiration".to_owned(), })?; let session = StreamSession { @@ -2234,6 +2236,7 @@ impl AdminService { .create_stream_session(CreateStreamSessionRequest { session: &session }) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "persist stream session", details: error.to_string(), })?; @@ -2267,6 +2270,7 @@ impl AdminService { streaming.max_session_lifetime_ms.unwrap_or(300_000) as i64, )) .ok_or_else(|| RuntimeError::SecretCrypto { + operation: "compute async job expiration", details: "failed to compute async job expiration".to_owned(), })?; let job = AsyncJobHandle { @@ -2287,6 +2291,7 @@ impl AdminService { .create_async_job(CreateAsyncJobRequest { job: &job }) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "persist async job", details: error.to_string(), })?; @@ -2376,6 +2381,7 @@ impl AdminService { .get_secret(workspace_id, secret_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load secret", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingSecret { @@ -2386,6 +2392,7 @@ impl AdminService { .get_current_secret_version(workspace_id, secret_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load current secret version", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingSecretVersion { @@ -2400,6 +2407,7 @@ impl AdminService { .touch_secret(workspace_id, secret_id, &used_at) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "touch secret", details: error.to_string(), })?; secrets.insert(secret_id.clone(), plaintext); @@ -4648,6 +4656,7 @@ async fn resolve_runtime_auth_for_task( .get_auth_profile(workspace_id, auth_profile_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load auth profile", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingAuthProfile { @@ -4662,6 +4671,7 @@ async fn resolve_runtime_auth_for_task( .get_secret(workspace_id, secret_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load secret", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingSecret { @@ -4671,6 +4681,7 @@ async fn resolve_runtime_auth_for_task( .get_current_secret_version(workspace_id, secret_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load current secret version", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingSecretVersion { @@ -4685,6 +4696,7 @@ async fn resolve_runtime_auth_for_task( .touch_secret(workspace_id, secret_id, &used_at) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "touch secret", details: error.to_string(), })?; secrets.insert(secret_id.clone(), plaintext); diff --git a/apps/mcp-server/src/app.rs b/apps/mcp-server/src/app.rs index 7a39333..fdad692 100644 --- a/apps/mcp-server/src/app.rs +++ b/apps/mcp-server/src/app.rs @@ -542,6 +542,7 @@ async fn resolve_runtime_auth_for_task( .get_auth_profile(workspace_id, auth_profile_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load auth profile", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingAuthProfile { @@ -567,6 +568,7 @@ async fn resolve_auth_profile( .get_secret(workspace_id, secret_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load secret", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingSecret { @@ -576,6 +578,7 @@ async fn resolve_auth_profile( .get_current_secret_version(workspace_id, secret_id) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "load current secret version", details: error.to_string(), })? .ok_or_else(|| RuntimeError::MissingSecretVersion { @@ -590,6 +593,7 @@ async fn resolve_auth_profile( .touch_secret(workspace_id, secret_id, &used_at) .await .map_err(|error| RuntimeError::SecretCrypto { + operation: "touch secret", details: error.to_string(), })?; secrets.insert(SecretId::new(secret_id.as_str()), plaintext); diff --git a/crates/crank-runtime/src/auth.rs b/crates/crank-runtime/src/auth.rs index e67f135..f802a4b 100644 --- a/crates/crank-runtime/src/auth.rs +++ b/crates/crank-runtime/src/auth.rs @@ -92,7 +92,7 @@ fn extract_secret_string( let Some(object) = value.as_object() else { return Err(RuntimeError::InvalidAuthSecretValue { secret_id: secret_id.as_str().to_owned(), - details: "secret payload must be a string or object".to_owned(), + reason: "secret payload must be a string or object".to_owned(), }); }; @@ -104,7 +104,7 @@ fn extract_secret_string( Err(RuntimeError::InvalidAuthSecretValue { secret_id: secret_id.as_str().to_owned(), - details: format!( + reason: format!( "secret payload must contain one of: {}", preferred_fields.join(", ") ), diff --git a/crates/crank-runtime/src/error.rs b/crates/crank-runtime/src/error.rs index 7ff625a..8e602c8 100644 --- a/crates/crank-runtime/src/error.rs +++ b/crates/crank-runtime/src/error.rs @@ -33,18 +33,21 @@ pub enum RuntimeError { operation_id: String, mode: ExecutionMode, }, - #[error("invalid prepared request: {details}")] - InvalidPreparedRequest { details: String }, - #[error("invalid streaming payload: {details}")] - InvalidStreamingPayload { details: String }, + #[error("invalid prepared request at {field}: {reason}")] + InvalidPreparedRequest { field: String, reason: String }, + #[error("invalid streaming payload at {field}: {reason}")] + InvalidStreamingPayload { field: String, reason: String }, #[error("auth profile {auth_profile_id} was not found")] MissingAuthProfile { auth_profile_id: String }, #[error("secret {secret_id} was not found")] MissingSecret { secret_id: String }, #[error("secret {secret_id} does not have current version {version}")] MissingSecretVersion { secret_id: String, version: u32 }, - #[error("invalid secret payload for {secret_id}: {details}")] - InvalidAuthSecretValue { secret_id: String, details: String }, - #[error("secret crypto error: {details}")] - SecretCrypto { details: String }, + #[error("invalid secret payload for {secret_id}: {reason}")] + InvalidAuthSecretValue { secret_id: String, reason: String }, + #[error("secret crypto error during {operation}: {details}")] + SecretCrypto { + operation: &'static str, + details: String, + }, } diff --git a/crates/crank-runtime/src/executor.rs b/crates/crank-runtime/src/executor.rs index 8acb89e..a373ec3 100644 --- a/crates/crank-runtime/src/executor.rs +++ b/crates/crank-runtime/src/executor.rs @@ -395,7 +395,8 @@ impl PreparedRequest { mapped .get("request") .ok_or_else(|| RuntimeError::InvalidPreparedRequest { - details: "mapped input must contain request root".to_owned(), + field: "request".to_owned(), + reason: "mapped input must contain request root".to_owned(), })?; Ok(Self { @@ -460,7 +461,8 @@ fn read_string_map( let Some(object) = value.as_object() else { return Err(RuntimeError::InvalidPreparedRequest { - details: format!("{field_name} must be an object"), + field: field_name.to_owned(), + reason: "must be an object".to_owned(), }); }; @@ -468,7 +470,10 @@ fn read_string_map( .iter() .map(|(key, value)| stringify_value(value).map(|value| (key.clone(), value))) .collect::, _>>() - .map_err(|details| RuntimeError::InvalidPreparedRequest { details }) + .map_err(|reason| RuntimeError::InvalidPreparedRequest { + field: field_name.to_owned(), + reason, + }) } fn stringify_value(value: &Value) -> Result { @@ -525,7 +530,7 @@ mod tests { use tokio::net::TcpListener; use tokio_tungstenite::accept_async; - use crate::{RuntimeError, RuntimeExecutor, RuntimeOperation}; + use crate::{PreparedRequest, RuntimeError, RuntimeExecutor, RuntimeOperation}; fn timestamp(value: &str) -> OffsetDateTime { OffsetDateTime::parse(value, &Rfc3339).unwrap() @@ -638,6 +643,19 @@ mod tests { assert!(matches!(error, RuntimeError::Schema(_))); } + #[test] + fn rejects_missing_request_root_with_structured_error() { + let error = PreparedRequest::from_mapping_output(&json!({})).unwrap_err(); + + match error { + RuntimeError::InvalidPreparedRequest { field, reason } => { + assert_eq!(field, "request"); + assert_eq!(reason, "mapped input must contain request root"); + } + other => panic!("unexpected error: {other}"), + } + } + #[tokio::test] async fn propagates_external_rest_errors() { let base_url = spawn_runtime_server().await; diff --git a/crates/crank-runtime/src/secret_crypto.rs b/crates/crank-runtime/src/secret_crypto.rs index 9314041..7ccd2a1 100644 --- a/crates/crank-runtime/src/secret_crypto.rs +++ b/crates/crank-runtime/src/secret_crypto.rs @@ -32,6 +32,7 @@ impl SecretCrypto { let trimmed = master_key.trim(); if trimmed.is_empty() { return Err(RuntimeError::SecretCrypto { + operation: "initialize secret crypto", details: "CRANK_MASTER_KEY must not be empty".to_owned(), }); } @@ -54,16 +55,19 @@ impl SecretCrypto { pub fn decrypt(&self, key_version: &str, ciphertext: &str) -> Result { let envelope: CipherEnvelope = serde_json::from_str(ciphertext).map_err(|error| RuntimeError::SecretCrypto { + operation: "decode secret envelope", details: format!("failed to decode secret envelope: {error}"), })?; let nonce_bytes = STANDARD .decode(envelope.nonce_b64) .map_err(|error| RuntimeError::SecretCrypto { + operation: "decode secret nonce", details: format!("failed to decode secret nonce: {error}"), })?; let ciphertext_bytes = STANDARD.decode(envelope.ciphertext_b64).map_err(|error| { RuntimeError::SecretCrypto { + operation: "decode secret payload", details: format!("failed to decode secret payload: {error}"), } })?; @@ -71,10 +75,12 @@ impl SecretCrypto { let plaintext = cipher .decrypt(Nonce::from_slice(&nonce_bytes), ciphertext_bytes.as_ref()) .map_err(|error| RuntimeError::SecretCrypto { + operation: "decrypt secret value", details: format!("failed to decrypt secret value: {error}"), })?; serde_json::from_slice(&plaintext).map_err(|error| RuntimeError::SecretCrypto { + operation: "deserialize secret value", details: format!("failed to deserialize secret value: {error}"), }) } @@ -84,6 +90,7 @@ impl SecretCrypto { LEGACY_KEY_VERSION => Ok(&self.legacy_cipher), CURRENT_KEY_VERSION => Ok(&self.current_cipher), other => Err(RuntimeError::SecretCrypto { + operation: "select secret key version", details: format!("unsupported secret key version: {other}"), }), } @@ -93,6 +100,7 @@ impl SecretCrypto { fn derive_legacy_cipher(master_key: &str) -> Result { let digest = Sha256::digest(master_key.as_bytes()); Aes256Gcm::new_from_slice(digest.as_slice()).map_err(|error| RuntimeError::SecretCrypto { + operation: "initialize legacy secret crypto", details: format!("failed to initialize legacy secret crypto: {error}"), }) } @@ -102,9 +110,11 @@ fn derive_hkdf_cipher(master_key: &str) -> Result { let mut key_bytes = [0_u8; 32]; hkdf.expand(SECRET_ENVELOPE_INFO, &mut key_bytes) .map_err(|error| RuntimeError::SecretCrypto { + operation: "derive secret key with hkdf", details: format!("failed to derive secret key with HKDF: {error}"), })?; Aes256Gcm::new_from_slice(&key_bytes).map_err(|error| RuntimeError::SecretCrypto { + operation: "initialize secret crypto", details: format!("failed to initialize secret crypto: {error}"), }) } @@ -115,6 +125,7 @@ fn encrypt_value_with_cipher( action: &str, ) -> Result { let plaintext = serde_json::to_vec(value).map_err(|error| RuntimeError::SecretCrypto { + operation: "serialize secret value", details: format!("failed to serialize secret value: {error}"), })?; let mut nonce_bytes = [0_u8; 12]; @@ -124,6 +135,7 @@ fn encrypt_value_with_cipher( cipher .encrypt(nonce, plaintext.as_ref()) .map_err(|error| RuntimeError::SecretCrypto { + operation: "encrypt secret value", details: format!("failed to {action}: {error}"), })?; let envelope = CipherEnvelope { @@ -132,6 +144,7 @@ fn encrypt_value_with_cipher( }; serde_json::to_string(&envelope).map_err(|error| RuntimeError::SecretCrypto { + operation: "encode secret ciphertext", details: format!("failed to encode secret ciphertext: {error}"), }) } @@ -148,6 +161,7 @@ fn current_key_bytes(master_key: &str) -> Result<[u8; 32], RuntimeError> { let mut key_bytes = [0_u8; 32]; hkdf.expand(SECRET_ENVELOPE_INFO, &mut key_bytes) .map_err(|error| RuntimeError::SecretCrypto { + operation: "derive secret key with hkdf", details: format!("failed to derive secret key with HKDF: {error}"), })?; Ok(key_bytes) @@ -163,6 +177,7 @@ fn legacy_key_bytes(master_key: &str) -> [u8; 32] { #[cfg(test)] mod tests { + use crate::RuntimeError; use serde_json::json; use super::{ @@ -202,11 +217,13 @@ mod tests { #[test] fn rejects_empty_master_key() { let error = SecretCrypto::new(" ").err().unwrap(); - assert!( - error - .to_string() - .contains("CRANK_MASTER_KEY must not be empty") - ); + match error { + RuntimeError::SecretCrypto { operation, details } => { + assert_eq!(operation, "initialize secret crypto"); + assert_eq!(details, "CRANK_MASTER_KEY must not be empty"); + } + other => panic!("unexpected error: {other}"), + } } #[test] @@ -246,6 +263,12 @@ mod tests { let ciphertext = crypto.encrypt(&json!({"token": "top-secret"})).unwrap(); let error = crypto.decrypt("v999", &ciphertext).unwrap_err(); - assert!(error.to_string().contains("unsupported secret key version")); + match error { + RuntimeError::SecretCrypto { operation, details } => { + assert_eq!(operation, "select secret key version"); + assert!(details.contains("unsupported secret key version")); + } + other => panic!("unexpected error: {other}"), + } } }