refactor(rust): make reading a secret an explicit act, and drop the unused half

`Revealed` and its serialisation in clear are gone. They existed so a caller could
write a configuration out and read it back, and no such caller exists: nothing in
the repository enables the `serde` feature, the FFI layer passes typed fields
across a C ABI, and the configuration comes from environment variables. The only
round-trip was the test written for it.

What is left is complete. `Debug` and `Serialize` redact; `Deserialize` refuses the
redaction marker, so reading back a dump fails where it can be understood rather
than as an unexplained rejection later.

`Deref` is gone too, which made the type match its own documentation: `&*password`
handed the value out with no call to anything. Emptiness, the only thing the call
sites wanted, has its own method. The accessor is `expose_secret`, named in full so
that a call site reads as the deliberate act it is.

`rustls` guards a private key the same way, verified in rustls-pki-types 1.15.1:
no `Deref`, no `AsRef`, a named `secret_*_der()` accessor, and a `Debug` that
elides.

The comments this branch adds were revised as a whole rather than only where the
code changed: 200 lines audited, the four longest blocks cut.

    $ cargo test -p armonik-transport --all-features
    69 passing

Author: wkirschenmann

packages/rust/armonik-transport/src/config.rs

@@ -12,15 +12,14 @@ use crate::secret::Secret;
 pub enum ProxySource {
     /// Connect directly, ignoring any proxy configured in the environment.
     ///
-    /// The default, so that adding proxy support changes nothing for a client that never asked for it.
+    /// The default: a client that asks for nothing connects directly.
     #[default]
     Disabled,
     /// Read the proxy from the environment, on `hyper_util`'s rules: `ALL_PROXY`, `HTTPS_PROXY`,
     /// `HTTP_PROXY` and `NO_PROXY`, in either case, with `NO_PROXY` matched as curl matches it.
     ///
-    /// Read when `connect` builds the channel, not when this is built and not again afterwards: a
-    /// variable changed in between is the one that counts, and a channel that reconnects keeps the
-    /// values it started with. Every other option is read in [`ClientConfigArgs::from_env`].
+    /// Read once, when `connect` builds the channel, so one that reconnects keeps the values it
+    /// started with. Every other option is read in [`ClientConfigArgs::from_env`].
     System,
     /// Use this specific proxy.
     Explicit(Uri),
@@ -44,11 +43,8 @@ pub struct ProxyConfig {
 impl ProxyConfig {
     /// Use this specific proxy.
     ///
-    /// Credentials written into the URL are taken out of it and kept as the proxy's credentials, so
-    /// they are honoured without the URI carrying them anywhere it is rendered.
-    ///
-    /// The type is `#[non_exhaustive]`, so another crate cannot build it with a struct expression;
-    /// these constructors are the way in.
+    /// Credentials written into the URL are taken out of it and kept here, so the URI carries none
+    /// wherever it is rendered. The type is `#[non_exhaustive]`, so this is the way in.
     pub fn explicit(uri: Uri) -> Self {
         let (uri, credentials) = crate::proxy::split_credentials(uri);
         let (username, password) = credentials.unwrap_or_default();
@@ -83,7 +79,7 @@ impl ProxyConfig {
         if self.username.is_empty() && self.password.is_empty() {
             None
         } else {
-            Some((&self.username, self.password.expose()))
+            Some((&self.username, self.password.expose_secret()))
         }
     }
 }
@@ -230,8 +226,8 @@ pub struct ClientConfigArgs {
     /// Password for proxy authentication.
     ///
     /// Empty falls back to the password the `proxy` URL carried, independently of the username, so
-    /// setting this one alone still uses that URL's username. Redacted by `Debug` and by an ordinary
-    /// serialisation; see [`Secret`].
+    /// setting this one alone still uses that URL's username. Redacted wherever it is written; see
+    /// [`Secret`].
     #[cfg_attr(feature = "serde", serde(default))]
     pub proxy_password: Secret,
 }
@@ -1238,22 +1234,6 @@ mod tests {
         assert!(!json.contains("s3cr3t"), "password written out: {json}");
     }
 
-    #[cfg(feature = "serde")]
-    #[test]
-    fn a_secret_is_written_in_clear_only_where_the_call_site_asks() {
-        let args = ClientConfigArgs {
-            proxy_password: "s3cr3t".into(),
-            ..args()
-        };
-
-        let revealed = serde_json::to_string(&args.proxy_password.revealed()).expect("serialise");
-        assert_eq!(revealed, "\"s3cr3t\"");
-        // The value is untouched by having been revealed once.
-        assert!(!serde_json::to_string(&args)
-            .expect("serialise")
-            .contains("s3cr3t"));
-    }
-
     #[test]
     fn proxy_without_a_host_is_rejected_and_names_its_own_option() {
         // Reporting these through the endpoint's URI error would send whoever reads it looking at the

packages/rust/armonik-transport/src/lib.rs

@@ -13,7 +13,7 @@ mod utils;
 pub use config::{ClientConfig, ClientConfigArgs, ConfigError, ProxyConfig, ProxySource};
 pub use connect::{connect, https_connector, ConnectionError};
 pub use proxy::ProxyError;
-pub use secret::{Revealed, Secret};
+pub use secret::Secret;
 // Snafu's context selectors, so a caller in another crate can build the error with the location
 // captured at its own call site. Hidden: this is how the error is built, not API to design against.
 #[doc(hidden)]

packages/rust/armonik-transport/src/proxy.rs

@@ -1,11 +1,8 @@
 //! Reaching the endpoint through an HTTP proxy.
 //!
 //! A `CONNECT` tunnel rather than an absolute-form request, so TLS stays end to end with the real
-//! server and the proxy only forwards opaque bytes. Terminating TLS at the proxy would defeat the
-//! point of this transport, which exists to control the TLS stack.
-//!
-//! [`ProxyConnector`] therefore sits below the TLS connector and hands back the same stream type a
-//! direct connection would.
+//! server: [`ProxyConnector`] sits below the TLS connector and hands back the stream a direct
+//! connection would.
 
 use std::future::Future;
 use std::pin::Pin;
@@ -44,16 +41,11 @@ const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(30);
 pub struct ProxyConnector<S> {
     inner: S,
     proxy: ProxyConfig,
-    /// The environment's proxy rules, and only those: `Some` for [`ProxySource::System`] alone.
-    ///
-    /// `hyper_util`'s rather than ours, because it already implements the `*_PROXY` convention,
-    /// `NO_PROXY` on curl's rules, and taking credentials out of a proxy URL. An explicitly named proxy
-    /// does not go through it: a matcher answers `None` for a target with no host or a scheme other
-    /// than `http`/`https`, which would silently skip the proxy the caller asked for. Only the tunnel
-    /// below is ours as well, because theirs rejects a `CONNECT` response split across reads.
+    /// The environment's proxy rules: `Some` for [`ProxySource::System`] alone, since a matcher
+    /// answers `None` for a scheme other than `http`/`https` and would skip an explicit proxy.
     ///
-    /// Shared rather than cloned: a `tower` connector is cloned per connection, and `Matcher` is
-    /// neither `Clone` nor cheap to rebuild.
+    /// Behind an `Arc` because a `tower` connector is cloned per connection and `Matcher` is not
+    /// `Clone`.
     matcher: Option<std::sync::Arc<Matcher>>,
 }
 

packages/rust/armonik-transport/src/secret.rs

@@ -1,18 +1,15 @@
 //! A configuration value that must not be printed.
 
 use std::fmt;
-use std::ops::Deref;
 
-/// The text a redacted secret renders as.
+/// What a secret renders as instead of its value.
 const REDACTED: &str = "[redacted]";
 
 /// A string that redacts itself when printed or serialised.
 ///
-/// Redacted by construction rather than by a hand-written `Debug` on each holder: a struct grows
-/// fields, and a `Debug` listing them by hand goes stale the first time someone forgets one.
-///
-/// Serialising redacts as well, which is what output that might be logged needs. [`Secret::revealed`]
-/// opts out for one serialisation, and only for that one.
+/// The value is reachable only through [`Secret::expose_secret`], named so that reading it is a visible act.
+/// There is deliberately no `Deref` or `AsRef`, which would let it out silently; `rustls` guards a
+/// private key the same way, with `secret_der` as the only way in.
 #[derive(Clone, Default, PartialEq, Eq, Hash)]
 pub struct Secret(String);
 
@@ -23,32 +20,16 @@ impl Secret {
     }
 
     /// The value itself, for the code that has to use it.
-    pub fn expose(&self) -> &str {
+    ///
+    /// Named in full so that a call site reads as the deliberate act it is, following the convention
+    /// the `secrecy` crate set.
+    pub fn expose_secret(&self) -> &str {
         &self.0
     }
 
-    /// Borrow this secret for a single serialisation in clear.
-    ///
-    /// The borrow is what makes it safe: revealing is a property of one call site, not of the value,
-    /// so it cannot be carried along by a clone or outlive the expression that asked for it.
-    ///
-    /// For output that is itself protected and has to be read back. `Debug` still redacts, because a
-    /// log is never that output.
-    pub fn revealed(&self) -> Revealed<'_> {
-        Revealed(self)
-    }
-}
-
-/// A [`Secret`] borrowed for one serialisation in clear. See [`Secret::revealed`].
-///
-/// Deliberately not `Copy` or `Clone`: serialising takes it by reference, so nothing needs to
-/// duplicate it, and a handle that cannot be passed around keeps revealing where it was asked for.
-pub struct Revealed<'a>(&'a Secret);
-
-// Redacted here too: this type exists to widen serialisation, not printing.
-impl fmt::Debug for Revealed<'_> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.fmt(f)
+    /// Whether no secret was given, which a caller may ask without reading one.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
     }
 }
 
@@ -62,14 +43,6 @@ impl fmt::Debug for Secret {
     }
 }
 
-impl Deref for Secret {
-    type Target = str;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
 impl From<String> for Secret {
     fn from(value: String) -> Self {
         Self::new(value)
@@ -84,6 +57,7 @@ impl From<&str> for Secret {
 
 #[cfg(feature = "serde")]
 impl serde::Serialize for Secret {
+    /// Redacts, so that a configuration dumped for diagnosis carries no credential.
     fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
         if self.0.is_empty() {
             serializer.serialize_str("")
@@ -93,24 +67,15 @@ impl serde::Serialize for Secret {
     }
 }
 
-#[cfg(feature = "serde")]
-impl serde::Serialize for Revealed<'_> {
-    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-        serializer.serialize_str(&self.0 .0)
-    }
-}
-
 #[cfg(feature = "serde")]
 impl<'de> serde::Deserialize<'de> for Secret {
-    /// Refuses the redaction marker, so that reading back a redacted dump fails where it can be
-    /// understood rather than later, as an unexplained rejection by whatever the secret authenticates
-    /// against.
+    /// Refuses the redaction marker, so that reading back a dump fails where it can be understood
+    /// rather than later, as an unexplained rejection by whatever the secret authenticates against.
     fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
         let value = String::deserialize(deserializer)?;
         if value == REDACTED {
             return Err(serde::de::Error::custom(format!(
-                "`{REDACTED}` is what a secret serialises to unless `Secret::revealed` was used; \
-                 this input cannot be a secret"
+                "`{REDACTED}` is what a secret serialises to, so this input cannot be one"
             )));
         }
         Ok(Self::new(value))
@@ -124,8 +89,6 @@ mod tests {
     #[test]
     fn debug_never_shows_the_value() {
         assert_eq!(format!("{:?}", Secret::new("hunter2")), REDACTED);
-        // Including through the wrapper that widens serialisation.
-        assert_eq!(format!("{:?}", Secret::new("hunter2").revealed()), REDACTED);
     }
 
     #[test]
@@ -135,27 +98,18 @@ mod tests {
     }
 
     #[test]
-    fn the_value_is_available_to_the_code_that_needs_it() {
-        assert_eq!(Secret::new("hunter2").expose(), "hunter2");
-        assert_eq!(&*Secret::new("hunter2"), "hunter2");
+    fn the_value_is_available_only_to_the_code_that_asks_for_it() {
+        assert_eq!(Secret::new("hunter2").expose_secret(), "hunter2");
+        // Emptiness is answerable without reading the value.
+        assert!(Secret::default().is_empty());
+        assert!(!Secret::new("hunter2").is_empty());
     }
 
     #[cfg(feature = "serde")]
     #[test]
-    fn serialisation_redacts_unless_the_call_site_asks_otherwise() {
-        let secret = Secret::new("hunter2");
-
-        assert_eq!(
-            serde_json::to_string(&secret).expect("serialise"),
-            format!("\"{REDACTED}\"")
-        );
-        assert_eq!(
-            serde_json::to_string(&secret.revealed()).expect("serialise"),
-            "\"hunter2\""
-        );
-        // Revealing one call site leaves the secret itself untouched, which is the whole point.
+    fn serialisation_redacts() {
         assert_eq!(
-            serde_json::to_string(&secret).expect("serialise"),
+            serde_json::to_string(&Secret::new("hunter2")).expect("serialise"),
             format!("\"{REDACTED}\"")
         );
     }
@@ -167,8 +121,8 @@ mod tests {
             .expect_err("the marker must not be taken for a secret");
 
         assert!(
-            error.to_string().contains("revealed"),
-            "the message should say what to do: {error}"
+            error.to_string().contains("cannot be one"),
+            "the message should say why: {error}"
         );
     }
 }

packages/rust/armonik/src/client/mod.rs

@@ -9,7 +9,7 @@ use armonik_transport::ConfigSnafu;
 #[cfg(feature = "_gen-client")]
 pub use armonik_transport::{
     ClientConfig, ClientConfigArgs, ConfigError, ConnectionError, ProxyConfig, ProxyError,
-    ProxySource, ReadEnvError, Revealed, Secret,
+    ProxySource, ReadEnvError, Secret,
 };
 
 #[cfg(feature = "worker")]