Auto merge of #12469 - arlosi:cred-stdio, r=weihanglo

cargo-credential: reset stdin & stdout to the Console

Credential providers run with stdin and stdout piped to Cargo to communicate. This makes it more difficult for providers to do anything interactive. The current workaround is for a provider to use the `cargo_credential::tty()` function when reading from the console by re-opening stdin using `/dev/tty` or `CONIN$`.

This PR makes the credential provider to re-attach itself to the current console so that reading from stdin and writing to stdout "just works" when inside the `perform` method of the provider. stderr is unaffected since it's not redirected by Cargo.

Only the `cargo-credential` crate is changed. No changes are needed to Cargo.

cc #8933

Author: bors

Cargo.lock

@@ -80,7 +80,6 @@ impl OnePasswordKeychain {
         let mut cmd = Command::new("op");
         cmd.args(["signin", "--raw"]);
         cmd.stdout(Stdio::piped());
-        cmd.stdin(cargo_credential::tty().map_err(Box::new)?);
         let mut child = cmd
             .spawn()
             .map_err(|e| format!("failed to spawn `op`: {}", e))?;
@@ -210,7 +209,7 @@ impl OnePasswordKeychain {
             Some(name) => format!("Cargo registry token for {}", name),
             None => "Cargo registry token".to_string(),
         };
-        let mut cmd = self.make_cmd(
+        let cmd = self.make_cmd(
             session,
             &[
                 "item",
@@ -225,10 +224,6 @@ impl OnePasswordKeychain {
                 CARGO_TAG,
             ],
         );
-        // For unknown reasons, `op item create` seems to not be happy if
-        // stdin is not a tty. Otherwise it returns with a 0 exit code without
-        // doing anything.
-        cmd.stdin(cargo_credential::tty().map_err(Box::new)?);
         self.run_cmd(cmd)?;
         Ok(())
     }

credential/cargo-credential-1password/src/lib.rs

@@ -8,10 +8,12 @@ description = "A library to assist writing Cargo credential helpers."
 
 [dependencies]
 anyhow.workspace = true
+libc.workspace = true
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
 thiserror.workspace = true
 time.workspace = true
+windows-sys = { workspace = true, features = ["Win32_System_Console", "Win32_Foundation"] }
 
 [dev-dependencies]
 snapbox = { workspace = true, features = ["examples"] }

credential/cargo-credential/Cargo.toml

@@ -0,0 +1,25 @@
+//! Provider used for testing redirection of stdout.
+
+use cargo_credential::{Action, Credential, CredentialResponse, Error, RegistryInfo};
+
+struct MyCredential;
+
+impl Credential for MyCredential {
+    fn perform(
+        &self,
+        _registry: &RegistryInfo,
+        _action: &Action,
+        _args: &[&str],
+    ) -> Result<CredentialResponse, Error> {
+        // Informational messages should be sent on stderr.
+        eprintln!("message on stderr should be sent the the parent process");
+
+        // Reading from stdin and writing to stdout will go to the attached console (tty).
+        println!("message from test credential provider");
+        Err(Error::OperationNotSupported)
+    }
+}
+
+fn main() {
+    cargo_credential::main(MyCredential);
+}

credential/cargo-credential/examples/stdout-redirected.rs

@@ -38,17 +38,16 @@
 //! ```
 
 use serde::{Deserialize, Serialize};
-use std::{
-    fmt::Display,
-    fs::File,
-    io::{self, BufRead, BufReader},
-};
+use std::{fmt::Display, io};
 use time::OffsetDateTime;
 
 mod error;
 mod secret;
+mod stdio;
+
 pub use error::Error;
 pub use secret::Secret;
+use stdio::stdin_stdout_to_console;
 
 /// Message sent by the credential helper on startup
 #[derive(Serialize, Deserialize, Clone, Debug)]
@@ -241,32 +240,20 @@ fn doit(
         if request.v != PROTOCOL_VERSION_1 {
             return Err(format!("unsupported protocol version {}", request.v).into());
         }
-        serde_json::to_writer(
-            std::io::stdout(),
-            &credential.perform(&request.registry, &request.action, &request.args),
-        )?;
+
+        let response = stdin_stdout_to_console(|| {
+            credential.perform(&request.registry, &request.action, &request.args)
+        })?;
+
+        serde_json::to_writer(std::io::stdout(), &response)?;
         println!();
     }
 }
 
-/// Open stdin from the tty
-pub fn tty() -> Result<File, io::Error> {
-    #[cfg(unix)]
-    const IN_DEVICE: &str = "/dev/tty";
-    #[cfg(windows)]
-    const IN_DEVICE: &str = "CONIN$";
-    let stdin = std::fs::OpenOptions::new()
-        .read(true)
-        .write(true)
-        .open(IN_DEVICE)?;
-    Ok(stdin)
-}
-
 /// Read a line of text from stdin.
 pub fn read_line() -> Result<String, io::Error> {
-    let mut reader = BufReader::new(tty()?);
     let mut buf = String::new();
-    reader.read_line(&mut buf)?;
+    io::stdin().read_line(&mut buf)?;
     Ok(buf.trim().to_string())
 }
 

credential/cargo-credential/src/lib.rs

@@ -0,0 +1,163 @@
+use std::{fs::File, io::Error};
+
+/// Reset stdin and stdout to the attached console / tty for the duration of the closure.
+/// If no console is available, stdin and stdout will be redirected to null.
+pub fn stdin_stdout_to_console<F, T>(f: F) -> Result<T, Error>
+where
+    F: FnOnce() -> T,
+{
+    let open_write = |f| std::fs::OpenOptions::new().write(true).open(f);
+
+    let mut stdin = File::open(imp::IN_DEVICE).or_else(|_| File::open(imp::NULL_DEVICE))?;
+    let mut stdout = open_write(imp::OUT_DEVICE).or_else(|_| open_write(imp::NULL_DEVICE))?;
+
+    let _stdin_guard = imp::ReplacementGuard::new(Stdio::Stdin, &mut stdin)?;
+    let _stdout_guard = imp::ReplacementGuard::new(Stdio::Stdout, &mut stdout)?;
+    Ok(f())
+}
+
+enum Stdio {
+    Stdin,
+    Stdout,
+}
+
+#[cfg(windows)]
+mod imp {
+    use super::Stdio;
+    use std::{fs::File, io::Error, os::windows::prelude::AsRawHandle};
+    use windows_sys::Win32::{
+        Foundation::{HANDLE, INVALID_HANDLE_VALUE},
+        System::Console::{
+            GetStdHandle, SetStdHandle, STD_HANDLE, STD_INPUT_HANDLE, STD_OUTPUT_HANDLE,
+        },
+    };
+    pub const OUT_DEVICE: &str = "CONOUT$";
+    pub const IN_DEVICE: &str = "CONIN$";
+    pub const NULL_DEVICE: &str = "NUL";
+
+    /// Restores previous stdio when dropped.
+    pub struct ReplacementGuard {
+        std_handle: STD_HANDLE,
+        previous: HANDLE,
+    }
+
+    impl ReplacementGuard {
+        pub(super) fn new(stdio: Stdio, replacement: &mut File) -> Result<ReplacementGuard, Error> {
+            let std_handle = match stdio {
+                Stdio::Stdin => STD_INPUT_HANDLE,
+                Stdio::Stdout => STD_OUTPUT_HANDLE,
+            };
+
+            let previous;
+            unsafe {
+                // Make a copy of the current handle
+                previous = GetStdHandle(std_handle);
+                if previous == INVALID_HANDLE_VALUE {
+                    return Err(std::io::Error::last_os_error());
+                }
+
+                // Replace stdin with the replacement handle
+                if SetStdHandle(std_handle, replacement.as_raw_handle() as HANDLE) == 0 {
+                    return Err(std::io::Error::last_os_error());
+                }
+            }
+
+            Ok(ReplacementGuard {
+                previous,
+                std_handle,
+            })
+        }
+    }
+
+    impl Drop for ReplacementGuard {
+        fn drop(&mut self) {
+            unsafe {
+                // Put previous handle back in to stdin
+                SetStdHandle(self.std_handle, self.previous);
+            }
+        }
+    }
+}
+
+#[cfg(unix)]
+mod imp {
+    use super::Stdio;
+    use libc::{close, dup, dup2, STDIN_FILENO, STDOUT_FILENO};
+    use std::{fs::File, io::Error, os::fd::AsRawFd};
+    pub const IN_DEVICE: &str = "/dev/tty";
+    pub const OUT_DEVICE: &str = "/dev/tty";
+    pub const NULL_DEVICE: &str = "/dev/null";
+
+    /// Restores previous stdio when dropped.
+    pub struct ReplacementGuard {
+        std_fileno: i32,
+        previous: i32,
+    }
+
+    impl ReplacementGuard {
+        pub(super) fn new(stdio: Stdio, replacement: &mut File) -> Result<ReplacementGuard, Error> {
+            let std_fileno = match stdio {
+                Stdio::Stdin => STDIN_FILENO,
+                Stdio::Stdout => STDOUT_FILENO,
+            };
+
+            let previous;
+            unsafe {
+                // Duplicate the existing stdin file to a new descriptor
+                previous = dup(std_fileno);
+                if previous == -1 {
+                    return Err(std::io::Error::last_os_error());
+                }
+                // Replace stdin with the replacement file
+                if dup2(replacement.as_raw_fd(), std_fileno) == -1 {
+                    return Err(std::io::Error::last_os_error());
+                }
+            }
+
+            Ok(ReplacementGuard {
+                previous,
+                std_fileno,
+            })
+        }
+    }
+
+    impl Drop for ReplacementGuard {
+        fn drop(&mut self) {
+            unsafe {
+                // Put previous file back in to stdin
+                dup2(self.previous, self.std_fileno);
+                // Close the file descriptor we used as a backup
+                close(self.previous);
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use std::fs::OpenOptions;
+    use std::io::{Seek, Write};
+
+    use super::imp::ReplacementGuard;
+    use super::Stdio;
+
+    #[test]
+    fn stdin() {
+        let tempdir = snapbox::path::PathFixture::mutable_temp().unwrap();
+        let file = tempdir.path().unwrap().join("stdin");
+        let mut file = OpenOptions::new()
+            .read(true)
+            .write(true)
+            .create(true)
+            .open(file)
+            .unwrap();
+
+        writeln!(&mut file, "hello").unwrap();
+        file.seek(std::io::SeekFrom::Start(0)).unwrap();
+        {
+            let _guard = ReplacementGuard::new(Stdio::Stdin, &mut file).unwrap();
+            let line = std::io::stdin().lines().next().unwrap().unwrap();
+            assert_eq!(line, "hello");
+        }
+    }
+}

credential/cargo-credential/src/stdio.rs

@@ -2,6 +2,23 @@ use std::path::Path;
 
 use snapbox::cmd::Command;
 
+#[test]
+fn stdout_redirected() {
+    let bin = snapbox::cmd::compile_example("stdout-redirected", []).unwrap();
+
+    let hello = r#"{"v":[1]}"#;
+    let get_request = r#"{"v": 1, "registry": {"index-url":"sparse+https://test/","name":"alternative"},"kind": "get","operation": "read","args": []}"#;
+    let err_not_supported = r#"{"Err":{"kind":"operation-not-supported"}}"#;
+
+    Command::new(bin)
+        .stdin(format!("{get_request}\n"))
+        .arg("--cargo-plugin")
+        .assert()
+        .stdout_eq(format!("{hello}\n{err_not_supported}\n"))
+        .stderr_eq("message on stderr should be sent the the parent process\n")
+        .success();
+}
+
 #[test]
 fn file_provider() {
     let bin = snapbox::cmd::compile_example("file-provider", []).unwrap();