docs(connectors): fix the inverted serde_secret redaction claim

.claude/skills/connectors-overview/SKILL.md

@@ -96,14 +96,24 @@ The connectors codebase is intentionally repetitive across plugins. Cross-plugin
 
 ### Secrets
 
-Any credential-bearing field (connection strings, API keys, bearer tokens, AWS keys) must be `SecretString` from the `secrecy` crate, with the workspace serde wrapper applied so `Debug` and serialization both redact. Runtime exposes plugin configs over the `/stats` HTTP surface via serialization - plain `String` leaks the secret to anyone who can hit the endpoint. Plain `String` for a credential is a review-blocker. Pattern (from `sinks/postgres_sink/src/lib.rs::PostgresSinkConfig`):
+Any credential-bearing field (connection strings, API keys, bearer tokens, AWS keys) must be `SecretString` from the `secrecy` crate. Plain `String` for a credential is a review-blocker: `SecretString` redacts on `Debug`, so it is what keeps a credential out of a log line that formats the whole config.
+
+**`serde_secret::serialize_secret` EXPOSES the secret. It does not redact.** It calls `expose_secret()` and writes the plaintext. `SecretString` deliberately has no `Serialize` impl, and that absence is the protection - so adding `serialize_with` is what *unblocks* the derive and turns a compile-time guarantee into plaintext output. Use it only where the plaintext is the point: a wire payload, a persisted config, an API response that exposes credentials by design.
+
+So the default for a plugin config struct is **derive `Deserialize`, but not `Serialize`**. `Deserialize` is required: the SDK glue deserializes the config into the plugin's own struct (`sdk/src/{sink,source}.rs` call `serde_json::from_str::<C>` under a `DeserializeOwned` bound).
+
+What never happens is the return trip. The runtime holds plugin configuration as a `serde_json::Value` - parsed from TOML, posted as JSON to the control API, or injected by env var - and hands that across the FFI, so nothing re-serializes the plugin's struct. Leaving `Serialize` off makes that compiler-enforced instead of convention-enforced (`sources/http_source/src/lib.rs::HttpSourceConfig` does this, and comments the omission so nobody adds it back).
+
+Pattern:
 
 ```rust
 use secrecy::{ExposeSecret, SecretString};
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+// `Deserialize` only. Nothing re-serializes a plugin config, and leaving
+// `Serialize` off is what makes the credential unserializable rather than
+// merely un-serialized.
+#[derive(Debug, Clone, Deserialize)]
 pub struct MyConfig {
-    #[serde(serialize_with = "iggy_common::serde_secret::serialize_secret")]
     pub connection_string: SecretString,
 }
 
@@ -113,7 +123,13 @@ let pool = PgPoolOptions::new()
     .await?;
 ```
 
-In-tree uses: `sinks/{postgres,mongodb,elasticsearch,influxdb,delta}_sink`, `sources/{postgres,elasticsearch,influxdb}_source`.
+If a config struct genuinely needs `Serialize`, `serde_secret::serialize_redacted` (and `serialize_optional_redacted`) write `[REDACTED]` in place of the value. Reach for `serialize_secret` only when the caller must get the real thing back. The sinks and sources listed below predate that helper and use the exposing one; the annotation is inert today, but it is not the protection it looks like.
+
+Note that none of this protects the credential from the runtime's own control API, which returns plugin configuration verbatim - see #3802. Plugin-side annotations are inert there because the runtime never routes through them.
+
+Plugin-side uses of the exposing helpers: `sinks/{postgres,mongodb,elasticsearch,influxdb,s3,surrealdb}_sink`, `sources/{postgres,elasticsearch,influxdb}_source`.
+
+That list is plugin-side only, not an inventory of every caller in the tree, and the others are not all mistakes: `runtime/src/api/config.rs` puts `serialize_secret` on `HttpConfig::api_key` (inert for the same reason), and several `core/common` wire-payload types (login, create-user, change-password, PAT) use these helpers by design, because there the credential *is* the payload.
 
 ### Errors
 
@@ -193,7 +209,7 @@ Each implemented in at least one in-tree plugin or runtime path.
 | `flume::unbounded()` channel                                            | `runtime/src/source.rs::spawn_source_handler` / `source_forwarding_loop`           | MPSC handoff from SDK async task to runtime loop  |
 | `tokio::sync::watch::channel(())`                                       | `sdk/src/{sink,source}.rs`, `runtime/src/sink.rs`, `runtime/src/manager/*`         | One-shot shutdown broadcast                       |
 | `dashmap::DashMap`                                                      | `runtime/src/manager/sink.rs`, `source.rs::SOURCE_SENDERS`, SDK `INSTANCES`        | Lock-free concurrent keyed access                 |
-| `secrecy::SecretString` + `iggy_common::serde_secret::serialize_secret` | `sinks/postgres_sink::PostgresSinkConfig::connection_string`                       | Auto-redact on Debug/Display + serialization      |
+| `secrecy::SecretString` + `iggy_common::serde_secret::serialize_secret` | `sinks/postgres_sink::PostgresSinkConfig::connection_string`                       | `Debug` redacts; `serialize_secret` EXPOSES       |
 
 ## Drop accounting
 

core/common/src/utils/serde_secret.rs

@@ -17,22 +17,42 @@
 
 //! Serde serialization helpers for `SecretString` fields.
 //!
-//! `SecretString` intentionally does not implement `Serialize` to prevent
-//! accidental secret exposure. These helpers are for fields that **must** be
-//! serialized (e.g., wire protocol payloads, persisted TOML configs, API
-//! responses that already expose credentials by design).
+//! `SecretString` intentionally does not implement `Serialize`, and that
+//! absence is the protection: a struct holding one cannot derive `Serialize`
+//! at all. Adding `serialize_with` is therefore what *unblocks* the derive, so
+//! reaching for a helper here is a decision to serialize a credential, never a
+//! way to avoid it.
+//!
+//! [`serialize_secret`] and [`serialize_optional_secret`] write the plaintext.
+//! Use them only where the plaintext is the point: wire protocol payloads,
+//! persisted configs, API responses that expose credentials by design.
 //!
-//! Usage:
 //! ```ignore
 //! #[serde(serialize_with = "crate::utils::serde_secret::serialize_secret")]
 //! pub password: SecretString,
 //! ```
 //!
-//! Do **not** add `serialize_with` to fields that should remain redacted in
-//! serialized output — rely on `SecretString`'s default behavior instead.
+//! [`serialize_redacted`] and [`serialize_optional_redacted`] write
+//! [`REDACTED`] in place of the value, for a struct that must be serializable
+//! for unrelated reasons but whose credential no reader is entitled to.
+//!
+//! **Redacted output is not a config.** Deserializing it hands back the literal
+//! [`REDACTED`] as the secret, silently, so a redact-then-reload round trip
+//! replaces the credential with the placeholder instead of failing. Nothing
+//! in-tree can reach that today: these helpers have no consumers, and the one
+//! persist/reload path round-trips a raw `serde_json::Value` rather than a
+//! typed struct. If a consumer ever needs the round trip closed mechanically,
+//! the shape that cannot be half-applied is a newtype owning both directions,
+//! not a paired `deserialize_with` that a caller can forget to add.
+//!
+//! If neither applies, leave `serialize_with` off and let the missing impl keep
+//! the field unserializable.
 
 use secrecy::{ExposeSecret, SecretString};
 
+/// Placeholder written in place of a redacted secret.
+pub const REDACTED: &str = "[REDACTED]";
+
 pub fn serialize_secret<S: serde::Serializer>(
     secret: &SecretString,
     serializer: S,
@@ -50,6 +70,28 @@ pub fn serialize_optional_secret<S: serde::Serializer>(
     }
 }
 
+/// Writes [`REDACTED`] instead of the secret.
+pub fn serialize_redacted<S: serde::Serializer>(
+    _secret: &SecretString,
+    serializer: S,
+) -> Result<S::Ok, S::Error> {
+    serializer.serialize_str(REDACTED)
+}
+
+/// Writes [`REDACTED`] instead of the secret, keeping `None` distinguishable.
+///
+/// Whether a credential is configured at all is not itself a secret, and
+/// collapsing `Some` to `null` would tell a reader the field is unset.
+pub fn serialize_optional_redacted<S: serde::Serializer>(
+    secret: &Option<SecretString>,
+    serializer: S,
+) -> Result<S::Ok, S::Error> {
+    match secret {
+        Some(_) => serializer.serialize_some(REDACTED),
+        None => serializer.serialize_none(),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -101,4 +143,43 @@ mod tests {
         let json = serde_json::to_string(&s).unwrap();
         assert_eq!(json, r#"{"token":null}"#);
     }
+
+    #[derive(Serialize)]
+    struct WithRedactedSecret {
+        #[serde(serialize_with = "serialize_redacted")]
+        password: SecretString,
+    }
+
+    #[derive(Serialize)]
+    struct WithOptionalRedactedSecret {
+        #[serde(serialize_with = "serialize_optional_redacted")]
+        token: Option<SecretString>,
+    }
+
+    #[test]
+    fn serialize_redacted_replaces_value_in_json() {
+        let s = WithRedactedSecret {
+            password: SecretString::from("my_password"),
+        };
+        let json = serde_json::to_string(&s).unwrap();
+        assert_eq!(json, r#"{"password":"[REDACTED]"}"#);
+        assert!(!json.contains("my_password"));
+    }
+
+    #[test]
+    fn serialize_optional_redacted_keeps_some_distinguishable_from_none() {
+        let present = WithOptionalRedactedSecret {
+            token: Some(SecretString::from("tok_123")),
+        };
+        let absent = WithOptionalRedactedSecret { token: None };
+
+        let present_json = serde_json::to_string(&present).unwrap();
+        assert_eq!(present_json, r#"{"token":"[REDACTED]"}"#);
+        assert!(!present_json.contains("tok_123"));
+        assert_eq!(
+            serde_json::to_string(&absent).unwrap(),
+            r#"{"token":null}"#,
+            "a configured credential must not read as an unset one"
+        );
+    }
 }