diff --git a/clients/rust/README.md b/clients/rust/README.md index 119b36f..963f915 100644 --- a/clients/rust/README.md +++ b/clients/rust/README.md @@ -78,7 +78,8 @@ cargo run -p mxgw-cli -- add-item --session-id --server-handle 1 -- cargo run -p mxgw-cli -- advise --session-id --server-handle 1 --item-handle 1 --json cargo run -p mxgw-cli -- stream-events --session-id --max-events 1 --json cargo run -p mxgw-cli -- stream-alarms --session-id --max-messages 4 --json -cargo run -p mxgw-cli -- acknowledge-alarm --session-id --alarm-reference "\\Galaxy\Area001.Pump001.PumpFault" --json +cargo run -p mxgw-cli -- acknowledge-alarm --reference "Area001.Pump001.PumpFault" --operator [--operator-domain ] --json +# operator password: MXGATEWAY_OPERATOR_PASSWORD, or --operator-password-env / --operator-password cargo run -p mxgw-cli -- write --session-id --server-handle 1 --item-handle 1 --value-type int32 --value 123 --json ``` @@ -124,6 +125,9 @@ creates an authenticated `tonic` client and attaches `authorization: Bearer `stream_alarms`, `acknowledge_alarm`, and `raw_client`. `stream_alarms` returns an `AlarmFeedStream` async stream of alarm-feed messages and shares the gateway's central alarm monitor with every other client. +`acknowledge_alarm` requires `operator_user` and carries `operator_password` (may be +empty) and `operator_domain`; the gateway authenticates as that operator so the Historian +records them. The password is scrubbed from any returned error's `Display`/`Debug`. The `from_truncated_snapshot` field on `ActiveAlarmSnapshot` reports that the record came from a provider fetch which hit the per-fetch cap: the snapshot set may omit active diff --git a/clients/rust/RustClientDesign.md b/clients/rust/RustClientDesign.md index 13056cd..140692f 100644 --- a/clients/rust/RustClientDesign.md +++ b/clients/rust/RustClientDesign.md @@ -174,6 +174,8 @@ alarm monitor — no worker session is opened — so any number of clients may attach. Dropping the stream cancels the gRPC call cooperatively. `acknowledge_alarm` is idempotent at the MxAccess layer; the returned `AcknowledgeAlarmReply` carries the native MxStatus from the worker. +`operator_user` is required; `operator_password` may be empty and is scrubbed from any +returned error via `Error::redact_secrets`. ## Galaxy Repository @@ -346,7 +348,7 @@ mxgw write-secured-bulk ... mxgw write-secured2-bulk ... mxgw stream-events --session-id --json mxgw stream-alarms [--filter-prefix ] [--max-events ] -mxgw acknowledge-alarm --reference [--comment ] [--operator ] +mxgw acknowledge-alarm --reference [--comment ] --operator [--operator-domain ] [--operator-password-env =MXGATEWAY_OPERATOR_PASSWORD] [--operator-password ] mxgw bench-read-bulk [--duration-seconds ] [--warmup-seconds ] [--bulk-size ] mxgw smoke --endpoint http://localhost:5000 --api-key-env MXGATEWAY_API_KEY --item TestChildObject.TestInt mxgw batch diff --git a/clients/rust/crates/mxgw-cli/src/main.rs b/clients/rust/crates/mxgw-cli/src/main.rs index 41393ec..075f0a3 100644 --- a/clients/rust/crates/mxgw-cli/src/main.rs +++ b/clients/rust/crates/mxgw-cli/src/main.rs @@ -376,6 +376,15 @@ enum Command { comment: String, #[arg(long, default_value = "")] operator: String, + /// Galaxy domain of the operator; empty authenticates the bare user name. + #[arg(long, default_value = "")] + operator_domain: String, + /// Operator password (prefer `--operator-password-env`; empty is allowed). + #[arg(long)] + operator_password: Option, + /// Environment variable holding the operator password. + #[arg(long, default_value = "MXGATEWAY_OPERATOR_PASSWORD")] + operator_password_env: String, #[arg(long)] json: bool, }, @@ -1081,8 +1090,16 @@ async fn dispatch(command: Command) -> Result<(), Error> { reference, comment, operator, + operator_domain, + operator_password, + operator_password_env, json, } => { + // An empty password is allowed (Galaxy accounts may have one), so + // an unset flag and env resolve to an empty string. + let operator_password = operator_password + .or_else(|| env::var(&operator_password_env).ok()) + .unwrap_or_default(); let client = connect(connection).await?; let reply = client .acknowledge_alarm(AcknowledgeAlarmRequest { @@ -1090,6 +1107,8 @@ async fn dispatch(command: Command) -> Result<(), Error> { alarm_full_reference: reference, comment, operator_user: operator, + operator_password, + operator_domain, }) .await?; print_acknowledge_alarm_reply(&reply, json); diff --git a/clients/rust/src/client.rs b/clients/rust/src/client.rs index aa3e9b1..0388915 100644 --- a/clients/rust/src/client.rs +++ b/clients/rust/src/client.rs @@ -327,6 +327,9 @@ impl GatewayClient { /// MXAccess session; the resulting native MxStatus is returned in the /// reply. Acks are idempotent at the MxAccess layer. /// + /// `operator_user` is required; `operator_password` may be empty. The + /// password is scrubbed from any returned error's `Display`/`Debug`. + /// /// # Errors /// /// Returns [`Error::ProtocolStatus`] when the gateway accepts the call but @@ -336,13 +339,19 @@ impl GatewayClient { &self, request: AcknowledgeAlarmRequest, ) -> Result { + let secret = request.operator_password.clone(); let mut client = self.inner.clone(); - let response = client - .acknowledge_alarm(self.unary_request(request)) - .await?; - let reply = response.into_inner(); - ensure_protocol_success("acknowledge alarm", reply.protocol_status.as_ref())?; - Ok(reply) + let result = async { + let response = client + .acknowledge_alarm(self.unary_request(request)) + .await?; + let reply = response.into_inner(); + ensure_protocol_success("acknowledge alarm", reply.protocol_status.as_ref())?; + Ok(reply) + } + .await; + // The operator password must never surface in a returned error. + result.map_err(|error: Error| error.redact_secrets(&[secret.as_str()])) } /// Open the server-streaming `QueryActiveAlarms` RPC — the gateway's diff --git a/clients/rust/src/error.rs b/clients/rust/src/error.rs index 80654c0..84a6782 100644 --- a/clients/rust/src/error.rs +++ b/clients/rust/src/error.rs @@ -314,6 +314,68 @@ impl std::fmt::Display for MxAccessError { impl std::error::Error for MxAccessError {} +impl Error { + /// Scrub exact caller-supplied secrets (for example an operator password) + /// from the error's rendered text and any wrapped `tonic::Status`, so that + /// neither `Display` nor `Debug` can reveal them. Empty secrets are ignored. + #[must_use] + pub fn redact_secrets(self, secrets: &[&str]) -> Self { + let scrub = |text: &str| { + let mut out = text.to_owned(); + for secret in secrets.iter().filter(|secret| !secret.is_empty()) { + out = out.replace(*secret, ""); + } + out + }; + let scrub_status = |status: Box| { + Box::new(tonic::Status::new(status.code(), scrub(status.message()))) + }; + match self { + Self::Authentication { message, status } => Self::Authentication { + message: scrub(&message), + status: scrub_status(status), + }, + Self::Authorization { message, status } => Self::Authorization { + message: scrub(&message), + status: scrub_status(status), + }, + Self::Timeout { message, status } => Self::Timeout { + message: scrub(&message), + status: scrub_status(status), + }, + Self::Cancelled { message, status } => Self::Cancelled { + message: scrub(&message), + status: scrub_status(status), + }, + Self::Unavailable { message, status } => Self::Unavailable { + message: scrub(&message), + status: scrub_status(status), + }, + Self::Status(status) => Self::Status(scrub_status(status)), + Self::ProtocolStatus { + operation, + code, + message, + } => Self::ProtocolStatus { + operation, + code, + message: scrub(&message), + }, + Self::MalformedReply { detail } => Self::MalformedReply { + detail: scrub(&detail), + }, + Self::InvalidArgument { name, detail } => Self::InvalidArgument { + name, + detail: scrub(&detail), + }, + Self::MxAccess(error) => Self::MxAccess(Box::new( + error.with_secrets(secrets.iter().map(|secret| (*secret).to_owned()).collect()), + )), + other => other, + } + } +} + impl From for Error { fn from(status: tonic::Status) -> Self { let message = redact_credentials(status.message()); @@ -461,6 +523,14 @@ fn redact_credentials(message: &str) -> String { #[cfg(test)] mod tests { + #[test] + fn redact_secrets_scrubs_status_display_and_debug() { + let error: super::Error = tonic::Status::internal("boom pw-secret").into(); + let error = error.redact_secrets(&["pw-secret", ""]); + assert!(!format!("{error:?}").contains("pw-secret")); + assert!(!error.to_string().contains("pw-secret")); + } + use tonic::{Code, Status}; use super::{ensure_mxaccess_success, Error}; diff --git a/clients/rust/tests/client_behavior.rs b/clients/rust/tests/client_behavior.rs index 4b0dba3..1fbeac8 100644 --- a/clients/rust/tests/client_behavior.rs +++ b/clients/rust/tests/client_behavior.rs @@ -229,6 +229,8 @@ async fn acknowledge_alarm_returns_reply_with_native_status() { alarm_full_reference: "Tank01.Level.HiHi".to_owned(), comment: "investigating".to_owned(), operator_user: "alice".to_owned(), + operator_password: String::new(), + operator_domain: String::new(), }) .await .unwrap(); @@ -1036,6 +1038,30 @@ async fn stream_alarms_emits_snapshot_then_complete_then_transition_in_order() { )); } +#[tokio::test] +async fn acknowledge_alarm_error_does_not_leak_operator_password() { + let state = Arc::new(FakeState::default()); + let endpoint = spawn_fake_gateway(state.clone()).await; + let client = GatewayClient::connect(ClientOptions::new(endpoint)) + .await + .unwrap(); + + let err = client + .acknowledge_alarm(AcknowledgeAlarmRequest { + client_correlation_id: "corr-pw".to_owned(), + alarm_full_reference: "Tank01.Level.HiHi".to_owned(), + comment: String::new(), + operator_user: "alice".to_owned(), + operator_password: "pw-secret".to_owned(), + operator_domain: String::new(), + }) + .await + .unwrap_err(); + + assert!(!format!("{err:?}").contains("pw-secret")); + assert!(!err.to_string().contains("pw-secret")); +} + #[tokio::test] async fn cli_subcommands_propagate_unique_correlation_ids_from_next_correlation_id() { // The CLI's `stream-alarms` and `acknowledge-alarm` paths used to @@ -1073,6 +1099,8 @@ async fn cli_subcommands_propagate_unique_correlation_ids_from_next_correlation_ alarm_full_reference: "Tank01.Level.HiHi".to_owned(), comment: String::new(), operator_user: String::new(), + operator_password: String::new(), + operator_domain: String::new(), }) .await .unwrap(); @@ -1424,8 +1452,11 @@ impl MxAccessGateway for FakeGateway { &self, request: Request, ) -> Result, Status> { - *self.state.last_correlation_id.lock().await = - Some(request.into_inner().client_correlation_id); + let request = request.into_inner(); + if request.operator_password == "pw-secret" { + return Err(Status::internal("boom pw-secret")); + } + *self.state.last_correlation_id.lock().await = Some(request.client_correlation_id); Ok(Response::new(AcknowledgeAlarmReply { correlation_id: "corr-1".to_owned(), protocol_status: Some(ok_status("ack ok")),