diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -499,6 +499,7 @@ dependencies = [ "enums", "env_logger", "fal_client", + "filesys", "image", "log", "midjourney_client", @@ -2858,6 +2859,7 @@ version = "0.0.1" dependencies = [ "errors", "log", + "tempfile", "test_utils", "walkdir", ] diff --git a/artcraft-native/Cargo.toml b/artcraft-native/Cargo.toml --- a/artcraft-native/Cargo.toml +++ b/artcraft-native/Cargo.toml @@ -16,6 +16,7 @@ artcraft_client.workspace = true artcraft_router.workspace = true cookie_store_wrapper.workspace = true fal_client.workspace = true +filesys.workspace = true midjourney_client.workspace = true tokens.workspace = true diff --git a/artcraft-native/src/backend/providers/accounts.rs b/artcraft-native/src/backend/providers/accounts.rs --- a/artcraft-native/src/backend/providers/accounts.rs +++ b/artcraft-native/src/backend/providers/accounts.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use log::warn; +use filesys::private_file::{ensure_private_directory, write_private_atomic}; use midjourney_client::credentials::credential_manager::MidjourneyCredentialManager; const MIDJOURNEY_FILENAME: &str = "midjourney_cookies.toml"; @@ -22,7 +23,7 @@ pub struct ProviderAccounts { impl ProviderAccounts { pub fn load(data_root: &Path) -> Self { let dir = data_root.join("credentials"); - if let Err(err) = std::fs::create_dir_all(&dir) { + if let Err(err) = ensure_private_directory(&dir) { warn!("Couldn't create {}: {err}", dir.display()); } let (path, legacy) = (dir.join(MIDJOURNEY_FILENAME), vec![dir.join(MIDJOURNEY_LEGACY_FILENAME)]); @@ -51,7 +52,7 @@ impl ProviderAccounts { } pub fn set_fal_key(&self, key: &str) -> std::io::Result<()> { - std::fs::write(&*self.fal_key_path, key.trim()) + write_private_atomic(&self.fal_key_path, key.trim().as_bytes()) } pub fn clear_fal_key(&self) -> std::io::Result<()> { diff --git a/artcraft-native/src/backend/session.rs b/artcraft-native/src/backend/session.rs --- a/artcraft-native/src/backend/session.rs +++ b/artcraft-native/src/backend/session.rs @@ -2,6 +2,7 @@ //! first run from the Tauri app's cookie jar so an existing ArtCraft login carries over. use std::fs; +use filesys::private_file::{ensure_private_directory, write_private_atomic}; use std::path::{Path, PathBuf}; use artcraft_client::credentials::storyteller_avt_cookie::StorytellerAvtCookie; @@ -69,7 +70,10 @@ pub fn load(root: &Path) -> Option { pub fn save(root: &Path, creds: &Credentials) { let path = credentials_path(root); - let result = path.parent().map_or(Ok(()), fs::create_dir_all).and_then(|_| fs::write(&path, serde_json::to_vec_pretty(creds).unwrap_or_default())); + let result = path.parent().map_or(Ok(()), ensure_private_directory).and_then(|_| { + let bytes = serde_json::to_vec_pretty(creds).map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?; + write_private_atomic(&path, &bytes) + }); if let Err(err) = result { warn!("Couldn't save the session to {}: {err}", path.display()); } diff --git a/crates/desktop/artcraft/src/core/providers/credentials/payload/api_key.rs b/crates/desktop/artcraft/src/core/providers/credentials/payload/api_key.rs --- a/crates/desktop/artcraft/src/core/providers/credentials/payload/api_key.rs +++ b/crates/desktop/artcraft/src/core/providers/credentials/payload/api_key.rs @@ -21,7 +21,7 @@ impl ApiKeyData { } pub fn save_to_file>(&self, file_path: P) -> Result<(), ApiKeyDataError> { - std::fs::write(file_path, self.0.trim()) + filesys::private_file::write_private_atomic(file_path.as_ref(), self.0.trim().as_bytes()) .map_err(ApiKeyDataError::IoError)?; Ok(()) } diff --git a/crates/desktop/artcraft/src/core/state/data_dir/app_credentials_dir.rs b/crates/desktop/artcraft/src/core/state/data_dir/app_credentials_dir.rs --- a/crates/desktop/artcraft/src/core/state/data_dir/app_credentials_dir.rs +++ b/crates/desktop/artcraft/src/core/state/data_dir/app_credentials_dir.rs @@ -15,6 +15,11 @@ impl DataSubdir for AppCredentialsDir { } } + fn get_or_create_dir>(dir: P) -> anyhow::Result { + filesys::private_file::ensure_private_directory(dir.as_ref())?; + Ok(Self::new_from(dir.as_ref().canonicalize()?)) + } + fn path(&self) -> &Path { &self.path } diff --git a/crates/lib/files/filesys/Cargo.toml b/crates/lib/files/filesys/Cargo.toml --- a/crates/lib/files/filesys/Cargo.toml +++ b/crates/lib/files/filesys/Cargo.toml @@ -22,6 +22,7 @@ errors.workspace = true # KEEP THIS TO A MINIMUM OR NIL. log.workspace = true walkdir.workspace = true +tempfile.workspace = true [dev-dependencies] test_utils = { path = "../../../testing/test_utils" } diff --git a/crates/lib/files/filesys/src/lib.rs b/crates/lib/files/filesys/src/lib.rs --- a/crates/lib/files/filesys/src/lib.rs +++ b/crates/lib/files/filesys/src/lib.rs @@ -34,3 +34,5 @@ pub mod path_to_string; pub mod read_to_trimmed_string; pub mod recursively_find_file_by_name; pub mod rename_across_devices; + +pub mod private_file; diff --git a/crates/lib/files/filesys/src/private_file.rs b/crates/lib/files/filesys/src/private_file.rs new file mode 100644 --- /dev/null +++ b/crates/lib/files/filesys/src/private_file.rs @@ -0,0 +1,93 @@ +//! Atomic replacement for small credential files. Unix permissions are explicit; +//! on other platforms files inherit the credential directory's access controls. + +use std::fs::{self, File}; +use std::io::{self, Write}; +use std::path::Path; + +/// Create or tighten a directory dedicated to credentials. Do not use on a +/// general-purpose parent (for example the user's home or temporary directory). +pub fn ensure_private_directory(path: &Path) -> io::Result<()> { + let mut builder = fs::DirBuilder::new(); + builder.recursive(true); + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt; + builder.mode(0o700); + } + builder.create(path)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(path, fs::Permissions::from_mode(0o700))?; + } + Ok(()) +} + +/// Replace a file only after the new contents have been completely written and +/// synced. A failed write leaves the old file intact and removes the temporary. +pub fn write_private_atomic(path: &Path, contents: &[u8]) -> io::Result<()> { + replace_with(path, |file| file.write_all(contents)) +} + +fn replace_with(path: &Path, write: impl FnOnce(&mut File) -> io::Result<()>) -> io::Result<()> { + let parent = path.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or_else(|| Path::new(".")); + let mut temp = tempfile::Builder::new().prefix(".credential-").tempfile_in(parent)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + temp.as_file().set_permissions(fs::Permissions::from_mode(0o600))?; + } + write(temp.as_file_mut())?; + temp.as_file().sync_all()?; + temp.persist(path).map_err(|err| err.error)?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + mod replacement_tests { + use super::*; + + #[test] + fn interrupted_write_keeps_previous_value_and_cleans_temporary() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + write_private_atomic(&path, b"old synthetic key").unwrap(); + assert!(replace_with(&path, |file| { + file.write_all(b"partial")?; + Err(io::Error::new(io::ErrorKind::Interrupted, "simulated interrupted write")) + }).is_err()); + assert_eq!(fs::read(&path).unwrap(), b"old synthetic key"); + assert_eq!(fs::read_dir(dir.path()).unwrap().count(), 1); + write_private_atomic(&path, b"new synthetic key").unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"new synthetic key"); + } + + #[cfg(unix)] + #[test] + fn creates_and_replaces_with_private_modes_under_permissive_umask() { + use std::os::unix::fs::PermissionsExt; + const CHILD: &str = "ARTCRAFT_PRIVATE_FILE_TEST_CHILD"; + if std::env::var_os(CHILD).is_none() { + let status = std::process::Command::new("sh").args(["-c", "umask 000; exec \"$1\" \"$2\" --exact", "private-file-test"]).arg(std::env::current_exe().unwrap()).arg("private_file::tests::replacement_tests::creates_and_replaces_with_private_modes_under_permissive_umask").env(CHILD, "1").status().unwrap(); + assert!(status.success()); + return; + } + let dir = tempfile::tempdir().unwrap(); + let credentials = dir.path().join("credentials"); + fs::create_dir(&credentials).unwrap(); + fs::set_permissions(&credentials, fs::Permissions::from_mode(0o777)).unwrap(); + ensure_private_directory(&credentials).unwrap(); + assert_eq!(fs::metadata(&credentials).unwrap().permissions().mode() & 0o777, 0o700); + let path = credentials.join("key"); + write_private_atomic(&path, b"first").unwrap(); + assert_eq!(fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o600); + fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); + write_private_atomic(&path, b"replacement").unwrap(); + assert_eq!(fs::metadata(&path).unwrap().permissions().mode() & 0o777, 0o600); + } + } +} diff --git a/netlify.toml b/netlify.toml new file mode 100644 --- /dev/null +++ b/netlify.toml @@ -0,0 +1,7 @@ +# The `storyteller-docs` Netlify site still builds this repository, but its +# build command (./script/netlify_build_docs.sh) and the docs themselves now +# live in storytold/artcraft-services, so every deploy preview here fails. +# Skip those builds until the Netlify site is repointed or disconnected, then +# delete this file. +[build] + ignore = "exit 0"