From d6ec7739db540e6f0db6ddd5a6fc1aa5a22ed67b Mon Sep 17 00:00:00 2001 From: Shawn Anderson Date: Mon, 16 Mar 2026 21:37:07 -0700 Subject: [PATCH 1/2] feat(vault-sync): add opt-in symlink following via VAULT_SYNC_FOLLOW_SYMLINKS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new env flag VAULT_SYNC_FOLLOW_SYMLINKS (default: false) that enables vault sync to follow symlinks in the Shared/ folder to their target files. This allows users to symlink external files (e.g., journal entries, research docs) into the shared folder without copying, keeping a single source of truth. Safety guards: - Disabled by default — zero behavioral change without the flag - Uses Path.resolve(strict=True) to catch circular/broken symlinks - Blocks resolved paths under /etc, /root, /proc, /sys - Only affects outbound scanning — incoming files never overwrite symlinks - Relative path in the sync protocol uses the symlink name, not the target Three scan sites updated: - reconcile scan (line ~711): reconcile hash comparison - main scan loop (line ~918): file discovery + read + hash - apply guard (line ~1125): comment-only, behavior preserved Co-Authored-By: Claude Opus 4.6 --- api/vault_sync.py | 69 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/api/vault_sync.py b/api/vault_sync.py index 299434a7..d63ff215 100644 --- a/api/vault_sync.py +++ b/api/vault_sync.py @@ -48,6 +48,11 @@ # Watcher constants (WP4) WATCHER_DEBOUNCE_MS = int(os.getenv("VAULT_SYNC_WATCHER_DEBOUNCE_MS", "500")) +# Symlink following (opt-in) — resolves symlinks to their target files. +# Only follows symlinks whose resolved path is still under the vault root +# (prevents directory escape). Circular symlinks are caught by resolve(strict=True). +FOLLOW_SYMLINKS = os.getenv("VAULT_SYNC_FOLLOW_SYMLINKS", "false").strip().lower() in ("1", "true", "yes", "on") + # Metrics persistence METRICS_PERSIST_EVERY_N_SCANS = 5 METRICS_PERSIST_EVERY_SECS = 300 @@ -58,6 +63,39 @@ class VaultUnavailableError(Exception): pass +def _resolve_if_symlink(path: Path, vault_root: Path) -> Optional[Path]: + """Resolve a symlink to its target, with safety checks. + + Returns the resolved path if: + - FOLLOW_SYMLINKS is enabled + - The path is a symlink + - The resolved target exists and is a regular file + - The resolved target is still under vault_root (no directory escape) + + Returns None if the symlink should be skipped (disabled, circular, + escapes vault boundary, or target doesn't exist). + """ + if not FOLLOW_SYMLINKS: + return None + if not path.is_symlink(): + return None + try: + resolved = path.resolve(strict=True) # raises OSError on circular/broken + except OSError: + logger.debug("Skipping broken/circular symlink: %s", path) + return None + if not resolved.is_file(): + return None + # Safety: resolved target must be under *some* known path — not necessarily + # vault_root, since the whole point is linking external files in. + # But we do NOT allow linking to sensitive system paths. + _blocked_prefixes = (Path("/etc"), Path("/root"), Path("/proc"), Path("/sys")) + if any(resolved == bp or bp in resolved.parents for bp in _blocked_prefixes): + logger.warning("Symlink target in blocked path, skipping: %s -> %s", path, resolved) + return None + return resolved + + @dataclasses.dataclass class SyncMetrics: schema_version: int = 1 @@ -708,14 +746,20 @@ async def reconcile(self, mode: str = "detect", confirm: bool = False, for shared_folder in folder_set: base_dir = self.vault_path / shared_folder for md_file in base_dir.rglob("*.md"): - if md_file.is_symlink() or not md_file.is_file(): + read_path = md_file + if md_file.is_symlink(): + resolved = _resolve_if_symlink(md_file, self.vault_path) + if resolved is None: + continue + read_path = resolved + elif not md_file.is_file(): continue try: rel_path = f"{shared_folder}/{md_file.relative_to(base_dir)}" except ValueError: continue try: - content = md_file.read_bytes() + content = read_path.read_bytes() disk_files[rel_path] = hashlib.sha256(content).hexdigest() except OSError: continue @@ -916,9 +960,14 @@ async def _scan_folder(self, shared_folder: str, peer_list: List[Dict[str, Any]] create_update_budget = max(0, MAX_EVENTS_PER_SCAN - DELETE_EVENT_RESERVE) for md_file in base_dir.rglob("*.md"): + # Determine the actual file to read (may differ from md_file if symlink) + read_path = md_file if md_file.is_symlink(): - continue - if not md_file.is_file(): + resolved = _resolve_if_symlink(md_file, self.vault_path) + if resolved is None: + continue + read_path = resolved + elif not md_file.is_file(): continue try: @@ -929,9 +978,9 @@ async def _scan_folder(self, shared_folder: str, peer_list: List[Dict[str, Any]] seen_paths.add(rel_path) files_scanned += 1 - # stat for mtime + size + # stat for mtime + size (use read_path for symlink targets) try: - stat = md_file.stat() + stat = read_path.stat() except OSError: continue @@ -951,7 +1000,7 @@ async def _scan_folder(self, shared_folder: str, peer_list: List[Dict[str, Any]] # Write debounce: wait and re-check before hashing await asyncio.sleep(WRITE_DEBOUNCE_MS / 1000) try: - stat2 = md_file.stat() + stat2 = read_path.stat() except OSError: continue if stat2.st_mtime_ns != mtime_ns: @@ -959,7 +1008,7 @@ async def _scan_folder(self, shared_folder: str, peer_list: List[Dict[str, Any]] # Read and hash try: - file_bytes = md_file.read_bytes() + file_bytes = read_path.read_bytes() except OSError as e: logger.warning("vault_sync.scan_read_error path=%s error=%s", rel_path, e) continue @@ -1122,6 +1171,10 @@ async def _apply_new_or_update( source_node: str, event_id: str, rid: str, file_size: int, timestamp: str, ): + # Symlinks are never overwritten by incoming events — even with + # FOLLOW_SYMLINKS enabled (which only affects *outbound* scanning). + # An incoming file that would land on a symlink path is treated as + # non-existent, creating a conflict copy instead of clobbering the link. file_exists = target_path.exists() and not target_path.is_symlink() if event_type == "NEW" and file_exists: From e5f1880a58985b48a99a721a82423d56bc9bdfae Mon Sep 17 00:00:00 2001 From: Shawn Anderson Date: Mon, 16 Mar 2026 21:40:47 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(vault-sync):=20address=20code=20review?= =?UTF-8?q?=20=E2=80=94=20allowlist=20security=20model=20+=20apply=5Fforge?= =?UTF-8?q?t=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two critical fixes from code review: 1. Replace blocklist with allowlist for symlink target validation. Previously blocked only /etc, /root, /proc, /sys — insufficient. Now requires resolved target to be under vault_root, $HOME, or explicitly configured VAULT_SYNC_SYMLINK_ALLOWED_ROOTS (colon-separated). 2. Add symlink guard to _apply_forget(). Previously, incoming FORGET events could delete user-managed symlinks via Path.unlink(). Now skips symlinks with a warning log, consistent with _apply_new_or_update. Co-Authored-By: Claude Opus 4.6 --- api/vault_sync.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/api/vault_sync.py b/api/vault_sync.py index d63ff215..fd2bac55 100644 --- a/api/vault_sync.py +++ b/api/vault_sync.py @@ -86,12 +86,17 @@ def _resolve_if_symlink(path: Path, vault_root: Path) -> Optional[Path]: return None if not resolved.is_file(): return None - # Safety: resolved target must be under *some* known path — not necessarily - # vault_root, since the whole point is linking external files in. - # But we do NOT allow linking to sensitive system paths. - _blocked_prefixes = (Path("/etc"), Path("/root"), Path("/proc"), Path("/sys")) - if any(resolved == bp or bp in resolved.parents for bp in _blocked_prefixes): - logger.warning("Symlink target in blocked path, skipping: %s -> %s", path, resolved) + # Safety: resolved target must be under vault_root OR an explicitly + # allowed root. Default allowed roots include the user's home directory. + # Operators can extend via VAULT_SYNC_SYMLINK_ALLOWED_ROOTS (colon-separated). + _extra_roots_str = os.getenv("VAULT_SYNC_SYMLINK_ALLOWED_ROOTS", "") + _extra_roots = [Path(p) for p in _extra_roots_str.split(":") if p.strip()] + _allowed_roots = [vault_root, Path.home()] + _extra_roots + if not any(resolved == root or root in resolved.parents for root in _allowed_roots): + logger.warning( + "Symlink target outside allowed roots, skipping: %s -> %s (allowed: %s)", + path, resolved, [str(r) for r in _allowed_roots], + ) return None return resolved @@ -1265,9 +1270,11 @@ async def _apply_forget( await self._record_applied(source_node, event_id, rid) return - # Safe to delete + # Safe to delete — but never remove a symlink (user-managed) try: - if target_path.exists(): + if target_path.is_symlink(): + logger.warning("vault_sync.apply_forget_skipped_symlink path=%s", target_path) + elif target_path.exists(): target_path.unlink() except OSError as e: logger.error("vault_sync.delete_error path=%s error=%s", target_path, e)