Skip to content

fix(security): create atomic-write temp files with restrictive permissions #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
qihan-bot wants to merge 1 commit into
base: main
Choose a base branch
from
+59 −2
Open

fix(security): create atomic-write temp files with restrictive permissions #531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/security-fix-atomic-write-permissions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Create atomic-write temp files with restrictive permissions before rename.
56 changes: 54 additions & 2 deletions src/fs_util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
//! File-system utilities.

use std::io;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;

/// Write `data` to `path` atomically.
Expand All @@ -40,7 +42,22 @@ pub fn atomic_write(path: &Path, data: &[u8]) -> io::Result<()> {
.map(|p| p.join(&tmp_name))
.unwrap_or_else(|| std::path::PathBuf::from(&tmp_name));

std::fs::write(&tmp_path, data)?;
{
use std::fs::OpenOptions;
use std::io::Write;

let mut opts = OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
opts.mode(0o600);
}
Comment on lines +51 to +54
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This correctly applies restrictive permissions on Unix. However, this security fix is platform-specific and does not apply to Windows. On Windows, the temporary file for sensitive data will still be created with default permissions, which could allow other users on the system to read it. This leaves the security vulnerability open on Windows systems.

For a robust, cross-platform solution, I recommend using the tempfile crate (which is already a dev-dependency). It handles secure temporary file creation on major platforms by default, which would fully resolve this security issue.


let mut file = opts.open(&tmp_path)?;
file.write_all(data)?;
file.sync_all()?;
}

std::fs::rename(&tmp_path, path)?;
Ok(())
}
Expand All @@ -56,7 +73,22 @@ pub async fn atomic_write_async(path: &Path, data: &[u8]) -> io::Result<()> {
.map(|p| p.join(&tmp_name))
.unwrap_or_else(|| std::path::PathBuf::from(&tmp_name));

tokio::fs::write(&tmp_path, data).await?;
{
use tokio::fs::OpenOptions;
use tokio::io::AsyncWriteExt;

let mut opts = OpenOptions::new();
opts.write(true).create(true).truncate(true);
#[cfg(unix)]
{
opts.mode(0o600);
}
Comment on lines +82 to +85
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to the synchronous version, this fix only applies restrictive permissions on Unix systems. The vulnerability of a readable temporary file remains on Windows.

To address this cross-platform, you could consider using a crate like async-tempfile, or refactor atomic_write to use the tempfile crate and then call it from this async function within a tokio::task::spawn_blocking block. This would ensure the temporary file is created securely on all supported platforms.


let mut file = opts.open(&tmp_path).await?;
file.write_all(data).await?;
file.sync_all().await?;
}

tokio::fs::rename(&tmp_path, path).await?;
Ok(())
}
Expand Down Expand Up @@ -99,4 +131,24 @@ mod tests {
atomic_write_async(&path, b"async hello").await.unwrap();
assert_eq!(fs::read(&path).unwrap(), b"async hello");
}

#[tokio::test]
async fn test_atomic_write_permissions() {
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;

let dir = tempfile::tempdir().unwrap();

let sync_path = dir.path().join("sync.txt");
atomic_write(&sync_path, b"sync").unwrap();
let sync_mode = fs::metadata(&sync_path).unwrap().permissions().mode() & 0o777;
assert_eq!(sync_mode, 0o600);

let async_path = dir.path().join("async.txt");
atomic_write_async(&async_path, b"async").await.unwrap();
let async_mode = fs::metadata(&async_path).unwrap().permissions().mode() & 0o777;
assert_eq!(async_mode, 0o600);
}
}
}
Loading