fix(database): keep the HA switchover credential out of curl's argv

src/controllers/cluster_probe.rs

@@ -134,22 +134,44 @@ pub async fn probe_nodes(
 
 /// Credential for the node's health server, resolved INSIDE the container the
 /// way the image resolves it: `HEALTH_API_PASSWORD` gates the mutating routes
-/// (blank = open) and `HEALTH_API_USERNAME` defaults to `railway`. Leaves `$@`
-/// holding curl's `-u user:pass`, or nothing when the node carries no password
-/// -- an open node ignores the header and an enforcing one requires it, so one
-/// command spans a cluster mid-rollout, and no secret enters the exec payload,
-/// this process, or a log line.
+/// (blank = open) and `HEALTH_API_USERNAME` defaults to `railway`. Stages it
+/// for curl as a one-line config document in `$HEALTH_API_CFG`
+/// (`user = "user:pass"`) with `$@` holding `-K -`, which makes curl read
+/// that document from stdin -- or leaves both empty when the node carries no
+/// password. An open node ignores the header and an enforcing one requires
+/// it, so one command spans a cluster mid-rollout, and no secret enters the
+/// exec payload, this process, or a log line.
+///
+/// Stdin rather than `-u user:pass` because argv is public inside the
+/// container: `/proc/<pid>/cmdline` (`ps`) shows every process's arguments
+/// to every other process in the PID namespace for as long as the request
+/// runs. `printf` is a builtin of the shells the data images ship (dash,
+/// bash), so the pipe spawns no process carrying the secret either.
+///
+/// `curl_cfg_quote` escapes a string for a double-quoted value in curl's
+/// config syntax the way curl's parser reads it back: `\` and `"` are
+/// backslashed and a newline becomes `\n`, since a raw newline would end
+/// the value. Everything else (`$`, `'`, `#`, whitespace) is literal inside
+/// the quotes. Pure POSIX sh, so it asks nothing of the image beyond the
+/// shell itself: `s` is the unread rest of the input, `c` the character in
+/// hand, `r` the input after it, `q` the escaped output. Twin of the Patroni
+/// prelude in [`super::patroni`].
 const HEALTH_API_AUTH_PRELUDE: &str = concat!(
     r#"HEALTH_API_PW="${HEALTH_API_PASSWORD:-}"; "#,
     r#"HEALTH_API_USER="${HEALTH_API_USERNAME:-railway}"; "#,
-    r#"if [ -n "$HEALTH_API_PW" ]; then set -- -u "$HEALTH_API_USER:$HEALTH_API_PW"; else set --; fi; "#,
+    r#"curl_cfg_quote() { s=$1; q=; nl=$(printf '\nx'); nl=${nl%x}; "#,
+    r#"while [ -n "$s" ]; do r=${s#?}; c=${s%"$r"}; s=$r; "#,
+    r#"case $c in \\) q="$q\\\\";; \") q="$q\\\"";; "$nl") q="$q\\n";; *) q="$q$c";; esac; done; "#,
+    r#"printf '%s' "$q"; }; "#,
+    r#"if [ -n "$HEALTH_API_PW" ]; then HEALTH_API_CFG="user = \"$(curl_cfg_quote "$HEALTH_API_USER:$HEALTH_API_PW")\""; set -- -K -; else HEALTH_API_CFG=; set --; fi; "#,
 );
 
 /// The exact shell text the switchover runs, so a test can pin both halves:
-/// the credential resolution and the request itself.
+/// the credential resolution and the request itself. The config document is
+/// piped into curl on every run; curl reads it only when `$@` says `-K -`.
 fn switchover_command(endpoint: &ResolvedEndpoint) -> String {
     format!(
-        "{prelude}curl -s --max-time 8 -w '\\nHTTP_STATUS:%{{http_code}}' \"$@\" -X POST localhost:{port}{path}",
+        r#"{prelude}printf '%s\n' "$HEALTH_API_CFG" | curl -s --max-time 8 -w '\nHTTP_STATUS:%{{http_code}}' "$@" -X POST localhost:{port}{path}"#,
         prelude = HEALTH_API_AUTH_PRELUDE,
         port = endpoint.port,
         path = endpoint.path,
@@ -266,14 +288,24 @@ mod tests {
         .unwrap()
     }
 
+    /// What the `curl` shim saw: its argv, one entry per element, and the
+    /// config document it was handed on stdin when that argv said `-K -`.
+    #[cfg(unix)]
+    struct CurlCall {
+        argv: Vec<String>,
+        config: Option<String>,
+    }
+
     #[cfg(unix)]
     /// Runs the emitted switchover text through a real `sh`, with a `curl`
-    /// shim that prints its argv one per line, so the assertions are about
+    /// shim that prints its argv one per line and, when told to read a
+    /// config from stdin, that config too -- so the assertions are about
     /// what curl is actually handed rather than about string shapes. The
     /// command only ever runs inside a Linux container, and Windows has no
-    /// shell to check it against -- `switchover_command_resolves_the_credential_inside_the_container`
+    /// shell to check it against --
+    /// `switchover_command_resolves_the_credential_inside_the_container`
     /// covers what can be asserted everywhere.
-    fn curl_argv_for(env: &[(&str, &str)]) -> Vec<String> {
+    fn curl_call_for(env: &[(&str, &str)]) -> CurlCall {
         use std::io::Write;
         let dir = std::env::temp_dir().join(format!(
             "cli-health-api-auth-{}-{:?}",
@@ -283,7 +315,19 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
         std::fs::create_dir_all(&dir).unwrap();
         let shim = dir.join("curl");
-        std::fs::write(&shim, "#!/bin/sh\nprintf '%s\\n' \"$@\"\n").unwrap();
+        std::fs::write(
+            &shim,
+            concat!(
+                "#!/bin/sh\n",
+                "printf '%s\\n' \"$@\"\n",
+                "prev=\n",
+                "for a in \"$@\"; do\n",
+                "  if [ \"$prev\" = -K ] && [ \"$a\" = - ]; then printf '%s\\n' '@@CONFIG@@'; cat; fi\n",
+                "  prev=$a\n",
+                "done\n",
+            ),
+        )
+        .unwrap();
         #[cfg(unix)]
         {
             use std::os::unix::fs::PermissionsExt;
@@ -323,11 +367,42 @@ mod tests {
             "{}",
             String::from_utf8_lossy(&out.stderr)
         );
-        String::from_utf8(out.stdout)
-            .unwrap()
-            .lines()
-            .map(str::to_string)
-            .collect()
+        let stdout = String::from_utf8(out.stdout).unwrap();
+        let (argv, config) = match stdout.split_once("@@CONFIG@@\n") {
+            Some((argv, config)) => (argv.to_string(), Some(config.to_string())),
+            None => (stdout, None),
+        };
+        CurlCall {
+            argv: argv.lines().map(str::to_string).collect(),
+            config,
+        }
+    }
+
+    /// How curl's config parser (`unslashquote` in `src/tool_parsecfg.c`)
+    /// reads a double-quoted value: `\\`, `\"`, `\t`, `\n`, `\r` and `\v`
+    /// are escapes, a backslash before anything else is dropped, and the
+    /// value ends at the closing quote. Pins that the shell-side escaper
+    /// speaks the dialect curl actually parses.
+    #[cfg(unix)]
+    fn curl_config_value(line: &str) -> String {
+        let (_, quoted) = line.split_once('"').expect("a double-quoted value");
+        let mut out = String::new();
+        let mut chars = quoted.chars();
+        while let Some(c) = chars.next() {
+            match c {
+                '"' => break,
+                '\\' => match chars.next() {
+                    Some('t') => out.push('\t'),
+                    Some('n') => out.push('\n'),
+                    Some('r') => out.push('\r'),
+                    Some('v') => out.push('\u{0B}'),
+                    Some(other) => out.push(other),
+                    None => break,
+                },
+                c => out.push(c),
+            }
+        }
+        out
     }
 
     #[test]
@@ -336,44 +411,86 @@ mod tests {
         assert!(cmd.starts_with(HEALTH_API_AUTH_PRELUDE));
         assert!(cmd.contains("${HEALTH_API_PASSWORD:-}"));
         assert!(cmd.contains("${HEALTH_API_USERNAME:-railway}"));
-        assert!(cmd.contains(r#"set -- -u "$HEALTH_API_USER:$HEALTH_API_PW""#));
+        assert!(cmd.contains(
+            r#"HEALTH_API_CFG="user = \"$(curl_cfg_quote "$HEALTH_API_USER:$HEALTH_API_PW")\""; set -- -K -"#
+        ));
+        assert!(cmd.contains("else HEALTH_API_CFG=; set --; fi"));
+        assert!(
+            !cmd.contains(" -u "),
+            "the credential must never be a curl argument"
+        );
+        // The config document is piped into curl, ahead of the request.
+        assert!(cmd.contains(r#"printf '%s\n' "$HEALTH_API_CFG" | curl "#));
         assert!(cmd.ends_with(r#""$@" -X POST localhost:8080/switchover"#));
     }
 
     #[cfg(unix)]
     #[test]
     fn an_open_node_gets_no_credential_flag_at_all() {
-        let argv = curl_argv_for(&[]);
-        // No password in the container => no `-u` at all; `-u "railway:"`
-        // would be a malformed credential rather than "none".
-        assert!(!argv.iter().any(|a| a == "-u"), "{argv:?}");
-        assert_eq!(argv.last().unwrap(), "localhost:8080/switchover");
+        let call = curl_call_for(&[]);
+        // No password in the container => no config document and no `-K` at
+        // all; `user = "railway:"` would be a malformed credential rather
+        // than "none".
+        assert!(call.config.is_none(), "{:?}", call.config);
+        assert!(
+            !call.argv.iter().any(|a| a == "-K" || a == "-u"),
+            "{:?}",
+            call.argv
+        );
+        assert_eq!(call.argv.last().unwrap(), "localhost:8080/switchover");
     }
 
     #[cfg(unix)]
     #[test]
     fn a_password_alone_authenticates_as_the_default_user() {
-        let argv = curl_argv_for(&[("HEALTH_API_PASSWORD", "s3cret")]);
-        let at = argv
+        let call = curl_call_for(&[("HEALTH_API_PASSWORD", "s3cret")]);
+        assert_eq!(call.config.as_deref(), Some("user = \"railway:s3cret\"\n"));
+        // argv is public inside the container; the credential never rides it.
+        assert!(
+            call.argv.iter().all(|a| !a.contains("s3cret")),
+            "{:?}",
+            call.argv
+        );
+        assert!(!call.argv.iter().any(|a| a == "-u"), "{:?}", call.argv);
+        // curl reads the config ahead of the request itself.
+        let k = call
+            .argv
             .iter()
-            .position(|a| a == "-u")
-            .expect("credential passed to curl");
-        assert_eq!(argv[at + 1], "railway:s3cret");
-        // curl carries the credential ahead of the request itself.
-        assert!(argv.iter().position(|a| a == "-X").unwrap() > at);
+            .position(|a| a == "-K")
+            .expect("curl told to read its config from stdin");
+        assert_eq!(call.argv[k + 1], "-");
+        assert!(call.argv.iter().position(|a| a == "-X").unwrap() > k);
     }
 
     #[cfg(unix)]
     #[test]
     fn an_explicit_username_wins_over_the_default() {
-        let argv = curl_argv_for(&[
+        let call = curl_call_for(&[
             ("HEALTH_API_PASSWORD", "s3cret"),
             ("HEALTH_API_USERNAME", "ops"),
         ]);
-        let at = argv
-            .iter()
-            .position(|a| a == "-u")
-            .expect("credential passed to curl");
-        assert_eq!(argv[at + 1], "ops:s3cret");
+        assert_eq!(call.config.as_deref(), Some("user = \"ops:s3cret\"\n"));
+    }
+
+    /// A password is arbitrary text, and curl's config parser gives `\` and
+    /// `"` meaning inside a double-quoted value and ends the value at a
+    /// newline -- so the shell escapes exactly those, and what curl reads
+    /// back (`curl_config_value`, its parser's rules) is the original.
+    #[cfg(unix)]
+    #[test]
+    fn the_credential_is_escaped_for_curls_config_parser() {
+        let password = "p\"a\\s$s' w#rd\nnext\ttab";
+        let call = curl_call_for(&[("HEALTH_API_PASSWORD", password)]);
+        let config = call.config.expect("config document piped to curl");
+        assert_eq!(
+            config,
+            "user = \"railway:p\\\"a\\\\s$s' w#rd\\nnext\ttab\"\n"
+        );
+        assert_eq!(curl_config_value(&config), format!("railway:{password}"));
+        assert!(
+            call.argv.iter().all(|a| !a.contains("w#rd")),
+            "{:?}",
+            call.argv
+        );
     }
 }