git: Intercept signing prompt from GPG when committing (#34096)

Closes #30111 

- [x] basic implementation
- [x] implementation for remote projects
- [x] surface error output from GPG if signing fails
- [ ] ~~Windows~~

Release Notes:

- git: Passphrase prompts from GPG to unlock commit signing keys are now
shown in Zed.

Cargo.lock

@@ -607,6 +607,7 @@ dependencies = [
  "parking_lot",
  "smol",
  "tempfile",
+ "unindent",
  "util",
  "workspace-hack",
 ]

crates/askpass/Cargo.toml

@@ -19,5 +19,6 @@ net.workspace = true
 parking_lot.workspace = true
 smol.workspace = true
 tempfile.workspace = true
+unindent.workspace = true
 util.workspace = true
 workspace-hack.workspace = true

crates/askpass/src/askpass.rs

@@ -40,11 +40,21 @@ impl AskPassDelegate {
         self.tx.send((prompt, tx)).await?;
         Ok(rx.await?)
     }
+
+    pub fn new_always_failing() -> Self {
+        let (tx, _rx) = mpsc::unbounded::<(String, oneshot::Sender<String>)>();
+        Self {
+            tx,
+            _task: Task::ready(()),
+        }
+    }
 }
 
 pub struct AskPassSession {
     #[cfg(not(target_os = "windows"))]
     script_path: std::path::PathBuf,
+    #[cfg(not(target_os = "windows"))]
+    gpg_script_path: std::path::PathBuf,
     #[cfg(target_os = "windows")]
     askpass_helper: String,
     #[cfg(target_os = "windows")]
@@ -59,6 +69,9 @@ const ASKPASS_SCRIPT_NAME: &str = "askpass.sh";
 #[cfg(target_os = "windows")]
 const ASKPASS_SCRIPT_NAME: &str = "askpass.ps1";
 
+#[cfg(not(target_os = "windows"))]
+const GPG_SCRIPT_NAME: &str = "gpg.sh";
+
 impl AskPassSession {
     /// This will create a new AskPassSession.
     /// You must retain this session until the master process exits.
@@ -72,6 +85,8 @@ impl AskPassSession {
         let temp_dir = tempfile::Builder::new().prefix("zed-askpass").tempdir()?;
         let askpass_socket = temp_dir.path().join("askpass.sock");
         let askpass_script_path = temp_dir.path().join(ASKPASS_SCRIPT_NAME);
+        #[cfg(not(target_os = "windows"))]
+        let gpg_script_path = temp_dir.path().join(GPG_SCRIPT_NAME);
         let (askpass_opened_tx, askpass_opened_rx) = oneshot::channel::<()>();
         let listener = UnixListener::bind(&askpass_socket).context("creating askpass socket")?;
         #[cfg(not(target_os = "windows"))]
@@ -135,9 +150,20 @@ impl AskPassSession {
             askpass_script_path.display()
         );
 
+        #[cfg(not(target_os = "windows"))]
+        {
+            let gpg_script = generate_gpg_script();
+            fs::write(&gpg_script_path, gpg_script)
+                .await
+                .with_context(|| format!("creating gpg wrapper script at {gpg_script_path:?}"))?;
+            make_file_executable(&gpg_script_path).await?;
+        }
+
         Ok(Self {
             #[cfg(not(target_os = "windows"))]
             script_path: askpass_script_path,
+            #[cfg(not(target_os = "windows"))]
+            gpg_script_path,
 
             #[cfg(target_os = "windows")]
             secret,
@@ -160,6 +186,19 @@ impl AskPassSession {
         &self.askpass_helper
     }
 
+    #[cfg(not(target_os = "windows"))]
+    pub fn gpg_script_path(&self) -> Option<impl AsRef<OsStr>> {
+        Some(&self.gpg_script_path)
+    }
+
+    #[cfg(target_os = "windows")]
+    pub fn gpg_script_path(&self) -> Option<impl AsRef<OsStr>> {
+        // TODO implement wrapping GPG on Windows. This is more difficult than on Unix
+        // because we can't use --passphrase-fd with a nonstandard FD, and both --passphrase
+        // and --passphrase-file are insecure.
+        None::<std::path::PathBuf>
+    }
+
     // This will run the askpass task forever, resolving as many authentication requests as needed.
     // The caller is responsible for examining the result of their own commands and cancelling this
     // future when this is no longer needed. Note that this can only be called once, but due to the
@@ -263,3 +302,23 @@ fn generate_askpass_script(zed_path: &std::path::Path, askpass_socket: &std::pat
         askpass_socket = askpass_socket.display(),
     )
 }
+
+#[inline]
+#[cfg(not(target_os = "windows"))]
+fn generate_gpg_script() -> String {
+    use unindent::Unindent as _;
+
+    r#"
+        #!/bin/sh
+        set -eu
+
+        unset GIT_CONFIG_PARAMETERS
+        GPG_PROGRAM=$(git config gpg.program || echo 'gpg')
+        PROMPT="Enter passphrase to unlock GPG key:"
+        PASSPHRASE=$(${GIT_ASKPASS} "${PROMPT}")
+
+        exec "${GPG_PROGRAM}" --batch --no-tty --yes --passphrase-fd 3 --pinentry-mode loopback "$@" 3<<EOF
+        ${PASSPHRASE}
+        EOF
+    "#.unindent()
+}

crates/fs/src/fake_git_repo.rs

@@ -375,8 +375,10 @@ impl GitRepository for FakeGitRepository {
         _message: gpui::SharedString,
         _name_and_email: Option<(gpui::SharedString, gpui::SharedString)>,
         _options: CommitOptions,
+        _ask_pass: AskPassDelegate,
         _env: Arc<HashMap<String, String>>,
-    ) -> BoxFuture<'_, Result<()>> {
+        _cx: AsyncApp,
+    ) -> BoxFuture<'static, Result<()>> {
         unimplemented!()
     }
 

crates/git/Cargo.toml

@@ -41,9 +41,9 @@ futures.workspace = true
 workspace-hack.workspace = true
 
 [dev-dependencies]
+gpui = { workspace = true, features = ["test-support"] }
 pretty_assertions.workspace = true
 serde_json.workspace = true
+tempfile.workspace = true
 text = { workspace = true, features = ["test-support"] }
 unindent.workspace = true
-gpui = { workspace = true, features = ["test-support"] }
-tempfile.workspace = true

crates/git/src/repository.rs

@@ -391,8 +391,12 @@ pub trait GitRepository: Send + Sync {
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
+        askpass: AskPassDelegate,
         env: Arc<HashMap<String, String>>,
-    ) -> BoxFuture<'_, Result<()>>;
+        // This method takes an AsyncApp to ensure it's invoked on the main thread,
+        // otherwise git-credentials-manager won't work.
+        cx: AsyncApp,
+    ) -> BoxFuture<'static, Result<()>>;
 
     fn push(
         &self,
@@ -1193,36 +1197,68 @@ impl GitRepository for RealGitRepository {
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
+        ask_pass: AskPassDelegate,
         env: Arc<HashMap<String, String>>,
-    ) -> BoxFuture<'_, Result<()>> {
+        cx: AsyncApp,
+    ) -> BoxFuture<'static, Result<()>> {
         let working_directory = self.working_directory();
-        self.executor
+        let executor = cx.background_executor().clone();
-            .spawn(async move {
+        async move {
-                let mut cmd = new_smol_command("git");
+            let working_directory = working_directory?;
-                cmd.current_dir(&working_directory?)
+            let have_user_git_askpass = env.contains_key("GIT_ASKPASS");
-                    .envs(env.iter())
+            let mut command = new_smol_command("git");
-                    .args(["commit", "--quiet", "-m"])
+            command.current_dir(&working_directory).envs(env.iter());
-                    .arg(&message.to_string())
-                    .arg("--cleanup=strip");
 
-                if options.amend {
+            let ask_pass = if have_user_git_askpass {
-                    cmd.arg("--amend");
+                None
-                }
+            } else {
+                Some(AskPassSession::new(&executor, ask_pass).await?)
+            };
 
-                if let Some((name, email)) = name_and_email {
+            if let Some(program) = ask_pass
-                    cmd.arg("--author").arg(&format!("{name} <{email}>"));
+                .as_ref()
-                }
+                .and_then(|ask_pass| ask_pass.gpg_script_path())
+            {
+                command.arg("-c").arg(format!(
+                    "gpg.program={}",
+                    program.as_ref().to_string_lossy()
+                ));
+            }
 
-                let output = cmd.output().await?;
+            command
+                .args(["commit", "-m"])
+                .arg(message.to_string())
+                .arg("--cleanup=strip")
+                .stdin(smol::process::Stdio::null())
+                .stdout(smol::process::Stdio::piped())
+                .stderr(smol::process::Stdio::piped());
 
+            if options.amend {
+                command.arg("--amend");
+            }
+
+            if let Some((name, email)) = name_and_email {
+                command.arg("--author").arg(&format!("{name} <{email}>"));
+            }
+
+            if let Some(ask_pass) = ask_pass {
+                command.env("GIT_ASKPASS", ask_pass.script_path());
+                let git_process = command.spawn()?;
+
+                run_askpass_command(ask_pass, git_process).await?;
+                Ok(())
+            } else {
+                let git_process = command.spawn()?;
+                let output = git_process.output().await?;
                 anyhow::ensure!(
                     output.status.success(),
-                    "Failed to commit:\n{}",
+                    "{}",
                     String::from_utf8_lossy(&output.stderr)
                 );
                 Ok(())
-            })
+            }
-            .boxed()
+        }
+        .boxed()
     }
 
     fn push(
@@ -2046,12 +2082,16 @@ mod tests {
         )
         .await
         .unwrap();
-        repo.commit(
+        cx.spawn(|cx| {
-            "Initial commit".into(),
+            repo.commit(
-            None,
+                "Initial commit".into(),
-            CommitOptions::default(),
+                None,
-            Arc::new(checkpoint_author_envs()),
+                CommitOptions::default(),
-        )
+                AskPassDelegate::new_always_failing(),
+                Arc::new(checkpoint_author_envs()),
+                cx,
+            )
+        })
         .await
         .unwrap();
 
@@ -2075,12 +2115,16 @@ mod tests {
         )
         .await
         .unwrap();
-        repo.commit(
+        cx.spawn(|cx| {
-            "Commit after checkpoint".into(),
+            repo.commit(
-            None,
+                "Commit after checkpoint".into(),
-            CommitOptions::default(),
+                None,
-            Arc::new(checkpoint_author_envs()),
+                CommitOptions::default(),
-        )
+                AskPassDelegate::new_always_failing(),
+                Arc::new(checkpoint_author_envs()),
+                cx,
+            )
+        })
         .await
         .unwrap();
 
@@ -2213,12 +2257,16 @@ mod tests {
         )
         .await
         .unwrap();
-        repo.commit(
+        cx.spawn(|cx| {
-            "Initial commit".into(),
+            repo.commit(
-            None,
+                "Initial commit".into(),
-            CommitOptions::default(),
+                None,
-            Arc::new(checkpoint_author_envs()),
+                CommitOptions::default(),
-        )
+                AskPassDelegate::new_always_failing(),
+                Arc::new(checkpoint_author_envs()),
+                cx,
+            )
+        })
         .await
         .unwrap();
 

crates/git_ui/src/git_panel.rs

@@ -1574,10 +1574,15 @@ impl GitPanel {
 
         let task = if self.has_staged_changes() {
             // Repository serializes all git operations, so we can just send a commit immediately
-            let commit_task = active_repository.update(cx, |repo, cx| {
+            cx.spawn_in(window, async move |this, cx| {
-                repo.commit(message.into(), None, options, cx)
+                let askpass_delegate = this.update_in(cx, |this, window, cx| {
-            });
+                    this.askpass_delegate("git commit", window, cx)
-            cx.background_spawn(async move { commit_task.await? })
+                })?;
+                let commit_task = active_repository.update(cx, |repo, cx| {
+                    repo.commit(message.into(), None, options, askpass_delegate, cx)
+                })?;
+                commit_task.await?
+            })
         } else {
             let changed_files = self
                 .entries
@@ -1594,10 +1599,13 @@ impl GitPanel {
 
             let stage_task =
                 active_repository.update(cx, |repo, cx| repo.stage_entries(changed_files, cx));
-            cx.spawn(async move |_, cx| {
+            cx.spawn_in(window, async move |this, cx| {
                 stage_task.await?;
+                let askpass_delegate = this.update_in(cx, |this, window, cx| {
+                    this.askpass_delegate("git commit".to_string(), window, cx)
+                })?;
                 let commit_task = active_repository.update(cx, |repo, cx| {
-                    repo.commit(message.into(), None, options, cx)
+                    repo.commit(message.into(), None, options, askpass_delegate, cx)
                 })?;
                 commit_task.await?
             })

crates/project/src/git_store.rs

@@ -1726,6 +1726,18 @@ impl GitStore {
         let repository_id = RepositoryId::from_proto(envelope.payload.repository_id);
         let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?;
 
+        let askpass = if let Some(askpass_id) = envelope.payload.askpass_id {
+            make_remote_delegate(
+                this,
+                envelope.payload.project_id,
+                repository_id,
+                askpass_id,
+                &mut cx,
+            )
+        } else {
+            AskPassDelegate::new_always_failing()
+        };
+
         let message = SharedString::from(envelope.payload.message);
         let name = envelope.payload.name.map(SharedString::from);
         let email = envelope.payload.email.map(SharedString::from);
@@ -1739,6 +1751,7 @@ impl GitStore {
                     CommitOptions {
                         amend: options.amend,
                     },
+                    askpass,
                     cx,
                 )
             })?
@@ -3462,11 +3475,14 @@ impl Repository {
         message: SharedString,
         name_and_email: Option<(SharedString, SharedString)>,
         options: CommitOptions,
+        askpass: AskPassDelegate,
         _cx: &mut App,
     ) -> oneshot::Receiver<Result<()>> {
         let id = self.id;
+        let askpass_delegates = self.askpass_delegates.clone();
+        let askpass_id = util::post_inc(&mut self.latest_askpass_id);
 
-        self.send_job(Some("git commit".into()), move |git_repo, _cx| async move {
+        self.send_job(Some("git commit".into()), move |git_repo, cx| async move {
             match git_repo {
                 RepositoryState::Local {
                     backend,
@@ -3474,10 +3490,16 @@ impl Repository {
                     ..
                 } => {
                     backend
-                        .commit(message, name_and_email, options, environment)
+                        .commit(message, name_and_email, options, askpass, environment, cx)
                         .await
                 }
                 RepositoryState::Remote { project_id, client } => {
+                    askpass_delegates.lock().insert(askpass_id, askpass);
+                    let _defer = util::defer(|| {
+                        let askpass_delegate = askpass_delegates.lock().remove(&askpass_id);
+                        debug_assert!(askpass_delegate.is_some());
+                    });
+
                     let (name, email) = name_and_email.unzip();
                     client
                         .request(proto::Commit {
@@ -3489,9 +3511,9 @@ impl Repository {
                             options: Some(proto::commit::CommitOptions {
                                 amend: options.amend,
                             }),
+                            askpass_id: Some(askpass_id),
                         })
-                        .await
+                        .await?;
-                        .context("sending commit request")?;
 
                     Ok(())
                 }

crates/proto/proto/git.proto

@@ -294,6 +294,7 @@ message Commit {
     optional string email = 5;
     string message = 6;
     optional CommitOptions options = 7;
+    optional uint64 askpass_id = 8;
 
     message CommitOptions {
         bool amend = 1;