From 5faef6c012158e59fd7b9ac15c4a43166b34fca7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 05:51:45 -0400 Subject: [PATCH] fix(CLI-03): Rust typed invoke validates HRESULT/MXSTATUS_PROXY Typed write/command wrappers now call ensure_mxaccess_success after ensure_command_success, failing on hresult < 0 (COM-correct, matching CLI-08/Python) or any MxStatusProxy.success == 0, via a new boxed Error::MxAccess variant with credential-safe formatting. The raw invoke escape hatch stays unvalidated. archreview: CLI-03 (P0). Verified: cargo fmt/check/test (28) and clippy --all-targets -D warnings clean. --- clients/rust/README.md | 12 +++ clients/rust/src/client.rs | 20 +++-- clients/rust/src/error.rs | 168 ++++++++++++++++++++++++++++++++++++- clients/rust/src/lib.rs | 2 +- 4 files changed, 194 insertions(+), 8 deletions(-) diff --git a/clients/rust/README.md b/clients/rust/README.md index e01e645..4548fee 100644 --- a/clients/rust/README.md +++ b/clients/rust/README.md @@ -125,6 +125,18 @@ preserving the raw message for parity diagnostics. Command replies whose protocol status is not `PROTOCOL_STATUS_CODE_OK` become `Error::Command` and retain the raw `MxCommandReply`. +The typed command helpers (`register`, `add_item`, `write`, the bulk variants, +etc.) also enforce MXAccess parity on an otherwise-OK reply: a reply that +reports a negative `hresult` (COM failure semantics — a positive code such as +`S_FALSE = 1` is a success) or a non-success `MXSTATUS_PROXY` status entry +becomes `Error::MxAccess`, which boxes an `MxAccessError` retaining the raw +`MxCommandReply` (recover it with `MxAccessError::reply` / `into_reply`). Its +message summarizes the `hresult` and status entries with credential-safe +redaction. Per-item bulk failures are reported inside each result entry +(`was_successful = false`) and do not raise `Error::MxAccess`. The raw +`invoke_raw` / `client.invoke_raw` escape hatch performs neither check and +returns the unvalidated reply. + ## Write Semantics And Common Pitfalls These are MXAccess parity behaviors that surprise new callers. The gateway diff --git a/clients/rust/src/client.rs b/clients/rust/src/client.rs index bdac543..493503c 100644 --- a/clients/rust/src/client.rs +++ b/clients/rust/src/client.rs @@ -11,7 +11,9 @@ use tonic::transport::Channel; use tonic::Request; use crate::auth::AuthInterceptor; -use crate::error::{ensure_command_success, ensure_protocol_success, Error}; +use crate::error::{ + ensure_command_success, ensure_mxaccess_success, ensure_protocol_success, Error, +}; use crate::generated::mxaccess_gateway::v1::mx_access_gateway_client::MxAccessGatewayClient; use crate::generated::mxaccess_gateway::v1::{ AcknowledgeAlarmReply, AcknowledgeAlarmRequest, ActiveAlarmSnapshot, AlarmFeedMessage, @@ -166,16 +168,24 @@ impl GatewayClient { Ok(response.into_inner()) } - /// Issue an `Invoke` RPC and surface a non-OK reply as - /// [`Error::Command`]. + /// Issue an `Invoke` RPC and surface a failing reply as a typed error. + /// + /// The reply is validated twice: [`ensure_command_success`] rejects a + /// non-OK protocol envelope as [`Error::Command`], then + /// [`ensure_mxaccess_success`] rejects an MXAccess-level failure (negative + /// `hresult` or a non-success `MXSTATUS_PROXY` entry) as + /// [`Error::MxAccess`], preserving MXAccess parity. Callers that need the + /// unvalidated reply should use [`invoke_raw`](Self::invoke_raw) instead. /// /// # Errors /// /// Returns [`Error::Command`] when the reply's `protocol_status` is not - /// `Ok`, plus any errors propagated by + /// `Ok`, [`Error::MxAccess`] when the reply reports an MXAccess-level + /// failure, plus any errors propagated by /// [`invoke_raw`](Self::invoke_raw). pub async fn invoke(&self, request: MxCommandRequest) -> Result { - ensure_command_success(self.invoke_raw(request).await?) + let reply = ensure_command_success(self.invoke_raw(request).await?)?; + ensure_mxaccess_success(reply) } /// Open the server-streaming `StreamEvents` RPC. diff --git a/clients/rust/src/error.rs b/clients/rust/src/error.rs index e7c8124..0c335b9 100644 --- a/clients/rust/src/error.rs +++ b/clients/rust/src/error.rs @@ -9,7 +9,9 @@ use thiserror::Error as ThisError; use tonic::Code; -use crate::generated::mxaccess_gateway::v1::{MxCommandReply, ProtocolStatus, ProtocolStatusCode}; +use crate::generated::mxaccess_gateway::v1::{ + MxCommandReply, MxStatusCategory, ProtocolStatus, ProtocolStatusCode, +}; /// Top-level error type returned by the Rust client wrappers. /// @@ -95,6 +97,15 @@ pub enum Error { #[error("gateway command failed: {0}")] Command(#[from] Box), + /// Gateway accepted the call and returned an `Ok` protocol envelope, but + /// the reply reported an MXAccess-level failure — a negative `hresult` + /// (COM failure semantics) or one or more `MXSTATUS_PROXY` entries that + /// did not indicate success. The wrapped [`MxAccessError`] preserves the + /// full reply so callers can inspect the native status payload. Boxed to + /// keep the containing enum small, matching [`Error::Command`]. + #[error("gateway command reported an MXAccess failure: {0}")] + MxAccess(#[from] Box), + /// Protocol-level operation (open/close session) returned a non-OK /// [`ProtocolStatus`] envelope. #[error("gateway {operation} failed: {code:?}: {message}")] @@ -175,6 +186,72 @@ impl std::fmt::Display for CommandError { impl std::error::Error for CommandError {} +/// Wrapper around an [`MxCommandReply`] whose protocol envelope succeeded but +/// whose MXAccess-level result reported a failure — a negative `hresult` or a +/// non-success `MXSTATUS_PROXY` entry. +/// +/// The wrapper is heap-allocated inside [`Error::MxAccess`] to keep the +/// containing enum small. Callers can recover the reply with +/// [`MxAccessError::reply`] or [`MxAccessError::into_reply`]. Its `Display` +/// summarizes the `hresult` and status entries and scrubs any credential-like +/// tokens from diagnostic text before it reaches a caller. +#[derive(Clone, Debug)] +pub struct MxAccessError { + reply: MxCommandReply, +} + +impl MxAccessError { + /// Wrap a reply whose MXAccess-level result reported a failure. + pub fn new(reply: MxCommandReply) -> Self { + Self { reply } + } + + /// Borrow the underlying reply (correlation id, hresult, statuses). + pub fn reply(&self) -> &MxCommandReply { + &self.reply + } + + /// Consume the error and return the underlying reply. + pub fn into_reply(self) -> MxCommandReply { + self.reply + } +} + +impl std::fmt::Display for MxAccessError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let hresult = match self.reply.hresult { + Some(value) => value.to_string(), + None => "none".to_owned(), + }; + + write!( + formatter, + "hresult={hresult}, {} status entr{}", + self.reply.statuses.len(), + if self.reply.statuses.len() == 1 { + "y" + } else { + "ies" + } + )?; + + for status in &self.reply.statuses { + let category = MxStatusCategory::try_from(status.category) + .unwrap_or(MxStatusCategory::Unspecified); + let diagnostic = redact_credentials(&status.diagnostic_text); + write!( + formatter, + "; [success={}, category={category:?}, detail={}, {}]", + status.success, status.detail, diagnostic + )?; + } + + Ok(()) + } +} + +impl std::error::Error for MxAccessError {} + impl From for Error { fn from(status: tonic::Status) -> Self { let message = redact_credentials(status.message()); @@ -225,6 +302,36 @@ pub fn ensure_command_success(reply: MxCommandReply) -> Result Result { + let hresult_failure = reply.hresult.is_some_and(|hresult| hresult < 0); + let status_failure = reply.statuses.iter().any(|status| status.success == 0); + + if hresult_failure || status_failure { + Err(Box::new(MxAccessError::new(reply)).into()) + } else { + Ok(reply) + } +} + /// Validate a [`ProtocolStatus`] envelope returned by an open/close-session /// reply. /// @@ -271,7 +378,20 @@ fn redact_credentials(message: &str) -> String { mod tests { use tonic::{Code, Status}; - use super::Error; + use super::{ensure_mxaccess_success, Error}; + use crate::generated::mxaccess_gateway::v1::{ + MxCommandReply, MxStatusCategory, MxStatusProxy, ProtocolStatus, ProtocolStatusCode, + }; + + fn ok_reply() -> MxCommandReply { + MxCommandReply { + protocol_status: Some(ProtocolStatus { + code: ProtocolStatusCode::Ok as i32, + message: String::new(), + }), + ..MxCommandReply::default() + } + } #[test] fn classifies_authentication_status() { @@ -286,4 +406,48 @@ mod tests { assert!(message.contains("")); assert!(!message.contains("visible_secret")); } + + #[test] + fn ensure_mxaccess_success_passes_clean_reply() { + let mut reply = ok_reply(); + // Positive hresult (e.g. S_FALSE = 1) is a success, not a failure. + reply.hresult = Some(1); + reply.statuses = vec![MxStatusProxy { + success: 1, + category: MxStatusCategory::Ok as i32, + ..MxStatusProxy::default() + }]; + + assert!(ensure_mxaccess_success(reply).is_ok()); + } + + #[test] + fn ensure_mxaccess_success_flags_failing_status_entry() { + let mut reply = ok_reply(); + reply.statuses = vec![MxStatusProxy { + success: 0, + category: MxStatusCategory::CommunicationError as i32, + detail: 42, + diagnostic_text: "write rejected for mxgw_visible_secret".to_owned(), + ..MxStatusProxy::default() + }]; + + let error = ensure_mxaccess_success(reply).expect_err("failing status must error"); + let message = error.to_string(); + + assert!(matches!(error, Error::MxAccess(_))); + assert!(message.contains("")); + assert!(!message.contains("visible_secret")); + } + + #[test] + fn ensure_mxaccess_success_flags_negative_hresult() { + let mut reply = ok_reply(); + // 0x80004005 (E_FAIL) as a signed 32-bit value. + reply.hresult = Some(-2_147_467_259); + + let error = ensure_mxaccess_success(reply).expect_err("negative hresult must error"); + + assert!(matches!(error, Error::MxAccess(_))); + } } diff --git a/clients/rust/src/lib.rs b/clients/rust/src/lib.rs index 38ffac8..44acce7 100644 --- a/clients/rust/src/lib.rs +++ b/clients/rust/src/lib.rs @@ -26,7 +26,7 @@ pub use auth::{ApiKey, AuthInterceptor}; #[doc(inline)] pub use client::{AlarmFeedStream, EventStream, GatewayClient}; #[doc(inline)] -pub use error::{CommandError, Error}; +pub use error::{CommandError, Error, MxAccessError}; #[doc(inline)] pub use galaxy::{DeployEventStream, GalaxyClient}; #[doc(inline)]