fix(desktop): restrict shared-agent sync to dev data dirs (#1597)

This commit is contained in:
Will Pfleger
2026-07-07 17:35:15 -04:00
committed by GitHub
parent cc42a49799
commit e5f831d2c2
6 changed files with 394 additions and 66 deletions
+3 -1
View File
@@ -88,7 +88,9 @@ const overrides = new Map([
// dev-build CLI symlink: cli_link_name helper + is_dev param on
// ensure_cli_symlink + prod/dev test variants add ~68 lines. Load-bearing;
// queued to split with the rest of this list.
["src-tauri/src/managed_agents/nest.rs", 1569],
// +4 lines: adopt shared create_symlink wrapper (behavior-preserving refactor
// for multi-line rustfmt expansion of the skills symlink call site).
["src-tauri/src/managed_agents/nest.rs", 1575],
// harness-persona-sync: persona-runtime resolution threaded into the spawn
// path here. Load-bearing feature growth; queued to split in the resolver
// unify refactor followup. +26 for resolve_effective_prompt_model_provider
+10 -5
View File
@@ -18,6 +18,8 @@ use std::path::{Path, PathBuf};
use tauri::{AppHandle, Manager};
use crate::managed_agents::discovery::known_skill_dirs;
#[cfg(unix)]
use crate::util::create_symlink;
/// Subdirectories created inside the nest.
/// `REPOS` is intentionally absent: it is provisioned by
@@ -295,7 +297,7 @@ fn ensure_skill_symlinks(root: &Path) -> Result<(), String> {
let depth = std::path::Path::new(skill_dir).components().count();
let prefix = "../".repeat(depth);
let target = format!("{prefix}{CANONICAL_SKILL_DIR}");
std::os::unix::fs::symlink(&target, &link)
create_symlink(std::path::Path::new(&target), &link)
.map_err(|e| format!("symlink {}{}: {e}", link.display(), target))?;
}
Ok(())
@@ -350,14 +352,14 @@ pub fn ensure_cli_symlink(exe_parent: &Path, is_dev: bool) -> Result<(), String>
match link.symlink_metadata() {
Ok(meta) if meta.file_type().is_symlink() => {
let _ = fs::remove_file(&link);
std::os::unix::fs::symlink(&buzz_bin, &link)
create_symlink(&buzz_bin, &link)
.map_err(|e| format!("symlink {}: {e}", link.display()))?;
}
Ok(_) => {
// Regular file or directory — don't clobber.
}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
std::os::unix::fs::symlink(&buzz_bin, &link)
create_symlink(&buzz_bin, &link)
.map_err(|e| format!("symlink {}: {e}", link.display()))?;
}
Err(e) => {
@@ -504,8 +506,11 @@ fn refresh_skill_md_if_stale(root: &Path) -> Result<(), String> {
fs::remove_file(&symlink_path)
.map_err(|e| format!("remove symlink {}: {e}", symlink_path.display()))?;
}
std::os::unix::fs::symlink("../../.agents/skills/buzz-cli", &symlink_path)
.map_err(|e| format!("symlink {}: {e}", symlink_path.display()))?;
create_symlink(
std::path::Path::new("../../.agents/skills/buzz-cli"),
&symlink_path,
)
.map_err(|e| format!("symlink {}: {e}", symlink_path.display()))?;
}
fs::write(&version_path, format!("{NEST_SKILL_VERSION}\n"))
@@ -9,6 +9,9 @@ use std::fs;
use std::io;
use std::path::{Path, PathBuf};
#[cfg(unix)]
use crate::util::{create_symlink, symlink_points_to};
/// Validate a user-supplied `repos_dir`, returning the canonical target path.
///
/// Requires an **existing absolute directory**. Rejects relative paths,
@@ -98,7 +101,7 @@ pub fn ensure_repos_symlink(nest_root: &Path, repos_dir: Option<&str>) -> Result
// Existing symlink → replace it if it points elsewhere. Re-pointing a
// symlink is data-safe; remove_file never follows the link.
Ok(meta) if meta.file_type().is_symlink() => {
if repos_path.read_link().ok().as_deref() == Some(target.as_path()) {
if symlink_points_to(&repos_path, &target) {
return Ok(()); // already correct
}
fs::remove_file(&repos_path)
@@ -134,7 +137,7 @@ pub fn ensure_repos_symlink(nest_root: &Path, repos_dir: Option<&str>) -> Result
#[cfg(unix)]
fn symlink_repos(target: &Path, link: &Path) -> Result<(), String> {
std::os::unix::fs::symlink(target, link)
create_symlink(target, link)
.map_err(|e| format!("symlink {}{}: {e}", link.display(), target.display()))
}
+41 -58
View File
@@ -18,6 +18,8 @@
use std::path::{Path, PathBuf};
use tauri::Manager;
use crate::util::replace_with_symlink;
const CANONICAL_DEV_IDENTIFIER: &str = "xyz.block.buzz.app.dev";
const LEGACY_CANONICAL_DEV_IDENTIFIER: &str = "xyz.block.sprout.app.dev";
const LEGACY_RELEASE_IDENTIFIER: &str = "xyz.block.sprout.app";
@@ -36,19 +38,17 @@ const SHARED_AGENT_FILES: &[&str] = &[
/// dev data directory. Each entry becomes a single directory symlink.
const SHARED_AGENT_DIRS: &[&str] = &["agents/teams"];
/// Create a symlink at `dst` pointing to `src`.
///
/// Worktree sync is a dev-only feature (`BUZZ_SHARE_IDENTITY=1`); on Windows
/// this is a no-op so the rest of `sync_shared_agent_data` keeps compiling and
/// running harmlessly.
#[cfg(unix)]
fn symlink(src: &Path, dst: &Path) -> std::io::Result<()> {
std::os::unix::fs::symlink(src, dst)
}
#[cfg(not(unix))]
fn symlink(_src: &Path, _dst: &Path) -> std::io::Result<()> {
Ok(())
/// Returns `true` when `name` is a dev data dir name — i.e. it is exactly the
/// canonical dev identifier or a worktree variant separated by a `.` (e.g.
/// `xyz.block.buzz.app.dev.my-branch`). Rejects prefix-collisions such as
/// `xyz.block.buzz.app.developer`. This is the authoritative dev/prod
/// discriminator shared by `run_boot_migrations`, `sync_shared_agent_data`,
/// and `reconcile_target_dir`.
fn is_dev_data_dir_name(name: &str) -> bool {
name == CANONICAL_DEV_IDENTIFIER
|| name
.strip_prefix(CANONICAL_DEV_IDENTIFIER)
.is_some_and(|rest| rest.starts_with('.'))
}
fn canonical_dev_data_dir(current: &Path) -> Option<PathBuf> {
@@ -81,7 +81,7 @@ fn copy_dir_all(src: &Path, dst: &Path) -> std::io::Result<()> {
if dst_path.exists() || dst_path.is_symlink() {
let _ = std::fs::remove_file(&dst_path);
}
std::os::unix::fs::symlink(target, &dst_path)?;
crate::util::create_symlink(&target, &dst_path)?;
}
#[cfg(not(unix))]
{
@@ -127,7 +127,7 @@ pub fn run_boot_migrations(app: &tauri::AppHandle) {
let dev = data_dir
.file_name()
.and_then(|n| n.to_str())
.is_some_and(|n| n.starts_with(CANONICAL_DEV_IDENTIFIER));
.is_some_and(is_dev_data_dir_name);
crate::managed_agents::init_nest_dir(dev);
dev
} else {
@@ -488,6 +488,23 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) {
}
};
// Guard: refuse to sync against a prod-identifier data directory, regardless
// of env vars. A release build launched from an env-armed shell (e.g. macOS
// `open` inherits the caller's env) must never overwrite real prod files with
// symlinks. Only data dirs whose name starts with CANONICAL_DEV_IDENTIFIER
// (the canonical dev dir and all worktree variants) are safe targets.
let is_dev = current_dir
.file_name()
.and_then(|n| n.to_str())
.is_some_and(is_dev_data_dir_name);
if !is_dev {
eprintln!(
"buzz-desktop: shared-agent-sync: skipping — data dir is not a dev dir ({})",
current_dir.display()
);
return;
}
let canonical_dir = match canonical_dev_data_dir(&current_dir) {
Some(dir) => dir,
None => {
@@ -576,26 +593,7 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) {
}
}
// Already a correct symlink — nothing to do.
if dst.is_symlink() {
if let Ok(target) = std::fs::read_link(&dst) {
if target == src {
continue;
}
}
}
// Remove whatever's at dst (regular file, wrong symlink, broken symlink).
if dst.exists() || dst.is_symlink() {
let _ = std::fs::remove_file(&dst);
}
match symlink(&src, &dst) {
Ok(_) => synced += 1,
Err(e) => {
eprintln!("buzz-desktop: shared-agent-sync: failed to symlink {rel}: {e}");
}
}
synced += replace_with_symlink(&src, &dst);
}
// Ensure shared directories exist in canonical before symlinking.
@@ -629,8 +627,8 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) {
}
}
// Replace the sibling's dir with a symlink to canonical.
let _ = std::fs::remove_dir_all(&sibling_dir);
let _ = symlink(&canonical_target, &sibling_dir);
// replace_with_symlink backs up any leftover real content.
replace_with_symlink(&canonical_target, &sibling_dir);
eprintln!(
"buzz-desktop: shared-agent-sync: migrated {rel} from {}",
sibling.display()
@@ -661,26 +659,7 @@ pub fn sync_shared_agent_data(app: &tauri::AppHandle) {
}
}
if dst.is_symlink() {
if let Ok(target) = std::fs::read_link(&dst) {
if target == src {
continue;
}
}
}
if dst.is_symlink() {
let _ = std::fs::remove_file(&dst);
} else if dst.exists() {
let _ = std::fs::remove_dir_all(&dst);
}
match symlink(&src, &dst) {
Ok(_) => synced += 1,
Err(e) => {
eprintln!("buzz-desktop: shared-agent-sync: failed to symlink {rel}: {e}");
}
}
synced += replace_with_symlink(&src, &dst);
}
if synced > 0 {
@@ -791,7 +770,7 @@ fn reconcile_target_dir(current_dir: &Path) -> PathBuf {
let is_dev_instance = current_dir
.file_name()
.and_then(|n| n.to_str())
.is_some_and(|n| n.starts_with(CANONICAL_DEV_IDENTIFIER));
.is_some_and(is_dev_data_dir_name);
if is_dev_instance {
match canonical_dev_data_dir(current_dir) {
Some(dir) if dir.exists() => dir,
@@ -1307,3 +1286,7 @@ mod command_tests;
#[cfg(test)]
#[path = "migration_team_dir_tests.rs"]
mod team_dir_tests;
#[cfg(test)]
#[path = "migration_sync_guard_tests.rs"]
mod sync_guard_tests;
@@ -0,0 +1,27 @@
use super::*;
// ── is_dev_data_dir_name predicate ──────────────────────────────────────────
#[test]
fn is_dev_data_dir_name_rejects_prod_identifier() {
assert!(!is_dev_data_dir_name("xyz.block.buzz.app"));
}
#[test]
fn is_dev_data_dir_name_accepts_canonical_dev_identifier() {
assert!(is_dev_data_dir_name("xyz.block.buzz.app.dev"));
}
#[test]
fn is_dev_data_dir_name_accepts_worktree_dev_identifier() {
assert!(is_dev_data_dir_name("xyz.block.buzz.app.dev.some-worktree"));
}
/// Prefix-collision guard: an identifier that merely starts with the dev
/// prefix but is not dot-separated must be treated as prod, not dev.
/// `xyz.block.buzz.app.developer` is a hypothetical prod variant, not a
/// worktree of `xyz.block.buzz.app.dev`.
#[test]
fn is_dev_data_dir_name_rejects_prefix_collision() {
assert!(!is_dev_data_dir_name("xyz.block.buzz.app.developer"));
}
+308
View File
@@ -42,6 +42,162 @@ pub fn slugify(name: &str, fallback: &str, max_len: usize) -> String {
raw.trim_end_matches('-').to_string()
}
// ── Safe symlink utilities ────────────────────────────────────────────────────
/// Create a symlink at `link` pointing to `target` on Unix; no-op on Windows.
///
/// Worktree sync and nest setup are Unix-only features. This wrapper lets
/// call sites compile and run harmlessly on non-Unix platforms.
#[cfg(unix)]
pub(crate) fn create_symlink(
target: &std::path::Path,
link: &std::path::Path,
) -> std::io::Result<()> {
std::os::unix::fs::symlink(target, link)
}
/// No-op on non-Unix platforms.
#[cfg(not(unix))]
pub(crate) fn create_symlink(
_target: &std::path::Path,
_link: &std::path::Path,
) -> std::io::Result<()> {
Ok(())
}
/// Returns `true` when `link` is a symlink whose stored target equals `target`.
///
/// Compares the raw stored link value — no canonicalization — so relative
/// targets (e.g. `../../.agents/skills/buzz-cli`) compare correctly against
/// the literal string used to create them.
pub(crate) fn symlink_points_to(link: &std::path::Path, target: &std::path::Path) -> bool {
link.is_symlink()
&& std::fs::read_link(link)
.map(|t| t == target)
.unwrap_or(false)
}
/// Compute a collision-safe backup path for `dst`.
///
/// The candidate is `<parent>/<full-filename>.bak.<ms-timestamp>`. If that
/// path already exists (rare — same-millisecond collision or leftover backup),
/// appends `-2`, `-3`, … up to 100. Returns `None` when all 100 candidates
/// are occupied, indicating a backup failure.
pub(crate) fn backup_path(dst: &std::path::Path) -> Option<std::path::PathBuf> {
let name = dst.file_name()?.to_str()?;
let stamp = Utc::now().format("%Y%m%d-%H%M%S%.3f");
let base = format!("{name}.bak.{stamp}");
let parent = dst.parent()?;
let candidate = parent.join(&base);
if !candidate.exists() {
return Some(candidate);
}
// Collision — try suffixes -2 … -100.
for n in 2u32..=100 {
let candidate = parent.join(format!("{base}-{n}"));
if !candidate.exists() {
return Some(candidate);
}
}
None
}
/// Replace `dst` with a symlink pointing to `src`, backing up any real
/// file or directory at `dst` first.
///
/// Behaviour by what `dst` currently is:
///
/// - **Already a correct symlink** (stored target == `src`): no-op, returns 0.
/// - **Wrong or broken symlink**: remove and replace; no backup — a symlink
/// holds no user data. Returns 1 on success, 0 if removal fails (the
/// subsequent `create_symlink` will surface EEXIST).
/// - **Real file or real directory**: rename to
/// `<full-filename>.bak.<ms-timestamp>` (collision-safe, up to suffix -100).
/// If backup fails, `dst` is left untouched and 0 is returned. If the
/// backup succeeds but `create_symlink` fails, the backup is renamed back to
/// restore `dst`; if that rollback also fails, an actionable error is logged.
/// Returns 1 on success, 0 on any failure.
/// - **Absent**: creates the symlink and returns 1.
#[cfg(unix)]
pub(crate) fn replace_with_symlink(src: &std::path::Path, dst: &std::path::Path) -> u32 {
if dst.is_symlink() {
if symlink_points_to(dst, src) {
return 0;
}
// Wrong or broken symlink — remove and replace, no backup.
if let Err(e) = std::fs::remove_file(dst) {
eprintln!(
"buzz-desktop: symlink-util: failed to remove stale symlink {}: {e}",
dst.display()
);
// Fall through — create_symlink will surface EEXIST.
}
} else if dst.exists() {
// Real file or real directory — back up before replacing.
let label = if dst.is_dir() { "dir" } else { "file" };
let Some(bak) = backup_path(dst) else {
eprintln!(
"buzz-desktop: symlink-util: all backup paths occupied for {}; skipping",
dst.display()
);
return 0;
};
match std::fs::rename(dst, &bak) {
Ok(()) => eprintln!(
"buzz-desktop: symlink-util: backed up real {label} {} → {}",
dst.display(),
bak.display()
),
Err(e) => {
eprintln!(
"buzz-desktop: symlink-util: failed to back up {label} {}: {e}",
dst.display()
);
return 0;
}
}
// Backup succeeded — attempt symlink creation.
if let Err(e) = create_symlink(src, dst) {
eprintln!(
"buzz-desktop: symlink-util: failed to symlink {} → {}: {e}; attempting rollback",
dst.display(),
src.display()
);
if let Err(rb_err) = std::fs::rename(&bak, dst) {
eprintln!(
"buzz-desktop: symlink-util: ROLLBACK FAILED ({rb_err}) — \
{dst_disp} is still at {bak_disp}; \
restore it manually: `mv {bak_disp} {dst_disp}`",
dst_disp = dst.display(),
bak_disp = bak.display(),
);
}
return 0;
}
return 1;
}
// dst was absent or was a symlink (already removed above).
match create_symlink(src, dst) {
Ok(()) => 1,
Err(e) => {
eprintln!(
"buzz-desktop: symlink-util: failed to symlink {} → {}: {e}",
dst.display(),
src.display()
);
0
}
}
}
/// No-op on non-Unix platforms — always returns 0.
#[cfg(not(unix))]
pub(crate) fn replace_with_symlink(_src: &std::path::Path, _dst: &std::path::Path) -> u32 {
0
}
#[cfg(test)]
mod tests {
use super::slugify;
@@ -90,4 +246,156 @@ mod tests {
// "abcde-----fghij" truncated at 10 → "abcde-----" → trimmed → "abcde"
assert_eq!(slugify("abcde fghij", "x", 10), "abcde");
}
// ── symlink_util ──────────────────────────────────────────────────────────
#[cfg(unix)]
#[test]
fn replace_with_symlink_backs_up_real_file_and_creates_symlink() {
let dir = tempfile::tempdir().unwrap();
let src = dir.path().join("source.json");
let dst = dir.path().join("dest.json");
std::fs::write(&src, r#"[{"id":"canonical"}]"#).unwrap();
std::fs::write(&dst, r#"[{"id":"real-local-data"}]"#).unwrap();
let created = super::replace_with_symlink(&src, &dst);
assert_eq!(created, 1);
assert!(dst.is_symlink());
assert_eq!(std::fs::read_link(&dst).unwrap(), src);
let bak_entry = std::fs::read_dir(dir.path()).unwrap().flatten().find(|e| {
e.file_name()
.to_str()
.map(|n| n.starts_with("dest.json.bak."))
.unwrap_or(false)
});
assert!(bak_entry.is_some(), "a .bak.* backup file must exist");
let bak_content = std::fs::read_to_string(bak_entry.unwrap().path()).unwrap();
assert_eq!(bak_content, r#"[{"id":"real-local-data"}]"#);
}
#[cfg(unix)]
#[test]
fn replace_with_symlink_backs_up_real_dir_and_creates_symlink() {
let dir = tempfile::tempdir().unwrap();
let src = dir.path().join("canonical-teams");
let dst = dir.path().join("local-teams");
std::fs::create_dir_all(&src).unwrap();
std::fs::create_dir_all(&dst).unwrap();
std::fs::write(dst.join("stale.txt"), "old-content").unwrap();
let created = super::replace_with_symlink(&src, &dst);
assert_eq!(created, 1);
assert!(dst.is_symlink());
assert_eq!(std::fs::read_link(&dst).unwrap(), src);
let bak_entry = std::fs::read_dir(dir.path()).unwrap().flatten().find(|e| {
e.file_name()
.to_str()
.map(|n| n.starts_with("local-teams.bak."))
.unwrap_or(false)
});
assert!(bak_entry.is_some(), "a .bak.* backup directory must exist");
let bak_path = bak_entry.unwrap().path();
assert!(bak_path.is_dir(), "backup must be a directory");
assert_eq!(
std::fs::read_to_string(bak_path.join("stale.txt")).unwrap(),
"old-content"
);
}
#[cfg(unix)]
#[test]
fn replace_with_symlink_noop_when_already_correct_symlink() {
let dir = tempfile::tempdir().unwrap();
let src = dir.path().join("source.json");
let dst = dir.path().join("dest.json");
std::fs::write(&src, "data").unwrap();
std::os::unix::fs::symlink(&src, &dst).unwrap();
let created = super::replace_with_symlink(&src, &dst);
assert_eq!(created, 0);
assert!(dst.is_symlink());
assert_eq!(std::fs::read_link(&dst).unwrap(), src);
let bak_count = std::fs::read_dir(dir.path())
.unwrap()
.flatten()
.filter(|e| {
e.file_name()
.to_str()
.map(|n| n.contains(".bak."))
.unwrap_or(false)
})
.count();
assert_eq!(
bak_count, 0,
"no backup should be created for a correct symlink"
);
}
#[cfg(unix)]
#[test]
fn replace_with_symlink_replaces_wrong_symlink_without_backup() {
let dir = tempfile::tempdir().unwrap();
let src = dir.path().join("source.json");
let dst = dir.path().join("dest.json");
let wrong_target = dir.path().join("wrong.json");
std::fs::write(&src, "data").unwrap();
std::fs::write(&wrong_target, "wrong").unwrap();
std::os::unix::fs::symlink(&wrong_target, &dst).unwrap();
let created = super::replace_with_symlink(&src, &dst);
assert_eq!(created, 1);
assert!(dst.is_symlink());
assert_eq!(std::fs::read_link(&dst).unwrap(), src);
let bak_count = std::fs::read_dir(dir.path())
.unwrap()
.flatten()
.filter(|e| {
e.file_name()
.to_str()
.map(|n| n.contains(".bak."))
.unwrap_or(false)
})
.count();
assert_eq!(bak_count, 0, "symlinks must not produce backups");
}
#[cfg(unix)]
#[test]
fn replace_with_symlink_replaces_broken_symlink_without_backup() {
let dir = tempfile::tempdir().unwrap();
let src = dir.path().join("source.json");
let dst = dir.path().join("dest.json");
std::fs::write(&src, "data").unwrap();
std::os::unix::fs::symlink(dir.path().join("nonexistent.json"), &dst).unwrap();
let created = super::replace_with_symlink(&src, &dst);
assert_eq!(created, 1);
assert!(dst.is_symlink());
assert_eq!(std::fs::read_link(&dst).unwrap(), src);
let bak_count = std::fs::read_dir(dir.path())
.unwrap()
.flatten()
.filter(|e| {
e.file_name()
.to_str()
.map(|n| n.contains(".bak."))
.unwrap_or(false)
})
.count();
assert_eq!(bak_count, 0, "broken symlinks must not produce backups");
}
}