Keep file permissions when extracting zip archives on Unix (#31304)

Follow-up of https://github.com/zed-industries/zed/pull/31080

Stop doing

```rs
#[cfg(not(windows))]
{
    file.set_permissions(<fs::Permissions as fs::unix::PermissionsExt>::from_mode(
        0o755,
    ))
    .await?;
}
```

after extracting zip archives on Unix, and use an API that provides the
file permissions data for each archive entry.

Release Notes:

- N/A

crates/dap/src/adapters.rs

@@ -309,7 +309,7 @@ pub async fn download_adapter_from_github(
             let mut file = File::create(&zip_path).await?;
             futures::io::copy(response.body_mut(), &mut file).await?;
             let file = File::open(&zip_path).await?;
-            extract_zip(&version_path, BufReader::new(file))
+            extract_zip(&version_path, file)
                 .await
                 // we cannot check the status as some adapter include files with names that trigger `Illegal byte sequence`
                 .ok();

crates/dap_adapters/src/codelldb.rs

@@ -399,34 +399,6 @@ impl DebugAdapter for CodeLldbDebugAdapter {
                 };
             let adapter_dir = version_path.join("extension").join("adapter");
             let path = adapter_dir.join("codelldb").to_string_lossy().to_string();
-            // todo("windows")
-            #[cfg(not(windows))]
-            {
-                use smol::fs;
-
-                fs::set_permissions(
-                    &path,
-                    <fs::Permissions as fs::unix::PermissionsExt>::from_mode(0o755),
-                )
-                .await
-                .with_context(|| format!("Settings executable permissions to {path:?}"))?;
-
-                let lldb_binaries_dir = version_path.join("extension").join("lldb").join("bin");
-                let mut lldb_binaries =
-                    fs::read_dir(&lldb_binaries_dir).await.with_context(|| {
-                        format!("reading lldb binaries dir contents {lldb_binaries_dir:?}")
-                    })?;
-                while let Some(binary) = lldb_binaries.next().await {
-                    let binary_entry = binary?;
-                    let path = binary_entry.path();
-                    fs::set_permissions(
-                        &path,
-                        <fs::Permissions as fs::unix::PermissionsExt>::from_mode(0o755),
-                    )
-                    .await
-                    .with_context(|| format!("Settings executable permissions to {path:?}"))?;
-                }
-            }
             self.path_to_codelldb.set(path.clone()).ok();
             command = Some(path);
         };

crates/languages/src/c.rs

@@ -7,7 +7,7 @@ pub use language::*;
 use lsp::{DiagnosticTag, InitializeParams, LanguageServerBinary, LanguageServerName};
 use project::lsp_store::clangd_ext;
 use serde_json::json;
-use smol::{fs, io::BufReader};
+use smol::fs;
 use std::{any::Any, env::consts, path::PathBuf, sync::Arc};
 use util::{ResultExt, archive::extract_zip, fs::remove_matching, maybe, merge_json_value_into};
 
@@ -83,20 +83,10 @@ impl super::LspAdapter for CLspAdapter {
                 "download failed with status {}",
                 response.status().to_string()
             );
-            extract_zip(&container_dir, BufReader::new(response.body_mut()))
+            extract_zip(&container_dir, response.body_mut())
                 .await
                 .with_context(|| format!("unzipping clangd archive to {container_dir:?}"))?;
             remove_matching(&container_dir, |entry| entry != version_dir).await;
-
-            // todo("windows")
-            #[cfg(not(windows))]
-            {
-                fs::set_permissions(
-                    &binary_path,
-                    <fs::Permissions as fs::unix::PermissionsExt>::from_mode(0o755),
-                )
-                .await?;
-            }
         }
 
         Ok(LanguageServerBinary {

crates/languages/src/json.rs

@@ -442,11 +442,7 @@ impl LspAdapter for NodeVersionAdapter {
                 .await
                 .context("downloading release")?;
             if version.url.ends_with(".zip") {
-                extract_zip(
-                    &destination_container_path,
-                    BufReader::new(response.body_mut()),
-                )
-                .await?;
+                extract_zip(&destination_container_path, response.body_mut()).await?;
             } else if version.url.ends_with(".tar.gz") {
                 let decompressed_bytes = GzipDecoder::new(BufReader::new(response.body_mut()));
                 let archive = Archive::new(decompressed_bytes);
@@ -462,15 +458,6 @@ impl LspAdapter for NodeVersionAdapter {
                 &destination_path,
             )
             .await?;
-            // todo("windows")
-            #[cfg(not(windows))]
-            {
-                fs::set_permissions(
-                    &destination_path,
-                    <fs::Permissions as fs::unix::PermissionsExt>::from_mode(0o755),
-                )
-                .await?;
-            }
             remove_matching(&container_dir, |entry| entry != destination_path).await;
         }
         Ok(LanguageServerBinary {

crates/languages/src/rust.rs

@@ -216,7 +216,7 @@ impl LspAdapter for RustLspAdapter {
                         })?;
                 }
                 AssetKind::Zip => {
-                    extract_zip(&destination_path, BufReader::new(response.body_mut()))
+                    extract_zip(&destination_path, response.body_mut())
                         .await
                         .with_context(|| {
                             format!("unzipping {} to {:?}", version.url, destination_path)

crates/languages/src/typescript.rs

@@ -490,7 +490,7 @@ impl LspAdapter for EsLintLspAdapter {
                         })?;
                 }
                 AssetKind::Zip => {
-                    extract_zip(&destination_path, BufReader::new(response.body_mut()))
+                    extract_zip(&destination_path, response.body_mut())
                         .await
                         .with_context(|| {
                             format!("unzipping {} to {:?}", version.url, destination_path)

crates/project/src/image_store.rs

@@ -376,12 +376,6 @@ impl ImageStore {
                 entry.insert(rx.clone());
 
                 let project_path = project_path.clone();
-                // TODO kb this is causing another error, and we also pass a worktree nearby — seems ok to pass "" here?
-                // let image_path = worktree
-                //     .read(cx)
-                //     .absolutize(&project_path.path)
-                //     .map(Arc::from)
-                //     .unwrap_or_else(|_| project_path.path.clone());
                 let load_image = self
                     .state
                     .open_image(project_path.path.clone(), worktree, cx);

crates/supermaven_api/src/supermaven_api.rs

@@ -271,14 +271,6 @@ pub async fn get_supermaven_agent_path(client: Arc<dyn HttpClient>) -> Result<Pa
         .await
         .with_context(|| format!("Unable to write binary to file at {:?}", binary_path))?;
 
-    #[cfg(not(windows))]
-    {
-        file.set_permissions(<fs::Permissions as fs::unix::PermissionsExt>::from_mode(
-            0o755,
-        ))
-        .await?;
-    }
-
     let mut old_binary_paths = fs::read_dir(supermaven_dir()).await?;
     while let Some(old_binary_path) = old_binary_paths.next().await {
         let old_binary_path = old_binary_path?;

crates/util/Cargo.toml

@@ -13,7 +13,7 @@ path = "src/util.rs"
 doctest = true
 
 [features]
-test-support = ["tempfile", "git2", "rand", "util_macros"]
+test-support = ["git2", "rand", "util_macros"]
 
 [dependencies]
 anyhow.workspace = true
@@ -35,7 +35,7 @@ serde_json.workspace = true
 serde_json_lenient.workspace = true
 smol.workspace = true
 take-until.workspace = true
-tempfile = { workspace = true, optional = true }
+tempfile.workspace = true
 unicase.workspace = true
 util_macros = { workspace = true, optional = true }
 walkdir.workspace = true
@@ -51,5 +51,4 @@ dunce = "1.0"
 [dev-dependencies]
 git2.workspace = true
 rand.workspace = true
-tempfile.workspace = true
 util_macros.workspace = true

crates/util/src/archive.rs

@@ -1,11 +1,12 @@
 use std::path::Path;
 
-use anyhow::Result;
-use async_zip::base::read::stream::ZipFileReader;
+use anyhow::{Context as _, Result};
+use async_zip::base::read;
 use futures::{AsyncRead, io::BufReader};
 
+#[cfg(windows)]
 pub async fn extract_zip<R: AsyncRead + Unpin>(destination: &Path, reader: R) -> Result<()> {
-    let mut reader = ZipFileReader::new(BufReader::new(reader));
+    let mut reader = read::stream::ZipFileReader::new(BufReader::new(reader));
 
     let destination = &destination
         .canonicalize()
@@ -14,18 +15,98 @@ pub async fn extract_zip<R: AsyncRead + Unpin>(destination: &Path, reader: R) ->
     while let Some(mut item) = reader.next_with_entry().await? {
         let entry_reader = item.reader_mut();
         let entry = entry_reader.entry();
-        let path = destination.join(entry.filename().as_str().unwrap());
+        let path = destination.join(
+            entry
+                .filename()
+                .as_str()
+                .context("reading zip entry file name")?,
+        );
 
-        if entry.dir().unwrap() {
-            std::fs::create_dir_all(&path)?;
+        if entry
+            .dir()
+            .with_context(|| format!("reading zip entry metadata for path {path:?}"))?
+        {
+            std::fs::create_dir_all(&path)
+                .with_context(|| format!("creating directory {path:?}"))?;
         } else {
-            let parent_dir = path.parent().expect("failed to get parent directory");
-            std::fs::create_dir_all(parent_dir)?;
-            let mut file = smol::fs::File::create(&path).await?;
-            futures::io::copy(entry_reader, &mut file).await?;
+            let parent_dir = path
+                .parent()
+                .with_context(|| format!("no parent directory for {path:?}"))?;
+            std::fs::create_dir_all(parent_dir)
+                .with_context(|| format!("creating parent directory {parent_dir:?}"))?;
+            let mut file = smol::fs::File::create(&path)
+                .await
+                .with_context(|| format!("creating file {path:?}"))?;
+            futures::io::copy(entry_reader, &mut file)
+                .await
+                .with_context(|| format!("extracting into file {path:?}"))?;
         }
 
-        reader = item.skip().await?;
+        reader = item.skip().await.context("reading next zip entry")?;
+    }
+
+    Ok(())
+}
+
+#[cfg(not(windows))]
+pub async fn extract_zip<R: AsyncRead + Unpin>(destination: &Path, reader: R) -> Result<()> {
+    // Unix needs file permissions copied when extracting.
+    // This is only possible to do when a reader impls `AsyncSeek` and `seek::ZipFileReader` is used.
+    // `stream::ZipFileReader` also has the `unix_permissions` method, but it will always return `Some(0)`.
+    //
+    // A typical `reader` comes from a streaming network response, so cannot be sought right away,
+    // and reading the entire archive into the memory seems wasteful.
+    //
+    // So, save the stream into a temporary file first and then get it read with a seeking reader.
+    let mut file = async_fs::File::from(tempfile::tempfile().context("creating a temporary file")?);
+    futures::io::copy(&mut BufReader::new(reader), &mut file)
+        .await
+        .context("saving archive contents into the temporary file")?;
+    let mut reader = read::seek::ZipFileReader::new(BufReader::new(file))
+        .await
+        .context("reading the zip archive")?;
+    let destination = &destination
+        .canonicalize()
+        .unwrap_or_else(|_| destination.to_path_buf());
+    for (i, entry) in reader.file().entries().to_vec().into_iter().enumerate() {
+        let path = destination.join(
+            entry
+                .filename()
+                .as_str()
+                .context("reading zip entry file name")?,
+        );
+
+        if entry
+            .dir()
+            .with_context(|| format!("reading zip entry metadata for path {path:?}"))?
+        {
+            std::fs::create_dir_all(&path)
+                .with_context(|| format!("creating directory {path:?}"))?;
+        } else {
+            let parent_dir = path
+                .parent()
+                .with_context(|| format!("no parent directory for {path:?}"))?;
+            std::fs::create_dir_all(parent_dir)
+                .with_context(|| format!("creating parent directory {parent_dir:?}"))?;
+            let mut file = smol::fs::File::create(&path)
+                .await
+                .with_context(|| format!("creating file {path:?}"))?;
+            let mut entry_reader = reader
+                .reader_with_entry(i)
+                .await
+                .with_context(|| format!("reading entry for path {path:?}"))?;
+            futures::io::copy(&mut entry_reader, &mut file)
+                .await
+                .with_context(|| format!("extracting into file {path:?}"))?;
+
+            if let Some(perms) = entry.unix_permissions() {
+                use std::os::unix::fs::PermissionsExt;
+                let permissions = std::fs::Permissions::from_mode(u32::from(perms));
+                file.set_permissions(permissions)
+                    .await
+                    .with_context(|| format!("setting permissions for file {path:?}"))?;
+            }
+        }
     }
 
     Ok(())
@@ -33,11 +114,9 @@ pub async fn extract_zip<R: AsyncRead + Unpin>(destination: &Path, reader: R) ->
 
 #[cfg(test)]
 mod tests {
-    use std::path::PathBuf;
-
     use async_zip::ZipEntryBuilder;
     use async_zip::base::write::ZipFileWriter;
-    use futures::AsyncWriteExt;
+    use futures::{AsyncSeek, AsyncWriteExt};
     use smol::io::Cursor;
     use tempfile::TempDir;
 
@@ -59,9 +138,23 @@ mod tests {
             let data = smol::fs::read(&path).await?;
 
             let filename = relative_path.display().to_string();
-            let builder = ZipEntryBuilder::new(filename.into(), async_zip::Compression::Deflate);
 
-            writer.write_entry_whole(builder, &data).await?;
+            #[cfg(unix)]
+            {
+                let mut builder =
+                    ZipEntryBuilder::new(filename.into(), async_zip::Compression::Deflate);
+                use std::os::unix::fs::PermissionsExt;
+                let metadata = std::fs::metadata(&path)?;
+                let perms = metadata.permissions().mode() as u16;
+                builder = builder.unix_permissions(perms);
+                writer.write_entry_whole(builder, &data).await?;
+            }
+            #[cfg(not(unix))]
+            {
+                let builder =
+                    ZipEntryBuilder::new(filename.into(), async_zip::Compression::Deflate);
+                writer.write_entry_whole(builder, &data).await?;
+            }
         }
 
         writer.close().await?;
@@ -91,7 +184,7 @@ mod tests {
         dir
     }
 
-    async fn read_archive(path: &PathBuf) -> impl AsyncRead + Unpin {
+    async fn read_archive(path: &Path) -> impl AsyncRead + AsyncSeek + Unpin {
         let data = smol::fs::read(&path).await.unwrap();
         Cursor::new(data)
     }
@@ -115,4 +208,36 @@ mod tests {
             assert_file_content(&dst.join("foo/bar/dar你好.txt"), "你好世界");
         });
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_extract_zip_preserves_executable_permissions() {
+        use std::os::unix::fs::PermissionsExt;
+
+        smol::block_on(async {
+            let test_dir = tempfile::tempdir().unwrap();
+            let executable_path = test_dir.path().join("my_script");
+
+            // Create an executable file
+            std::fs::write(&executable_path, "#!/bin/bash\necho 'Hello'").unwrap();
+            let mut perms = std::fs::metadata(&executable_path).unwrap().permissions();
+            perms.set_mode(0o755); // rwxr-xr-x
+            std::fs::set_permissions(&executable_path, perms).unwrap();
+
+            // Create zip
+            let zip_file = test_dir.path().join("test.zip");
+            compress_zip(test_dir.path(), &zip_file).await.unwrap();
+
+            // Extract to new location
+            let extract_dir = tempfile::tempdir().unwrap();
+            let reader = read_archive(&zip_file).await;
+            extract_zip(extract_dir.path(), reader).await.unwrap();
+
+            // Check permissions are preserved
+            let extracted_path = extract_dir.path().join("my_script");
+            assert!(extracted_path.exists());
+            let extracted_perms = std::fs::metadata(&extracted_path).unwrap().permissions();
+            assert_eq!(extracted_perms.mode() & 0o777, 0o755);
+        });
+    }
 }