From e79fc055d46e9a8fe6059bfe27561158d3828198 Mon Sep 17 00:00:00 2001 From: CleverWild Date: Sun, 9 Aug 2026 00:11:03 +0200 Subject: [PATCH] fix(errors)!: forbid interpolated details in wire-facing internal errors --- server/Cargo.lock | 130 +----------------- server/Cargo.toml | 5 +- server/crates/arbiter-client/src/auth.rs | 1 + server/crates/arbiter-client/src/client.rs | 1 + server/crates/arbiter-client/src/storage.rs | 1 + server/crates/arbiter-client/src/transport.rs | 1 + server/crates/arbiter-proto/src/url.rs | 1 + .../arbiter-server/src/crypto/integrity/v1.rs | 16 ++- .../src/peers/operator/auth/mod.rs | 13 +- .../src/peers/operator/vault_gate/mod.rs | 33 +++-- server/crates/arbiter-server/src/utils.rs | 44 ++++++ 11 files changed, 94 insertions(+), 152 deletions(-) diff --git a/server/Cargo.lock b/server/Cargo.lock index dcf3fa1..a19268d 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -2,15 +2,6 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "addr2line" -version = "0.25.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b5d307320b3181d6d7954e663bd7c774a838b8220fe0593c86d9fb09f498b4b" -dependencies = [ - "gimli", -] - [[package]] name = "adler2" version = "2.0.1" @@ -1166,30 +1157,6 @@ dependencies = [ "tower-service", ] -[[package]] -name = "backtrace" -version = "0.3.76" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb531853791a215d7c62a30daf0dde835f381ab5de4589cfe7c649d2cbe92bd6" -dependencies = [ - "addr2line", - "cfg-if", - "libc", - "miniz_oxide", - "object", - "rustc-demangle", - "windows-link", -] - -[[package]] -name = "backtrace-ext" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "537beee3be4a18fb023b570f80e3ae28003db9167a751266b259926e25539d50" -dependencies = [ - "backtrace", -] - [[package]] name = "base16ct" version = "0.2.0" @@ -2365,12 +2332,6 @@ dependencies = [ "wasip3", ] -[[package]] -name = "gimli" -version = "0.32.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e629b9b98ef3dd8afe6ca2bd0f89306cec16d43d907889945bc5d6687f2f13c7" - [[package]] name = "glob" version = "0.3.3" @@ -2828,12 +2789,6 @@ dependencies = [ "serde", ] -[[package]] -name = "is_ci" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7655c9839580ee829dfacba1d1278c2b7883e50a277ff7541299489d6bdfdc45" - [[package]] name = "itertools" version = "0.10.5" @@ -3169,18 +3124,9 @@ version = "7.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5f98efec8807c63c752b5bd61f862c165c115b0a35685bdcfd9238c7aeb592b7" dependencies = [ - "backtrace", - "backtrace-ext", "cfg-if", "miette-derive", - "owo-colors", - "serde", - "supports-color", - "supports-hyperlinks", - "supports-unicode", - "terminal_size", - "textwrap", - "unicode-width 0.1.14", + "unicode-width", ] [[package]] @@ -3402,15 +3348,6 @@ dependencies = [ "smallvec", ] -[[package]] -name = "object" -version = "0.37.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff76201f031d8863c38aa7f905eca4f53abbfa15f609db4277d44cd8938f33fe" -dependencies = [ - "memchr", -] - [[package]] name = "oid-registry" version = "0.8.1" @@ -3438,12 +3375,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" -[[package]] -name = "owo-colors" -version = "4.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d211803b9b6b570f68772237e415a029d5a50c65d382910b879fb19d3271f94d" - [[package]] name = "parity-scale-codec" version = "3.7.5" @@ -4247,12 +4178,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "48fd7bd8a6377e15ad9d42a8ec25371b94ddc67abe7c8b9127bec79bebaaae18" -[[package]] -name = "rustc-demangle" -version = "0.1.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b50b8869d9fc858ce7266cce0194bd74df58b9d0e3f6df3a9fc8eb470d95c09d" - [[package]] name = "rustc-hash" version = "2.1.2" @@ -4892,27 +4817,6 @@ version = "2.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" -[[package]] -name = "supports-color" -version = "3.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c64fc7232dd8d2e4ac5ce4ef302b1d81e0b80d055b9d77c7c4f51f6aa4c867d6" -dependencies = [ - "is_ci", -] - -[[package]] -name = "supports-hyperlinks" -version = "3.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e396b6523b11ccb83120b115a0b7366de372751aa6edf19844dfb13a6af97e91" - -[[package]] -name = "supports-unicode" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7401a30af6cb5818bb64852270bb722533397edcfc7344954a38f420819ece2" - [[package]] name = "syn" version = "1.0.109" @@ -4986,16 +4890,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "terminal_size" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "230a1b821ccbd75b185820a1f1ff7b14d21da1e442e22c0863ea5f08771a8874" -dependencies = [ - "rustix", - "windows-sys 0.61.2", -] - [[package]] name = "test-log" version = "0.2.20" @@ -5027,16 +4921,6 @@ dependencies = [ "test-log-core", ] -[[package]] -name = "textwrap" -version = "0.16.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c13547615a44dc9c452a8a534638acdf07120d4b6847c8178705da06306a3057" -dependencies = [ - "unicode-linebreak", - "unicode-width 0.2.2", -] - [[package]] name = "thiserror" version = "2.0.18" @@ -5476,12 +5360,6 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" -[[package]] -name = "unicode-linebreak" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" - [[package]] name = "unicode-segmentation" version = "1.13.2" @@ -5494,12 +5372,6 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" -[[package]] -name = "unicode-width" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" - [[package]] name = "unicode-xid" version = "0.2.6" diff --git a/server/Cargo.toml b/server/Cargo.toml index ab81ee3..51cda79 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -15,7 +15,10 @@ k256 = { version = "0.13.4", features = ["ecdsa", "pkcs8"] } kameo = {git = "https://github.com/hdbg/kameo.git", rev = "805b417"} kameo_actors = {git = "https://github.com/hdbg/kameo.git", rev = "805b417"} hmac = "0.13.0" -miette = { version = "7.6.0", features = ["fancy", "serde"] } +# `derive` only: nothing renders a miette Report yet, so `fancy` would drag the +# terminal-formatting stack (owo-colors, supports-color, textwrap, terminal_size) +# into the headless daemon. A CLI that wants pretty output enables `fancy` itself. +miette = "7.6.0" ml-dsa = { version = "0.1.0-rc.9", features = ["zeroize"] } mutants = "0.0.4" prost = "0.14.3" diff --git a/server/crates/arbiter-client/src/auth.rs b/server/crates/arbiter-client/src/auth.rs index 176cd13..4d5dcd4 100644 --- a/server/crates/arbiter-client/src/auth.rs +++ b/server/crates/arbiter-client/src/auth.rs @@ -23,6 +23,7 @@ use arbiter_proto::{ use chrono::DateTime; #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum AuthError { #[error("Server sent invalid auth challenge")] InvalidChallenge, diff --git a/server/crates/arbiter-client/src/client.rs b/server/crates/arbiter-client/src/client.rs index 7269772..3f76a9f 100644 --- a/server/crates/arbiter-client/src/client.rs +++ b/server/crates/arbiter-client/src/client.rs @@ -17,6 +17,7 @@ use tokio_stream::wrappers::ReceiverStream; use tonic::transport::ClientTlsConfig; #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum ArbiterClientError { #[error("Authentication error")] Authentication(#[from] AuthError), diff --git a/server/crates/arbiter-client/src/storage.rs b/server/crates/arbiter-client/src/storage.rs index 69d80c2..bf35ede 100644 --- a/server/crates/arbiter-client/src/storage.rs +++ b/server/crates/arbiter-client/src/storage.rs @@ -4,6 +4,7 @@ use arbiter_proto::home_path; use std::path::{Path, PathBuf}; #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum StorageError { #[error("Invalid signing key length in storage: expected {expected} bytes, got {actual} bytes")] InvalidKeyLength { expected: usize, actual: usize }, diff --git a/server/crates/arbiter-client/src/transport.rs b/server/crates/arbiter-client/src/transport.rs index 0e1e44e..c04702d 100644 --- a/server/crates/arbiter-client/src/transport.rs +++ b/server/crates/arbiter-client/src/transport.rs @@ -11,6 +11,7 @@ pub fn next_request_id() -> i32 { } #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum ClientSignError { #[error("Transport channel closed")] ChannelClosed, diff --git a/server/crates/arbiter-proto/src/url.rs b/server/crates/arbiter-proto/src/url.rs index abcf50c..db77027 100644 --- a/server/crates/arbiter-proto/src/url.rs +++ b/server/crates/arbiter-proto/src/url.rs @@ -30,6 +30,7 @@ impl Display for ArbiterUrl { } #[derive(Debug, thiserror::Error, miette::Diagnostic)] +#[non_exhaustive] pub enum Error { #[error("Invalid URL scheme, expected '{ARBITER_URL_SCHEME}://'")] #[diagnostic( diff --git a/server/crates/arbiter-server/src/crypto/integrity/v1.rs b/server/crates/arbiter-server/src/crypto/integrity/v1.rs index 0b053d9..790f4a7 100644 --- a/server/crates/arbiter-server/src/crypto/integrity/v1.rs +++ b/server/crates/arbiter-server/src/crypto/integrity/v1.rs @@ -13,6 +13,7 @@ use diesel_async::{AsyncConnection, RunQueryDsl}; use hmac::Hmac; use kameo::{actor::ActorRef, error::SendError}; use sha2::{Digest as _, Sha256}; +use tracing::error; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -121,7 +122,10 @@ pub async fn sign_entity( .await .map_err(|err| match err { SendError::HandlerError(inner) => Error::Vault(inner), - _ => Error::VaultSend, + other => { + error!(?other, "Vault unreachable while signing integrity envelope"); + Error::VaultSend + } })?; insert_into(integrity_envelope::table) @@ -195,12 +199,18 @@ pub async fn verify_entity( Err(SendError::HandlerError( vault::Error::Sealed | vault::Error::KeyVersionMismatch { .. }, )) => Ok(AttestationStatus::Unavailable), - Err(_) => Err(Error::VaultSend), + Err(other) => { + error!(?other, "Vault unreachable while verifying integrity envelope"); + Err(Error::VaultSend) + } } } pub async fn is_signing_available(vault: &ActorRef) -> Result { - let state = vault.ask(GetState).await.map_err(|_| Error::VaultSend)?; + let state = vault.ask(GetState).await.map_err(|err| { + error!(?err, "Vault unreachable while querying signing availability"); + Error::VaultSend + })?; Ok(matches!(state, vault::VaultState::Unsealed)) } diff --git a/server/crates/arbiter-server/src/peers/operator/auth/mod.rs b/server/crates/arbiter-server/src/peers/operator/auth/mod.rs index 4b2ecb6..d7516b5 100644 --- a/server/crates/arbiter-server/src/peers/operator/auth/mod.rs +++ b/server/crates/arbiter-server/src/peers/operator/auth/mod.rs @@ -26,21 +26,22 @@ pub enum Error { UnregisteredPublicKey, InvalidChallengeSolution, InvalidBootstrapToken, - Internal { details: String }, + /// Reaches the operator verbatim via `Status::internal`, so the payload is + /// `&'static str`: the type makes it impossible to interpolate an inner + /// error. Log the cause, send the constant. + Internal { details: &'static str }, Transport, } impl Error { - fn internal(details: impl Into) -> Self { - Self::Internal { - details: details.into(), - } + const fn internal(details: &'static str) -> Self { + Self::Internal { details } } } impl From for Error { fn from(e: diesel::result::Error) -> Self { - error!(?e, "Database error"); + error!(error = %crate::utils::error_chain(&e), "Database error"); Self::internal("Database error") } } diff --git a/server/crates/arbiter-server/src/peers/operator/vault_gate/mod.rs b/server/crates/arbiter-server/src/peers/operator/vault_gate/mod.rs index d0e38e3..ed784b2 100644 --- a/server/crates/arbiter-server/src/peers/operator/vault_gate/mod.rs +++ b/server/crates/arbiter-server/src/peers/operator/vault_gate/mod.rs @@ -31,13 +31,11 @@ pub enum Error { #[error("State transition failed")] State, + /// Reaches the operator verbatim via `Status::internal`, so the payload is + /// `&'static str`: the type makes it impossible to interpolate an inner + /// error. Log the cause, send the constant. #[error("Internal error: {0}")] - Internal(String), -} -impl Error { - fn internal(message: impl Into) -> Self { - Self::Internal(message.into()) - } + Internal(&'static str), } pub struct HandshakeResponse { @@ -179,7 +177,7 @@ impl VaultGate { } Err(err) => { error!(?err, "Failed to send unseal request to vault"); - Err(Error::internal("Vault actor error")) + Err(Error::Internal("Vault actor error")) } } } @@ -221,7 +219,7 @@ impl VaultGate { } Err(err) => { error!(?err, "Failed to send bootstrap request to vault"); - Err(Error::internal("Vault error")) + Err(Error::Internal("Vault error")) } } } @@ -233,7 +231,10 @@ impl VaultGate { .vault .ask(GetState {}) .await - .map_err(|_| Error::internal("failed to query vault"))?; + .map_err(|err| { + error!(?err, "Failed to query vault state"); + Error::Internal("failed to query vault") + })?; Ok(answer) } @@ -252,7 +253,10 @@ impl Message for VaultGate { .db .get() .await - .map_err(|_| Error::internal("DB unavailable"))?; + .map_err(|err| { + error!(error = %crate::utils::error_chain(&err), "DB unavailable on bootstrap"); + Error::Internal("DB unavailable") + })?; integrity::sign_entity( &mut conn, &self.actors.vault, @@ -260,9 +264,12 @@ impl Message for VaultGate { self.auth_creds.id, ) .await - .map_err(|e| { - error!(?e, "Failed to sign integrity envelope on bootstrap"); - Error::internal("Integrity sign failed") + .map_err(|err| { + error!( + error = %crate::utils::error_chain(&err), + "Failed to sign integrity envelope on bootstrap" + ); + Error::Internal("Integrity sign failed") })?; Ok(()) } diff --git a/server/crates/arbiter-server/src/utils.rs b/server/crates/arbiter-server/src/utils.rs index d072aa7..e335377 100644 --- a/server/crates/arbiter-server/src/utils.rs +++ b/server/crates/arbiter-server/src/utils.rs @@ -14,3 +14,47 @@ impl Drop for DeferClosure { pub fn defer(f: F) -> impl Drop + Sized { DeferClosure { f: Some(f) } } + +/// Renders an error together with its full `source` chain as `outer: inner: root`. +/// +/// Error variants in this crate deliberately keep `Display` terse so that no +/// internal detail can leak across the gRPC boundary. That same terseness would +/// hide the cause in the logs, so use this for `tracing` fields, never in a +/// wire payload. +pub fn error_chain(err: &dyn core::error::Error) -> String { + let mut out = err.to_string(); + let mut current = err.source(); + while let Some(source) = current { + out.push_str(": "); + out.push_str(&source.to_string()); + current = source.source(); + } + out +} + +#[cfg(test)] +mod tests { + use super::error_chain; + + #[derive(Debug, thiserror::Error)] + #[error("root")] + struct Root; + + #[derive(Debug, thiserror::Error)] + #[error("middle")] + struct Middle(#[source] Root); + + #[derive(Debug, thiserror::Error)] + #[error("outer")] + struct Outer(#[source] Middle); + + #[test] + fn walks_the_whole_source_chain() { + assert_eq!(error_chain(&Root), "root", "a leaf error renders alone"); + assert_eq!( + error_chain(&Outer(Middle(Root))), + "outer: middle: root", + "every source link must appear, in order" + ); + } +}