diff --git a/.changeset/fix-atomic-credential-writes.md b/.changeset/fix-atomic-credential-writes.md new file mode 100644 index 00000000..2f049c45 --- /dev/null +++ b/.changeset/fix-atomic-credential-writes.md @@ -0,0 +1,5 @@ +--- +"gws": patch +--- + +fix: atomic credential file writes to prevent corruption on crash or Ctrl-C diff --git a/src/credential_store.rs b/src/credential_store.rs index 6b4eb9cc..fb532e5f 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -233,18 +233,16 @@ pub fn save_encrypted(json: &str) -> anyhow::Result { let encrypted = encrypt(json.as_bytes())?; + // Write atomically via a sibling .tmp file + rename so the credentials + // file is never left in a corrupt partial-write state on crash/Ctrl-C. + crate::fs_util::atomic_write(&path, &encrypted) + .map_err(|e| anyhow::anyhow!("Failed to write credentials: {e}"))?; + + // Set permissions to 600 on Unix (contains secrets) #[cfg(unix)] { - use std::os::unix::fs::OpenOptionsExt; - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true).mode(0o600); - let mut file = options.open(&path)?; - use std::io::Write; - file.write_all(&encrypted)?; - } - #[cfg(not(unix))] - { - std::fs::write(&path, encrypted)?; + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); } Ok(path) diff --git a/src/fs_util.rs b/src/fs_util.rs new file mode 100644 index 00000000..b387565e --- /dev/null +++ b/src/fs_util.rs @@ -0,0 +1,102 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! File-system utilities. + +use std::io; +use std::path::Path; + +/// Write `data` to `path` atomically. +/// +/// The data is first written to a temporary file alongside the target +/// (e.g. `credentials.enc.tmp`) and then renamed into place. On POSIX +/// systems `rename(2)` is atomic with respect to crashes, so a reader of +/// `path` will always see either the old or the new content — never a +/// partially-written file. +/// +/// # Errors +/// +/// Returns an `io::Error` if the temporary file cannot be written or if the +/// rename fails. +pub fn atomic_write(path: &Path, data: &[u8]) -> io::Result<()> { + // Derive a sibling tmp path, e.g. `/home/user/.config/gws/credentials.enc.tmp` + let file_name = path + .file_name() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "path has no file name"))?; + let tmp_name = format!("{}.tmp", file_name.to_string_lossy()); + let tmp_path = path + .parent() + .map(|p| p.join(&tmp_name)) + .unwrap_or_else(|| std::path::PathBuf::from(&tmp_name)); + + std::fs::write(&tmp_path, data)?; + std::fs::rename(&tmp_path, path)?; + Ok(()) +} + +/// Async variant of [`atomic_write`] for use with tokio. +pub async fn atomic_write_async(path: &Path, data: &[u8]) -> io::Result<()> { + let file_name = path + .file_name() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "path has no file name"))?; + let tmp_name = format!("{}.tmp", file_name.to_string_lossy()); + let tmp_path = path + .parent() + .map(|p| p.join(&tmp_name)) + .unwrap_or_else(|| std::path::PathBuf::from(&tmp_name)); + + tokio::fs::write(&tmp_path, data).await?; + tokio::fs::rename(&tmp_path, path).await?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + #[test] + fn test_atomic_write_creates_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.enc"); + atomic_write(&path, b"hello").unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"hello"); + } + + #[test] + fn test_atomic_write_overwrites_existing() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.enc"); + fs::write(&path, b"old").unwrap(); + atomic_write(&path, b"new").unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"new"); + } + + #[test] + fn test_atomic_write_leaves_no_tmp_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("credentials.enc"); + atomic_write(&path, b"data").unwrap(); + let tmp = dir.path().join("credentials.enc.tmp"); + assert!(!tmp.exists(), "tmp file should be cleaned up by rename"); + } + + #[tokio::test] + async fn test_atomic_write_async_creates_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("token_cache.json"); + atomic_write_async(&path, b"async hello").await.unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"async hello"); + } +} diff --git a/src/main.rs b/src/main.rs index 9df91c7e..37d53255 100644 --- a/src/main.rs +++ b/src/main.rs @@ -28,6 +28,7 @@ mod discovery; mod error; mod executor; mod formatter; +mod fs_util; mod generate_skills; mod helpers; mod oauth_config; diff --git a/src/oauth_config.rs b/src/oauth_config.rs index 210725f0..02154b58 100644 --- a/src/oauth_config.rs +++ b/src/oauth_config.rs @@ -81,7 +81,8 @@ pub fn save_client_config( } let json = serde_json::to_string_pretty(&config)?; - std::fs::write(&path, &json)?; + crate::fs_util::atomic_write(&path, json.as_bytes()) + .map_err(|e| anyhow::anyhow!("Failed to write client config: {e}"))?; // Set file permissions to 600 on Unix (contains secrets) #[cfg(unix)] diff --git a/src/token_storage.rs b/src/token_storage.rs index fc5117a9..cf63ce5c 100644 --- a/src/token_storage.rs +++ b/src/token_storage.rs @@ -60,19 +60,8 @@ impl EncryptedTokenStorage { } } - #[cfg(unix)] - { - let mut options = tokio::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true).mode(0o600); - if let Ok(mut file) = options.open(&self.file_path).await { - use tokio::io::AsyncWriteExt; - let _ = file.write_all(encrypted.as_slice()).await; - } - } - #[cfg(not(unix))] - { - tokio::fs::write(&self.file_path, encrypted).await?; - } + // Write atomically via a sibling .tmp file + rename. + let _ = crate::fs_util::atomic_write_async(&self.file_path, encrypted.as_slice()).await; Ok(()) }