Skip to content

Implement Native 2-Way Live Sync with Recursive File Watching#35

Open
EdoSag wants to merge 6 commits intoDonnieDice:mainfrom
EdoSag:feature/two-way-sync
Open

Implement Native 2-Way Live Sync with Recursive File Watching#35
EdoSag wants to merge 6 commits intoDonnieDice:mainfrom
EdoSag:feature/two-way-sync

Conversation

@EdoSag
Copy link
Copy Markdown

@EdoSag EdoSag commented Feb 7, 2026

PR: Harden live 2-way sync: path safety, bounded suppression cache, and safer logging

Summary

This updates the experimental native 2-way sync implementation with security and reliability hardening, while preserving the same cross-package behavior (AUR/AppImage/Flatpak/snap/deb/rpm).

What changed

1) Remote write safety: validate before mutation

  • For create/update, target path validation now executes before any directory/file mutation.
  • This prevents side effects before rejection when a path fails root/symlink safety checks.

2) Bounded ping-pong suppression cache

  • Replaced unbounded suppression tracking with a bounded + expiring cache.
  • Added:
    • TTL-based expiration
    • size cap with oldest-entry eviction
  • Prevents long-running sessions from accumulating stale suppression entries.

3) Event emission reliability

  • emit("live-sync://local-change", ...) failures are now logged explicitly.
  • Avoids silent sync-signal loss.

4) Existing hardening included

  • Generic/sanitized user-facing error strings.
  • Added remote write/delete audit logs.
  • Added sync command origin checks and root-path constraints.
  • Added notify dependency for native recursive watcher.

Files changed

  • src-tauri/src/live_sync.rs
  • src-tauri/src/main.rs
  • src-tauri/Cargo.toml
  • README.md

Why

This addresses reviewer concerns around:

  • path traversal / symlink escape risk
  • unbounded suppression state growth
  • silent event propagation failures
  • sensitive error/log exposure

Notes

  • No package-specific runtime code paths were introduced.
  • Changes stay in shared app logic and remain package-format agnostic.
  • I could not run cargo check in my current shell because Rust/Cargo is not installed there.
    "@

$bodyFile = Join-Path $PWD "pr-body.md"
Set-Content -Path $bodyFile -Value $prBody -Encoding UTF8

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Implement native 2-way live sync with event-driven file watching

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implement native event-driven 2-way file sync with recursive filesystem watching
• Add sync suppression cache to prevent infinite sync loops between local and remote changes
• Introduce four new Tauri commands for sync lifecycle management and remote updates
• Modernize base64 API to use base64::Engine for compatibility with current Rust ecosystem
• Consolidate command registration into single unified invoke handler block
Diagram
flowchart LR
  A["Local Filesystem Events"] -->|"notify crate watcher"| B["Rust Event Handler"]
  B -->|"filter known_files cache"| C["Emit live-sync://local-change"]
  C -->|"JS bridge"| D["Frontend Upload Logic"]
  E["Remote File Changes"] -->|"JS interceptor"| F["handle_remote_update command"]
  F -->|"mark_known_file suppression"| G["Write to Local Folder"]
  G -->|"watcher detects event"| H["Cache suppresses echo"]
Loading

Grey Divider

File Changes

1. src-tauri/src/live_sync.rs ✨ Enhancement +209/-0

Core live sync manager with event-driven watcher

• New module implementing LiveSyncManager struct with event-driven filesystem watching using
 notify::RecommendedWatcher
• Implements sync suppression cache (known_files HashSet) to prevent ping-pong loops between local
 and remote changes
• Provides apply_remote_change method with path validation using std::path::Component to prevent
 directory traversal attacks
• Includes start, stop, and status methods for managing the watcher lifecycle and thread
 safety with Arc/Mutex primitives

src-tauri/src/live_sync.rs


2. src-tauri/src/main.rs ✨ Enhancement +88/-15

Add sync commands and consolidate handler registration

• Add live_sync module import and integrate LiveSyncManager into AppState struct
• Implement four new Tauri commands: start_sync, stop_sync, get_sync_status, and
 handle_remote_update for sync control
• Consolidate all command registrations into single unified invoke_handler block to fix
 overwriting bug
• Apply code formatting improvements including line breaks for readability and proper indentation
• Modernize base64 decoding to use base64::engine::general_purpose::STANDARD API

src-tauri/src/main.rs


3. README.md 📝 Documentation +1/-0

Document experimental live 2-way sync feature

• Add feature description for experimental live 2-way sync hooks in features list
• Document native commands availability for watching local folders and applying remote updates

README.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Feb 7, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (6) 📎 Requirement gaps (0)

Grey Divider


Action required

1. apply_remote_change leaks OS errors📘 Rule violation ⛨ Security
Description
• Multiple Tauri-facing operations convert low-level errors directly to strings (via e.to_string()
/ format!("...: {}", e)), which can expose internal implementation details and filesystem
information to the UI. • This violates the requirement that user-facing errors be generic, with
detailed diagnostics written only to internal logs. • It also makes it hard to consistently control
what details are exposed across different failure modes (watcher init, base64 decode, filesystem
writes).
Code

src-tauri/src/live_sync.rs[R58-63]

+        let mut watcher =
+            RecommendedWatcher::new(tx, Config::default()).map_err(|e| e.to_string())?;
+
+        watcher
+            .watch(&folder, RecursiveMode::Recursive)
+            .map_err(|e| e.to_string())?;
Evidence
PR Compliance ID 4 requires user-facing errors to avoid exposing internal details. The Tauri command
path returns String errors built from raw underlying errors (e.to_string() / `format!("... {}",
e)`), which are typically propagated to the frontend as-is.

Rule 4: Generic: Secure Error Handling
src-tauri/src/live_sync.rs[58-63]
src-tauri/src/live_sync.rs[150-160]
src-tauri/src/main.rs[100-105]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
User-facing `Result<_, String>` errors currently include raw underlying error strings (OS/fs/base64/notify/url parsing), which can leak internal details to the frontend.
## Issue Context
These functions are invoked via Tauri commands, so their `Err(String)` values can be surfaced to the user/UI. Compliance requires generic user-facing errors with detailed diagnostics only in internal logs.
## Fix Focus Areas
- src-tauri/src/live_sync.rs[50-170]
- src-tauri/src/main.rs[86-106]
- src-tauri/src/main.rs[117-132]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. emit() error ignored📘 Rule violation ⛯ Reliability
Description
• The filesystem watcher suppresses any failure from app_handle.emit(...) by assigning the result
to _. • If event emission fails, local changes may silently stop propagating to the frontend,
undermining sync correctness and making failures difficult to detect. • This violates robust error
handling expectations for critical sync signaling.
Code

src-tauri/src/live_sync.rs[R93-100]

+                            let _ = app_handle.emit(
+                                "live-sync://local-change",
+                                LiveSyncEvent {
+                                    kind: kind.to_string(),
+                                    paths: filtered_paths,
+                                },
+                            );
+                        }
Evidence
PR Compliance ID 3 requires identifying and handling failure points without silent failures. The
code discards the result of emitting the sync event, providing no handling or logging if emission
fails.

Rule 3: Generic: Robust Error Handling and Edge Case Management
src-tauri/src/live_sync.rs[93-100]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`app_handle.emit(...)` failures are ignored, causing silent loss of local-change events.
## Issue Context
Event emission is a critical step in the Local → Cloud sync path. If it fails, users will see missing uploads without any actionable diagnostics.
## Fix Focus Areas
- src-tauri/src/live_sync.rs[93-102]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. println! logs proxy body📘 Rule violation ⛨ Security
Description
• The code logs full URLs, return URLs, and (conditionally) full HTTP response bodies using
unstructured println!, which can include sensitive content (session data, account metadata,
tokens, etc.). • This violates the requirement for secure logging that avoids sensitive data
exposure and favors structured logs for auditing. • These logs may be collected in CI/system logs,
increasing the blast radius of accidental leakage.
Code

src-tauri/src/main.rs[R244-245]

       println!("[Proxy] Body: {}", body);
   }
Evidence
PR Compliance ID 5 requires that logs are structured and contain no sensitive data. The current code
prints potentially sensitive URLs and response bodies directly to stdout in plaintext.

Rule 5: Generic: Secure Logging Practices
src-tauri/src/main.rs[92-93]
src-tauri/src/main.rs[242-245]
src-tauri/src/live_sync.rs[101-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Current logging prints sensitive values (URLs, return URLs, HTTP bodies) to stdout in plaintext and without redaction.
## Issue Context
These logs can include tokens/cookies/user content and may end up in systemd logs, CI artifacts, or user bug reports.
## Fix Focus Areas
- src-tauri/src/main.rs[92-93]
- src-tauri/src/main.rs[240-245]
- src-tauri/src/live_sync.rs[101-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (4)
4. No audit logs for writes📘 Rule violation ✓ Correctness
Description
• The new remote→local sync path performs filesystem writes and deletes without any audit logging of
the action, outcome, or associated user/session context. • This makes it difficult to reconstruct
what changes were applied to the local filesystem and when, especially when troubleshooting or
investigating incidents. • This violates the audit trail requirement for critical actions like
write/delete operations.
Code

src-tauri/src/live_sync.rs[R145-166]

+        match change.action.as_str() {
+            "create" | "update" => {
+                let encoded = change
+                    .content_base64
+                    .ok_or("contentBase64 is required for create/update")?;
+                let data = base64::engine::general_purpose::STANDARD
+                    .decode(encoded)
+                    .map_err(|e| e.to_string())?;
+
+                if let Some(parent) = target.parent() {
+                    fs::create_dir_all(parent).map_err(|e| e.to_string())?;
+                }
+
+                self.mark_known_file(&target)?;
+                fs::write(&target, data).map_err(|e| e.to_string())?;
+            }
+            "delete" => {
+                if target.exists() {
+                    self.mark_known_file(&target)?;
+                    fs::remove_file(&target).map_err(|e| e.to_string())?;
+                }
+            }
Evidence
PR Compliance ID 1 requires logging critical actions (including write/delete) with sufficient
context to reconstruct events. The remote update handler executes write/delete operations with no
corresponding audit log entries.

Rule 1: Generic: Comprehensive Audit Trails
src-tauri/src/live_sync.rs[145-166]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remote-applied filesystem mutations (create/update/delete) lack audit logging.
## Issue Context
This PR adds a new capability to write/delete local files based on remote events. Auditability is required for post-incident analysis and user support.
## Fix Focus Areas
- src-tauri/src/live_sync.rs[122-171]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Symlink escape in target📘 Rule violation ⛨ Security
Description
• The code validates relative_path components but does not prevent writes through symlinks inside
the sync root (e.g., root/symlink_to_/etc/...). • A remote change could therefore write/delete
files outside the intended sync directory if a symlink exists within the root, violating
security-first input handling. • Additional canonicalization and root containment checks are needed
to ensure the final resolved path stays within the sync folder.
Code

src-tauri/src/live_sync.rs[R130-160]

+        let relative = Path::new(&change.relative_path);
+        if relative.as_os_str().is_empty() {
+            return Err("Invalid relative path".into());
+        }
+        if relative.components().any(|c| {
+            matches!(
+                c,
+                Component::ParentDir | Component::RootDir | Component::Prefix(_)
+            )
+        }) {
+            return Err("Invalid relative path".into());
+        }
+
+        let target = root.join(relative);
+
+        match change.action.as_str() {
+            "create" | "update" => {
+                let encoded = change
+                    .content_base64
+                    .ok_or("contentBase64 is required for create/update")?;
+                let data = base64::engine::general_purpose::STANDARD
+                    .decode(encoded)
+                    .map_err(|e| e.to_string())?;
+
+                if let Some(parent) = target.parent() {
+                    fs::create_dir_all(parent).map_err(|e| e.to_string())?;
+                }
+
+                self.mark_known_file(&target)?;
+                fs::write(&target, data).map_err(|e| e.to_string())?;
+            }
Evidence
PR Compliance ID 6 requires secure validation/handling of external inputs to prevent
vulnerabilities. While the code blocks .. and absolute paths, it does not verify that the resolved
filesystem target remains under root after symlink resolution, allowing potential path escape via
symlinks.

Rule 6: Generic: Security-First Input Validation and Data Handling
src-tauri/src/live_sync.rs[130-144]
src-tauri/src/live_sync.rs[154-160]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Current path validation prevents `..` and absolute paths but does not prevent symlink-based escape from the sync root.
## Issue Context
If the sync directory contains a symlink, remote updates may write/delete outside the intended folder. This is an external-input-to-filesystem sink.
## Fix Focus Areas
- src-tauri/src/live_sync.rs[122-171]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Missing notify dependency🐞 Bug ✓ Correctness
Description
live_sync.rs imports notify, but src-tauri/Cargo.toml does not declare it under
[dependencies]. • This should fail compilation with an unresolved import error, blocking the PR
from building.
Code

src-tauri/src/live_sync.rs[2]

+use notify::{Config, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
Evidence
The new module directly imports the notify crate, but the crate manifest has no notify = ...
entry in dependencies, so compilation will fail.

src-tauri/src/live_sync.rs[1-3]
src-tauri/Cargo.toml[17-29]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src-tauri/src/live_sync.rs` imports and uses the `notify` crate, but `src-tauri/Cargo.toml` does not list `notify` under `[dependencies]`, which will cause a compilation failure.
## Issue Context
Live sync relies on `notify::{RecommendedWatcher, Watcher, ...}`.
## Fix Focus Areas
- src-tauri/Cargo.toml[17-29]
- src-tauri/src/live_sync.rs[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. WebView can write files🐞 Bug ⛨ Security
Description
• New Tauri commands allow WebView JS to (a) pick an arbitrary directory to watch (`start_sync(path:
String)`) and (b) write/delete arbitrary relative paths with attacker-controlled bytes
(handle_remote_update → base64 decode → fs::write). • Given the app config disables CSP and
enables global Tauri injection, any XSS/untrusted script running in the WebView could invoke these
commands to modify user files (and potentially persist malware within writable locations).
Code

src-tauri/src/main.rs[R134-165]

+#[tauri::command]
+fn start_sync(
+    app: tauri::AppHandle,
+    state: tauri::State<'_, Arc<AppState>>,
+    path: String,
+) -> Result<live_sync::LiveSyncStatus, String> {
+    state
+        .sync_manager
+        .start(app, std::path::PathBuf::from(path))?;
+    state.sync_manager.status()
+}
+
+#[tauri::command]
+fn stop_sync(state: tauri::State<'_, Arc<AppState>>) -> Result<live_sync::LiveSyncStatus, String> {
+    state.sync_manager.stop()?;
+    state.sync_manager.status()
+}
+
+#[tauri::command]
+fn get_sync_status(
+    state: tauri::State<'_, Arc<AppState>>,
+) -> Result<live_sync::LiveSyncStatus, String> {
+    state.sync_manager.status()
+}
+
+#[tauri::command]
+fn handle_remote_update(
+    state: tauri::State<'_, Arc<AppState>>,
+    change: live_sync::RemoteSyncChange,
+) -> Result<String, String> {
+    state.sync_manager.apply_remote_change(change)
+}
Evidence
start_sync takes a raw string path from the frontend and starts watching it;
handle_remote_update takes a change struct from the frontend and performs direct filesystem
writes/deletes. The Tauri config shows CSP disabled and withGlobalTauri enabled, increasing the
blast radius if WebView content is compromised.

src-tauri/src/main.rs[134-165]
src-tauri/src/live_sync.rs[122-165]
src-tauri/src/main.rs[1083-1097]
src-tauri/tauri.conf.json[50-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new live-sync commands provide a powerful filesystem write/delete primitive to WebView JS:
- `start_sync(path: String)` lets JS select the watched/root directory.
- `handle_remote_update(change)` decodes attacker-controlled bytes and writes them to disk.
With CSP disabled and global Tauri injection enabled, any compromised script context can potentially invoke these commands.
## Issue Context
These commands are registered in the main invoke handler and write directly via `fs::write` / `fs::remove_file`.
## Fix Focus Areas
- src-tauri/src/main.rs[134-165]
- src-tauri/src/main.rs[1083-1097]
- src-tauri/src/live_sync.rs[122-170]
- src-tauri/tauri.conf.json[50-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

8. data variable too generic 📘 Rule violation ✓ Correctness
Description
• The decoded file bytes are stored in a variable named data, which is a generic identifier that
does not convey intent. • Given this is sync/write logic, clearer naming (e.g., file_bytes,
decoded_bytes) would improve readability and reduce ambiguity.
Code

src-tauri/src/live_sync.rs[R150-152]

+                let data = base64::engine::general_purpose::STANDARD
+                    .decode(encoded)
+                    .map_err(|e| e.to_string())?;
Evidence
PR Compliance ID 2 requires meaningful, self-documenting identifiers and calls out generic names
like data as a failure case. Here data specifically represents decoded file contents, and a more
specific name would better communicate intent.

Rule 2: Generic: Meaningful Naming and Self-Documenting Code
src-tauri/src/live_sync.rs[150-152]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The variable name `data` is too generic for decoded file contents.
## Issue Context
This is security-sensitive sync code; clearer naming helps reviewers and maintainers understand what is being written to disk.
## Fix Focus Areas
- src-tauri/src/live_sync.rs[147-160]ృష

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Known-files suppression leak🐞 Bug ⛯ Reliability
Description
• Remote updates add a path to known_files before attempting write/remove_file, and the marker
is only cleared when the watcher later sees an event for that exact path. • If the remote operation
fails without producing a filesystem event, the marker can persist and may suppress the next
unrelated local event on that path (e.g., user later creates/edits the file).
Code

src-tauri/src/live_sync.rs[R158-165]

+                self.mark_known_file(&target)?;
+                fs::write(&target, data).map_err(|e| e.to_string())?;
+            }
+            "delete" => {
+                if target.exists() {
+                    self.mark_known_file(&target)?;
+                    fs::remove_file(&target).map_err(|e| e.to_string())?;
+                }
Evidence
mark_known_file inserts into a set and should_ignore_known_file removes it only when a matching
watcher event arrives. Because marking happens before the filesystem operation, failed operations
can leave markers behind until a future event occurs.

src-tauri/src/live_sync.rs[154-166]
src-tauri/src/live_sync.rs[194-209]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`known_files` is used to suppress watcher echoes of remote-applied changes, but marking occurs before the filesystem operation and markers are only removed when a subsequent watcher event occurs. If the operation fails without generating an event, the marker can persist and suppress a later unrelated local change.
## Issue Context
The marker insertion is unconditional before `fs::write`/`fs::remove_file`, while removal only happens via `should_ignore_known_file`.
## Fix Focus Areas
- src-tauri/src/live_sync.rs[145-166]
- src-tauri/src/live_sync.rs[194-209]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant