feat: smart sync with change detection

[arimxyer]

Summary

• Replace blind rclone sync with smart change detection using rclone lsjson metadata checks and local SHA-256 hashing
• Track sync state in .sync-state file so syncs only trigger when local or remote actually changed
• Move sync into vault layer (VaultService.SyncPull() + auto-push in save()) so both CLI and TUI get sync automatically
• Add conflict detection: when both local and remote changed, warn user instead of overwriting
• Remove session-scoped maybeSyncPull/Push from CLI commands — sync is now handled by the vault layer

Test plan

• Unit tests for SyncState load/save/hash operations (state_test.go)
• Unit tests for SmartPull skip, pull, conflict detection with mock executor
• Unit tests for SmartPush skip and push with mock executor
• Build passes
• Lint passes
• Full test suite passes

🤖 Generated with Claude Code

General approach: Introduce centralized validation/normalization for vault paths and directories, and apply it before using them in file I/O and hashing. The validation should (a) normalize to absolute paths, (b) reject or sanitize obviously dangerous patterns (.., empty path, embedded NUL), and (c) be applied consistently where vaultPath enters the system (CLI GetVaultPath, TUI getDefaultVaultPath, and vault.New), and where vault directories are used for sync state (LoadState, SaveState, StatePath, SmartPush/HashFile).

Best concrete fix with minimal behavior change:

1. In internal/sync/state.go, add:

   • A helper sanitizeVaultDir(vaultDir string) (string, error) which resolves the directory to an absolute path and rejects empty strings and directories that resolve to root if that’s considered unsafe.
   • A helper sanitizeFilePath(path string) (string, error) used by HashFile to ensure we’re hashing an absolute, normalized path.
   • Update LoadState, SaveState, StatePath, and HashFile to call these helpers before constructing or using filesystem paths.

2. In cmd/root.go, add a small validator normalizeVaultPath(vaultPath string) (string, error) that:

   • Expands environment variables and ~ as today, then uses filepath.Abs to normalize.
   • Rejects empty results or paths containing .. components when cleaned (filepath.Clean), to avoid odd relative traversals.
   • Replace the inline expansion logic in GetVaultPath with a call to this helper, and on failure print an error and exit (similar to the existing validation block).

3. In cmd/tui/main.go, update getDefaultVaultPath to reuse the same normalization logic as GetVaultPath (or a TUI-local equivalent) instead of manually joining home and ~. This keeps behavior consistent between CLI and TUI and ensures the TUI also returns a normalized, safe path.

4. In internal/vault/vault.go, tighten handling of the vaultPath argument in New:

   • Normalize to an absolute path using filepath.Abs after the existing ~ expansion.
   • Optionally reject vaultPath that cleans to root or contains .. in any awkward way.

Because you asked for a fix centered on internal/sync/state.go and related flows, and to avoid touching more files than necessary, the concrete edits below focus on path normalization inside internal/sync/state.go itself: ensuring that the tainted path argument passed into HashFile is normalized and validated before opening.

---

General strategy: Normalize and validate the vault path at a single entry point, then only use the normalized value throughout the rest of the application. Ensure that the path is absolute, does not contain attempts to escape via .., and is located inside a designated base directory (e.g. under the user’s home directory such as $HOME/.pass-cli). Use standard library helpers like filepath.Abs, filepath.Clean, and prefix checks to enforce this. Then update SyncPull to use the validated vault path and its directory only after this normalization.

Best concrete fix with minimal behavioral change:

1. Centralize vault-path normalization and validation in cmd/root.go:GetVaultPath():

   • Keep the existing logic (config, defaults, ~ expansion, os.ExpandEnv) but then:
   • Resolve to an absolute, cleaned path via filepath.Abs.
   • Compute a safe base directory (e.g. the user’s home dir) and ensure the vault path is either:
     • exactly the default $HOME/.pass-cli/vault.enc, or
     • located under the user’s home directory.
       In practice, we’ll allow any path under the home directory, which is a reasonable security boundary for a user-local CLI tool.
   • If normalization fails or the path is outside the home directory, print an error to stderr and os.Exit(1) so we don’t proceed with an unsafe path.

   This addresses the variants that involve os.UserHomeDir(), os.ExpandEnv, and configuration.

2. Ensure the vault path passed into vault.New (and thus into VaultService.SyncPull and Service.SmartPull) is already validated:

   • cmd/tui.go uses GetVaultPath(), so by fixing GetVaultPath, this path is now safe.
   • For the separate TUI entrypoint in cmd/tui/main.go, getDefaultVaultPath() duplicates some of the vault-path logic. We should reuse GetVaultPath behavior by:
     • Normalizing and validating the path in getDefaultVaultPath similarly (absolute, under home directory).
   • This ensures both CLI and TUI variants use a normalized, constrained vaultPath.

3. No change is needed inside internal/sync/sync.go smart pull logic beyond relying on the now-validated vaultPath. SmartPull simply derives vaultDir := filepath.Dir(vaultPath) and uses HashFile and os.Stat on vaultPath. Once vaultPath is validated, these operations are safe.

Concretely:

• In cmd/root.go:
  • After computing vaultPath but before returning, use filepath.Abs and filepath.Clean to normalize it.
  • Determine the user’s home directory once, compute the home-based prefix (e.g. home + string(os.PathSeparator)), and ensure the vault path has that prefix or equals the default path. If not, exit with a clear error.
• In cmd/tui/main.go’s getDefaultVaultPath:
  • After building the vault path (config or default), normalize to filepath.Abs and explicitly ensure that:
    • if from config, it is under the user’s home directory; if not, exit with an error; and
    • for the default, keep using $HOME/.pass-cli/vault.enc (already under home).

This adds validation but does not change normal behavior for valid configurations. It also resolves all CodeQL flows since the tainted inputs are now sanitized before being used as filesystem paths.

---

In general, to fix uncontrolled path usage, we should constrain or validate the user-supplied path before using it for filesystem operations, ensuring it is either: (a) within a designated safe base directory, or (b) at least a normalized, absolute path without traversal tricks. Here, vaultPath is the path to a single encrypted vault file. The sensitive usage is the directory containing that file (vaultDir in SmartPull). The least invasive fix is to normalize vaultPath to an absolute path and then verify that the directory is safe to operate in, or at least that it is a valid absolute path; this addresses CodeQL’s concern and reduces the risk of surprising locations being used.

The best, low-impact fix within the shown code is:

1. Normalize vaultPath to an absolute path when constructing the VaultService in internal/vault/vault.go::New. That ensures every use of v.vaultPath (including in sync) is absolute and canonical.
2. In internal/sync/sync.go::SmartPull, after computing vaultDir := filepath.Dir(vaultPath), enforce that vaultDir is an absolute path and reject clearly invalid values (empty, rootless relative, etc.) before calling os.MkdirAll and rclone. If invalid, return an error instead of operating on that path.
3. Optionally, avoid duplicating path-expansion logic in multiple places by trusting GetVaultPath to return something sensible; however, since we must not change behavior significantly, we’ll keep existing behavior but add the normalization and simple validation.

Concretely:

• In internal/vault/vault.go inside New, after the existing ~ expansion block, call filepath.Abs on vaultPath and, on failure, return an error. This ensures v.vaultPath is always absolute and normalized.
• In internal/sync/sync.go inside SmartPull, replace the plain vaultDir := filepath.Dir(vaultPath) with logic that:
  • calls filepath.Abs(vaultPath) to get a canonical path (without changing the effective target),
  • computes vaultDir := filepath.Dir(absPath),
  • checks that vaultDir is non-empty and filepath.IsAbs(vaultDir), and returns a wrapped error if not.
• These two localized changes keep existing functionality but ensure that the paths we pass to os.MkdirAll and rclone are well-formed and not relative/traversal-based, addressing all the alert variants that ultimately depend on vaultDir in SmartPull.

No changes are required in cmd/root.go or cmd/tui/main.go for validation, because we normalize/validate at the lower layers where the sink occurs.

---