[QUALITY-1958] Use unique temp files when writing git credential stores

app/src/ai/agent_sdk/driver/git_credentials.rs

@@ -147,6 +147,49 @@ fn write_secret_file(path: &std::path::Path, content: &str) -> Result<()> {
     Ok(())
 }
 
+/// Atomically replace `path` with `content` through a uniquely named, owner-only (0600 on Unix)
+/// temporary file in the same directory, so concurrent writers never share a temporary path.
+fn replace_secret_file(path: &Path, content: &str) -> Result<()> {
+    use std::io::Write as _;
+
+    let dir = path
+        .parent()
+        .with_context(|| format!("{} has no parent directory", path.display()))?;
+    let file_name = path
+        .file_name()
+        .with_context(|| format!("{} has no file name", path.display()))?
+        .to_string_lossy();
+    let mut temp_file = tempfile::Builder::new()
+        .prefix(&format!(".{}.tmp-", file_name.trim_start_matches('.')))
+        .tempfile_in(dir)
+        .with_context(|| format!("Failed to create a temporary file in {}", dir.display()))?;
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        temp_file
+            .as_file()
+            .set_permissions(std::fs::Permissions::from_mode(0o600))
+            .with_context(|| {
+                format!(
+                    "Failed to set permissions on a temporary file in {}",
+                    dir.display()
+                )
+            })?;
+    }
+    temp_file
+        .write_all(content.as_bytes())
+        .with_context(|| format!("Failed to write a temporary file in {}", dir.display()))?;
+    temp_file
+        .as_file()
+        .sync_all()
+        .with_context(|| format!("Failed to sync a temporary file in {}", dir.display()))?;
+    temp_file
+        .persist(path)
+        .map_err(|error| error.error)
+        .with_context(|| format!("Failed to replace {}", path.display()))?;
+    Ok(())
+}
+
 fn azure_devops_auth_dir(home: &Path) -> PathBuf {
     home.join(".warp").join(AZURE_DEVOPS_AUTH_DIR)
 }
@@ -173,43 +216,7 @@ fn prepare_azure_devops_auth_dir(home: &Path) -> Result<PathBuf> {
 }
 
 fn write_azure_cli_token(auth_dir: &Path, token: &str) -> Result<()> {
-    use std::io::Write as _;
-
-    let token_path = auth_dir.join(AZURE_DEVOPS_TOKEN_FILENAME);
-    let mut temp_file = tempfile::Builder::new()
-        .prefix(&format!(".{AZURE_DEVOPS_TOKEN_FILENAME}.tmp-"))
-        .tempfile_in(auth_dir)
-        .with_context(|| {
-            format!(
-                "Failed to create a temporary token in {}",
-                auth_dir.display()
-            )
-        })?;
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::PermissionsExt as _;
-        temp_file
-            .as_file()
-            .set_permissions(std::fs::Permissions::from_mode(0o600))
-            .with_context(|| {
-                format!(
-                    "Failed to set permissions on temporary token in {}",
-                    auth_dir.display()
-                )
-            })?;
-    }
-    temp_file
-        .write_all(token.as_bytes())
-        .with_context(|| format!("Failed to write temporary token in {}", auth_dir.display()))?;
-    temp_file
-        .as_file()
-        .sync_all()
-        .with_context(|| format!("Failed to sync temporary token in {}", auth_dir.display()))?;
-    temp_file
-        .persist(&token_path)
-        .map_err(|error| error.error)
-        .with_context(|| format!("Failed to replace {}", token_path.display()))?;
-    Ok(())
+    replace_secret_file(&auth_dir.join(AZURE_DEVOPS_TOKEN_FILENAME), token)
 }
 
 fn shell_single_quote(value: &Path) -> String {
@@ -398,26 +405,15 @@ fn merge_git_credentials_file_content(existing: &str, credentials: &[GitCredenti
 /// - `https://x-access-token:{token}@{host}` for service-account tokens
 ///
 /// The write is done atomically: a temporary file is written then renamed.
-fn write_git_credentials_file(credentials: &[GitCredential]) -> Result<()> {
+fn write_git_credentials_file(credentials: &[GitCredential], home: &Path) -> Result<()> {
     if credentials.is_empty() {
         return Ok(());
     }
 
-    let home = home_dir()?;
     let path = home.join(".git-credentials");
-    let tmp_path = home.join(".git-credentials.tmp");
     let existing = std::fs::read_to_string(&path).unwrap_or_default();
     let content = merge_git_credentials_file_content(&existing, credentials);
-    write_secret_file(&tmp_path, &content)?;
-    std::fs::rename(&tmp_path, &path).with_context(|| {
-        format!(
-            "Failed to rename {} to {}",
-            tmp_path.display(),
-            path.display()
-        )
-    })?;
-
-    Ok(())
+    replace_secret_file(&path, &content)
 }
 
 /// The directory `gh` reads its config from, in `gh`'s own precedence order. On Windows `gh`
@@ -470,7 +466,6 @@ fn write_gh_hosts_yml(credentials: &[GitCredential], gh_config_dir: &Path) -> Re
     std::fs::create_dir_all(gh_config_dir)
         .with_context(|| format!("Failed to create {}", gh_config_dir.display()))?;
     let path = gh_config_dir.join(GH_HOSTS_FILENAME);
-    let tmp_path = gh_config_dir.join(format!("{GH_HOSTS_FILENAME}.tmp"));
 
     let mut yaml = String::new();
     for cred in github_credentials {
@@ -482,16 +477,7 @@ fn write_gh_hosts_yml(credentials: &[GitCredential], gh_config_dir: &Path) -> Re
         }
     }
 
-    write_secret_file(&tmp_path, &yaml)?;
-    std::fs::rename(&tmp_path, &path).with_context(|| {
-        format!(
-            "Failed to rename {} to {}",
-            tmp_path.display(),
-            path.display()
-        )
-    })?;
-
-    Ok(())
+    replace_secret_file(&path, &yaml)
 }
 
 /// Write `~/.config/glab-cli/config.yml` so the `glab` CLI is authenticated.
@@ -518,7 +504,6 @@ fn write_glab_config(credentials: &[GitCredential], home: &std::path::Path) -> R
     std::fs::create_dir_all(&glab_config_dir)
         .with_context(|| format!("Failed to create {}", glab_config_dir.display()))?;
     let path = glab_config_dir.join(GLAB_CONFIG_FILENAME);
-    let tmp_path = glab_config_dir.join(format!("{GLAB_CONFIG_FILENAME}.tmp"));
 
     let mut yaml = String::new();
     yaml.push_str("hosts:\n");
@@ -529,16 +514,7 @@ fn write_glab_config(credentials: &[GitCredential], home: &std::path::Path) -> R
         yaml.push_str("        api_protocol: https\n");
     }
 
-    write_secret_file(&tmp_path, &yaml)?;
-    std::fs::rename(&tmp_path, &path).with_context(|| {
-        format!(
-            "Failed to rename {} to {}",
-            tmp_path.display(),
-            path.display()
-        )
-    })?;
-
-    Ok(())
+    replace_secret_file(&path, &yaml)
 }
 
 /// Formats non-sensitive metadata for verifying local credential injection.
@@ -586,7 +562,7 @@ pub(crate) fn write_git_credentials_with_failures(
     }
     let home = home_dir()?;
     let outcomes = [
-        write_git_credentials_file(credentials),
+        write_git_credentials_file(credentials, &home),
         write_gh_hosts_yml(credentials, &gh_config_dir(&home)),
         write_glab_config(credentials, &home),
         write_azure_cli_auth(credentials, &home),

app/src/ai/agent_sdk/driver/git_credentials_tests.rs

@@ -419,15 +419,9 @@ fn write_gh_hosts_yml_uses_gh_cli_filename() -> Result<()> {
         &gh_config_dir,
     )?;
 
-    let hosts_path = gh_config_dir.join(GH_HOSTS_FILENAME);
-    assert!(hosts_path.exists());
-    assert!(
-        !gh_config_dir
-            .join(format!("{GH_HOSTS_FILENAME}.tmp"))
-            .exists()
-    );
+    assert_eq!(sorted_file_names(&gh_config_dir)?, [GH_HOSTS_FILENAME]);
 
-    let hosts = std::fs::read_to_string(hosts_path)?;
+    let hosts = std::fs::read_to_string(gh_config_dir.join(GH_HOSTS_FILENAME))?;
     assert!(hosts.contains("github.com:"));
     assert!(hosts.contains("    oauth_token: token"));
     assert!(hosts.contains("    git_protocol: https"));
@@ -579,6 +573,75 @@ fn gitlab_credential() -> GitCredential {
     }
 }
 
+fn sorted_file_names(dir: &Path) -> Result<Vec<String>> {
+    let mut names = std::fs::read_dir(dir)?
+        .map(|entry| Ok(entry?.file_name().to_string_lossy().into_owned()))
+        .collect::<Result<Vec<_>>>()?;
+    names.sort();
+    Ok(names)
+}
+
+/// Direct-backend sandboxes on one host share `HOME` and `GH_CONFIG_DIR`, so concurrently starting
+/// tasks write the same credential stores at the same time.
+#[test]
+fn concurrent_credential_store_writes_share_directories_safely() -> Result<()> {
+    const WRITERS: usize = 8;
+    const WRITES_PER_WRITER: usize = 25;
+    let home = tempfile::tempdir()?;
+    let home = home.path();
+    let gh_config_dir = home.join(".config").join("gh");
+    let barrier = std::sync::Barrier::new(WRITERS);
+
+    std::thread::scope(|scope| {
+        let writers = (0..WRITERS)
+            .map(|writer| {
+                let barrier = &barrier;
+                let gh_config_dir = &gh_config_dir;
+                scope.spawn(move || -> Result<()> {
+                    let credentials = [
+                        GitCredential {
+                            token: format!("github-token-{writer}"),
+                            ..github_credential()
+                        },
+                        GitCredential {
+                            token: format!("gitlab-token-{writer}"),
+                            ..gitlab_credential()
+                        },
+                    ];
+                    barrier.wait();
+                    for _ in 0..WRITES_PER_WRITER {
+                        write_git_credentials_file(&credentials, home)?;
+                        write_gh_hosts_yml(&credentials, gh_config_dir)?;
+                        write_glab_config(&credentials, home)?;
+                    }
+                    Ok(())
+                })
+            })
+            .collect::<Vec<_>>();
+        writers
+            .into_iter()
+            .try_for_each(|writer| writer.join().expect("credential writer panicked"))
+    })?;
+
+    assert_eq!(sorted_file_names(home)?, [".config", ".git-credentials"]);
+    assert_eq!(sorted_file_names(&gh_config_dir)?, [GH_HOSTS_FILENAME]);
+    assert_eq!(
+        sorted_file_names(&home.join(".config").join("glab-cli"))?,
+        [GLAB_CONFIG_FILENAME]
+    );
+    assert_eq!(
+        std::fs::read_to_string(home.join(".git-credentials"))?
+            .lines()
+            .count(),
+        2
+    );
+    assert!(
+        std::fs::read_to_string(gh_config_dir.join(GH_HOSTS_FILENAME))?
+            .contains("oauth_token: github-token-")
+    );
+    Ok(())
+}
+
 #[test]
 fn merged_credentials_include_each_provider_host() {
     let content = merge_git_credentials_file_content(
@@ -976,15 +1039,9 @@ fn write_glab_config_uses_glab_cli_filename() -> Result<()> {
         temp_dir.path(),
     )?;
 
-    let config_path = glab_config_dir.join(GLAB_CONFIG_FILENAME);
-    assert!(config_path.exists());
-    assert!(
-        !glab_config_dir
-            .join(format!("{GLAB_CONFIG_FILENAME}.tmp"))
-            .exists()
-    );
+    assert_eq!(sorted_file_names(&glab_config_dir)?, [GLAB_CONFIG_FILENAME]);
 
-    let config = std::fs::read_to_string(config_path)?;
+    let config = std::fs::read_to_string(glab_config_dir.join(GLAB_CONFIG_FILENAME))?;
     assert!(config.contains("hosts:"));
     assert!(config.contains("    gitlab.com:"));
     assert!(config.contains("        token: gitlab-token"));