diff --git a/docs/next/website/src/content/docs/integrations.mdx b/docs/next/website/src/content/docs/integrations.mdx index 4cbd0a631b..71baa66ce0 100644 --- a/docs/next/website/src/content/docs/integrations.mdx +++ b/docs/next/website/src/content/docs/integrations.mdx @@ -51,6 +51,35 @@ herdr integration uninstall antigravity-cli herdr integration uninstall grok ``` +On Linux and macOS, shared agent configuration is staged in a temporary file and atomically replaced after a complete write. Existing Windows configs are instead backed up before being updated in place, preserving their ownership and access rules; interrupted updates may require recovery. New files use staged replacement on every platform. Herdr follows symlinks and rejects configurations with multiple hard links before changing hook files; use a separate file or a symlink before retrying. Protection is per file, not an installation transaction, concurrency control, or a power-loss durability guarantee. Later failures can leave earlier installation changes in place. + +
+Windows config recovery + +Before changing an existing config, Herdr saves and syncs its original contents in a private `.herdr-backup` file. A failed write retains this recovery copy and reports both paths. Further updates refuse to overwrite it, even if the config is missing or invalid. Existing EFS-encrypted configs are rejected unchanged rather than creating plaintext backups. + +A `.herdr-backup.pending` file is an unfinished backup, **not a recovery source**. The in-place write has not started for that backup. Inspect the config before removing the unfinished file and retrying. A completed backup can also remain after a successful update if cleanup failed or was interrupted; its presence alone does not mean the current config is damaged. + +If recovery is needed, close the agent and restore the backup's **contents into the existing config**, not by moving the backup over it. In PowerShell, set `$config` to the config path reported by Herdr: + +```powershell +$ErrorActionPreference = 'Stop' +$config = 'C:\path\to\cli.json' +$backup = "$config.herdr-backup" +$bytes = [System.IO.File]::ReadAllBytes($backup) +$file = [System.IO.File]::Open($config, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Write) +try { + $file.SetLength(0) + $file.Write($bytes, 0, $bytes.Length) + $file.Flush($true) +} finally { + $file.Dispose() +} +``` + +Keep the backup if restoration fails. After verifying the config is correct, remove the completed backup with `Remove-Item -LiteralPath $backup` before retrying the integration update. If the original file is missing, restore its intended permissions as well; creating a replacement file does not preserve them automatically. +
+ ## How Herdr uses integrations Herdr uses integrations in two ways: diff --git a/docs/next/website/src/content/docs/ja/integrations.mdx b/docs/next/website/src/content/docs/ja/integrations.mdx index 124010e177..136d74a1e7 100644 --- a/docs/next/website/src/content/docs/ja/integrations.mdx +++ b/docs/next/website/src/content/docs/ja/integrations.mdx @@ -53,6 +53,35 @@ herdr integration uninstall antigravity-cli herdr integration uninstall grok ``` +Linux と macOS では、共有エージェント設定を一時ファイルに完全に書き込んでからアトミックに置き換えます。Windows の既存設定は、所有者とアクセス規則を維持するため、バックアップ後に同じファイルを更新します。中断された更新には復旧が必要な場合があります。新規ファイルはすべてのプラットフォームで一時ファイルを使って作成します。Herdr はシンボリックリンクをたどり、複数のハードリンクがある設定はフックファイルの変更前に拒否します。独立したファイルまたはシンボリックリンクに変更して再試行してください。これはファイル単位の保護であり、インストール全体のトランザクション、同時編集の制御、停電時の永続性は保証しません。後から失敗すると、それまでのインストール変更が残る場合があります。 + +
+Windows の設定の復旧 + +既存設定を変更する前に、Herdr は元の内容を非公開の `.herdr-backup` に保存して同期します。書き込みが失敗すると、この復旧用コピーを保持し、両方のパスを通知します。設定が存在しない場合や無効な場合も、コピーが残っている間は再更新を拒否します。EFS で暗号化された既存設定は、平文バックアップを作らず、変更せずに拒否します。 + +`.herdr-backup.pending` は未完了のバックアップであり、**復旧元として使わないでください**。そのバックアップに対応する設定への書き込みはまだ始まっていません。設定を確認してから未完了ファイルを削除し、再試行してください。更新成功後でも、後処理の失敗や中断により完成済みバックアップが残ることがあります。バックアップの存在だけでは、現在の設定が壊れているとは判断できません。 + +復旧が必要ならエージェントを終了し、バックアップを移動して置き換えるのではなく、**既存設定ファイルに内容を書き戻してください**。PowerShell で `$config` を Herdr が通知した設定パスに変更します。 + +```powershell +$ErrorActionPreference = 'Stop' +$config = 'C:\path\to\cli.json' +$backup = "$config.herdr-backup" +$bytes = [System.IO.File]::ReadAllBytes($backup) +$file = [System.IO.File]::Open($config, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Write) +try { + $file.SetLength(0) + $file.Write($bytes, 0, $bytes.Length) + $file.Flush($true) +} finally { + $file.Dispose() +} +``` + +復旧に失敗したらバックアップを保持してください。設定が正しいことを確認した後、`Remove-Item -LiteralPath $backup` で完成済みバックアップを削除してから統合の更新を再試行します。元のファイルが存在しない場合は、意図したアクセス権も復元してください。新しいファイルを作るだけでは元の権限は保持されません。 +
+ ## Herdr がインテグレーションをどう使うか Herdr はインテグレーションを 2 つの異なる方法で使います: diff --git a/docs/next/website/src/content/docs/zh-cn/integrations.mdx b/docs/next/website/src/content/docs/zh-cn/integrations.mdx index e5535808a1..1cad193201 100644 --- a/docs/next/website/src/content/docs/zh-cn/integrations.mdx +++ b/docs/next/website/src/content/docs/zh-cn/integrations.mdx @@ -53,6 +53,35 @@ herdr integration uninstall antigravity-cli herdr integration uninstall grok ``` +在 Linux 和 macOS 上，共享的智能体配置会完整写入临时文件，再进行原子替换。Windows 上的现有配置则先备份，再原地更新，以保留文件所有者和访问规则；更新中断后可能需要恢复。所有平台的新文件都使用临时文件方式创建。Herdr 会跟随符号链接，并在修改钩子文件之前拒绝具有多个硬链接的配置；请改用独立文件或符号链接后重试。这是单文件保护，不是整个安装的事务、并发编辑控制或断电持久性保证。后续失败仍可能留下之前已完成的安装变更。 + +
+Windows 配置恢复 + +修改现有配置前，Herdr 会将原始内容保存并同步到私有的 `.herdr-backup` 文件。写入失败时会保留此恢复副本并报告两个路径。即使配置已丢失或无效，后续更新也不会覆盖尚未处理的恢复副本。对于现有的 EFS 加密配置，Herdr 会拒绝更新并保持原样，而不是创建明文备份。 + +`.herdr-backup.pending` 是未完成的备份，**不能作为恢复来源**。对应的原地写入尚未开始。请检查配置后再删除未完成文件并重试。更新成功后，如果清理失败或中断，完整备份也可能留下；仅凭备份存在不能判断当前配置已损坏。 + +需要恢复时，请先关闭智能体，将备份的**内容写回现有配置文件**，不要移动备份来替换它。在 PowerShell 中，将 `$config` 设置为 Herdr 报告的配置路径： + +```powershell +$ErrorActionPreference = 'Stop' +$config = 'C:\path\to\cli.json' +$backup = "$config.herdr-backup" +$bytes = [System.IO.File]::ReadAllBytes($backup) +$file = [System.IO.File]::Open($config, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Write) +try { + $file.SetLength(0) + $file.Write($bytes, 0, $bytes.Length) + $file.Flush($true) +} finally { + $file.Dispose() +} +``` + +恢复失败时请保留备份。确认配置正确后，运行 `Remove-Item -LiteralPath $backup` 删除完整备份，再重试集成更新。如果原文件已丢失，还需恢复其预期权限；创建替代文件不会自动保留原权限。 +
+ ## Herdr 如何使用集成 Herdr 以两种不同方式使用集成: diff --git a/src/integration/config_file.rs b/src/integration/config_file.rs new file mode 100644 index 0000000000..11e34c3d25 --- /dev/null +++ b/src/integration/config_file.rs @@ -0,0 +1,147 @@ +//! Protected writes for user-owned integration configuration, not managed assets. + +use std::fs::{self, OpenOptions}; +use std::io; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +#[cfg(test)] +mod tests; + +static NEXT_TEMP: AtomicU64 = AtomicU64::new(0); + +/// Check before changing assets as well as immediately before replacing a config. +/// This is deliberately not config parsing or a transaction across multiple files. +pub(super) fn check_config_targets(dir: &Path, names: &[&str]) -> io::Result<()> { + for name in names { + check_config_target(&dir.join(name))?; + } + Ok(()) +} + +pub(super) fn check_config_target(path: &Path) -> io::Result<()> { + reject_hard_links(path)?; + crate::platform::check_config_write_target(&resolve_target(path)?) +} + +fn reject_hard_links(path: &Path) -> io::Result<()> { + let metadata = match fs::metadata(path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => return Err(error), + }; + if metadata.is_file() && crate::platform::config_file_link_count(path)? > 1 { + return Err(io::Error::other(format!( + "cannot update {}: config has multiple hard links; use a separate file or a symlink before retrying", + path.display() + ))); + } + Ok(()) +} + +// Unlike canonicalize, this also follows dangling symlinks on a first install. +fn resolve_target(path: &Path) -> io::Result { + let mut current = path.to_path_buf(); + for _ in 0..40 { + match fs::symlink_metadata(¤t) { + Ok(metadata) if metadata.file_type().is_symlink() => { + let link = fs::read_link(¤t)?; + current = if link.is_absolute() { + link + } else { + current.parent().unwrap_or(Path::new(".")).join(link) + }; + } + Ok(metadata) if !metadata.is_file() => { + return Err(io::Error::other(format!( + "cannot update {}: config is not a regular file", + path.display() + ))); + } + Ok(_) => return Ok(current), + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(current), + Err(error) => return Err(error), + } + } + Err(io::Error::other(format!( + "cannot update {}: too many symbolic links", + path.display() + ))) +} + +pub(super) fn write_config(path: &Path, contents: impl AsRef<[u8]>) -> io::Result<()> { + check_config_target(path)?; + let target = resolve_target(path)?; + if crate::platform::write_existing_config(&target, contents.as_ref())? { + return Ok(()); + } + let replacement = Replacement::prepare(&target, contents.as_ref())?; + replacement.commit() +} + +struct Replacement { + target: PathBuf, + temporary: PathBuf, +} + +impl Replacement { + fn prepare(path: &Path, contents: &[u8]) -> io::Result { + reject_hard_links(path)?; + let target = resolve_target(path)?; + let existing = match fs::metadata(&target) { + Ok(_) => { + // A writable parent must not let rename bypass file write permissions. + OpenOptions::new().read(true).write(true).open(&target)?; + Some(target.as_path()) + } + Err(error) if error.kind() == io::ErrorKind::NotFound => None, + Err(error) => return Err(error), + }; + let parent = target + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or(Path::new(".")); + for _ in 0..128 { + let sequence = NEXT_TEMP.fetch_add(1, Ordering::Relaxed); + let temporary = parent.join(format!( + ".herdr-config-{}-{sequence}.tmp", + std::process::id() + )); + // Existing configs can contain secrets. Start their staging file private; + // the platform writer preserves the original permissions before publication. + // New configs retain ordinary create/umask/inherited-ACL defaults. + let created = crate::platform::create_config_temporary(&temporary, existing.is_some()); + match created { + Ok(file) => drop(file), + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(error), + } + let replacement = Self { + target: target.clone(), + temporary, + }; + crate::platform::write_config_temporary(existing, &replacement.temporary, contents)?; + return Ok(replacement); + } + Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "could not allocate a unique config temporary file", + )) + } + + fn commit(self) -> io::Result<()> { + reject_hard_links(&self.target)?; + crate::platform::replace_file(&self.temporary, &self.target) + } +} + +impl Drop for Replacement { + fn drop(&mut self) { + // After publication the temporary name is absent. Never remove the target. + if let Err(error) = fs::remove_file(&self.temporary) { + if error.kind() != io::ErrorKind::NotFound { + tracing::warn!(path = %self.temporary.display(), %error, "failed to remove integration config temporary file"); + } + } + } +} diff --git a/src/integration/config_file/tests.rs b/src/integration/config_file/tests.rs new file mode 100644 index 0000000000..916edc67b5 --- /dev/null +++ b/src/integration/config_file/tests.rs @@ -0,0 +1,243 @@ +use super::*; + +struct Directory(PathBuf); + +impl Directory { + fn new() -> Self { + let path = std::env::temp_dir().join(format!( + "herdr-config-write-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir(&path).unwrap(); + Self(path) + } +} + +impl Drop for Directory { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +// Windows existing files have separate native backup/recovery coverage. +#[cfg(unix)] +const ATOMIC_CASES: &[bool] = &[false, true]; +#[cfg(windows)] +const ATOMIC_CASES: &[bool] = &[false]; + +#[test] +fn config_publication_keeps_old_content_until_commit() { + let dir = Directory::new(); + for &existing in ATOMIC_CASES { + let path = dir.0.join(if existing { "existing" } else { "new" }); + if existing { + fs::write(&path, b"old preferences").unwrap(); + } + let staged = Replacement::prepare(&path, b"complete new preferences").unwrap(); + if existing { + assert_eq!(fs::read(&path).unwrap(), b"old preferences"); + } else { + assert!(!path.exists()); + } + assert_eq!( + fs::read(&staged.temporary).unwrap(), + b"complete new preferences" + ); + staged.commit().unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"complete new preferences"); + } + assert_eq!(fs::read_dir(&dir.0).unwrap().count(), ATOMIC_CASES.len()); +} + +#[test] +fn abandoned_and_failed_publication_leave_config_unchanged() { + for &existing in ATOMIC_CASES { + let dir = Directory::new(); + let path = dir.0.join("config"); + if existing { + fs::write(&path, b"original").unwrap(); + } + drop(Replacement::prepare(&path, b"new").unwrap()); + let staged = Replacement::prepare(&path, b"new").unwrap(); + fs::remove_file(&staged.temporary).unwrap(); + assert_eq!(staged.commit().unwrap_err().kind(), io::ErrorKind::NotFound); + if existing { + assert_eq!(fs::read(&path).unwrap(), b"original"); + } else { + assert!(!path.exists()); + } + assert_eq!(fs::read_dir(&dir.0).unwrap().count(), usize::from(existing)); + assert!(write_config(&dir.0, b"not a file").is_err()); + assert!(dir.0.is_dir()); + } +} + +#[test] +fn hard_links_are_rejected_before_staging_and_rechecked_before_commit() { + let dir = Directory::new(); + let path = dir.0.join("config"); + let alias = dir.0.join("alias"); + fs::write(&path, b"original").unwrap(); + #[cfg(unix)] + let staged = Replacement::prepare(&path, b"new").unwrap(); + fs::hard_link(&path, &alias).unwrap(); + #[cfg(unix)] + assert!(staged + .commit() + .unwrap_err() + .to_string() + .contains("multiple hard links")); + for candidate in [&path, &alias] { + let error = write_config(candidate, b"new").unwrap_err().to_string(); + assert!(error.contains(&candidate.display().to_string())); + assert_eq!(fs::read(candidate).unwrap(), b"original"); + assert_eq!( + crate::platform::config_file_link_count(candidate).unwrap(), + 2 + ); + } + assert_eq!(fs::read_dir(&dir.0).unwrap().count(), 2); +} + +#[cfg(unix)] +fn symlink(target: &Path, link: &Path) { + std::os::unix::fs::symlink(target, link).unwrap(); +} +#[cfg(windows)] +fn symlink(target: &Path, link: &Path) { + std::os::windows::fs::symlink_file(target, link).unwrap(); +} + +#[test] +fn symlink_chains_and_dangling_targets_preserve_links() { + let dir = Directory::new(); + let other = dir.0.join("other"); + fs::create_dir(&other).unwrap(); + let target = other.join("preferences"); + let intermediate = other.join("link"); + let entry = dir.0.join("config"); + symlink(&target, &intermediate); + // A Windows reparse target needs native separators, unlike ordinary Win32 paths. + let relative_target = Path::new("other").join("link"); + symlink(&relative_target, &entry); + let original_intermediate = fs::read_link(&intermediate).unwrap(); + assert_eq!( + fs::metadata(&entry).unwrap_err().kind(), + io::ErrorKind::NotFound + ); + write_config(&entry, b"first install").unwrap(); + write_config(&entry, b"second install").unwrap(); + assert_eq!(fs::read(&target).unwrap(), b"second install"); + assert_eq!(fs::read_link(&entry).unwrap(), relative_target); + assert_eq!(fs::read_link(&intermediate).unwrap(), original_intermediate); + assert_eq!(fs::read_dir(&other).unwrap().count(), 2); + let alias = other.join("hard-link"); + fs::hard_link(&target, &alias).unwrap(); + assert!(write_config(&entry, b"must not change").is_err()); + assert_eq!(fs::read(&alias).unwrap(), b"second install"); + assert_eq!(fs::read_link(&entry).unwrap(), relative_target); + + let cycle = dir.0.join("cycle"); + symlink(Path::new("cycle"), &cycle); + assert!(write_config(&cycle, b"must not replace the link").is_err()); + assert_eq!(fs::read_link(&cycle).unwrap(), Path::new("cycle")); +} + +#[test] +fn existing_permissions_and_new_file_defaults_are_preserved() { + let dir = Directory::new(); + let path = dir.0.join("existing"); + fs::write(&path, b"original").unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&path, fs::Permissions::from_mode(0o640)).unwrap(); + } + let permissions = fs::metadata(&path).unwrap().permissions(); + write_config(&path, b"new").unwrap(); + assert_eq!(fs::metadata(&path).unwrap().permissions(), permissions); + let ordinary = dir.0.join("ordinary"); + let new = dir.0.join("new"); + fs::write(&ordinary, b"ordinary creation").unwrap(); + write_config(&new, b"atomic creation").unwrap(); + assert_eq!( + fs::metadata(&ordinary).unwrap().permissions(), + fs::metadata(&new).unwrap().permissions() + ); +} + +#[cfg(unix)] +#[test] +fn writable_directory_does_not_bypass_read_only_config() { + use std::os::unix::{fs::PermissionsExt, process::CommandExt}; + const CHILD: &str = "HERDR_CONFIG_READ_ONLY_TEST"; + if let Some(path) = std::env::var_os(CHILD) { + let error = write_config(Path::new(&path), b"must not replace").unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::PermissionDenied); + println!("read-only rejection executed"); + return; + } + let dir = Directory::new(); + let path = dir.0.join("read-only"); + fs::write(&path, b"original").unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o444)).unwrap(); + fs::set_permissions(&dir.0, fs::Permissions::from_mode(0o777)).unwrap(); + let mut child = std::process::Command::new(std::env::current_exe().unwrap()); + child + .args([ + "--exact", + "integration::config_file::tests::writable_directory_does_not_bypass_read_only_config", + "--nocapture", + ]) + .env(CHILD, &path); + // Root bypasses Unix mode checks. Test the real user path in a child instead. + if unsafe { libc::geteuid() } == 0 { + child.gid(65534).uid(65534); + } + let output = child.output().unwrap(); + assert!(output.status.success(), "child failed: {output:?}"); + assert!(String::from_utf8_lossy(&output.stdout).contains("read-only rejection executed")); + assert_eq!(fs::read(&path).unwrap(), b"original"); + assert_eq!(fs::read_dir(&dir.0).unwrap().count(), 1); +} + +#[cfg(unix)] +#[test] +fn partial_write_errors_preserve_files_and_do_not_remove_collisions() { + const CHILD: &str = "HERDR_CONFIG_PARTIAL_WRITE_TEST"; + if let Some(path) = std::env::var_os(CHILD) { + let dir = PathBuf::from(path); + // This process runs only this test. A collision must neither be used nor removed. + NEXT_TEMP.store(0, Ordering::Relaxed); + let collision = dir.join(format!(".herdr-config-{}-0.tmp", std::process::id())); + fs::write(&collision, b"unrelated file").unwrap(); + for name in ["existing", "new"] { + let error = write_config(&dir.join(name), vec![b'x'; 8192]).unwrap_err(); + assert_eq!(error.raw_os_error(), Some(libc::EFBIG)); + } + assert_eq!(fs::read(collision).unwrap(), b"unrelated file"); + println!("partial-write paths executed"); + return; + } + let dir = Directory::new(); + fs::write(dir.0.join("existing"), b"original").unwrap(); + let output = std::process::Command::new("bash") + .args(["-c", "trap '' XFSZ; ulimit -f 1; exec \"$@\"", "herdr-test"]) + .arg(std::env::current_exe().unwrap()) + .args(["--exact", "integration::config_file::tests::partial_write_errors_preserve_files_and_do_not_remove_collisions", "--nocapture"]) + .env(CHILD, &dir.0) + .output().unwrap(); + assert!(output.status.success(), "child failed: {output:?}"); + assert!(String::from_utf8_lossy(&output.stdout).contains("partial-write paths executed")); + assert_eq!(fs::read(dir.0.join("existing")).unwrap(), b"original"); + assert!(!dir.0.join("new").exists()); + assert_eq!( + fs::read_dir(&dir.0).unwrap().count(), + 2, + "only original and collision may remain" + ); +} diff --git a/src/integration/mod.rs b/src/integration/mod.rs index d956c7bcd7..1e053b8155 100644 --- a/src/integration/mod.rs +++ b/src/integration/mod.rs @@ -2,6 +2,7 @@ mod actions; mod claude_settings; mod command; mod config_edit; +mod config_file; mod env; mod file_ops; mod opencode_config; diff --git a/src/integration/opencode_config.rs b/src/integration/opencode_config.rs index 2ab91e844c..b062365b19 100644 --- a/src/integration/opencode_config.rs +++ b/src/integration/opencode_config.rs @@ -6,6 +6,8 @@ use jsonc_parser::cst::{CstInputValue, CstRootNode}; use jsonc_parser::ParseOptions; use serde_json::Value; +use super::config_file::{check_config_target, write_config}; + const TUI_CONFIG_NAME: &str = "tui.jsonc"; pub(crate) fn tui_config_path(config_dir: &Path) -> PathBuf { @@ -44,6 +46,7 @@ pub(crate) fn add_cli_plugin( plugin_spec: &str, ) -> io::Result> { let path = config_dir.join("cli.json"); + check_config_target(&path)?; // OpenCode imports V1 TUI preferences (`tui.json`, `kv.json`) into cli.json on // its first V2 start, but only while cli.json is absent. Defer registration // while those sources still exist so we do not skip the migration; otherwise @@ -60,6 +63,7 @@ fn cli_migration_pending(config_dir: &Path, state_dir: &Path) -> bool { } fn add_plugin(config_path: PathBuf, key: &str, plugin_spec: &str) -> io::Result { + check_config_target(&config_path)?; let content = if config_path.is_file() { fs::read_to_string(&config_path)? } else { @@ -90,7 +94,7 @@ fn add_plugin(config_path: PathBuf, key: &str, plugin_spec: &str) -> io::Result< } } - fs::write(&config_path, root.to_string())?; + write_config(&config_path, root.to_string())?; Ok(config_path) } @@ -103,6 +107,7 @@ pub(crate) fn remove_cli_plugin(config_dir: &Path, plugin_spec: &str) -> io::Res } fn remove_plugin(config_path: &Path, key: &str, plugin_spec: &str) -> io::Result { + check_config_target(config_path)?; if !config_path.is_file() { return Ok(false); } @@ -133,7 +138,7 @@ fn remove_plugin(config_path: &Path, key: &str, plugin_spec: &str) -> io::Result property.remove(); } - fs::write(config_path, root.to_string())?; + write_config(config_path, root.to_string())?; Ok(true) } @@ -245,6 +250,48 @@ mod tests { .unwrap() } + #[cfg(unix)] + #[test] + fn failed_cli_registration_preserves_existing_config() { + const CHILD_CONFIG: &str = "HERDR_TEST_3970_CONFIG_DIR"; + if let Some(dir) = std::env::var_os(CHILD_CONFIG) { + let dir = PathBuf::from(dir); + let result = add_cli_plugin(&dir, &dir.join("state"), "./herdr-opencode"); + assert_eq!(result.unwrap_err().raw_os_error(), Some(libc::EFBIG)); + println!("registration reached the file-size limit"); + return; + } + + let dir = unique_dir(); + let path = dir.join("cli.json"); + let original = r#"{"theme":{"name":"catppuccin"},"plugins":["example"]}"#; + fs::write(&path, original).unwrap(); + // Apply the limit only to a child, after seeding the existing preferences. + // Ignoring SIGXFSZ makes the kernel return EFBIG instead of killing it. + let output = std::process::Command::new("bash") + .args(["-c", "trap '' XFSZ; ulimit -f 0; exec \"$@\"", "herdr-test"]) + .arg(std::env::current_exe().unwrap()) + .args([ + "--exact", + "integration::opencode_config::tests::failed_cli_registration_preserves_existing_config", + "--nocapture", + ]) + .env(CHILD_CONFIG, &dir) + .output() + .unwrap(); + let actual = fs::read_to_string(&path).unwrap(); + let remaining_files = fs::read_dir(&dir).unwrap().count(); + fs::remove_dir_all(&dir).unwrap(); + assert!(output.status.success(), "child failed: {output:?}"); + assert!(String::from_utf8_lossy(&output.stdout) + .contains("registration reached the file-size limit")); + assert_eq!( + actual, original, + "failed registration must preserve preferences" + ); + assert_eq!(remaining_files, 1, "temporary files must be cleaned up"); + } + #[test] fn add_and_remove_tui_plugin_preserves_jsonc_config() { let dir = unique_dir(); diff --git a/src/integration/targets.rs b/src/integration/targets.rs index d204c55293..cdd31317c4 100644 --- a/src/integration/targets.rs +++ b/src/integration/targets.rs @@ -19,6 +19,7 @@ use super::config_edit::{ remove_direct_hook_commands, remove_flat_command_hook, remove_hermes_plugin_enabled, remove_hook_commands, remove_kimi_config_block, remove_simple_command_hook, }; +use super::config_file::{check_config_targets, write_config}; use super::env::{ antigravity_cli_dir, claude_dir, codex_dir, copilot_dir, cursor_dir, devin_dir, droid_dir, grok_dir, hermes_dir, hermes_plugin_dir, kilo_dir, kimi_dir, mastracode_dir, omp_extension_dir, @@ -121,6 +122,7 @@ pub(crate) fn remove_legacy_pi_extension_from_omp_dir(dir: &Path) -> io::Result< pub(crate) fn install_claude() -> io::Result { let dir = claude_dir()?; + check_config_targets(&dir, &["settings.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "claude directory not found at {}. install claude code first", @@ -145,7 +147,7 @@ pub(crate) fn install_claude() -> io::Result { remove_legacy_bash_hook_file(&hook_path)?; if updated_settings != existing_settings { - fs::write(&settings_path, updated_settings)?; + write_config(&settings_path, updated_settings)?; } Ok(ClaudeInstallPaths { @@ -156,6 +158,7 @@ pub(crate) fn install_claude() -> io::Result { pub(crate) fn install_codex() -> io::Result { let dir = codex_dir()?; + check_config_targets(&dir, &["hooks.json", "config.toml"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "codex config directory not found at {}. install codex first", @@ -197,7 +200,7 @@ pub(crate) fn install_codex() -> io::Result { )?; remove_legacy_bash_hook_file(&hook_path)?; - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; let config_path = dir.join("config.toml"); let existing_config = if config_path.is_file() { @@ -207,7 +210,7 @@ pub(crate) fn install_codex() -> io::Result { }; let new_config = build_codex_config_with_hooks(&existing_config); if new_config != existing_config { - fs::write(&config_path, new_config)?; + write_config(&config_path, new_config)?; } Ok(CodexInstallPaths { @@ -219,6 +222,7 @@ pub(crate) fn install_codex() -> io::Result { pub(crate) fn install_kimi() -> io::Result { let dir = kimi_dir()?; + check_config_targets(&dir, &["config.toml"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "kimi code config directory not found at {}. install kimi code first", @@ -241,7 +245,7 @@ pub(crate) fn install_kimi() -> io::Result { }; let new_config = build_kimi_config_with_hooks(&existing_config, &hook_path); if new_config != existing_config { - fs::write(&config_path, new_config)?; + write_config(&config_path, new_config)?; } remove_legacy_bash_hook_file(&hook_path)?; @@ -253,6 +257,7 @@ pub(crate) fn install_kimi() -> io::Result { pub(crate) fn install_copilot() -> io::Result { let dir = copilot_dir()?; + check_config_targets(&dir, &["settings.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "copilot config directory not found at {}. install github copilot cli first", @@ -297,7 +302,7 @@ pub(crate) fn install_copilot() -> io::Result { } remove_legacy_bash_hook_file(&hook_path)?; - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; Ok(CopilotInstallPaths { hook_path, @@ -307,6 +312,7 @@ pub(crate) fn install_copilot() -> io::Result { pub(crate) fn install_devin() -> io::Result { let dir = devin_dir()?; + check_config_targets(&dir, &["config.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "devin config directory not found at {}. install devin cli first", @@ -353,7 +359,7 @@ pub(crate) fn install_devin() -> io::Result { } remove_legacy_bash_hook_file(&hook_path)?; - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; Ok(DevinInstallPaths { hook_path, @@ -363,6 +369,7 @@ pub(crate) fn install_devin() -> io::Result { pub(crate) fn install_droid() -> io::Result { let dir = droid_dir()?; + check_config_targets(&dir, &["settings.json", "hooks.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "droid config directory not found at {}. install droid first", @@ -413,7 +420,7 @@ pub(crate) fn install_droid() -> io::Result { } remove_legacy_bash_hook_file(&hook_path)?; - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; let hooks_path = dir.join("hooks.json"); let mut updated_legacy_hooks = false; @@ -439,7 +446,7 @@ pub(crate) fn install_droid() -> io::Result { } } if updated_legacy_hooks { - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; } } @@ -453,6 +460,7 @@ pub(crate) fn install_droid() -> io::Result { pub(crate) fn install_opencode() -> io::Result { let dir = opencode_dir()?; + check_config_targets(&dir, &["tui.jsonc", "cli.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "opencode config directory not found at {}. install opencode first", @@ -506,6 +514,7 @@ pub(crate) fn install_kilo() -> io::Result { pub(crate) fn install_hermes() -> io::Result { let dir = hermes_dir()?; + check_config_targets(&dir, &["config.yaml"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "hermes config directory not found at {}. install hermes agent first", @@ -532,7 +541,7 @@ pub(crate) fn install_hermes() -> io::Result { }; let new_config = ensure_hermes_plugin_enabled(&existing_config); if new_config != existing_config { - fs::write(&config_path, new_config)?; + write_config(&config_path, new_config)?; } Ok(HermesInstallPaths { @@ -562,8 +571,10 @@ pub(crate) fn uninstall_omp() -> io::Result { } pub(crate) fn uninstall_claude() -> io::Result { - let hook_path = claude_dir()?.join("hooks").join(CLAUDE_HOOK_INSTALL_NAME); - let settings_path = claude_dir()?.join("settings.json"); + let dir = claude_dir()?; + check_config_targets(&dir, &["settings.json"])?; + let hook_path = dir.join("hooks").join(CLAUDE_HOOK_INSTALL_NAME); + let settings_path = dir.join("settings.json"); let mut updated_settings = false; if settings_path.is_file() { @@ -572,7 +583,7 @@ pub(crate) fn uninstall_claude() -> io::Result { uninstall_claude_settings(&existing_settings, &settings_path, &hook_path)?; updated_settings = new_settings != existing_settings; if updated_settings { - fs::write(&settings_path, new_settings)?; + write_config(&settings_path, new_settings)?; } } @@ -589,6 +600,7 @@ pub(crate) fn uninstall_claude() -> io::Result { pub(crate) fn uninstall_codex() -> io::Result { let codex_dir = codex_dir()?; + check_config_targets(&codex_dir, &["hooks.json"])?; let hook_path = codex_dir.join(CODEX_HOOK_INSTALL_NAME); let hooks_path = codex_dir.join("hooks.json"); let config_path = codex_dir.join("config.toml"); @@ -619,7 +631,7 @@ pub(crate) fn uninstall_codex() -> io::Result { } if updated_hooks { - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; } } @@ -637,6 +649,7 @@ pub(crate) fn uninstall_codex() -> io::Result { pub(crate) fn uninstall_kimi() -> io::Result { let kimi_dir = kimi_dir()?; + check_config_targets(&kimi_dir, &["config.toml"])?; let hook_path = kimi_dir.join("hooks").join(KIMI_HOOK_INSTALL_NAME); let config_path = kimi_dir.join("config.toml"); let mut updated_config = false; @@ -645,7 +658,7 @@ pub(crate) fn uninstall_kimi() -> io::Result { let existing_config = fs::read_to_string(&config_path)?; let new_config = remove_kimi_config_block(&existing_config); if new_config != existing_config { - fs::write(&config_path, new_config)?; + write_config(&config_path, new_config)?; updated_config = true; } } @@ -663,6 +676,7 @@ pub(crate) fn uninstall_kimi() -> io::Result { pub(crate) fn uninstall_copilot() -> io::Result { let copilot_dir = copilot_dir()?; + check_config_targets(&copilot_dir, &["settings.json"])?; let hook_path = copilot_dir.join("hooks").join(COPILOT_HOOK_INSTALL_NAME); let settings_path = copilot_dir.join("settings.json"); let mut updated_settings = false; @@ -691,7 +705,7 @@ pub(crate) fn uninstall_copilot() -> io::Result { } if updated_settings { - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; } } @@ -708,6 +722,7 @@ pub(crate) fn uninstall_copilot() -> io::Result { pub(crate) fn uninstall_devin() -> io::Result { let devin_dir = devin_dir()?; + check_config_targets(&devin_dir, &["config.json"])?; let hook_path = devin_dir.join(DEVIN_HOOK_INSTALL_NAME); let settings_path = devin_dir.join("config.json"); let mut updated_settings = false; @@ -736,7 +751,7 @@ pub(crate) fn uninstall_devin() -> io::Result { } if updated_settings { - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; } } @@ -753,6 +768,7 @@ pub(crate) fn uninstall_devin() -> io::Result { pub(crate) fn uninstall_droid() -> io::Result { let droid_dir = droid_dir()?; + check_config_targets(&droid_dir, &["settings.json", "hooks.json"])?; let hook_path = droid_dir.join("hooks").join(DROID_HOOK_INSTALL_NAME); let hooks_path = droid_dir.join("hooks.json"); let settings_path = droid_dir.join("settings.json"); @@ -780,7 +796,7 @@ pub(crate) fn uninstall_droid() -> io::Result { } if updated_hooks { - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; } } @@ -808,7 +824,7 @@ pub(crate) fn uninstall_droid() -> io::Result { } if updated_settings { - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; } } @@ -827,6 +843,7 @@ pub(crate) fn uninstall_droid() -> io::Result { pub(crate) fn uninstall_opencode() -> io::Result { let dir = opencode_dir()?; + check_config_targets(&dir, &["tui.jsonc", "cli.json"])?; let tui_config_path = tui_config_path(&dir); let plugin_path = dir.join("plugins").join(OPENCODE_PLUGIN_INSTALL_NAME); let tui_plugin_path = dir.join(OPENCODE_TUI_PLUGIN_INSTALL_NAME); @@ -882,6 +899,7 @@ pub(crate) fn uninstall_kilo() -> io::Result { pub(crate) fn uninstall_hermes() -> io::Result { let dir = hermes_dir()?; + check_config_targets(&dir, &["config.yaml"])?; let plugin_dir = hermes_plugin_dir()?; let config_path = dir.join("config.yaml"); @@ -891,7 +909,7 @@ pub(crate) fn uninstall_hermes() -> io::Result { let existing_config = fs::read_to_string(&config_path)?; let new_config = remove_hermes_plugin_enabled(&existing_config); if new_config != existing_config { - fs::write(&config_path, new_config)?; + write_config(&config_path, new_config)?; updated_config = true; } } @@ -906,6 +924,7 @@ pub(crate) fn uninstall_hermes() -> io::Result { pub(crate) fn install_qodercli() -> io::Result { let dir = qodercli_dir()?; + check_config_targets(&dir, &["settings.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "qodercli config directory not found at {}. install qodercli first", @@ -960,7 +979,7 @@ pub(crate) fn install_qodercli() -> io::Result { } remove_legacy_bash_hook_file(&hook_path)?; - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; Ok(QodercliInstallPaths { hook_path, @@ -970,6 +989,7 @@ pub(crate) fn install_qodercli() -> io::Result { pub(crate) fn install_qwen() -> io::Result { let dir = qwen_dir()?; + check_config_targets(&dir, &["settings.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "qwen code config directory not found at {}. install qwen code first", @@ -1013,7 +1033,7 @@ pub(crate) fn install_qwen() -> io::Result { )?; } - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; Ok(QwenInstallPaths { hook_path, @@ -1023,6 +1043,7 @@ pub(crate) fn install_qwen() -> io::Result { pub(crate) fn install_cursor() -> io::Result { let dir = cursor_dir()?; + check_config_targets(&dir, &["hooks.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "cursor config directory not found at {}. install cursor agent cli first", @@ -1069,7 +1090,7 @@ pub(crate) fn install_cursor() -> io::Result { remove_simple_command_hook(hooks, "sessionEnd", &session_command)?; ensure_simple_command_hook(hooks, "sessionStart", session_command)?; - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; Ok(CursorInstallPaths { hook_path, @@ -1078,10 +1099,10 @@ pub(crate) fn install_cursor() -> io::Result { } pub(crate) fn uninstall_qodercli() -> io::Result { - let hook_path = qodercli_dir()? - .join("hooks") - .join(QODERCLI_HOOK_INSTALL_NAME); - let settings_path = qodercli_dir()?.join("settings.json"); + let dir = qodercli_dir()?; + check_config_targets(&dir, &["settings.json"])?; + let hook_path = dir.join("hooks").join(QODERCLI_HOOK_INSTALL_NAME); + let settings_path = dir.join("settings.json"); let mut updated_settings = false; if settings_path.is_file() { @@ -1108,7 +1129,7 @@ pub(crate) fn uninstall_qodercli() -> io::Result { } if updated_settings { - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; } } @@ -1124,8 +1145,10 @@ pub(crate) fn uninstall_qodercli() -> io::Result { } pub(crate) fn uninstall_qwen() -> io::Result { - let hook_path = qwen_dir()?.join("hooks").join(QWEN_HOOK_INSTALL_NAME); - let settings_path = qwen_dir()?.join("settings.json"); + let dir = qwen_dir()?; + check_config_targets(&dir, &["settings.json"])?; + let hook_path = dir.join("hooks").join(QWEN_HOOK_INSTALL_NAME); + let settings_path = dir.join("settings.json"); let mut updated_settings = false; if settings_path.is_file() { @@ -1149,7 +1172,7 @@ pub(crate) fn uninstall_qwen() -> io::Result { } if updated_settings { - fs::write(&settings_path, serde_json::to_string_pretty(&settings)?)?; + write_config(&settings_path, serde_json::to_string_pretty(&settings)?)?; } } @@ -1165,6 +1188,7 @@ pub(crate) fn uninstall_qwen() -> io::Result { pub(crate) fn uninstall_cursor() -> io::Result { let cursor_home = cursor_dir()?; + check_config_targets(&cursor_home, &["hooks.json"])?; let hook_path = cursor_home.join(CURSOR_HOOK_INSTALL_NAME); let hooks_path = cursor_home.join("hooks.json"); let mut updated_hooks = false; @@ -1194,7 +1218,7 @@ pub(crate) fn uninstall_cursor() -> io::Result { } if updated_hooks { - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; } } @@ -1221,6 +1245,7 @@ pub(crate) fn mastracode_hook_command(hook_path: &Path, action: &str) -> String pub(crate) fn install_mastracode() -> io::Result { let mastracode_home = mastracode_dir()?; + check_config_targets(&mastracode_home, &["hooks.json"])?; let hook_dir = mastracode_home.join("hooks"); fs::create_dir_all(&hook_dir)?; @@ -1258,7 +1283,7 @@ pub(crate) fn install_mastracode() -> io::Result { )?; } - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; Ok(MastracodeInstallPaths { hook_path, @@ -1268,6 +1293,7 @@ pub(crate) fn install_mastracode() -> io::Result { pub(crate) fn uninstall_mastracode() -> io::Result { let mastracode_home = mastracode_dir()?; + check_config_targets(&mastracode_home, &["hooks.json"])?; let hook_path = mastracode_home .join("hooks") .join(MASTRACODE_HOOK_INSTALL_NAME); @@ -1300,7 +1326,7 @@ pub(crate) fn uninstall_mastracode() -> io::Result { } if updated_hooks { - fs::write(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; + write_config(&hooks_path, serde_json::to_string_pretty(&hooks_file)?)?; } } @@ -1316,6 +1342,7 @@ pub(crate) fn uninstall_mastracode() -> io::Result { pub(crate) fn install_antigravity_cli() -> io::Result { let dir = antigravity_cli_dir()?; + check_config_targets(&dir, &["hooks.json"])?; if !dir.is_dir() { return Err(io::Error::other(format!( "antigravity cli config directory not found at {}. install antigravity cli first", @@ -1353,7 +1380,7 @@ pub(crate) fn install_antigravity_cli() -> io::Result Value { pub(crate) fn uninstall_antigravity_cli() -> io::Result { let dir = antigravity_cli_dir()?; + check_config_targets(&dir, &["hooks.json"])?; let hook_path = dir.join("hooks").join(ANTIGRAVITY_CLI_HOOK_INSTALL_NAME); let hooks_path = dir.join("hooks.json"); let mut updated_hooks = false; @@ -1411,7 +1439,7 @@ pub(crate) fn uninstall_antigravity_cli() -> io::Result std::io::Result { + use std::os::unix::fs::MetadataExt; + Ok(std::fs::metadata(path)?.nlink()) +} + +pub(crate) fn check_config_write_target(_target: &std::path::Path) -> std::io::Result<()> { + Ok(()) +} + +pub(crate) fn write_existing_config( + _target: &std::path::Path, + _contents: &[u8], +) -> std::io::Result { + // Unix keeps atomic replacement for existing files too. + Ok(false) +} + +pub(crate) fn create_config_temporary( + path: &std::path::Path, + private: bool, +) -> std::io::Result { + use std::os::unix::fs::OpenOptionsExt; + std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(if private { 0o600 } else { 0o666 }) + .open(path) +} + +pub(crate) fn write_config_temporary( + source: Option<&std::path::Path>, + temporary: &std::path::Path, + contents: &[u8], +) -> std::io::Result<()> { + use std::os::{fd::AsRawFd, unix::fs::MetadataExt}; + let mut output = std::fs::OpenOptions::new() + .write(true) + .truncate(true) + .open(temporary)?; + if let Some(source) = source { + let input = std::fs::File::open(source)?; + let metadata = input.metadata()?; + let current = output.metadata()?; + if (metadata.uid(), metadata.gid()) != (current.uid(), current.gid()) { + // Keep ownership before restoring mode/ACLs; chown can clear mode bits. + if unsafe { libc::fchown(output.as_raw_fd(), metadata.uid(), metadata.gid()) } != 0 { + return Err(std::io::Error::last_os_error()); + } + } + // Replace inherited ACLs before enabling the original mode. Prepare all + // access controls while the temporary is empty, before writing secrets. + copy_config_xattrs(input.as_raw_fd(), output.as_raw_fd())?; + output.set_permissions(metadata.permissions())?; + } + output.write_all(contents)?; + output.sync_all() +} + +// Access ACLs and security labels live in xattrs on Linux. Mode bits alone can +// silently broaden access, especially with a default ACL on the parent directory. +fn copy_config_xattrs(source: RawFd, destination: RawFd) -> std::io::Result<()> { + use std::ffi::CStr; + fn names(fd: RawFd) -> std::io::Result> { + let size = unsafe { libc::flistxattr(fd, std::ptr::null_mut(), 0) }; + if size < 0 { + let error = std::io::Error::last_os_error(); + if error.raw_os_error() == Some(libc::ENOTSUP) { + return Ok(Vec::new()); + } + return Err(error); + } + let mut buffer = vec![0; size as usize]; + let read = unsafe { libc::flistxattr(fd, buffer.as_mut_ptr().cast(), buffer.len()) }; + if read < 0 { + return Err(std::io::Error::last_os_error()); + } + buffer.truncate(read as usize); + Ok(buffer) + } + fn value(fd: RawFd, name: &CStr) -> std::io::Result> { + let size = unsafe { libc::fgetxattr(fd, name.as_ptr(), std::ptr::null_mut(), 0) }; + if size < 0 { + return Err(std::io::Error::last_os_error()); + } + let mut buffer = vec![0; size as usize]; + let read = + unsafe { libc::fgetxattr(fd, name.as_ptr(), buffer.as_mut_ptr().cast(), buffer.len()) }; + if read < 0 { + return Err(std::io::Error::last_os_error()); + } + buffer.truncate(read as usize); + Ok(buffer) + } + let source_names = names(source)?; + for bytes in names(destination)?.split_inclusive(|byte| *byte == 0) { + if !source_names + .split_inclusive(|byte| *byte == 0) + .any(|name| name == bytes) + { + let name = CStr::from_bytes_with_nul(bytes).map_err(std::io::Error::other)?; + if unsafe { libc::fremovexattr(destination, name.as_ptr()) } != 0 { + return Err(std::io::Error::last_os_error()); + } + } + } + for bytes in source_names.split_inclusive(|byte| *byte == 0) { + let name = CStr::from_bytes_with_nul(bytes).map_err(std::io::Error::other)?; + let original = value(source, name)?; + // Avoid requiring relabel privileges when the inherited label already matches. + if value(destination, name).is_ok_and(|current| current == original) { + continue; + } + if unsafe { + libc::fsetxattr( + destination, + name.as_ptr(), + original.as_ptr().cast(), + original.len(), + 0, + ) + } != 0 + { + return Err(std::io::Error::last_os_error()); + } + } + Ok(()) +} + pub fn raise_server_nofile_limit() {} pub(crate) fn should_draw_host_cursor_by_default() -> bool { diff --git a/src/platform/linux/config_file_tests.rs b/src/platform/linux/config_file_tests.rs new file mode 100644 index 0000000000..b11c117daa --- /dev/null +++ b/src/platform/linux/config_file_tests.rs @@ -0,0 +1,96 @@ +use super::*; +use std::ffi::CStr; +use std::os::{fd::AsRawFd, unix::fs::MetadataExt}; + +fn set_attribute(file: &std::fs::File, name: &CStr, value: &[u8]) { + assert_eq!( + unsafe { + libc::fsetxattr( + file.as_raw_fd(), + name.as_ptr(), + value.as_ptr().cast(), + value.len(), + 0, + ) + }, + 0, + "{}", + std::io::Error::last_os_error() + ); +} + +fn attribute(file: &std::fs::File, name: &CStr) -> Option> { + let mut value = vec![0; 1024]; + let read = unsafe { + libc::fgetxattr( + file.as_raw_fd(), + name.as_ptr(), + value.as_mut_ptr().cast(), + value.len(), + ) + }; + if read < 0 { + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ENODATA) + ); + return None; + } + value.truncate(read as usize); + Some(value) +} + +#[test] +fn config_metadata_preserves_ownership_and_acl_without_inheriting_extra_access() { + let dir = std::env::temp_dir().join(format!("herdr-config-acl-{}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + // Linux UAPI posix_acl_xattr_header/entry, version 2, little-endian fields. + // Owner rw, named user 65534 read, group none, mask read, other none. + let mut acl = 2_u32.to_le_bytes().to_vec(); + for (tag, permissions, id) in [ + (1_u16, 6_u16, u32::MAX), + (2, 4, 65534), + (4, 0, u32::MAX), + (16, 4, u32::MAX), + (32, 0, u32::MAX), + ] { + acl.extend(tag.to_le_bytes()); + acl.extend(permissions.to_le_bytes()); + acl.extend(id.to_le_bytes()); + } + for has_acl in [false, true] { + let source = dir.join(format!("source-{has_acl}")); + let target = dir.join(format!("target-{has_acl}")); + std::fs::write(&source, b"old").unwrap(); + let input = std::fs::File::open(&source).unwrap(); + if unsafe { libc::geteuid() } == 0 { + assert_eq!(unsafe { libc::fchown(input.as_raw_fd(), 1001, 1002) }, 0); + } + if has_acl { + set_attribute(&input, c"system.posix_acl_access", &acl); + } + set_attribute(&input, c"user.herdr-test", b"preserve this attribute"); + let original = input.metadata().unwrap(); + drop(create_config_temporary(&target, true).unwrap()); + let output = std::fs::File::open(&target).unwrap(); + // Model a default ACL inherited from the destination's parent directory. + set_attribute(&output, c"system.posix_acl_access", &acl); + write_config_temporary(Some(&source), &target, b"new").unwrap(); + let actual = output.metadata().unwrap(); + assert_eq!( + (actual.uid(), actual.gid(), actual.mode()), + (original.uid(), original.gid(), original.mode()) + ); + assert_eq!( + attribute(&output, c"system.posix_acl_access"), + attribute(&input, c"system.posix_acl_access") + ); + assert_eq!( + attribute(&output, c"user.herdr-test"), + Some(b"preserve this attribute".to_vec()) + ); + assert_eq!(std::fs::read(source).unwrap(), b"old"); + assert_eq!(std::fs::read(target).unwrap(), b"new"); + } + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/src/platform/macos.rs b/src/platform/macos.rs index 8840c7ed9b..83c493d889 100644 --- a/src/platform/macos.rs +++ b/src/platform/macos.rs @@ -20,6 +20,149 @@ pub(crate) use super::unix_common::{ wait_client_stream_readable, StatusCommandGuard, }; +#[cfg(test)] +mod config_file_tests; + +pub(crate) fn config_file_link_count(path: &Path) -> std::io::Result { + use std::os::unix::fs::MetadataExt; + Ok(std::fs::metadata(path)?.nlink()) +} + +pub(crate) fn check_config_write_target(_target: &std::path::Path) -> std::io::Result<()> { + Ok(()) +} + +pub(crate) fn write_existing_config( + _target: &std::path::Path, + _contents: &[u8], +) -> std::io::Result { + // Unix keeps atomic replacement for existing files too. + Ok(false) +} + +pub(crate) fn create_config_temporary( + path: &Path, + private: bool, +) -> std::io::Result { + if !private { + return std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(path); + } + use std::os::fd::FromRawFd; + // Darwin's opaque ACL/filesec APIs (, ) are not + // exposed by libc. Supply a non-inheriting empty ACL at creation: clearing + // inherited ACEs later cannot revoke descriptors opened in the meantime. + unsafe extern "C" { + fn acl_init(count: libc::c_int) -> *mut libc::c_void; + fn acl_free(acl: *mut libc::c_void) -> libc::c_int; + fn acl_get_flagset_np(acl: *mut libc::c_void, flags: *mut *mut libc::c_void) + -> libc::c_int; + fn acl_add_flag_np(flags: *mut libc::c_void, flag: libc::c_uint) -> libc::c_int; + fn filesec_init() -> *mut libc::c_void; + fn filesec_free(security: *mut libc::c_void); + fn filesec_set_property( + security: *mut libc::c_void, + property: libc::c_int, + value: *const libc::c_void, + ) -> libc::c_int; + fn openx_np( + path: *const libc::c_char, + flags: libc::c_int, + security: *mut libc::c_void, + ) -> libc::c_int; + } + const FILESEC_MODE: libc::c_int = 4; + const FILESEC_ACL: libc::c_int = 5; + const ACL_FLAG_NO_INHERIT: libc::c_uint = 1 << 17; + let path = + std::ffi::CString::new(path.as_os_str().as_bytes()).map_err(std::io::Error::other)?; + let acl = unsafe { acl_init(0) }; + if acl.is_null() { + return Err(std::io::Error::last_os_error()); + } + let security = unsafe { filesec_init() }; + if security.is_null() { + let error = std::io::Error::last_os_error(); + unsafe { + acl_free(acl); + } + return Err(error); + } + let result = (|| { + let mut flags = std::ptr::null_mut(); + let mode: libc::mode_t = 0o600; + if unsafe { acl_get_flagset_np(acl, &mut flags) } != 0 + || unsafe { acl_add_flag_np(flags, ACL_FLAG_NO_INHERIT) } != 0 + || unsafe { + filesec_set_property(security, FILESEC_MODE, std::ptr::from_ref(&mode).cast()) + } != 0 + || unsafe { + filesec_set_property(security, FILESEC_ACL, std::ptr::from_ref(&acl).cast()) + } != 0 + { + return Err(std::io::Error::last_os_error()); + } + let fd = unsafe { + openx_np( + path.as_ptr(), + libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_CLOEXEC, + security, + ) + }; + if fd < 0 { + return Err(std::io::Error::last_os_error()); + } + // The successful exclusive create returns one owned descriptor. + Ok(unsafe { std::fs::File::from_raw_fd(fd) }) + })(); + unsafe { + filesec_free(security); + acl_free(acl); + } + result +} + +pub(crate) fn write_config_temporary( + source: Option<&Path>, + temporary: &Path, + contents: &[u8], +) -> std::io::Result<()> { + use std::os::{fd::AsRawFd, unix::fs::MetadataExt}; + let mut output = std::fs::OpenOptions::new() + .write(true) + .truncate(true) + .open(temporary)?; + if let Some(source) = source { + let input = std::fs::File::open(source)?; + let metadata = input.metadata()?; + let current = output.metadata()?; + if (metadata.uid(), metadata.gid()) != (current.uid(), current.gid()) + && unsafe { libc::fchown(output.as_raw_fd(), metadata.uid(), metadata.gid()) } != 0 + { + return Err(std::io::Error::last_os_error()); + } + // Prepare access controls while the temporary is still empty. Copy ACLs + // before mode bits so no inherited/default grant can expose the content. + // Do not copy data or old timestamps. + if unsafe { + libc::fcopyfile( + input.as_raw_fd(), + output.as_raw_fd(), + std::ptr::null_mut(), + libc::COPYFILE_ACL | libc::COPYFILE_XATTR, + ) + } != 0 + { + return Err(std::io::Error::last_os_error()); + } + output.set_permissions(metadata.permissions())?; + } + output.write_all(contents)?; + output.sync_all() +} + const PROC_PGRP_ONLY: u32 = 2; const SERVER_NOFILE_LIMIT_TARGET: libc::rlim_t = 8192; diff --git a/src/platform/macos/config_file_tests.rs b/src/platform/macos/config_file_tests.rs new file mode 100644 index 0000000000..f08d6c2aa4 --- /dev/null +++ b/src/platform/macos/config_file_tests.rs @@ -0,0 +1,44 @@ +use super::*; + +#[test] +fn config_replacement_preserves_macos_acl() { + let dir = std::env::temp_dir().join(format!("herdr-config-acl-{}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + let source = dir.join("source"); + let temporary = dir.join("temporary"); + std::fs::write(&source, b"original").unwrap(); + assert!(Command::new("chmod") + .args(["+a", "everyone deny execute"]) + .arg(&source) + .status() + .unwrap() + .success()); + let acl = |path: &Path| { + let output = Command::new("ls").arg("-le").arg(path).output().unwrap(); + assert!(output.status.success()); + String::from_utf8(output.stdout) + .unwrap() + .lines() + .skip(1) + .collect::>() + .join("\n") + }; + let original = acl(&source); + assert!(original.contains("everyone deny execute")); + assert!(Command::new("chmod") + .args(["+a", "everyone allow read,file_inherit"]) + .arg(&dir) + .status() + .unwrap() + .success()); + drop(create_config_temporary(&temporary, true).unwrap()); + assert!( + acl(&temporary).is_empty(), + "staging must not inherit allow ACEs" + ); + write_config_temporary(Some(&source), &temporary, b"new").unwrap(); + assert_eq!(acl(&temporary), original); + assert_eq!(std::fs::read(&source).unwrap(), b"original"); + assert_eq!(std::fs::read(&temporary).unwrap(), b"new"); + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/src/platform/windows.rs b/src/platform/windows.rs index 8344be2dc2..4f96ec7709 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -14,6 +14,7 @@ use std::{ }; mod clipboard_image; +mod config_backup; pub(crate) fn classify_child_exit(status: &portable_pty::ExitStatus) -> super::ChildExitReason { // STATUS_CONTROL_C_EXIT is reported without a Unix signal by portable-pty. @@ -137,6 +138,159 @@ pub(crate) fn replace_file( } } +pub(crate) fn config_file_link_count(path: &std::path::Path) -> std::io::Result { + use windows_sys::Win32::Storage::FileSystem::{ + GetFileInformationByHandle, BY_HANDLE_FILE_INFORMATION, + }; + let file = std::fs::File::open(path)?; + let mut info = BY_HANDLE_FILE_INFORMATION::default(); + if unsafe { GetFileInformationByHandle(file.as_raw_handle(), &mut info) } == 0 { + return Err(std::io::Error::last_os_error()); + } + Ok(u64::from(info.nNumberOfLinks)) +} + +pub(crate) fn create_config_temporary( + path: &std::path::Path, + private: bool, +) -> std::io::Result { + if !private { + return std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(path); + } + use interprocess::os::windows::security_descriptor::{ + AsSecurityDescriptorExt as _, SecurityDescriptor, + }; + use widestring::U16CString; + use windows_sys::Win32::{ + Foundation::GENERIC_WRITE, + Storage::FileSystem::{ + CreateFileW, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_DELETE, FILE_SHARE_READ, + FILE_SHARE_WRITE, + }, + }; + let sddl = + U16CString::from_str("D:P(A;;GA;;;SY)(A;;GA;;;OW)").map_err(std::io::Error::other)?; + let descriptor = SecurityDescriptor::deserialize(&sddl)?; + let mut attributes = SECURITY_ATTRIBUTES { + nLength: size_of::() as u32, + lpSecurityDescriptor: null_mut(), + bInheritHandle: 0, + }; + descriptor.write_to_security_attributes(&mut attributes); + let path = extended_length_path(path)?; + let handle = unsafe { + CreateFileW( + path.as_ptr(), + GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + &attributes, + CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, + null_mut(), + ) + }; + if handle == INVALID_HANDLE_VALUE { + return Err(std::io::Error::last_os_error()); + } + // CreateFileW returned an owned handle; File closes it exactly once. + Ok(unsafe { std::fs::File::from_raw_handle(handle) }) +} + +pub(crate) fn write_config_temporary( + source: Option<&std::path::Path>, + temporary: &std::path::Path, + contents: &[u8], +) -> std::io::Result<()> { + use std::io::Write; + if source.is_some() { + // If preparation finds an existing file, leave it to the recovery-backed + // path instead of applying replacement-file permissions. + return Err(std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + "config appeared while preparing a new file; retry the update", + )); + } + let mut output = std::fs::OpenOptions::new() + .write(true) + .truncate(true) + .open(temporary)?; + output.write_all(contents)?; + output.sync_all() +} + +pub(crate) fn check_config_write_target(target: &std::path::Path) -> std::io::Result<()> { + config_backup::check_recovery(target) +} + +pub(crate) fn write_existing_config( + target: &std::path::Path, + contents: &[u8], +) -> std::io::Result { + config_backup::write_existing(target, contents) +} + +#[cfg(test)] +fn config_security_descriptor( + path: &std::path::Path, + information: windows_sys::Win32::Security::OBJECT_SECURITY_INFORMATION, +) -> std::io::Result> { + use windows_sys::Win32::Security::GetFileSecurityW; + let path = extended_length_path(path)?; + let mut needed = 0; + unsafe { GetFileSecurityW(path.as_ptr(), information, null_mut(), 0, &mut needed) }; + if needed == 0 { + return Err(std::io::Error::last_os_error()); + } + let mut descriptor = vec![0_u8; needed as usize]; + if unsafe { + GetFileSecurityW( + path.as_ptr(), + information, + descriptor.as_mut_ptr().cast(), + needed, + &mut needed, + ) + } == 0 + { + return Err(std::io::Error::last_os_error()); + } + Ok(descriptor) +} + +#[cfg(test)] +fn config_security_sddl( + descriptor: &mut [u8], + information: windows_sys::Win32::Security::OBJECT_SECURITY_INFORMATION, +) -> std::io::Result> { + use windows_sys::Win32::Security::{ + Authorization::{ConvertSecurityDescriptorToStringSecurityDescriptorW, SDDL_REVISION_1}, + SACL_SECURITY_INFORMATION, + }; + let mut text = null_mut(); + if unsafe { + ConvertSecurityDescriptorToStringSecurityDescriptorW( + descriptor.as_mut_ptr().cast(), + SDDL_REVISION_1, + // Only labels were queried from the SACL. Serialize that returned + // SACL too; this does not request audit access to either file. + information | SACL_SECURITY_INFORMATION, + &mut text, + null_mut(), + ) + } == 0 + { + return Err(std::io::Error::last_os_error()); + } + let result = unsafe { widestring::U16CStr::from_ptr_str(text) } + .as_slice() + .to_vec(); + unsafe { LocalFree(text.cast()) }; + Ok(result) +} + pub(crate) fn set_default_plugin_pane_pwd( _env: &mut Vec<(String, String)>, _cwd: &std::path::Path, diff --git a/src/platform/windows/config_backup.rs b/src/platform/windows/config_backup.rs new file mode 100644 index 0000000000..8e7390abf8 --- /dev/null +++ b/src/platform/windows/config_backup.rs @@ -0,0 +1,165 @@ +//! Recovery-backed writes for existing Windows integration configs. + +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Read, Seek, Write}; +use std::os::windows::{fs::MetadataExt, io::AsRawHandle}; +use std::path::{Path, PathBuf}; +use windows_sys::Win32::Storage::FileSystem::{ + GetFileInformationByHandle, MoveFileExW, BY_HANDLE_FILE_INFORMATION, FILE_ATTRIBUTE_ENCRYPTED, +}; + +#[cfg(test)] +mod tests; + +fn backup_paths(target: &Path) -> (PathBuf, PathBuf) { + let mut name = target.as_os_str().to_os_string(); + name.push(".herdr-backup"); + let complete = PathBuf::from(&name); + name.push(".pending"); + (complete, PathBuf::from(name)) +} + +pub(super) fn check_recovery(target: &Path) -> io::Result<()> { + let (complete, pending) = backup_paths(target); + for (path, description) in [ + (&complete, "recovery copy"), + (&pending, "unfinished backup"), + ] { + match fs::symlink_metadata(path) { + Ok(_) => { + return Err(io::Error::other(format!( + "cannot update {}: {description} exists at {}; inspect the config and resolve this backup before retrying", + target.display(), path.display() + ))); + } + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(error), + } + } + Ok(()) +} + +struct PendingBackup(PathBuf); +impl Drop for PendingBackup { + fn drop(&mut self) { + // Only the incomplete name is owned by this cleanup. The completed + // recovery copy must survive errors and must never be removed here. + if let Err(error) = fs::remove_file(&self.0) { + if error.kind() != io::ErrorKind::NotFound { + tracing::warn!(path = %self.0.display(), %error, "failed to remove incomplete config backup"); + } + } + } +} + +// Internal observation points let native tests terminate a child at actual I/O +// boundaries. The production callback is a no-op; there is no runtime failpoint. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Phase { + BackupChunk, + BackupReady, + Truncated, + Committed, +} + +pub(super) fn write_existing(target: &Path, contents: &[u8]) -> io::Result { + write_observed(target, contents, |_| Ok(())) +} + +fn check_source(file: &File, target: &Path) -> io::Result<()> { + if file.metadata()?.file_attributes() & FILE_ATTRIBUTE_ENCRYPTED != 0 { + return Err(io::Error::other(format!( + "cannot update {}: encrypted configs are not supported by recovery-backed writes; the original was not changed", + target.display() + ))); + } + let mut info = BY_HANDLE_FILE_INFORMATION::default(); + if unsafe { GetFileInformationByHandle(file.as_raw_handle(), &mut info) } == 0 { + return Err(io::Error::last_os_error()); + } + if info.nNumberOfLinks > 1 { + return Err(io::Error::other(format!( + "cannot update {}: config has multiple hard links; use a separate file or a symlink before retrying", + target.display() + ))); + } + Ok(()) +} + +fn write_observed( + target: &Path, + contents: &[u8], + mut observe: impl FnMut(Phase) -> io::Result<()>, +) -> io::Result { + check_recovery(target)?; + // Keep this handle for both backup reads and the destructive write. Do not + // reopen the pathname after saving the original contents. + let mut source = match OpenOptions::new().read(true).write(true).open(target) { + Ok(source) => source, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(false), + Err(error) => return Err(error), + }; + check_source(&source, target)?; + let (complete, pending) = backup_paths(target); + let mut backup = super::create_config_temporary(&pending, true)?; + let pending = PendingBackup(pending); + let preparation = (|| { + let mut remaining = source.metadata()?.len(); + let mut buffer = [0_u8; 8192]; + while remaining != 0 { + let requested = remaining.min(buffer.len() as u64) as usize; + let count = source.read(&mut buffer[..requested])?; + if count == 0 { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "config shortened during backup", + )); + } + backup.write_all(&buffer[..count])?; + remaining -= count as u64; + observe(Phase::BackupChunk)?; + } + backup.sync_all()?; + drop(backup); + let from = super::extended_length_path(&pending.0)?; + let to = super::extended_length_path(&complete)?; + // No REPLACE_EXISTING: never overwrite a previous recovery copy. + if unsafe { MoveFileExW(from.as_ptr(), to.as_ptr(), 0) } == 0 { + return Err(io::Error::last_os_error()); + } + Ok::<_, io::Error>(()) + })(); + preparation.map_err(|error| { + io::Error::other(format!( + "cannot back up {}; original not changed (unfinished backup: {}): {error}", + target.display(), + pending.0.display() + )) + })?; + + let update = (|| { + observe(Phase::BackupReady)?; + check_source(&source, target)?; + source.rewind()?; + source.set_len(0)?; + observe(Phase::Truncated)?; + source.write_all(contents)?; + source.sync_all()?; + Ok::<_, io::Error>(()) + })(); + update.map_err(|error| { + io::Error::other(format!( + "could not update {}; config may be incomplete; original contents retained at {}: {error}", + target.display(), complete.display() + )) + })?; + // Success here is after the new contents were synced. A cleanup failure is + // not an unchanged-original failure, and must not trigger automatic rollback. + observe(Phase::Committed).and_then(|()| fs::remove_file(&complete)).map_err(|error| { + io::Error::other(format!( + "updated {}, but recovery copy remains at {}; verify the config and remove that copy before retrying: {error}", + target.display(), complete.display() + )) + })?; + Ok(true) +} diff --git a/src/platform/windows/config_backup/tests.rs b/src/platform/windows/config_backup/tests.rs new file mode 100644 index 0000000000..966ba2a9d0 --- /dev/null +++ b/src/platform/windows/config_backup/tests.rs @@ -0,0 +1,375 @@ +use super::super::{config_security_descriptor, config_security_sddl}; +use super::*; +use std::process::Command; +use windows_sys::Win32::{ + Security::{ + DACL_SECURITY_INFORMATION, GROUP_SECURITY_INFORMATION, LABEL_SECURITY_INFORMATION, + OWNER_SECURITY_INFORMATION, + }, + Storage::FileSystem::{ + EncryptFileW, LockFileEx, LOCKFILE_EXCLUSIVE_LOCK, LOCKFILE_FAIL_IMMEDIATELY, + }, + System::IO::OVERLAPPED, +}; + +struct Directory(PathBuf); +impl Directory { + fn new(case: &str) -> Self { + let path = + std::env::temp_dir().join(format!("herdr-config-backup-{case}-{}", std::process::id())); + fs::create_dir(&path).unwrap(); + Self(path) + } +} +impl Drop for Directory { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} +fn powershell(script: &str, path: &Path) { + let output = Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", script]) + .env("HERDR_TEST_CONFIG_SOURCE", path) + .output() + .unwrap(); + assert!( + output.status.success() && output.stderr.is_empty(), + "{output:?}" + ); +} +fn security(path: &Path) -> Vec { + let flags = OWNER_SECURITY_INFORMATION + | GROUP_SECURITY_INFORMATION + | DACL_SECURITY_INFORMATION + | LABEL_SECURITY_INFORMATION; + let mut descriptor = config_security_descriptor(path, flags).unwrap(); + config_security_sddl(&mut descriptor, flags).unwrap() +} +fn identity(path: &Path) -> (u32, u32, u32) { + let file = File::open(path).unwrap(); + let mut info = BY_HANDLE_FILE_INFORMATION::default(); + assert_ne!( + unsafe { GetFileInformationByHandle(file.as_raw_handle(), &mut info) }, + 0 + ); + ( + info.dwVolumeSerialNumber, + info.nFileIndexHigh, + info.nFileIndexLow, + ) +} +fn lock_range(path: &Path, offset: u32) -> File { + let file = OpenOptions::new() + .read(true) + .write(true) + .open(path) + .unwrap(); + let mut overlapped = OVERLAPPED::default(); + overlapped.Anonymous.Anonymous.Offset = offset; + assert_ne!( + unsafe { + LockFileEx( + file.as_raw_handle(), + LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, + 0, + 1, + 0, + &mut overlapped, + ) + }, + 0, + "{}", + io::Error::last_os_error() + ); + file +} +fn successful_update(case: &str) { + let dir = Directory::new(case); + let source = dir.0.join("config"); + fs::write(&source, b"original preferences").unwrap(); + fs::write(dir.0.join("config:private"), b"original stream").unwrap(); + if case == "protected" || case == "unprotected" || case == "moved" { + powershell( + &format!( + r#" +$ErrorActionPreference = 'Stop' +$acl = [System.IO.File]::GetAccessControl($env:HERDR_TEST_CONFIG_SOURCE) +$acl.SetAccessRuleProtection(${}, $false) +$sid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User +$rule = [System.Security.AccessControl.FileSystemAccessRule]::new($sid, [System.Security.AccessControl.FileSystemRights]::FullControl, [System.Security.AccessControl.AccessControlType]::Allow) +$acl.AddAccessRule($rule) +[System.IO.File]::SetAccessControl($env:HERDR_TEST_CONFIG_SOURCE, $acl) +"#, + case == "protected" + ), + &source, + ); + } + if case == "metadata" { + powershell( + r#" +$ErrorActionPreference = 'Stop' +$acl = [System.IO.File]::GetAccessControl($env:HERDR_TEST_CONFIG_SOURCE) +$acl.SetOwner([System.Security.Principal.WindowsIdentity]::GetCurrent().User) +$acl.SetGroup([System.Security.Principal.SecurityIdentifier]::new('S-1-5-32-545')) +[System.IO.File]::SetAccessControl($env:HERDR_TEST_CONFIG_SOURCE, $acl) +$null = & icacls.exe $env:HERDR_TEST_CONFIG_SOURCE /setintegritylevel L +if ($LASTEXITCODE -ne 0) { throw 'could not install low integrity label' } +"#, + &source, + ); + } + let source = if case == "moved" { + let original = security(&source); + let parent = dir.0.join("broader-parent"); + fs::create_dir(&parent).unwrap(); + powershell( + r#" +$ErrorActionPreference = 'Stop' +$acl = [System.IO.Directory]::GetAccessControl($env:HERDR_TEST_CONFIG_SOURCE) +$sid = [System.Security.Principal.SecurityIdentifier]::new('S-1-5-32-546') +$rule = [System.Security.AccessControl.FileSystemAccessRule]::new($sid, [System.Security.AccessControl.FileSystemRights]::Read, [System.Security.AccessControl.InheritanceFlags]::ObjectInherit, [System.Security.AccessControl.PropagationFlags]::None, [System.Security.AccessControl.AccessControlType]::Allow) +$acl.AddAccessRule($rule) +[System.IO.Directory]::SetAccessControl($env:HERDR_TEST_CONFIG_SOURCE, $acl) +"#, + &parent, + ); + let moved = parent.join("config"); + fs::rename(&source, &moved).unwrap(); + assert_eq!(security(&moved), original); + assert!(!String::from_utf16_lossy(&original).contains(";;;BG)")); + let inherited = parent.join("inherited"); + fs::write(&inherited, b"").unwrap(); + assert!(String::from_utf16_lossy(&security(&inherited)).contains(";;;BG)")); + moved + } else { + source + }; + let before = security(&source); + let before_id = identity(&source); + let text = String::from_utf16_lossy(&before); + match case { + "legacy" => assert!(text.contains("D:("), "{text}"), + "protected" => assert!(text.contains("D:PAI"), "{text}"), + "unprotected" | "moved" => assert!(text.contains("D:AI"), "{text}"), + "metadata" => assert!(text.contains("G:BU") && text.contains(";;;LW)"), "{text}"), + _ => unreachable!(), + } + let (complete, pending) = backup_paths(&source); + let mut phases = Vec::new(); + assert!(write_observed(&source, b"new", |phase| { + phases.push(phase); + if phase == Phase::BackupReady { + assert_eq!(fs::read(&complete).unwrap(), b"original preferences"); + assert_eq!(fs::read(&source).unwrap(), b"original preferences"); + assert!(!pending.exists()); + let private = String::from_utf16_lossy(&security(&complete)); + assert!( + private.contains("D:P") && !private.contains(";;;BG)"), + "{private}" + ); + } + if phase == Phase::Truncated { + assert!(fs::read(&source).unwrap().is_empty()); + } + Ok(()) + }) + .unwrap()); + assert_eq!( + phases, + [ + Phase::BackupChunk, + Phase::BackupReady, + Phase::Truncated, + Phase::Committed + ] + ); + assert_eq!(fs::read(&source).unwrap(), b"new"); + assert_eq!( + fs::read(source.with_file_name("config:private")).unwrap(), + b"original stream" + ); + assert_eq!(security(&source), before); + assert_eq!(identity(&source), before_id); + assert!(!complete.exists() && !pending.exists()); +} +#[test] +fn backup_preserves_legacy() { + successful_update("legacy"); +} +#[test] +fn backup_preserves_protected() { + successful_update("protected"); +} +#[test] +fn backup_preserves_unprotected() { + successful_update("unprotected"); +} +#[test] +fn backup_preserves_moved() { + successful_update("moved"); +} +#[test] +fn backup_preserves_metadata() { + successful_update("metadata"); +} + +#[test] +fn native_io_failures_preserve_recovery_ordering() { + let dir = Directory::new("io-failures"); + for during_backup in [true, false] { + let path = dir.0.join(if during_backup { "backup" } else { "write" }); + let original = vec![b'a'; if during_backup { 32768 } else { 16 }]; + fs::write(&path, &original).unwrap(); + let before = security(&path); + let before_id = identity(&path); + // First case fails a later backup read after one real chunk. In the + // second, lock after truncation so the actual new WriteFile must fail. + let mut locked = during_backup.then(|| lock_range(&path, 8192)); + let mut phases = Vec::new(); + let error = write_observed(&path, &vec![b'x'; 16384], |phase| { + phases.push(phase); + if !during_backup && phase == Phase::Truncated { + locked = Some(lock_range(&path, 4096)); + } + Ok(()) + }) + .unwrap_err(); + eprintln!("during_backup={during_backup}: {error}; phases={phases:?}"); + assert!(error.to_string().contains("os error 33")); + drop(locked); + let (complete, pending) = backup_paths(&path); + assert!(!pending.exists()); + assert_eq!(security(&path), before); + assert_eq!(identity(&path), before_id); + if during_backup { + assert_eq!(phases, [Phase::BackupChunk]); + assert_eq!(fs::read(&path).unwrap(), original); + assert!(!complete.exists()); + } else { + assert!(phases.contains(&Phase::Truncated)); + assert_eq!(fs::read(&complete).unwrap(), original); + assert!(error.to_string().contains(&complete.display().to_string())); + assert!(write_existing(&path, b"retry") + .unwrap_err() + .to_string() + .contains("recovery copy")); + // Recovery is contents-only, never rename over the original file. + fs::write(&path, fs::read(&complete).unwrap()).unwrap(); + assert_eq!(identity(&path), before_id); + assert_eq!(security(&path), before); + fs::remove_file(&complete).unwrap(); + assert!(write_existing(&path, b"resolved").unwrap()); + } + } +} + +#[test] +fn interrupted_backups_and_writes_are_distinguishable() { + const CHILD: &str = "HERDR_BACKUP_CRASH_TEST"; + if let Some(path) = std::env::var_os(CHILD) { + let path = PathBuf::from(path); + let crash_phase = if path.file_name().unwrap() == "copy" { + Phase::BackupChunk + } else { + Phase::Truncated + }; + let _ = write_observed(&path, b"new", |phase| { + if phase == crash_phase { + std::process::exit(23); + } + Ok(()) + }); + panic!("crash phase not reached"); + } + let dir = Directory::new("crash"); + for name in ["copy", "write"] { + let path = dir.0.join(name); + let original = vec![b'a'; 32768]; + fs::write(&path, &original).unwrap(); + let output = Command::new(std::env::current_exe().unwrap()).args(["--exact", "platform::windows::config_backup::tests::interrupted_backups_and_writes_are_distinguishable", "--nocapture"]) + .env(CHILD, &path).output().unwrap(); + assert_eq!(output.status.code(), Some(23), "{output:?}"); + let (complete, pending) = backup_paths(&path); + if name == "copy" { + assert_eq!(fs::read(&pending).unwrap().len(), 8192); + assert!(!complete.exists()); + assert_eq!(fs::read(&path).unwrap(), original); + assert!(check_recovery(&path) + .unwrap_err() + .to_string() + .contains("unfinished backup")); + } else { + assert!(!pending.exists()); + assert_eq!(fs::read(&complete).unwrap(), original); + assert!(fs::read(&path).unwrap().is_empty()); + fs::remove_file(&path).unwrap(); + assert!(write_existing(&path, b"must not recreate").is_err()); + assert!(!path.exists()); + assert_eq!(fs::read(&complete).unwrap(), original); + } + } +} + +#[test] +fn completed_write_cleanup_failure_and_foreign_backups_are_reported() { + use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::{FILE_SHARE_READ, FILE_SHARE_WRITE}; + let dir = Directory::new("cleanup"); + let path = dir.0.join("config"); + fs::write(&path, b"original").unwrap(); + let (complete, _) = backup_paths(&path); + fs::write(&complete, b"foreign contents").unwrap(); + assert!(write_existing(&path, b"rejected").is_err()); + assert_eq!(fs::read(&complete).unwrap(), b"foreign contents"); + assert_eq!(fs::read(&path).unwrap(), b"original"); + fs::remove_file(&complete).unwrap(); + let mut held = None; + let error = write_observed(&path, b"saved", |phase| { + if phase == Phase::Committed { + held = Some( + OpenOptions::new() + .read(true) + .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE) + .open(&complete)?, + ); + } + Ok(()) + }) + .unwrap_err(); + assert!( + error.to_string().contains("updated ") + && error.to_string().contains("recovery copy remains") + ); + assert_eq!(fs::read(&path).unwrap(), b"saved"); + assert_eq!(fs::read(&complete).unwrap(), b"original"); + assert!(write_existing(&path, b"retry").is_err()); + drop(held); +} + +#[test] +fn encrypted_config_is_rejected_without_plaintext_backup() { + let dir = Directory::new("encrypted"); + let path = dir.0.join("config"); + fs::write(&path, b"encrypted original").unwrap(); + let wide = super::super::extended_length_path(&path).unwrap(); + assert_ne!( + unsafe { EncryptFileW(wide.as_ptr()) }, + 0, + "cannot establish EFS fixture: {}", + io::Error::last_os_error() + ); + assert_ne!( + fs::metadata(&path).unwrap().file_attributes() & FILE_ATTRIBUTE_ENCRYPTED, + 0 + ); + let before = security(&path); + let before_id = identity(&path); + let error = write_existing(&path, b"new").unwrap_err(); + assert!(error.to_string().contains("encrypted configs")); + assert_eq!(fs::read(&path).unwrap(), b"encrypted original"); + assert_eq!(security(&path), before); + assert_eq!(identity(&path), before_id); + let (complete, pending) = backup_paths(&path); + assert!(!complete.exists() && !pending.exists()); +}