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.
This commit is contained in:
@@ -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
|
protocol status is not `PROTOCOL_STATUS_CODE_OK` become `Error::Command` and
|
||||||
retain the raw `MxCommandReply`.
|
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
|
## Write Semantics And Common Pitfalls
|
||||||
|
|
||||||
These are MXAccess parity behaviors that surprise new callers. The gateway
|
These are MXAccess parity behaviors that surprise new callers. The gateway
|
||||||
|
|||||||
@@ -11,7 +11,9 @@ use tonic::transport::Channel;
|
|||||||
use tonic::Request;
|
use tonic::Request;
|
||||||
|
|
||||||
use crate::auth::AuthInterceptor;
|
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::mx_access_gateway_client::MxAccessGatewayClient;
|
||||||
use crate::generated::mxaccess_gateway::v1::{
|
use crate::generated::mxaccess_gateway::v1::{
|
||||||
AcknowledgeAlarmReply, AcknowledgeAlarmRequest, ActiveAlarmSnapshot, AlarmFeedMessage,
|
AcknowledgeAlarmReply, AcknowledgeAlarmRequest, ActiveAlarmSnapshot, AlarmFeedMessage,
|
||||||
@@ -166,16 +168,24 @@ impl GatewayClient {
|
|||||||
Ok(response.into_inner())
|
Ok(response.into_inner())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Issue an `Invoke` RPC and surface a non-OK reply as
|
/// Issue an `Invoke` RPC and surface a failing reply as a typed error.
|
||||||
/// [`Error::Command`].
|
///
|
||||||
|
/// 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
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// Returns [`Error::Command`] when the reply's `protocol_status` is not
|
/// 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).
|
/// [`invoke_raw`](Self::invoke_raw).
|
||||||
pub async fn invoke(&self, request: MxCommandRequest) -> Result<MxCommandReply, Error> {
|
pub async fn invoke(&self, request: MxCommandRequest) -> Result<MxCommandReply, Error> {
|
||||||
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.
|
/// Open the server-streaming `StreamEvents` RPC.
|
||||||
|
|||||||
+166
-2
@@ -9,7 +9,9 @@
|
|||||||
use thiserror::Error as ThisError;
|
use thiserror::Error as ThisError;
|
||||||
use tonic::Code;
|
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.
|
/// Top-level error type returned by the Rust client wrappers.
|
||||||
///
|
///
|
||||||
@@ -95,6 +97,15 @@ pub enum Error {
|
|||||||
#[error("gateway command failed: {0}")]
|
#[error("gateway command failed: {0}")]
|
||||||
Command(#[from] Box<CommandError>),
|
Command(#[from] Box<CommandError>),
|
||||||
|
|
||||||
|
/// 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<MxAccessError>),
|
||||||
|
|
||||||
/// Protocol-level operation (open/close session) returned a non-OK
|
/// Protocol-level operation (open/close session) returned a non-OK
|
||||||
/// [`ProtocolStatus`] envelope.
|
/// [`ProtocolStatus`] envelope.
|
||||||
#[error("gateway {operation} failed: {code:?}: {message}")]
|
#[error("gateway {operation} failed: {code:?}: {message}")]
|
||||||
@@ -175,6 +186,72 @@ impl std::fmt::Display for CommandError {
|
|||||||
|
|
||||||
impl std::error::Error 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<tonic::Status> for Error {
|
impl From<tonic::Status> for Error {
|
||||||
fn from(status: tonic::Status) -> Self {
|
fn from(status: tonic::Status) -> Self {
|
||||||
let message = redact_credentials(status.message());
|
let message = redact_credentials(status.message());
|
||||||
@@ -225,6 +302,36 @@ pub fn ensure_command_success(reply: MxCommandReply) -> Result<MxCommandReply, E
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Promote an MXAccess-level failure carried inside an otherwise-successful
|
||||||
|
/// [`MxCommandReply`] to an [`Error::MxAccess`].
|
||||||
|
///
|
||||||
|
/// This is the second reply check applied to the typed command path, after
|
||||||
|
/// [`ensure_command_success`] confirms the protocol envelope is `Ok`. It
|
||||||
|
/// enforces MXAccess parity: a reply can carry an `Ok` protocol envelope while
|
||||||
|
/// MXAccess itself rejected the operation. Following COM semantics (and the
|
||||||
|
/// Python client), only a **negative** `hresult` is a failure — positive codes
|
||||||
|
/// such as `S_FALSE = 1` are success. A `MXSTATUS_PROXY` entry is treated as a
|
||||||
|
/// failure when its `success` member is `0`.
|
||||||
|
///
|
||||||
|
/// Per-item bulk failures are reported inside each result entry
|
||||||
|
/// (`was_successful = false`) rather than in the top-level `hresult`/`statuses`
|
||||||
|
/// fields, so this check does not trip on them — matching the other clients.
|
||||||
|
///
|
||||||
|
/// # Errors
|
||||||
|
///
|
||||||
|
/// Returns [`Error::MxAccess`] when `reply.hresult` is negative or any
|
||||||
|
/// `reply.statuses` entry reports a non-success `success` member.
|
||||||
|
pub fn ensure_mxaccess_success(reply: MxCommandReply) -> Result<MxCommandReply, Error> {
|
||||||
|
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
|
/// Validate a [`ProtocolStatus`] envelope returned by an open/close-session
|
||||||
/// reply.
|
/// reply.
|
||||||
///
|
///
|
||||||
@@ -271,7 +378,20 @@ fn redact_credentials(message: &str) -> String {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use tonic::{Code, Status};
|
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]
|
#[test]
|
||||||
fn classifies_authentication_status() {
|
fn classifies_authentication_status() {
|
||||||
@@ -286,4 +406,48 @@ mod tests {
|
|||||||
assert!(message.contains("<redacted>"));
|
assert!(message.contains("<redacted>"));
|
||||||
assert!(!message.contains("visible_secret"));
|
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("<redacted>"));
|
||||||
|
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(_)));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ pub use auth::{ApiKey, AuthInterceptor};
|
|||||||
#[doc(inline)]
|
#[doc(inline)]
|
||||||
pub use client::{AlarmFeedStream, EventStream, GatewayClient};
|
pub use client::{AlarmFeedStream, EventStream, GatewayClient};
|
||||||
#[doc(inline)]
|
#[doc(inline)]
|
||||||
pub use error::{CommandError, Error};
|
pub use error::{CommandError, Error, MxAccessError};
|
||||||
#[doc(inline)]
|
#[doc(inline)]
|
||||||
pub use galaxy::{DeployEventStream, GalaxyClient};
|
pub use galaxy::{DeployEventStream, GalaxyClient};
|
||||||
#[doc(inline)]
|
#[doc(inline)]
|
||||||
|
|||||||
Reference in New Issue
Block a user