diff --git a/README.md b/README.md index 111b0ee..e08f44c 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ If you want to add another target later, follow [Adding Strava](#adding-strava) ```bash eufy-sync # sync new measurements to every configured target -eufy-sync --status # show the last sync and token health +eufy-sync --status # show the latest synced weigh-in and token health eufy-sync --dry-run # preview a sync without uploading eufy-sync --doctor # check the setup and print fixes eufy-sync --history # show recent sync history diff --git a/docs/command-reference.md b/docs/command-reference.md index b1993d3..6d2e2ea 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -5,7 +5,7 @@ Running `eufy-sync` with no options syncs new measurements to every configured t ## Check and preview ```bash -eufy-sync --status # last sync and token health +eufy-sync --status # latest synced weigh-in and token health eufy-sync --history # the last 14 sync-history entries eufy-sync --history 30 # choose how many entries to show eufy-sync --dry-run # preview without uploading diff --git a/docs/headless-linux.md b/docs/headless-linux.md index abf4e87..583c9df 100644 --- a/docs/headless-linux.md +++ b/docs/headless-linux.md @@ -65,6 +65,22 @@ A user timer normally runs only while that user's systemd manager is active. If sudo loginctl enable-linger "$USER" ``` +## Set the timezone + +eufy-sync uses the machine's timezone to decide which day a weigh-in belongs to when it checks Garmin for an existing entry, and to show dates in its summaries and notifications. Servers, NAS boxes, and containers often run on UTC, which can put an evening weigh-in on the next day. Set `TZ` to your local zone for the scheduled run. + +For the systemd service, add this line under `[Service]`: + +```ini +Environment=TZ=America/New_York +``` + +For Docker, pass it when starting the container: + +```bash +docker run -e TZ=America/New_York ... +``` + ## Check a scheduled run ```bash diff --git a/eufy_sync/__init__.py b/eufy_sync/__init__.py index 1ce380c..b5a26ca 100644 --- a/eufy_sync/__init__.py +++ b/eufy_sync/__init__.py @@ -1,6 +1,6 @@ """Sync Eufy smart scale body composition data to Garmin Connect and Strava.""" -__version__ = "1.13.2" +__version__ = "1.14.0" # Public API for programmatic use from eufy_sync.eufy_client import EufyClient, EufyMeasurement diff --git a/eufy_sync/cli/app.py b/eufy_sync/cli/app.py index 58efb4f..61f8762 100644 --- a/eufy_sync/cli/app.py +++ b/eufy_sync/cli/app.py @@ -1,6 +1,7 @@ """The eufy-sync command line entry point and sync driver.""" from __future__ import annotations +import contextlib import logging import sys from pathlib import Path @@ -64,6 +65,20 @@ def _password_failure_title(failures: list) -> str: return "eufy-sync: login failed" +@contextlib.contextmanager +def _credential_lock(command: str): + """Hold the sync lock for a command that changes credentials, tokens, or + config, or exit with a retry hint when a sync or another such command has + it. A lock file that cannot be opened also refuses, unlike the sync path: + an unserialized credential write can strand a token a sync just rotated.""" + from eufy_sync.cli import lock + with lock.single_instance(require_lock=True) as acquired: + if not acquired: + print(f"Another eufy-sync run is in progress. Retry {command} when it finishes.") + sys.exit(1) + yield + + def _sync_with_network_retry(user, state, **kwargs): """Give scheduled network failures one delayed retry, keeping partial counts.""" from eufy_sync.network import is_transient_network_error @@ -91,6 +106,29 @@ def _sync_with_network_retry(user, state, **kwargs): sleep(NETWORK_RETRY_DELAY) +def _exit_could_not_start(error: Exception) -> None: + msg = f"eufy-sync could not start: {error}" + print(msg) + platform_support.notify("eufy-sync failed", str(error)[:200]) + sys.exit(1) + + +def _load_config_or_exit(config_path: Path, migrate: bool = True): + from eufy_sync.config import load_config + try: + return load_config(config_path, migrate=migrate) + except SystemExit: + raise + except Exception as e: + _exit_could_not_start(e) + + +def _check_target(args, config) -> None: + if args.target and not any(getattr(u, args.target, None) for u in config.users): + print(f"Target '{args.target}' is not configured. Check your config.") + sys.exit(1) + + def main() -> None: try: _main() @@ -169,14 +207,22 @@ def _main() -> None: # Handle full uninstall if args.uninstall: - maintenance._uninstall(shared.DATA_DIR, config_path=config_path, db_path=db_path) + from eufy_sync.cli import lock + with _credential_lock("--uninstall"): + removed = maintenance._uninstall(shared.DATA_DIR, config_path=config_path, db_path=db_path) + if removed: + # POSIX: delete the sync lock file before releasing it. + lock.unlink_while_held() + if removed: + maintenance._remove_lock_files(shared.DATA_DIR) return # Handle credential store mode switches if args.use_file_store: from eufy_sync import credentials try: - credentials.use_file_store() + with _credential_lock("--use-file-store"): + credentials.use_file_store() except RuntimeError as e: print(str(e)) sys.exit(1) @@ -186,7 +232,8 @@ def _main() -> None: if args.use_keychain: from eufy_sync import credentials try: - credentials.use_keychain_store() + with _credential_lock("--use-keychain"): + credentials.use_keychain_store() except RuntimeError as e: print(str(e)) sys.exit(1) @@ -209,55 +256,43 @@ def _main() -> None: # Handle self-update if args.update: - updater._self_update() + # A reinstall mid-sync swaps the code under the running process. + with _credential_lock("--update"): + updater._self_update() return # Handle Strava setup if args.setup_strava: - setup._setup_strava(config_path) + with _credential_lock("--setup-strava"): + setup._setup_strava(config_path) return if args.setup_zwift: - from eufy_sync.cli import lock - with lock.single_instance(require_lock=True) as acquired: - if not acquired: - print("Another eufy-sync run is in progress. Retry --setup-zwift when it finishes.") - sys.exit(1) + with _credential_lock("--setup-zwift"): setup._setup_zwift(config_path) return if args.disconnect_zwift: - from eufy_sync.cli import lock - with lock.single_instance(require_lock=True) as acquired: - if not acquired: - print("Another eufy-sync run is in progress. Retry --disconnect-zwift when it finishes.") - sys.exit(1) + with _credential_lock("--disconnect-zwift"): maintenance._disconnect_zwift(config_path) return # Handle profile selection if args.select_profile: - profiles._select_profile(config_path) + with _credential_lock("--select-profile"): + profiles._select_profile(config_path) return # Handle password update if args.update_password: - from eufy_sync.cli import lock - with lock.single_instance(require_lock=True) as acquired: - if not acquired: - print("Another eufy-sync run is in progress. Retry --update-password when it finishes.") - sys.exit(1) + with _credential_lock("--update-password"): maintenance._update_password(config_path) return # Handle reauth if args.reauth is not None: target = None if args.reauth == "all" else args.reauth - from eufy_sync.cli import lock - with lock.single_instance(require_lock=True) as acquired: - if not acquired: - print("Another eufy-sync run is in progress. Retry --reauth when it finishes.") - sys.exit(1) + with _credential_lock("--reauth"): maintenance._reauth(config_path, force=True, target=target) return @@ -277,55 +312,36 @@ def _main() -> None: platform_support.notify("eufy-sync", msg) sys.exit(1) - try: - if first_run: - setup._first_run_setup(config_path) - else: - # Migrate existing plaintext passwords to keychain (one-time) - setup._migrate_config_passwords(config_path) - # One-time upgrade notice for users coming from eufy-garmin-sync - setup._show_upgrade_notice() - - # Load config (passwords resolved from keychain or YAML fallback) - from eufy_sync.config import load_config - config = load_config(config_path) - except SystemExit: - raise - except Exception as e: - msg = f"eufy-sync could not start: {e}" - print(msg) - platform_support.notify("eufy-sync failed", str(e)[:200]) - sys.exit(1) - - has_garmin = any(u.garmin for u in config.users) - - if args.target and not any(getattr(u, args.target, None) for u in config.users): - print(f"Target '{args.target}' is not configured. Check your config.") - sys.exit(1) - - # Handle status - if args.status: + if args.status or args.history is not None: + # Read-only and unlocked, so it must not write the vault: no config + # password migration, and legacy keychain items are read in place + # rather than moved. A sync holding the lock may be saving the vault + # right now, and a second writer could undo its token refresh. + config = _load_config_or_exit(config_path, migrate=False) + _check_target(args, config) from eufy_sync.state import SyncState try: state = SyncState(db_path) except Exception as e: print(f"Could not read sync state: {e}") sys.exit(1) - status._show_status(state, config.users) + if args.status: + status._show_status(state, config.users) + else: + status._show_history(state, config.users, limit=args.history) state.close() return - # Handle history - if args.history is not None: - from eufy_sync.state import SyncState + if first_run: + # Setup stores passwords and can log in to Garmin and Zwift. The + # lock is released before the first sync takes it again below. try: - state = SyncState(db_path) + with _credential_lock("eufy-sync"): + setup._first_run_setup(config_path) + except SystemExit: + raise except Exception as e: - print(f"Could not read sync state: {e}") - sys.exit(1) - status._show_history(state, config.users, limit=args.history) - state.close() - return + _exit_could_not_start(e) # Run sync from eufy_sync.cli import lock @@ -344,6 +360,24 @@ def _main() -> None: print("Another eufy-sync run is in progress; skipping.") return + # The password migrations below write the credential vault, so they + # run only while this process holds the lock. + if not first_run: + try: + # Migrate existing plaintext passwords to keychain (one-time) + setup._migrate_config_passwords(config_path) + # One-time upgrade notice for users coming from eufy-garmin-sync + setup._show_upgrade_notice() + except SystemExit: + raise + except Exception as e: + _exit_could_not_start(e) + # Load config (passwords resolved from keychain or YAML fallback) + config = _load_config_or_exit(config_path) + + has_garmin = any(u.garmin for u in config.users) + _check_target(args, config) + updater._check_for_updates() backfill = args.backfill_days diff --git a/eufy_sync/cli/doctor.py b/eufy_sync/cli/doctor.py index b6421cc..3bbffcc 100644 --- a/eufy_sync/cli/doctor.py +++ b/eufy_sync/cli/doctor.py @@ -153,7 +153,9 @@ def _check_keychain(report) -> None: return report("PASS", "keychain", active_store_label()) except Exception as e: - report("PASS", "keychain", f"file store (no keychain prompts) ({e})") + # A damaged or unreadable credentials file now raises here instead + # of quietly falling back to the keychain. + report("FAIL", "keychain", str(e)) def _check_eufy_token(report, user): @@ -281,9 +283,9 @@ def _check_state_db(report, db_path: Path, user) -> None: days = ago.days hours = int(ago.total_seconds() / 3600) if days > 0: - report("PASS", "state db", f"last sync {days}d ago") + report("PASS", "state db", f"latest weigh-in {days}d ago") else: - report("PASS", "state db", f"last sync {hours}h ago") + report("PASS", "state db", f"latest weigh-in {hours}h ago") except Exception as e: report("FAIL", "state db", str(e)) finally: diff --git a/eufy_sync/cli/lock.py b/eufy_sync/cli/lock.py index 1ab5f0b..f00b8b0 100644 --- a/eufy_sync/cli/lock.py +++ b/eufy_sync/cli/lock.py @@ -11,7 +11,9 @@ The lock is a courtesy, not a guarantee. If the file cannot be created the run proceeds unlocked rather than failing: an overlap is a rare annoyance, while a sync that refuses to start is a real one (same trade-off failure_notify makes -with its counter file). +with its counter file). The credential vault does not depend on this: every +vault write separately takes the vault lock in eufy_sync.credentials, which +refuses to write rather than run unlocked. There is no PID or staleness handling on purpose. The OS drops the lock when the handle closes or the process dies, so a killed run leaves nothing behind. @@ -19,62 +21,21 @@ from __future__ import annotations import contextlib -import os import sys from pathlib import Path from typing import Iterator +from eufy_sync import file_lock from eufy_sync.cli import shared LOCK_NAME = "sync.lock" -if sys.platform == "win32": - import msvcrt -else: - import fcntl - def lock_path() -> Path: # Read at call time: the data dir is redirected in tests. return shared.DATA_DIR / LOCK_NAME -def _open() -> int | None: - """Open (creating if needed) the lock file. None if it cannot be made.""" - try: - shared.DATA_DIR.mkdir(parents=True, exist_ok=True, mode=0o700) - return os.open(str(lock_path()), os.O_RDWR | os.O_CREAT, 0o600) - except OSError: - return None - - -def _try_acquire(fd: int) -> bool: - """Take the exclusive lock without blocking. False when someone holds it.""" - try: - if sys.platform == "win32": - # msvcrt locks a byte range from the current position; one byte - # past EOF is fine and keeps the file empty. - os.lseek(fd, 0, os.SEEK_SET) - msvcrt.locking(fd, msvcrt.LK_NBLCK, 1) - else: - fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) - except OSError: - return False - return True - - -def _release(fd: int) -> None: - try: - if sys.platform == "win32": - os.lseek(fd, 0, os.SEEK_SET) - msvcrt.locking(fd, msvcrt.LK_UNLCK, 1) - else: - fcntl.flock(fd, fcntl.LOCK_UN) - except OSError: - # Closing the handle below releases it anyway. - pass - - @contextlib.contextmanager def single_instance(require_lock: bool = False) -> Iterator[bool]: """Hold the sync lock for the block. @@ -84,17 +45,30 @@ def single_instance(require_lock: bool = False) -> Iterator[bool]: require_lock=True, that failure yields False so credential and token mutations cannot proceed without serialization. """ - fd = _open() - if fd is None: + try: + fd = file_lock.acquire(lock_path()) + except OSError: yield not require_lock return + if fd is None: + yield False + return try: - if not _try_acquire(fd): - yield False - return - try: - yield True - finally: - _release(fd) + yield True finally: - os.close(fd) + file_lock.release(fd) + + +def unlink_while_held() -> None: + """Delete the lock file while this process still holds it (call inside + single_instance). Deleting it after release would let another process + lock the old file while a third creates and locks a new one at the same + path. POSIX only: Windows refuses to delete an open file, so there the + caller deletes it after release, and a process that has it open blocks + that delete.""" + if sys.platform == "win32": + return + try: + lock_path().unlink(missing_ok=True) + except OSError: + pass diff --git a/eufy_sync/cli/maintenance.py b/eufy_sync/cli/maintenance.py index d950313..3a10bdc 100644 --- a/eufy_sync/cli/maintenance.py +++ b/eufy_sync/cli/maintenance.py @@ -14,7 +14,7 @@ def _update_password(config_path: Path) -> None: - """Update stored passwords.""" + """Update stored passwords, each one only after it logs in.""" if not config_path.exists(): print("No config found. Run eufy-sync first to set up.") sys.exit(1) @@ -36,46 +36,77 @@ def _update_password(config_path: Path) -> None: print("No changes made.") return - from eufy_sync.credentials import delete_token, store_password - - if eufy_pw: - store_password(f"{user_name}:eufy", eufy_pw) - - if garmin_pw: - store_password(f"{user_name}:garmin", garmin_pw) - if zwift_pw: - store_password(f"{user_name}:zwift", zwift_pw) - - # Clear cached tokens for changed services - if eufy_pw: - delete_token("eufy") - eufy_token = shared.DATA_DIR / "eufy_token.json" - if eufy_token.exists(): - eufy_token.unlink() - - if garmin_pw: - delete_token("garmin") - garmin_session = shared.DATA_DIR / "session.json" - if garmin_session.exists(): - garmin_session.unlink() - if zwift_pw: - delete_token("zwift") - - changed = [] - if eufy_pw: - changed.append("Eufy") - if garmin_pw: - changed.append("Garmin") - if zwift_pw: - changed.append("Zwift") - print(f"{' and '.join(changed)} password{'s' if len(changed) > 1 else ''} updated.") - - if garmin_pw: - print("Garmin password changed - re-authenticating...") - _reauth(config_path, config, target="garmin") - if zwift_pw: - print("Zwift password changed - re-authenticating...") - _reauth(config_path, config, target="zwift") + from eufy_sync.credentials import store_password + + # Each new password logs in before it is stored. The login saves its own + # token on success; a failure (a typo, a cancelled MFA prompt, no network) + # leaves that service's stored password and token as they were, so a + # mistake here cannot end a session that was still working. + changes = [ + (label, password, verify) + for label, password, verify in ( + ("Eufy", eufy_pw, _verify_eufy_password), + ("Garmin", garmin_pw, _verify_garmin_password), + ("Zwift", zwift_pw, _verify_zwift_password), + ) + if password + ] + updated = [] + for label, password, verify in changes: + print(f"Checking the new {label} password...") + try: + verify(user, password) + except Exception as e: + print(f"{label} login with the new password failed: {e}") + print(f"The stored {label} password and login were left unchanged.") + if updated: + print(f"Already updated: {' and '.join(updated)}.") + print("Retry with: eufy-sync --update-password") + sys.exit(1) + store_password(f"{user_name}:{label.lower()}", password) + updated.append(label) + + print(f"{' and '.join(updated)} password{'s' if len(updated) > 1 else ''} updated.") + + +def _verify_eufy_password(user: dict, password: str) -> None: + """Log in to Eufy with password; the token is replaced only on success.""" + from eufy_sync.config import EufyConfig + from eufy_sync.eufy_client import EufyClient + + client = EufyClient( + EufyConfig(email=user["eufy"]["email"], password=password), + token_path=shared.DATA_DIR / "eufy_token.json", + ) + try: + # Skip the cached token on purpose: it says nothing about the password. + client._fresh_login() + finally: + client.close() + + +def _verify_garmin_password(user: dict, password: str) -> None: + """Log in to Garmin with password, prompting for MFA if Garmin asks. + force_reauth replaces the stored token only after the login succeeds.""" + from eufy_sync.garmin_auth import GarminAuth + + auth = GarminAuth(user["garmin"]["email"], password, session_path=shared.DATA_DIR / "session.json") + auth.force_reauth() + print("Done - Garmin tokens saved.") + + +def _verify_zwift_password(user: dict, password: str) -> None: + """Log in to Zwift with password. A forced login reads the profile before + it saves anything, so the cached token is replaced only on success.""" + from eufy_sync.config import ZwiftConfig + from eufy_sync.zwift_client import ZwiftClient + + client = ZwiftClient(ZwiftConfig(email=user["zwift"]["email"], password=password)) + try: + client.authenticate(force=True) + finally: + client.close() + print("Done - Zwift token saved.") def _reauth(config_path: Path, config: dict | None = None, force: bool = False, target: str | None = None) -> None: @@ -214,12 +245,14 @@ def _uninstall_launch_agent() -> None: platform_support.uninstall_agent() -def _uninstall(data_dir: Path, config_path: Path | None = None, db_path: Path | None = None) -> None: +def _uninstall(data_dir: Path, config_path: Path | None = None, db_path: Path | None = None) -> bool: """Remove all eufy-sync data: Launch Agent, config, tokens, state DB. config_path/db_path default to the standard files under data_dir, but a custom --config/--db location (outside data_dir) is also deleted so --uninstall does not leave those files behind. + + Returns True when the data was removed, False when the user cancelled. """ if not sys.stdin.isatty(): print("Error: --uninstall requires an interactive terminal.") @@ -236,7 +269,7 @@ def _uninstall(data_dir: Path, config_path: Path | None = None, db_path: Path | answer = input("Are you sure? [y/N] ").strip() if not answer.lower().startswith("y"): print("Cancelled.") - return + return False default_config_path = data_dir / "config.yaml" default_db_path = data_dir / "state.db" @@ -266,51 +299,75 @@ def _uninstall(data_dir: Path, config_path: Path | None = None, db_path: Path | except Exception: pass - # Clear the keychain vault. On a file-backend machine this gate skips the - # deletes, which is safe only because credentials.json lives inside data_dir - # and is erased by the rmtree below; keep them together if CRED_FILE ever - # moves outside ~/.garmin-sync. - from eufy_sync.credentials import _keyring_available, delete_password, delete_token - if _keyring_available(): - # Best-effort: a locked keychain makes the vault read raise, and a - # half-finished uninstall that leaves the data dir behind (the rmtree - # is below) plus a raw traceback is worse than skipping this. The - # rmtree still erases a file-backed vault under data_dir. - try: - for name in user_names: - # "strava" here is the API app's client secret, not an account - # password; it moved into the vault alongside the other two. - for suffix in ["eufy", "garmin", "strava", "zwift"]: - delete_password(f"{name}:{suffix}") - delete_token("eufy") - delete_token("garmin") - delete_token("strava") - delete_token("zwift") - delete_token("zwift_probe") - except Exception: - print("Note: could not clear keychain entries (the keychain may be locked).") - - # Remove data directory. A kept DB at a custom --db path lives outside - # data_dir, so only the default location needs the selective sweep. - preserve_default_db = keep_db and db_path == default_db_path and db_path.exists() - if data_dir.exists(): - if not preserve_default_db: - shutil.rmtree(data_dir) - else: + from eufy_sync.cli.lock import LOCK_NAME + from eufy_sync.credentials import ( + VAULT_LOCK_NAME, + _keyring_available, + delete_password, + delete_token, + vault_lock, + ) + + # One vault lock across clearing the vault and deleting credentials.json, + # so no credential write can land in between. + with vault_lock(): + # Clear the keychain vault. On a file-backend machine this gate skips the + # deletes, which is safe only because credentials.json lives inside data_dir + # and is erased by the rmtree below; keep them together if CRED_FILE ever + # moves outside ~/.garmin-sync. + if _keyring_available(): + # Best-effort: a locked keychain makes the vault read raise, and a + # half-finished uninstall that leaves the data dir behind (the rmtree + # is below) plus a raw traceback is worse than skipping this. The + # rmtree still erases a file-backed vault under data_dir. + try: + for name in user_names: + # "strava" here is the API app's client secret, not an account + # password; it moved into the vault alongside the other two. + for suffix in ["eufy", "garmin", "strava", "zwift"]: + delete_password(f"{name}:{suffix}") + delete_token("eufy") + delete_token("garmin") + delete_token("strava") + delete_token("zwift") + delete_token("zwift_probe") + except Exception: + print("Note: could not clear keychain entries (the keychain may be locked).") + + # Remove data directory. A kept DB at a custom --db path lives outside + # data_dir, so only the default location needs the selective sweep. + # Both lock files are skipped: --uninstall holds them, and a lock file + # deleted mid-sweep lets another process create and lock a fresh one + # while credentials are still being removed. The vault lock file goes + # below, after everything else; the caller handles the sync lock file + # (see lock.unlink_while_held and _remove_lock_files). + preserve_default_db = keep_db and db_path == default_db_path and db_path.exists() + if data_dir.exists(): + keep = {LOCK_NAME, VAULT_LOCK_NAME} + if preserve_default_db: + keep.add("state.db") for item in data_dir.iterdir(): - if item.name == "state.db": + if item.name in keep: continue if item.is_dir(): shutil.rmtree(item) else: item.unlink() - # A custom --config/--db path lives outside data_dir, so it survives the - # rmtree above and must be removed explicitly. - if config_path != default_config_path and config_path.exists(): - config_path.unlink() - if db_path != default_db_path and not keep_db and db_path.exists(): - db_path.unlink() + # A custom --config/--db path lives outside data_dir, so it survives the + # sweep above and must be removed explicitly. + if config_path != default_config_path and config_path.exists(): + config_path.unlink() + if db_path != default_db_path and not keep_db and db_path.exists(): + db_path.unlink() + + # Last step under the vault lock. On POSIX the lock file is deleted + # while still held, so no other process can lock it between release + # and delete. Windows refuses to delete an open file, so there + # _remove_lock_files deletes it after release. + if sys.platform != "win32": + (data_dir / VAULT_LOCK_NAME).unlink(missing_ok=True) + _remove_dir_if_empty(data_dir) print("") if keep_db: @@ -319,3 +376,30 @@ def _uninstall(data_dir: Path, config_path: Path | None = None, db_path: Path | print("Removed all eufy-sync data.") print(f"To remove the package itself, run: {install.uninstall_command()}") + return True + + +def _remove_dir_if_empty(path: Path) -> None: + try: + path.rmdir() + except OSError: + pass + + +def _remove_lock_files(data_dir: Path) -> None: + """Finish --uninstall once its locks are released. + + On Windows the lock files could not be deleted while open, so they go + now; another process that has one open blocks the delete, which keeps it + safe. On POSIX they were already deleted while still held, and deleting + now could remove a lock file another process has just created and + locked. Either way the data dir goes too when nothing else is left.""" + from eufy_sync.cli.lock import LOCK_NAME + from eufy_sync.credentials import VAULT_LOCK_NAME + if sys.platform == "win32": + for name in (LOCK_NAME, VAULT_LOCK_NAME): + try: + (data_dir / name).unlink(missing_ok=True) + except OSError: + pass + _remove_dir_if_empty(data_dir) diff --git a/eufy_sync/cli/setup.py b/eufy_sync/cli/setup.py index b7bc6f8..78248cf 100644 --- a/eufy_sync/cli/setup.py +++ b/eufy_sync/cli/setup.py @@ -217,19 +217,10 @@ def _connect_zwift(user: dict, retry_command: str = "eufy-sync --setup-zwift") - from eufy_sync import credentials vault = credentials._load_vault() - probe = vault.get("tokens", {}).get("zwift_probe") password_account = f"{user_name}:zwift" existing = user.get("zwift") or {} existing_email = existing.get("email") existing_password = vault.get("passwords", {}).get(password_account) - probe_is_current_user = ( - isinstance(probe, dict) - and probe.get("user_name") == user_name - and probe.get("password_account") == password_account - and (not existing_email or probe.get("email") == existing_email) - ) - probe_email = probe.get("email") if probe_is_current_user else None - probe_password = vault.get("passwords", {}).get(password_account) if probe_is_current_user else None print("") print(" Experimental Zwift weight sync") @@ -242,13 +233,9 @@ def _connect_zwift(user: dict, retry_command: str = "eufy-sync --setup-zwift") - email = existing_email password = existing_password print(f"Using the configured Zwift account for {email}.") - elif probe_email and probe_password: - email = probe_email - password = probe_password - print(f"Using the validated Zwift account for {email}.") else: if not sys.stdin.isatty(): - print("No validated Zwift credentials were found. Run setup in an interactive terminal.") + print("No saved Zwift credentials were found. Run setup in an interactive terminal.") sys.exit(1) email = input("Zwift email: ").strip() if not email: diff --git a/eufy_sync/cli/status.py b/eufy_sync/cli/status.py index fa4d452..77626e5 100644 --- a/eufy_sync/cli/status.py +++ b/eufy_sync/cli/status.py @@ -48,14 +48,15 @@ def _print_summary( user = users[0] ts = state.get_latest_sync_timestamp(user.name) if ts: - last_sync = datetime.fromtimestamp(ts, tz=timezone.utc) - ago = datetime.now(timezone.utc) - last_sync + # The newest synced measurement's time, not when sync last ran. + latest = datetime.fromtimestamp(ts, tz=timezone.utc) + ago = datetime.now(timezone.utc) - latest days = ago.days hours = int(ago.total_seconds() / 3600) % 24 if days > 0: - parts.append(f"last sync: {days}d ago") + parts.append(f"latest weigh-in: {days}d ago") else: - parts.append(f"last sync: {hours}h ago") + parts.append(f"latest weigh-in: {hours}h ago") from eufy_sync.eufy_client import EufyClient eufy_status = EufyClient(user.eufy).token_status() diff --git a/eufy_sync/config.py b/eufy_sync/config.py index 5c9b74a..6c504a2 100644 --- a/eufy_sync/config.py +++ b/eufy_sync/config.py @@ -72,12 +72,17 @@ def _walk_and_interpolate(obj: dict | list | str) -> dict | list | str: return obj -def _get_password(user_name: str, service: str, email: str, yaml_password: str | None) -> str: - """Resolve password: credential store first, then YAML fallback.""" +def _get_password( + user_name: str, service: str, email: str, yaml_password: str | None, migrate: bool = True +) -> str: + """Resolve password: credential store first, then YAML fallback. + + migrate=False reads a legacy keychain item without moving it into the + vault, for callers that run without the sync lock.""" from eufy_sync.credentials import get_password key = f"{user_name}:{service}" - stored = get_password(key) + stored = get_password(key, migrate=migrate) if stored: return stored @@ -90,7 +95,7 @@ def _get_password(user_name: str, service: str, email: str, yaml_password: str | ) -def _get_strava_secret(user_name: str, yaml_secret: str | None) -> str: +def _get_strava_secret(user_name: str, yaml_secret: str | None, migrate: bool = True) -> str: """Resolve the Strava API client secret: credential store first, then the YAML fallback for configs not yet migrated. @@ -100,7 +105,7 @@ def _get_strava_secret(user_name: str, yaml_secret: str | None) -> str: """ from eufy_sync.credentials import get_password - stored = get_password(f"{user_name}:strava") + stored = get_password(f"{user_name}:strava", migrate=migrate) if stored: return stored @@ -113,7 +118,12 @@ def _get_strava_secret(user_name: str, yaml_secret: str | None) -> str: ) -def load_config(path: Path) -> AppConfig: +def load_config(path: Path, migrate: bool = True) -> AppConfig: + """Parse the config and resolve its secrets. + + The default migrates legacy keychain items into the vault, which writes + the vault, so it belongs inside the sync lock. Unlocked read-only + commands (--status, --history) pass migrate=False.""" with open(path) as f: raw = yaml.safe_load(f) @@ -143,21 +153,21 @@ def load_config(path: Path) -> AppConfig: if "garmin" in u: garmin = GarminConfig( email=u["garmin"]["email"], - password=_get_password(name, "garmin", u["garmin"]["email"], u["garmin"].get("password")), + password=_get_password(name, "garmin", u["garmin"]["email"], u["garmin"].get("password"), migrate), ) strava = None if "strava" in u: strava = StravaConfig( client_id=str(u["strava"]["client_id"]), - client_secret=_get_strava_secret(name, u["strava"].get("client_secret")), + client_secret=_get_strava_secret(name, u["strava"].get("client_secret"), migrate), ) zwift = None if "zwift" in u: zwift = ZwiftConfig( email=u["zwift"]["email"], - password=_get_password(name, "zwift", u["zwift"]["email"], u["zwift"].get("password")), + password=_get_password(name, "zwift", u["zwift"]["email"], u["zwift"].get("password"), migrate), ) if not garmin and not strava and not zwift: @@ -170,7 +180,7 @@ def load_config(path: Path) -> AppConfig: name=name, eufy=EufyConfig( email=u["eufy"]["email"], - password=_get_password(name, "eufy", u["eufy"]["email"], u["eufy"].get("password")), + password=_get_password(name, "eufy", u["eufy"]["email"], u["eufy"].get("password"), migrate), customer_id=str(u["eufy"]["customer_id"]) if u["eufy"].get("customer_id") is not None else None, ), garmin=garmin, diff --git a/eufy_sync/credentials.py b/eufy_sync/credentials.py index 62d2c09..9af6925 100644 --- a/eufy_sync/credentials.py +++ b/eufy_sync/credentials.py @@ -24,18 +24,39 @@ A keychain that exists but cannot be read (locked, access denied) does raise, so a failed read can never be saved back over the real vault, and --use-file-store aborts rather than write an empty marker file that would -orphan the unread keychain secrets. +orphan the unread keychain secrets. A vault that is present but damaged +(unparseable JSON, a missing chunk) raises VaultCorruptError, a RuntimeError, +for the same reason: reading it as empty would let the next save wipe it. A lazy, one-time migration promotes secrets from the old per-item keychain layout (one keyring account per password/token) into the vault the first time each one is looked up. + +Every change to stored credentials (store, delete, migration, switching +stores) holds the vault lock, an exclusive OS lock on ~/.garmin-sync/ +vault.lock, from the moment it reads the vault until its cleanup is done. +Two processes therefore never interleave a read-modify-write: the second one +waits (up to VAULT_LOCK_TIMEOUT seconds) and then works on the first one's +result. A lock that cannot be created or acquired raises VaultLockError and +nothing is written. Reads do not take the lock; a read that lands on a store +switch in progress retries once on the new backend (see _load_vault). """ from __future__ import annotations +import contextlib +import functools +import hashlib import json import logging import os +import re +import secrets +import threading +import time from pathlib import Path +from typing import Callable, Iterator + +from eufy_sync import file_lock logger = logging.getLogger(__name__) @@ -44,14 +65,106 @@ # Windows Credential Manager caps one entry at ~2,560 bytes stored as UTF-16, # roughly 1,280 characters. A vault larger than CHUNK_LIMIT characters is split -# across numbered "vault:i" entries so set_password never fails on Windows. -# MAX_CHUNKS bounds how far a save probes for stale leftover chunks to delete, -# so a corrupt store can never make that scan run away. +# across "vault::" entries so set_password never fails on Windows; the +# "vault" entry then holds a small header naming the tag, the chunk count and a +# checksum. Each save writes its chunks under a fresh tag (the generation +# number plus a random suffix) and switches the header last, so a save killed +# at any step leaves either the old vault or the new one readable, never a +# splice of the two. +# +# The keychain cannot list entries, so the "vault:journal" entry records every +# tag a save may have left chunks under. Saves only run under the vault lock, +# so no other save can be in progress: every journal tag the live header does +# not name is a leftover of a finished or crashed save, and each save deletes +# them all once its header is in place. Released versions wrote chunks as +# "vault:"; those are still read and are deleted by the next save. +# +# MAX_CHUNKS caps how many chunks a save writes (a larger vault is refused +# before anything is written) and so how many a header of this layout may +# claim. Released versions had no cap, so a "vault:" header of any size is +# still read; its chunks end at the first missing one either way. CHUNK_LIMIT = 1200 MAX_CHUNKS = 40 +JOURNAL_ACCOUNT = "vault:journal" +# Each entry is ~20 characters, so this stays well under CHUNK_LIMIT. +MAX_JOURNAL = 24 CRED_FILE = Path.home() / ".garmin-sync" / "credentials.json" +VAULT_LOCK_NAME = "vault.lock" +VAULT_LOCK_TIMEOUT = 30.0 + + +class VaultLockError(RuntimeError): + """The vault lock could not be created or was not released in time. + Nothing was written.""" + + +def vault_lock_path() -> Path: + # Next to CRED_FILE, read at call time: tests redirect CRED_FILE. The + # file is never deleted in normal use; only --uninstall removes it. + return CRED_FILE.parent / VAULT_LOCK_NAME + + +# One holder per process: the thread lock serializes threads (and makes the +# lock reentrant for nested calls such as a migration inside a store), and +# the first entry takes the OS lock that serializes processes. +_thread_lock = threading.RLock() +_lock_depth = 0 +_lock_fd: int | None = None + + +@contextlib.contextmanager +def vault_lock() -> Iterator[None]: + """Hold the exclusive vault lock for the block. Reentrant within a + process. Raises VaultLockError, writing nothing, when the lock file + cannot be opened or another process keeps the lock past + VAULT_LOCK_TIMEOUT seconds.""" + global _lock_depth, _lock_fd + deadline = time.monotonic() + VAULT_LOCK_TIMEOUT + if not _thread_lock.acquire(timeout=VAULT_LOCK_TIMEOUT): + raise VaultLockError(_VAULT_BUSY) + try: + if _lock_depth == 0: + path = vault_lock_path() + try: + fd = file_lock.acquire(path, max(0.0, deadline - time.monotonic())) + except OSError as e: + raise VaultLockError( + f"The credential lock file {path} could not be created " + f"({e.strerror or e}), so nothing was saved. Check that " + f"{path.parent} is writable and retry." + ) from e + if fd is None: + raise VaultLockError(_VAULT_BUSY) + _lock_fd = fd + _lock_depth += 1 + try: + yield + finally: + _lock_depth -= 1 + if _lock_depth == 0: + fd, _lock_fd = _lock_fd, None + file_lock.release(fd) + finally: + _thread_lock.release() + + +_VAULT_BUSY = ( + "Another eufy-sync process kept the stored credentials locked for " + f"{int(VAULT_LOCK_TIMEOUT)} seconds, so nothing was saved. Retry when it " + "finishes." +) + + +def _locked(fn: Callable) -> Callable: + """Run fn under the vault lock.""" + @functools.wraps(fn) + def wrapper(*args, **kwargs): + with vault_lock(): + return fn(*args, **kwargs) + return wrapper + def _keyring_available() -> bool: try: @@ -71,15 +184,50 @@ def _keyring_available() -> bool: return False +def _file_corrupt(detail: str) -> VaultCorruptError: + return VaultCorruptError( + f"The credentials file {CRED_FILE} is damaged ({detail}). " + "It was left untouched so nothing is saved over it. Repair it or " + "move it aside, then run eufy-sync to sign in again." + ) + + +def _read_cred_file() -> dict | None: + """The parsed CRED_FILE object, or None when there is no file. + + Raises RuntimeError when the file exists but cannot be read, and + VaultCorruptError when it is not a JSON object. Treating either as + empty would let the next save replace the file with an empty vault, + and treating it as unmarked would silently switch to the keychain and + hide every secret in the file.""" + try: + text = CRED_FILE.read_text() + except FileNotFoundError: + return None + except ValueError: + # UnicodeDecodeError: non-UTF-8 bytes. + raise _file_corrupt("not a JSON object") from None + except OSError as e: + raise RuntimeError( + f"The credentials file {CRED_FILE} could not be read " + f"({e.strerror or e}). Check its permissions and retry." + ) from e + try: + parsed = json.loads(text) + except ValueError: + raise _file_corrupt("not a JSON object") from None + if not isinstance(parsed, dict): + raise _file_corrupt("not a JSON object") + return parsed + + def _file_store_is_explicit() -> bool: """True when CRED_FILE carries the opt-in marker that only - use_file_store() writes. Malformed content counts as no marker. - ValueError covers both bad JSON and non-UTF-8 bytes in the file.""" - try: - data = json.loads(CRED_FILE.read_text()) - except (ValueError, TypeError, OSError): - return False - return isinstance(data, dict) and bool(data.get("explicit")) + use_file_store() writes. Only a file that parses and has no marker + counts as unmarked; an existing file that cannot be read or parsed + raises (see _read_cred_file).""" + data = _read_cred_file() + return data is not None and bool(data.get("explicit")) def _active_backend() -> str: @@ -110,16 +258,21 @@ def _empty_vault() -> dict: return {"passwords": {}, "tokens": {}} -def _normalize_vault(vault: dict | None) -> dict: - """Tolerate a partially-shaped or missing vault dict.""" +def _normalize_vault(vault: dict | None, corrupt: Callable[[str], Exception] | None = None) -> dict: + """Fill in absent sections of a vault dict. + + A section that is present but not an object raises (via `corrupt`, the + store's own VaultCorruptError factory): reading it as empty would let the + next save overwrite whatever is still recoverable in it.""" if not isinstance(vault, dict): return _empty_vault() - passwords = vault.get("passwords") - tokens = vault.get("tokens") - normalized = { - "passwords": passwords if isinstance(passwords, dict) else {}, - "tokens": tokens if isinstance(tokens, dict) else {}, - } + corrupt = corrupt or _keychain_corrupt + normalized = {} + for section in ("passwords", "tokens"): + value = vault.get(section, {}) + if not isinstance(value, dict): + raise corrupt(f'its "{section}" section is not a JSON object') + normalized[section] = value # The opt-in marker must survive every load/save round trip of the file # backend, or the first store after --use-file-store would drop it and # silently flip the backend back to the keychain. @@ -128,106 +281,396 @@ def _normalize_vault(vault: dict | None) -> dict: return normalized -def _load_vault_from_keychain() -> dict: +_KEYCHAIN_UNREADABLE = ( + "The system keychain could not be read (it may be locked or " + "access was denied). Unlock it and retry, or run: " + "eufy-sync --use-file-store" +) + + +class VaultCorruptError(RuntimeError): + """The stored vault exists but cannot be parsed or reassembled. + + Raised instead of returning an empty vault: an empty result would be + saved back by the next store_*() call, wiping every stored secret. It is + a RuntimeError, so callers that already handle an unreadable keychain + handle this the same way.""" + + +def _keychain_corrupt(detail: str) -> VaultCorruptError: + return VaultCorruptError( + f"The credential vault in the system keychain is damaged ({detail}). " + "It was left untouched so nothing is saved over it. To start over, " + f'delete the "{VAULT_ACCOUNT}" item for "{SERVICE_NAME}" in your ' + "keychain app, then run eufy-sync to sign in again." + ) + + +def _keychain_get(account: str) -> str | None: import keyring try: - raw = keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT) + return keyring.get_password(SERVICE_NAME, account) except Exception as e: # Returning an empty vault here would let the next read-modify-write # save a near-empty vault over the real one. Raising keeps every # caller safe; sync/doctor/startup already report exceptions cleanly. - raise RuntimeError( - "The system keychain could not be read (it may be locked or " - "access was denied). Unlock it and retry, or run: " - "eufy-sync --use-file-store" - ) from e - if raw is None: - return _empty_vault() + raise RuntimeError(_KEYCHAIN_UNREADABLE) from e + + +def _valid_count(value, limit: int | None = MAX_CHUNKS) -> bool: + # bool is an int subclass; a header saying {"__chunks__": true} is junk. + return type(value) is int and value >= 1 and (limit is None or value <= limit) + + +# "" (written by pre-release builds of the generation layout) or +# ".<8 hex>". Anything else is never used to build an account name, so a +# damaged header or journal cannot point a sweep at unrelated entries. +_TAG_RE = re.compile(r"[0-9]{1,9}(\.[0-9a-f]{8})?") + + +def _valid_tag(value) -> bool: + return isinstance(value, str) and _TAG_RE.fullmatch(value) is not None + + +def _parse_header(raw: str) -> tuple: + """Classify the "vault" entry. Returns one of: + + ("single", vault) the whole vault in one entry + ("legacy", count) released-version chunks "vault:1".."vault:" + ("gen", tag, count, sha256, gen) chunks "vault::1".."vault::" + + Raises VaultCorruptError for anything else.""" try: parsed = json.loads(raw) - # An oversized vault is stored as a header pointing at numbered chunks; - # reassemble the payload before normalizing. A missing chunk means the - # header outlived its payload, which is as unusable as malformed JSON. - if isinstance(parsed, dict) and "__chunks__" in parsed: - count = parsed["__chunks__"] - pieces = [] - for i in range(1, count + 1): - try: - piece = keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{i}") - except Exception as e: - # A keyring failure mid-reassembly (locked, access denied) is - # the same unreadable-keychain condition as the initial read, - # not a missing chunk. Translate it so a partial read can - # never be saved back over the real vault. A chunk that - # returns None (below) is a genuinely missing chunk and keeps - # its malformed-vault handling. - raise RuntimeError( - "The system keychain could not be read (it may be locked or " - "access was denied). Unlock it and retry, or run: " - "eufy-sync --use-file-store" - ) from e - if piece is None: - raise ValueError("missing vault chunk") - pieces.append(piece) - parsed = json.loads("".join(pieces)) - return _normalize_vault(parsed) except (ValueError, TypeError): - logger.warning("Keychain vault contained malformed JSON; treating as empty") - return _empty_vault() + raise _keychain_corrupt("the vault entry is not valid JSON") from None + if not isinstance(parsed, dict): + raise _keychain_corrupt("the vault entry is not a JSON object") + if "__vault__" in parsed: + meta = parsed["__vault__"] + if ( + isinstance(meta, dict) + and type(meta.get("gen")) is int + and meta["gen"] >= 1 + and _valid_count(meta.get("chunks")) + and isinstance(meta.get("sha256"), str) + ): + # Headers written before chunk names carried a random suffix + # have no "tag"; their chunks are named by the generation alone. + tag = meta.get("tag", str(meta["gen"])) + if _valid_tag(tag): + return ("gen", tag, meta["chunks"], meta["sha256"], meta["gen"]) + raise _keychain_corrupt("the chunk header is malformed") + if "__chunks__" in parsed: + # Released versions wrote any number of chunks; reading stops at the + # first missing one, so no cap is needed to keep this bounded. + if _valid_count(parsed["__chunks__"], limit=None): + return ("legacy", parsed["__chunks__"]) + raise _keychain_corrupt("the chunk header is malformed") + return ("single", parsed) + + +def _chunk_account(tag: str | None, i: int) -> str: + # tag None is the layout released versions wrote ("vault:1", "vault:2"). + if tag is None: + return f"{VAULT_ACCOUNT}:{i}" + return f"{VAULT_ACCOUNT}:{tag}:{i}" + + +def _assemble(layout: tuple) -> dict: + """Turn a parsed header into a vault dict, reading chunks as needed.""" + if layout[0] == "single": + return _normalize_vault(layout[1]) + if layout[0] == "legacy": + tag, count, digest = None, layout[1], None + else: + _, tag, count, digest, _ = layout + pieces = [] + for i in range(1, count + 1): + # A keyring failure here (locked, access denied) raises the + # unreadable-keychain RuntimeError, same as the header read. A chunk + # that comes back None is genuinely missing: the vault is damaged. + piece = _keychain_get(_chunk_account(tag, i)) + if piece is None: + raise _keychain_corrupt(f"chunk {i} of {count} is missing") + pieces.append(piece) + payload = "".join(pieces) + if digest is not None and hashlib.sha256(payload.encode()).hexdigest() != digest: + raise _keychain_corrupt("the chunks do not match their header") + try: + parsed = json.loads(payload) + except (ValueError, TypeError): + raise _keychain_corrupt("the reassembled vault is not valid JSON") from None + if not isinstance(parsed, dict): + raise _keychain_corrupt("the reassembled vault is not a JSON object") + return _normalize_vault(parsed) -def _delete_stale_chunks(start: int) -> None: - # A previous save may have used more chunks than this one. Delete numbered - # entries from `start` upward until the first gap, so a later read can never - # reassemble a stale tail. Bounded by MAX_CHUNKS. +def _load_vault_from_keychain() -> dict: + raw = _keychain_get(VAULT_ACCOUNT) + if raw is None: + return _empty_vault() + try: + return _assemble(_parse_header(raw)) + except VaultCorruptError: + # Reads do not take the vault lock, so a save in another process may + # have switched the header and deleted the chunks this read was + # partway through. If the header moved on, read the new vault once; + # if it did not, the vault really is damaged. + fresh = _keychain_get(VAULT_ACCOUNT) + if fresh is None or fresh == raw: + raise + return _assemble(_parse_header(fresh)) + + +def _delete_chunk_run(tag: str | None) -> None: + # Delete one tag's chunk entries from 1 up to the first gap. Chunks are + # always written 1..N in order, so leftovers form an unbroken run. + # Deleting from the top down, and stopping at the first failed delete, + # keeps what is left unbroken, so a later sweep still finds all of it. + # Raises on that failure so the caller keeps the tag in the journal. + # Tagged runs are bounded by MAX_CHUNKS; released-version runs had no + # cap and end at their first gap. import keyring - for i in range(start, MAX_CHUNKS + 1): - account = f"{VAULT_ACCOUNT}:{i}" + run = [] + i = 1 + while tag is None or i <= MAX_CHUNKS: + account = _chunk_account(tag, i) if keyring.get_password(SERVICE_NAME, account) is None: break + run.append(account) + i += 1 + for account in reversed(run): + keyring.delete_password(SERVICE_NAME, account) + + +def _read_header() -> tuple[str | None, tuple | None]: + """The raw "vault" entry and its parsed layout. The layout is None when + the entry is absent or this module cannot parse it.""" + raw = _keychain_get(VAULT_ACCOUNT) + if raw is None: + return None, None + try: + return raw, _parse_header(raw) + except VaultCorruptError: + return raw, None + + +def _layout_tag(layout: tuple | None) -> str | None: + """The chunk tag a parsed header points at, if any. A single-entry vault + written by a pre-release build may carry "__gen__", the generation whose + chunks it had just replaced; those may still need sweeping.""" + if layout is None: + return None + if layout[0] == "gen": + return layout[1] + if layout[0] == "single": + gen = layout[1].get("__gen__") + if type(gen) is int and gen >= 1: + return str(gen) + return None + + +def _layout_gen(layout: tuple | None) -> int: + if layout is not None and layout[0] == "gen": + return layout[4] + return 0 + + +def _read_journal() -> list[str]: + """Tags that may still have chunk entries, oldest first. Best-effort: an + unreadable or malformed journal reads as empty. Pre-release builds + stored {tag: timestamp}; its keys are read the same way.""" + import keyring + try: + raw = keyring.get_password(SERVICE_NAME, JOURNAL_ACCOUNT) + parsed = json.loads(raw) if raw else [] + except Exception: + return [] + if isinstance(parsed, dict): + parsed = list(parsed) + if not isinstance(parsed, list): + return [] + return list(dict.fromkeys(tag for tag in parsed if _valid_tag(tag))) + + +def _write_journal(tags: list[str]) -> None: + import keyring + if not tags: try: - keyring.delete_password(SERVICE_NAME, account) + keyring.delete_password(SERVICE_NAME, JOURNAL_ACCOUNT) except Exception: pass + return + keyring.set_password(SERVICE_NAME, JOURNAL_ACCOUNT, json.dumps(tags[-MAX_JOURNAL:])) + + +def _journal_add(tags: list[str]) -> None: + journal = _read_journal() + added = [tag for tag in tags if tag not in journal] + if added: + _write_journal(journal + added) + +def _sweep_chunks() -> None: + """Best-effort removal of chunk entries that no header points at. + Runs under the vault lock after a save has switched the header. No other + save can be running, so every journal tag except the one the header + names is a leftover and its chunks are deleted, along with the + released-version "vault:i" chunks unless the header still uses them. A + tag whose chunks could not all be deleted stays in the journal for the + next save.""" + try: + raw, layout = _read_header() + if layout is None and raw is not None: + # A header this module cannot parse might still point somewhere; + # deleting nothing is the safe answer. + return + current = layout[1] if layout is not None and layout[0] == "gen" else None + if layout is None or layout[0] != "legacy": + _delete_chunk_run(None) + remaining = [] + for tag in _read_journal(): + if tag == current: + continue + try: + _delete_chunk_run(tag) + except Exception: + remaining.append(tag) + _write_journal(remaining) + except Exception: + # The new vault is already in place; leftovers cost tidiness, not + # data, so a failed cleanup must not fail the save. + pass + + +class VaultTooLargeError(RuntimeError): + """The vault is too large to store in the keychain. Nothing was written.""" + + +@_locked def _save_vault_to_keychain(vault: dict) -> None: import keyring + # json.dumps escapes non-ASCII by default, so each character is one + # UTF-16 unit and a CHUNK_LIMIT-character entry stays under the Windows cap. payload = json.dumps(vault) - if len(payload) <= CHUNK_LIMIT: + chunks = [] + if len(payload) > CHUNK_LIMIT: + chunks = [payload[i:i + CHUNK_LIMIT] for i in range(0, len(payload), CHUNK_LIMIT)] + if len(chunks) > MAX_CHUNKS: + # Checked before any write: a header claiming more chunks than the + # reader accepts would commit a vault nobody can read. + raise VaultTooLargeError( + f"The stored credentials are too large for the system keychain " + f"({len(payload)} characters; the limit is " + f"{MAX_CHUNKS * CHUNK_LIMIT}). Nothing was changed. Run " + "eufy-sync --use-file-store to keep them in a file instead." + ) + + _, start_layout = _read_header() + pending = [] + prev_tag = _layout_tag(start_layout) + if prev_tag is not None: + # Recorded before the switch, so a sweep a crash cuts short is + # finished by a later save. + pending.append(prev_tag) + if "." not in prev_tag: + # Pre-release generation layout: a crashed save there left its + # chunks at the neighbouring generation numbers. + gen = int(prev_tag) + pending.extend(str(n) for n in (gen - 1, gen + 1) if n >= 1) + + if not chunks: + if pending: + _journal_add(pending) keyring.set_password(SERVICE_NAME, VAULT_ACCOUNT, payload) - _delete_stale_chunks(1) + _sweep_chunks() return - chunks = [payload[i:i + CHUNK_LIMIT] for i in range(0, len(payload), CHUNK_LIMIT)] - # Chunks first, header last: a reader that races the write sees either the - # old vault or a complete new one, never a header pointing at a chunk that - # has not been written yet. - for i, chunk in enumerate(chunks, start=1): - keyring.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{i}", chunk) - keyring.set_password( - SERVICE_NAME, VAULT_ACCOUNT, json.dumps({"__chunks__": len(chunks)}) - ) - _delete_stale_chunks(len(chunks) + 1) + + # The new chunks go under a tag no header references, and the header + # write is the commit point. A save killed before it leaves the old vault + # readable; one killed after it leaves the new vault readable. Either way + # the journal names the leftovers for the next save to delete. + gen = _layout_gen(start_layout) + 1 + tag = f"{gen}.{secrets.token_hex(4)}" + _journal_add(pending + [tag]) + try: + for i, chunk in enumerate(chunks, start=1): + keyring.set_password(SERVICE_NAME, _chunk_account(tag, i), chunk) + except Exception: + try: + _delete_chunk_run(tag) + _write_journal([t for t in _read_journal() if t != tag]) + except Exception: + pass + raise + header = { + "__vault__": { + "gen": gen, + "tag": tag, + "chunks": len(chunks), + "sha256": hashlib.sha256(payload.encode()).hexdigest(), + } + } + keyring.set_password(SERVICE_NAME, VAULT_ACCOUNT, json.dumps(header)) + _sweep_chunks() -def _load_vault_from_file() -> dict: - if not CRED_FILE.exists(): - return _empty_vault() +@_locked +def _delete_keychain_vault() -> None: + """Best-effort removal of the vault header, every chunk entry this + module can find, and the journal.""" + import keyring + try: + _, layout = _read_header() + except Exception: + layout = None + try: + keyring.delete_password(SERVICE_NAME, VAULT_ACCOUNT) + except Exception: + pass + tags = set() + tag = _layout_tag(layout) + if tag is not None: + tags.add(tag) + if "." not in tag: + tags.update(str(g) for g in (int(tag) - 1, int(tag) + 1) if g >= 1) + try: + tags.update(_read_journal()) + except Exception: + pass + for tag in tags: + try: + _delete_chunk_run(tag) + except Exception: + pass + try: + # Released-version chunks an earlier install left behind. + _delete_chunk_run(None) + except Exception: + pass try: - return _normalize_vault(json.loads(CRED_FILE.read_text())) - except (ValueError, TypeError, OSError): - logger.warning("Credentials file contained malformed JSON; treating as empty") + keyring.delete_password(SERVICE_NAME, JOURNAL_ACCOUNT) + except Exception: + pass + + +def _load_vault_from_file() -> dict: + parsed = _read_cred_file() + if parsed is None: return _empty_vault() + return _normalize_vault(parsed, _file_corrupt) +@_locked def _save_vault_to_file(vault: dict) -> None: CRED_FILE.parent.mkdir(parents=True, exist_ok=True, mode=0o700) # Temp file + atomic rename: an interrupted in-place write would truncate # the vault, destroying the secrets and the opt-in marker (which would # silently flip the backend to an empty keychain on the next run). The - # temp name carries the pid so two concurrent writers (e.g. the 4-hourly - # Launch Agent and an interactive command) never share one temp inode and - # truncate each other's partial write before the rename. + # pid in the temp name is a second guard behind the vault lock, which + # already keeps two writers from running at once. tmp = CRED_FILE.with_name(f"{CRED_FILE.name}.{os.getpid()}.tmp") fd = os.open(str(tmp), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) try: @@ -245,12 +688,40 @@ def _save_vault_to_file(vault: dict) -> None: raise -def _load_vault() -> dict: - if _active_backend() == "file": +def _load_from(backend: str) -> dict: + if backend == "file": return _load_vault_from_file() return _load_vault_from_keychain() +def _load_vault() -> dict: + """Load the vault from the active backend. + + Reads do not take the vault lock, so a store switch in another process + can land between choosing the backend and reading it: use_keychain_store + unlinks the file this read picked, or use_file_store deletes the keychain + vault this read picked. Both switches write the new store before clearing + the old one, so in either case the old store reads as empty (or, for a + keychain half-deleted mid-read, damaged) while the new one already holds + everything. When the result is empty or damaged and the backend has + changed since it was chosen, the read is redone once on the new backend. + A populated read needs no recheck: it saw a complete vault.""" + backend = _active_backend() + try: + vault = _load_from(backend) + except VaultCorruptError: + current = _active_backend() + if current == backend: + raise + return _load_from(current) + if not vault["passwords"] and not vault["tokens"]: + current = _active_backend() + if current != backend: + return _load_from(current) + return vault + + +@_locked def _save_vault(vault: dict) -> None: if _active_backend() == "file": _save_vault_to_file(vault) @@ -261,9 +732,30 @@ def _save_vault(vault: dict) -> None: # --- Public API: passwords --------------------------------------------------- -def get_password(account: str) -> str | None: +def _legacy_get(account: str) -> str | None: + """A legacy per-item keychain value, or None when it is absent or the + keychain cannot be read.""" + try: + import keyring + return keyring.get_password(SERVICE_NAME, account) + except Exception: + return None + + +def _delete_legacy(account: str) -> None: + try: + import keyring + keyring.delete_password(SERVICE_NAME, account) + except Exception: + pass + + +def get_password(account: str, migrate: bool = True) -> str | None: """Return a stored password, migrating it from the legacy keychain item - (one account per password) into the vault on first access.""" + (one account per password) into the vault on first access. + + migrate=False still returns a legacy item but leaves it where it is, so a + caller that does not hold the sync lock never writes the vault.""" vault = _load_vault() if account in vault["passwords"]: return vault["passwords"][account] @@ -275,17 +767,29 @@ def get_password(account: str) -> str | None: except Exception: legacy = None if legacy is not None: - vault["passwords"][account] = legacy - _save_vault(vault) - try: - keyring.delete_password(SERVICE_NAME, account) - except Exception: - pass - return legacy + if not migrate: + return legacy + with vault_lock(): + # Re-read both under the lock: another process may have + # migrated, changed, or deleted this password since the reads + # above. Saving the cached legacy value after a delete_password + # finished would bring the deleted secret back. + vault = _load_vault() + if account in vault["passwords"]: + _delete_legacy(account) + return vault["passwords"][account] + legacy = _legacy_get(account) + if legacy is None: + return None + vault["passwords"][account] = legacy + _save_vault(vault) + _delete_legacy(account) + return legacy return None +@_locked def store_password(account: str, password: str) -> None: """Store a password in the vault (keychain or file, whichever is active).""" vault = _load_vault() @@ -293,6 +797,7 @@ def store_password(account: str, password: str) -> None: _save_vault(vault) +@_locked def delete_password(account: str) -> None: """Remove a password from the vault, and best-effort from the legacy keychain item if one is still lingering.""" @@ -331,17 +836,29 @@ def get_token(name: str) -> dict | None: legacy = json.loads(legacy_raw) except (json.JSONDecodeError, TypeError): return None - vault["tokens"][name] = legacy - _save_vault(vault) - try: - keyring.delete_password(SERVICE_NAME, f"token:{name}") - except Exception: - pass - return legacy + with vault_lock(): + # Re-read both under the lock, as in get_password. + account = f"token:{name}" + vault = _load_vault() + if name in vault["tokens"]: + _delete_legacy(account) + return vault["tokens"][name] + legacy_raw = _legacy_get(account) + if legacy_raw is None: + return None + try: + legacy = json.loads(legacy_raw) + except (json.JSONDecodeError, TypeError): + return None + vault["tokens"][name] = legacy + _save_vault(vault) + _delete_legacy(account) + return legacy return None +@_locked def store_token(name: str, data: dict) -> None: """Store a token dict in the vault (keychain or file, whichever is active).""" vault = _load_vault() @@ -349,6 +866,7 @@ def store_token(name: str, data: dict) -> None: _save_vault(vault) +@_locked def delete_token(name: str) -> None: """Remove a token from the vault, and best-effort from the legacy keychain item if one is still lingering.""" @@ -368,6 +886,7 @@ def delete_token(name: str) -> None: # --- Mode switching ----------------------------------------------------------- +@_locked def use_file_store() -> None: """Adopt the 0o600 file as the permanent credential store. @@ -388,6 +907,9 @@ def use_file_store() -> None: if _keyring_available(): try: keychain_vault = _load_vault_from_keychain() + except VaultCorruptError: + # Already says what is damaged and that nothing was changed. + raise except Exception as e: raise RuntimeError( "The system keychain could not be read (it may be locked or " @@ -414,22 +936,15 @@ def use_file_store() -> None: _save_vault_to_file(merged) if _keyring_available(): - try: - import keyring - keyring.delete_password(SERVICE_NAME, VAULT_ACCOUNT) - except Exception: - pass - # An oversized vault also left numbered "vault:i" entries behind, each - # holding a slice of the same plaintext secrets. Deleting only the - # header hides them from every reader but leaves full copies in the - # keychain the user just opted out of. Best-effort: the file already + # An oversized vault also has chunk entries, each holding a slice of + # the same plaintext secrets. Deleting only the header hides them from + # every reader but leaves full copies in the keychain the user just + # opted out of, so the chunks go too. Best-effort: the file already # holds the merged vault, so a failure here must not fail the switch. - try: - _delete_stale_chunks(1) - except Exception: - pass + _delete_keychain_vault() +@_locked def use_keychain_store() -> None: """Move the vault into the system keychain and stop using the file. diff --git a/eufy_sync/eufy_client.py b/eufy_sync/eufy_client.py index e82d388..df100a9 100644 --- a/eufy_sync/eufy_client.py +++ b/eufy_sync/eufy_client.py @@ -2,6 +2,7 @@ import json import logging +import re import time from dataclasses import dataclass from datetime import datetime, timezone @@ -14,6 +15,20 @@ logger = logging.getLogger(__name__) BASE_URL = "https://api.eufylife.com/v1" +LOGIN_URL = f"{BASE_URL}/user/v2/email/login" +# EufyLife 3.3.12 for Android logs in here instead. Scale data still comes +# from BASE_URL. Request shape follows m4ary/eufylife-api-hacs (MIT). +FALLBACK_LOGIN_URL = "https://home-api.eufylife.com/v1/user/v2/email/login/" +FALLBACK_APP_VERSION = "3.3.12" +DEFAULT_COUNTRY = "US" + +CLIENT_ID = "eufy-app" +CLIENT_SECRET = "8FHf22gaTKu7MZXqz5zytw" # Public app identifier from EufyLife APK, not a per-user secret +DEFAULT_TOKEN_TTL = 2592000 # 30 days + +# Error messages that mean the login endpoint itself is retired, as opposed to +# the credentials being wrong. +_ENDPOINT_RETIRED = re.compile(r"deprecat|upgrade|no longer supported|obsolete", re.I) COMMON_HEADERS = { "Accept": "*/*", @@ -87,28 +102,110 @@ def authenticate(self) -> None: self._fresh_login() def _fresh_login(self) -> None: - resp = self._client.post(f"{BASE_URL}/user/v2/email/login", json={ - "client_id": "eufy-app", - "client_secret": "8FHf22gaTKu7MZXqz5zytw", # Public app identifier from EufyLife APK, not a per-user secret - "email": self.config.email, - "password": self.config.password, - }) + data, moved_reason = self._primary_login() + host = "api.eufylife.com" + if moved_reason is not None: + logger.info("Eufy login endpoint unavailable (%s); trying home-api.eufylife.com", moved_reason) + data = self._fallback_login() + host = "home-api.eufylife.com" + + self._apply_login(data) + logger.info("Authenticated to Eufy as user %s via %s", self.user_id, host) + + def _primary_login(self) -> tuple[dict | None, str | None]: + """Log in at the original host. + + Returns (data, None) on success, or (None, reason) when the endpoint + looks moved or gone and the newer host is worth one try. Rejected + credentials and rate limits raise instead, so a bad password is never + sent twice. + """ + try: + resp = self._client.post(LOGIN_URL, json={ + "client_id": CLIENT_ID, + "client_secret": CLIENT_SECRET, + "email": self.config.email, + "password": self.config.password, + }) + except httpx.ConnectError as e: # DNS failure or connection refused + return None, type(e).__name__ + + if resp.status_code in (404, 410): + return None, f"HTTP {resp.status_code}" resp.raise_for_status() - data = resp.json() + + try: + data = resp.json() + except ValueError: + return None, "non-JSON response" + if not isinstance(data, dict): + return None, "non-JSON response" if data.get("res_code") != 1: - from eufy_sync.sync import PermanentSyncError - msg = data.get("message", "unknown error") - raise PermanentSyncError( - f"Eufy login failed: {msg}. " - "If you changed your Eufy password, run: eufy-sync --update-password" - ) + msg = str(data.get("message") or "") + if _ENDPOINT_RETIRED.search(msg): + return None, f"res_code {data.get('res_code')}" + self._raise_login_rejected(data) + return data, None + + def _fallback_login(self) -> dict: + """Log in at home-api.eufylife.com the way EufyLife 3.3.12 for Android does.""" + country = DEFAULT_COUNTRY + resp = self._client.post( + FALLBACK_LOGIN_URL, + headers={ + "User-Agent": f"EufyLife-Android-{FALLBACK_APP_VERSION}", + "Category": "Health", + "Country": country.upper(), + }, + json={ + "client_id": CLIENT_ID, + "client_Secret": CLIENT_SECRET, # the app capitalizes this key on this host + "email": self.config.email, + "password": self.config.password, + "ab": country.lower(), + "un_subscribe_flag": True, + }, + ) + resp.raise_for_status() + try: + data = resp.json() + except ValueError: + raise RuntimeError("Eufy login failed: home-api.eufylife.com returned a non-JSON response") from None + if not isinstance(data, dict): + raise RuntimeError("Eufy login failed: home-api.eufylife.com returned an unexpected response") + if data.get("res_code") != 1: + self._raise_login_rejected(data) + return data + + @staticmethod + def _raise_login_rejected(data: dict) -> None: + from eufy_sync.sync import PermanentSyncError + msg = data.get("message", "unknown error") + raise PermanentSyncError( + f"Eufy login failed: {msg}. " + "If you changed your Eufy password, run: eufy-sync --update-password" + ) + + def _apply_login(self, data: dict) -> None: + """Store the token and user id from either host's login response. + + Both hosts put access_token and user_id at the top level; a nested + `data` object is accepted too in case Eufy wraps it later. + """ + nested = data.get("data") if isinstance(data.get("data"), dict) else {} + token = data.get("access_token") or nested.get("access_token") + user_id = data.get("user_id") or nested.get("user_id") + if not token or not user_id: + raise RuntimeError("Eufy login response is missing the access token or user id") + + expires_in = data.get("expires_in", nested.get("expires_in")) + if not isinstance(expires_in, int) or expires_in <= 0: + expires_in = DEFAULT_TOKEN_TTL - self.access_token = data["access_token"] - self.user_id = data["user_id"] - expires_in = data.get("expires_in", 2592000) # default 30 days + self.access_token = str(token) + self.user_id = str(user_id) self._save_token(expires_in) - logger.info("Authenticated to Eufy as user %s", self.user_id) def token_status(self) -> dict: """Return token health without authenticating.""" diff --git a/eufy_sync/file_lock.py b/eufy_sync/file_lock.py new file mode 100644 index 0000000..b1b7b1d --- /dev/null +++ b/eufy_sync/file_lock.py @@ -0,0 +1,104 @@ +"""Cross-platform exclusive locks on a file (fcntl.flock or msvcrt.locking). + +Shared by the sync lock (eufy_sync.cli.lock) and the credential vault lock +(eufy_sync.credentials). The OS drops a lock when its handle closes or the +process dies, so a killed process never leaves a stale lock behind. + +On POSIX a lock belongs to the file, not the path. If a holder unlinks the +file (only --uninstall does), a process that opened the old file before the +unlink could lock it after the release while a third process creates and +locks a new file at the same path. acquire() rejects a lock on a file that is +no longer linked at its path and retries on the new one, so at most one +process ever holds the lock for a path. +""" +from __future__ import annotations + +import os +import sys +import time +from pathlib import Path + +if sys.platform == "win32": + import msvcrt +else: + import fcntl + +POLL_INTERVAL = 0.05 +MAX_ORPHAN_RETRIES = 5 + + +def _open(path: Path) -> int: + """Open (creating if needed) the lock file. Raises OSError if it cannot.""" + path.parent.mkdir(parents=True, exist_ok=True, mode=0o700) + return os.open(str(path), os.O_RDWR | os.O_CREAT, 0o600) + + +def try_lock(fd: int) -> bool: + """Take the exclusive lock without blocking. False when someone holds it.""" + try: + if sys.platform == "win32": + # msvcrt locks a byte range from the current position; one byte + # past EOF is fine and keeps the file empty. + os.lseek(fd, 0, os.SEEK_SET) + msvcrt.locking(fd, msvcrt.LK_NBLCK, 1) + else: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + return False + return True + + +def unlock(fd: int) -> None: + try: + if sys.platform == "win32": + os.lseek(fd, 0, os.SEEK_SET) + msvcrt.locking(fd, msvcrt.LK_UNLCK, 1) + else: + fcntl.flock(fd, fcntl.LOCK_UN) + except OSError: + # Closing the handle releases it anyway. + pass + + +def _still_linked(fd: int, path: Path) -> bool: + if sys.platform == "win32": + # Windows cannot delete a file another process has open. + return True + try: + on_disk = os.stat(path) + except OSError: + return False + held = os.fstat(fd) + return (on_disk.st_dev, on_disk.st_ino) == (held.st_dev, held.st_ino) + + +def acquire(path: Path, timeout: float = 0.0) -> int | None: + """Open `path` and take its exclusive lock, waiting up to `timeout` + seconds for another holder to let go. + + Returns the open handle holding the lock (pass it to release()), or None + if someone else still held it when the wait ran out. Raises OSError when + the lock file cannot be created or opened.""" + deadline = time.monotonic() + timeout + orphans = 0 + while True: + fd = _open(path) + if try_lock(fd): + if _still_linked(fd, path): + return fd + # The previous holder unlinked the file while holding it; this + # lock is on an orphan. Start over on whatever is at the path now. + release(fd) + orphans += 1 + if orphans <= MAX_ORPHAN_RETRIES: + continue + else: + os.close(fd) + if time.monotonic() >= deadline: + return None + time.sleep(POLL_INTERVAL) + + +def release(fd: int) -> None: + unlock(fd) + os.close(fd) diff --git a/eufy_sync/garmin_auth.py b/eufy_sync/garmin_auth.py index f9bc421..1f34202 100644 --- a/eufy_sync/garmin_auth.py +++ b/eufy_sync/garmin_auth.py @@ -302,8 +302,12 @@ def login(self, interactive: bool = True) -> Garmin: return garmin def force_reauth(self) -> Garmin: - """Clear the stored token and do a fresh interactive login.""" - self._clear_token() + """Do a fresh interactive login and store its token. + + The login runs on a new client and the stored token is replaced only + once it succeeds. A failed attempt (a cancelled MFA prompt, a wrong + password, a passing Garmin or Cloudflare error) leaves the old token + where it was, since that session may still be good.""" garmin = Garmin(self.email, self.password, prompt_mfa=_mfa_prompt) self._fresh_login(garmin) self._save_token(garmin) @@ -311,12 +315,13 @@ def force_reauth(self) -> Garmin: return garmin def silent_reauth(self) -> Garmin: - """Clear the stored token and log in again with nobody watching. + """Log in again with nobody watching and store the new token. The scheduled counterpart to force_reauth: same recovery from a session Garmin has stopped honoring, but it never prompts and never opens a - browser.""" - self._clear_token() + browser. The old token stays stored unless this login succeeds: a 403 + that only looked like a dead session would otherwise cost an MFA user + a working session and force an interactive --reauth.""" garmin = Garmin(self.email, self.password, prompt_mfa=_headless_mfa_prompt) self._silent_login(garmin) self._save_token(garmin) @@ -453,9 +458,3 @@ def _save_token(self, garmin: Garmin) -> None: store_token("garmin", blob) if self.session_path.exists(): self.session_path.unlink() - - def _clear_token(self) -> None: - from eufy_sync.credentials import delete_token - delete_token("garmin") - if self.session_path.exists(): - self.session_path.unlink() diff --git a/eufy_sync/garmin_client.py b/eufy_sync/garmin_client.py index 28a94fb..160d653 100644 --- a/eufy_sync/garmin_client.py +++ b/eufy_sync/garmin_client.py @@ -68,9 +68,15 @@ def _match_uploaded_entry(entries: list[dict], uploaded_at: datetime) -> dict | def _is_garmin_auth_failure(exc: Exception) -> bool: - """True when a Garmin call failed because the session is dead. Covers the - dedicated auth error and the 401/403 that the library reports as a generic - connection error ("API Error 401 - ...").""" + """True when a Garmin call failed because the session may be dead. Covers + the dedicated auth error and the 401/403 that the library reports as a + generic connection error ("API Error 401 - ..."). + + A Cloudflare 403 matches too: the library drops the response body before + raising, so the message is the same "API Error 403" either way. That is + safe only because a relogin keeps the stored token until the new login + succeeds, so a 403 that was only a passing block costs one login attempt, + not the session.""" if isinstance(exc, GarminConnectAuthenticationError): return True return isinstance(exc, GarminConnectConnectionError) and ( @@ -84,6 +90,15 @@ def __init__(self, config: GarminConfig): self._auth = GarminAuth(config.email, config.password) self._garmin = None self._allow_interactive = True + # At most one relogin per run, whether or not it worked. After a + # failed one, later calls raise its error again: a second attempt + # minutes later meets the same MFA demand or wrong password. After a + # successful one, a later 401/403 means the fresh session did not help + # (a Cloudflare block on the API while SSO still lets logins through), + # so the call's own error is raised. Every extra login raises the odds + # of a Garmin 429. + self._reauth_attempted = False + self._reauth_error: Exception | None = None def authenticate(self, allow_interactive: bool = True) -> None: self._allow_interactive = allow_interactive @@ -96,13 +111,21 @@ def _reauth(self) -> None: usually needs no input at all, so it tries once silently rather than ending the run on a re-auth nag the run could have fixed itself. When even that cannot proceed (MFA demanded, password wrong), the error - already names the command to run and travels to the caller unchanged.""" - if not self._allow_interactive: - logger.info("Garmin session expired; re-authenticating without prompts") - self._garmin = self._auth.silent_reauth() - else: - logger.info("Garmin session expired; re-authenticating") - self._garmin = self._auth.force_reauth() + already names the command to run and travels to the caller unchanged. + + Only one relogin is tried per run. After a failure, every later call + that needs one gets the same error back without contacting Garmin.""" + self._reauth_attempted = True + try: + if not self._allow_interactive: + logger.info("Garmin session expired; re-authenticating without prompts") + self._garmin = self._auth.silent_reauth() + else: + logger.info("Garmin session expired; re-authenticating") + self._garmin = self._auth.force_reauth() + except Exception as e: + self._reauth_error = e + raise def _call_with_reauth(self, call): """Run a Garmin call, re-logging in once when the session is dead. @@ -116,6 +139,15 @@ def _call_with_reauth(self, call): except (GarminConnectAuthenticationError, GarminConnectConnectionError) as e: if not _is_garmin_auth_failure(e): raise + if self._reauth_attempted: + if self._reauth_error is not None: + # This run already tried to log in again and failed. + # Report that failure, which names the fix, rather than + # this call's 401 or 403. + raise self._reauth_error from e + # The relogin worked and Garmin still refuses: another login + # would not change that. + raise self._reauth() return call() @@ -139,6 +171,8 @@ def read() -> bool: return self._call_with_reauth(read) except Exception as e: # Fail open: let the upload proceed; Garmin de-dupes by timestamp. + # A failed relogin is remembered, so the upload reports it with its + # fix-it hint instead of logging in a second time. logger.warning("Garmin duplicate-check failed for %s: %s", date_str, e) return False diff --git a/eufy_sync/install.py b/eufy_sync/install.py index 96700da..e0e3bd3 100644 --- a/eufy_sync/install.py +++ b/eufy_sync/install.py @@ -31,7 +31,10 @@ def install_argv(spec: str) -> list[str]: "eufy-sync[browser]") through the installer that owns this copy.""" which = installer() if which == "uv": - return ["uv", "tool", "install", "--force", spec] + # uv trusts its cached package index, which can predate a release + # published minutes ago. pip revalidates index pages on every + # install, so the pipx and pip paths need no equivalent. + return ["uv", "tool", "install", "--force", "--refresh-package", "eufy-sync", spec] if which == "pipx": return ["pipx", "install", "--force", spec] return [sys.executable, "-m", "pip", "install", "--upgrade", spec] diff --git a/eufy_sync/state.py b/eufy_sync/state.py index 1bc4b84..e5a13d4 100644 --- a/eufy_sync/state.py +++ b/eufy_sync/state.py @@ -205,12 +205,22 @@ def clear_pending_upgrade(self, user_name: str, measurement_id: str) -> None: (user_name, measurement_id), ) - def get_oldest_weight_only_timestamp(self, user_name: str, target: str) -> int | None: + def get_oldest_weight_only_timestamp( + self, user_name: str, target: str, since: int | None = None, + ) -> int | None: + """Oldest weight-only sync still waiting for its full record. With + since (unix seconds), older rows are ignored: past that age the full + record is not coming, and reaching back for it would refetch all + history since then on every run. The rows themselves stay, so a + backfill that does return the full record can still upgrade them. + Compared in Python because the stored strings mix UTC offsets.""" rows = self._conn.execute( "SELECT measurement_timestamp FROM sync_log WHERE user_name = ? AND target = ? AND weight_only = 1", (user_name, target), ) timestamps = [datetime.fromisoformat(row[0]).timestamp() for row in rows] + if since is not None: + timestamps = [ts for ts in timestamps if ts >= since] return int(min(timestamps)) if timestamps else None def get_latest_sync_timestamp(self, user_name: str, target: str | None = None) -> int | None: diff --git a/eufy_sync/strava_client.py b/eufy_sync/strava_client.py index cdd0cc8..cbf6f72 100644 --- a/eufy_sync/strava_client.py +++ b/eufy_sync/strava_client.py @@ -6,7 +6,9 @@ capture the callback code via a local HTTP server, and exchange for tokens. 2. Tokens stored in keychain (file fallback). 3. Access tokens expire after 6 hours; refresh tokens are indefinite. -4. On each sync, we PUT /api/v3/athlete with the latest weight. +4. On each sync, we PUT /athlete with the latest weight. +5. The API host follows Strava's published move to api-v3.strava.com + (see _api_bases). """ from __future__ import annotations @@ -15,6 +17,8 @@ import secrets import time import webbrowser +from collections.abc import Callable +from datetime import date, datetime, timezone from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import Path from threading import Thread @@ -28,12 +32,31 @@ STRAVA_AUTH_URL = "https://www.strava.com/oauth/authorize" STRAVA_TOKEN_URL = "https://www.strava.com/oauth/token" -STRAVA_API_BASE = "https://www.strava.com/api/v3" +STRAVA_API_BASE_OLD = "https://www.strava.com/api/v3" +STRAVA_API_BASE_NEW = "https://api-v3.strava.com" +# Strava's changelog (2026-06-01) says the new base is available from this date. +NEW_API_BASE_FROM = date(2027, 1, 4) CALLBACK_PORT = 8089 REDIRECT_URI = f"http://localhost:{CALLBACK_PORT}/callback" REFRESH_SAFETY_MARGIN = 300 # seconds before expiry to trigger refresh +def _utc_today() -> date: + return datetime.now(timezone.utc).date() + + +def _api_bases(today: date) -> tuple[str, ...]: + """API bases to try, in order, for a request made on ``today``. + + Before the new host exists we use the old one. After that we try the new + host first and keep the old one as a fallback. Strava hasn't published a + shutdown date for the old host, so the fallback has no end date either. + """ + if today < NEW_API_BASE_FROM: + return (STRAVA_API_BASE_OLD,) + return (STRAVA_API_BASE_NEW, STRAVA_API_BASE_OLD) + + def _auth_url(client_id: str, state_value: str) -> str: """The authorization URL, with every query value percent-encoded.""" query = urlencode({ @@ -163,12 +186,25 @@ def _serve_until_done(): class StravaClient: - """Syncs weight to Strava via PUT /api/v3/athlete.""" + """Syncs weight to Strava via PUT /athlete.""" - def __init__(self, config: StravaConfig): + def __init__(self, config: StravaConfig, today: Callable[[], date] = _utc_today): self.config = config self._client = httpx.Client(timeout=30.0) self._tokens: dict | None = None + self._today = today + + def _api_request(self, method: str, path: str, **kwargs) -> httpx.Response: + """Send an API request, falling back to the next base only when the + connection itself fails. Any HTTP response, including 401, 403 and + 429, comes from a live host and is returned as is.""" + bases = _api_bases(self._today()) + for base in bases[:-1]: + try: + return self._client.request(method, f"{base}{path}", **kwargs) + except (httpx.ConnectError, httpx.ConnectTimeout) as e: + logger.info("Could not reach %s (%s); trying %s", base, e, bases[-1]) + return self._client.request(method, f"{bases[-1]}{path}", **kwargs) def authenticate(self) -> None: """Load tokens and refresh if needed.""" @@ -220,7 +256,7 @@ def _refresh_access_token(self) -> None: def check_connection(self) -> None: """Verify the saved session with a read-only athlete request.""" - resp = self._client.get(f"{STRAVA_API_BASE}/athlete") + resp = self._api_request("GET", "/athlete") if resp.status_code in (401, 403): from eufy_sync.sync import PermanentSyncError raise PermanentSyncError("Strava rejected the session. Run: eufy-sync --setup-strava") @@ -232,8 +268,9 @@ def update_weight(self, weight_kg: float) -> dict: Strava only accepts current weight, without a timestamp or body composition fields. The caller selects the newest valid reading. """ - resp = self._client.put( - f"{STRAVA_API_BASE}/athlete", + resp = self._api_request( + "PUT", + "/athlete", data={"weight": round(weight_kg, 2)}, ) diff --git a/eufy_sync/sync.py b/eufy_sync/sync.py index 1adf55f..99026cd 100644 --- a/eufy_sync/sync.py +++ b/eufy_sync/sync.py @@ -24,12 +24,21 @@ # only a unique match close in both time and weight is safe to replace. UPGRADE_MAX_SECONDS = 120 UPGRADE_MAX_WEIGHT_KG = 0.1 +# How far back a run reaches for weight-only Garmin entries that still lack +# their full record. One that has not matched within two weeks never will, +# and an unbounded reach-back would refetch everything since it on every run. +UPGRADE_LOOKBACK_DAYS = 14 class PermanentSyncError(RuntimeError): """Raised for failures that retries can't fix (bad password, revoked token).""" +class UnsupportedMeasurementError(PermanentSyncError): + """A target cannot accept this one measurement (e.g. outside its weight + range). The measurement is skipped; the target itself stays healthy.""" + + def _is_permanent(exc: BaseException) -> bool: # GarminConnectTooManyRequestsError: a 429 on login/refresh won't clear by # retrying seconds later, and retrying makes the IP rate limit worse, so @@ -145,7 +154,9 @@ def sync_user(user: UserConfig, state: SyncState, backfill_days: int | None = No if ts is None: logger.info("No prior syncs to %s for %s, backfilling 7 days", name, user.name) if name == "garmin": - pending_ts = state.get_oldest_weight_only_timestamp(user.name, name) + pending_ts = state.get_oldest_weight_only_timestamp( + user.name, name, since=int(time.time()) - UPGRADE_LOOKBACK_DAYS * 86400, + ) if pending_ts is not None: # Processed data can arrive after newer weigh-ins have # advanced the cursor. Include small timestamp shifts. @@ -366,6 +377,9 @@ def sync_user(user: UserConfig, state: SyncState, backfill_days: int | None = No logger.info("Upgraded weight-only entry to full body comp for %s", m.timestamp.astimezone().date()) if target_name == "garmin" and (upgrade_row is not None or m.measurement_id in pending): state.clear_pending_upgrade(user.name, m.measurement_id) + except UnsupportedMeasurementError as e: + logger.warning("Skipping %s for %s: %s", target_name.capitalize(), user.name, e) + continue except Exception as e: logger.error("Upload to %s failed for %s: %s", target_name, user.name, e) # str(e) carries the actionable text the CLI keys its diff --git a/eufy_sync/transform.py b/eufy_sync/transform.py index afcd671..1eb0346 100644 --- a/eufy_sync/transform.py +++ b/eufy_sync/transform.py @@ -41,11 +41,13 @@ def transform(measurement: EufyMeasurement) -> GarminBodyComposition | None: return None return GarminBodyComposition( - # garminconnect's add_body_composition encodes the FIT timestamp with - # mktime(timetuple()), which reads the wall-clock fields as LOCAL - # time. measurement.timestamp is UTC-aware, so convert to local time - # first (same instant, local wall-clock fields) or the upload lands - # shifted by the machine's UTC offset. + # Before garminconnect 0.3.17 (PR #438), add_body_composition encoded + # the FIT timestamp with mktime(timetuple()), which reads the + # wall-clock fields as LOCAL time and ignores the offset. We still + # support 0.3.10+, so convert the UTC-aware timestamp to local time + # first (same instant, local wall-clock fields); otherwise older + # versions shift the upload by the machine's UTC offset. Newer + # versions handle the offset and get the same instant either way. timestamp=measurement.timestamp.astimezone().isoformat(), weight=measurement.weight_kg, percent_fat=_clamp_or_none(measurement.body_fat_pct, 3.0, 60.0), diff --git a/eufy_sync/zwift_client.py b/eufy_sync/zwift_client.py index 8c3e357..83715e6 100644 --- a/eufy_sync/zwift_client.py +++ b/eufy_sync/zwift_client.py @@ -213,12 +213,14 @@ def check_connection(self) -> None: @staticmethod def _grams(weight_kg: float) -> int: + # Retrying cannot change the weight, so these are skipped, not retried. + from eufy_sync.sync import UnsupportedMeasurementError try: value = Decimal(str(weight_kg)) except (InvalidOperation, ValueError) as exc: - raise ValueError("Zwift weight must be a finite number") from exc + raise UnsupportedMeasurementError("Zwift weight must be a finite number") from exc if not value.is_finite() or value < Decimal("30") or value > Decimal("300"): - raise ValueError("Zwift weight must be between 30 and 300 kg") + raise UnsupportedMeasurementError(f"Zwift only accepts 30 to 300 kg, got {weight_kg} kg") return int((value * 1000).quantize(Decimal("1"), rounding=ROUND_HALF_UP)) @staticmethod diff --git a/pyproject.toml b/pyproject.toml index 65fddb5..9b017b9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "eufy-sync" -version = "1.13.2" +version = "1.14.0" description = "Sync Eufy smart scale data to Garmin Connect, Strava, and Zwift" readme = "README.md" license = "MIT" @@ -45,6 +45,9 @@ dependencies = [ # several times the size of everything else, and most installs never need it. browser = ["playwright>=1.40.0"] +[dependency-groups] +dev = ["pytest", "ruff"] + [project.scripts] eufy-sync = "eufy_sync.cli:main" diff --git a/tests/test_cli.py b/tests/test_cli.py index 5887b2d..410eaaa 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1797,3 +1797,121 @@ def test_dunder_version_matches_pyproject(): with open(pyproject, "rb") as f: declared = tomllib.load(f)["project"]["version"] assert eufy_sync.__version__ == declared + + +# --- --update-password logs in before it stores anything --- + +_OLD_GARMIN_TOKEN = {"di_token": "old", "di_refresh_token": "old-r", "di_client_id": "cid"} +_OLD_ZWIFT_TOKEN = {"access_token": "old-zwift"} +_OLD_EUFY_TOKEN = {"access_token": "old-eufy", "user_id": "u", "expires_at": 9e12} + + +def _password_update_setup(tmp_path: Path) -> Path: + from eufy_sync.credentials import store_password, store_token + config_path = tmp_path / "config.yaml" + _write_config(config_path, {"users": [{ + "name": "default", + "eufy": {"email": "e@example.com"}, + "garmin": {"email": "g@example.com"}, + "zwift": {"email": "z@example.com"}, + }]}) + for service in ("eufy", "garmin", "zwift"): + store_password(f"default:{service}", f"old-{service}") + store_token("eufy", dict(_OLD_EUFY_TOKEN)) + store_token("garmin", dict(_OLD_GARMIN_TOKEN)) + store_token("zwift", dict(_OLD_ZWIFT_TOKEN)) + return config_path + + +def _stored(service: str) -> tuple: + from eufy_sync.credentials import get_password, get_token + return get_password(f"default:{service}"), get_token(service) + + +def _garmin_rejecting_login(): + from garminconnect import GarminConnectAuthenticationError + fake = MagicMock() + fake.login.side_effect = GarminConnectAuthenticationError("401 Unauthorized") + return patch("eufy_sync.garmin_auth.Garmin", return_value=fake) + + +def test_update_password_typo_keeps_garmin_password_and_session(tmp_path, capsys): + from eufy_sync.cli.maintenance import _update_password + config_path = _password_update_setup(tmp_path) + + with patch("getpass.getpass", side_effect=["", "typo", ""]), _garmin_rejecting_login(): + with pytest.raises(SystemExit) as exc: + _update_password(config_path) + + assert exc.value.code == 1 + assert _stored("garmin") == ("old-garmin", _OLD_GARMIN_TOKEN) + assert _stored("zwift") == ("old-zwift", _OLD_ZWIFT_TOKEN) + assert _stored("eufy") == ("old-eufy", _OLD_EUFY_TOKEN) + assert "left unchanged" in capsys.readouterr().out + + +def test_update_password_typo_keeps_zwift_password_and_session(tmp_path): + from eufy_sync.cli.maintenance import _update_password + from eufy_sync.sync import PermanentSyncError + config_path = _password_update_setup(tmp_path) + + zwift = MagicMock() + zwift.authenticate.side_effect = PermanentSyncError("Zwift login was rejected") + with patch("getpass.getpass", side_effect=["", "", "typo"]), \ + patch("eufy_sync.zwift_client.ZwiftClient", return_value=zwift): + with pytest.raises(SystemExit): + _update_password(config_path) + + zwift.authenticate.assert_called_once_with(force=True) + assert _stored("zwift") == ("old-zwift", _OLD_ZWIFT_TOKEN) + assert _stored("garmin") == ("old-garmin", _OLD_GARMIN_TOKEN) + + +def test_update_password_typo_keeps_eufy_password_and_session(tmp_path): + from eufy_sync.cli.maintenance import _update_password + from eufy_sync.sync import PermanentSyncError + config_path = _password_update_setup(tmp_path) + + with patch("getpass.getpass", side_effect=["typo", "", ""]), \ + patch("eufy_sync.eufy_client.EufyClient._fresh_login", + side_effect=PermanentSyncError("Eufy login failed")): + with pytest.raises(SystemExit): + _update_password(config_path) + + assert _stored("eufy") == ("old-eufy", _OLD_EUFY_TOKEN) + + +def test_update_password_stores_garmin_password_and_new_session_after_login(tmp_path): + import json + + from eufy_sync.cli.maintenance import _update_password + config_path = _password_update_setup(tmp_path) + + new_token = {"di_token": "new", "di_refresh_token": "new-r", "di_client_id": "cid"} + fake = MagicMock() + fake.client.dumps.return_value = json.dumps(new_token) + with patch("getpass.getpass", side_effect=["", "new-garmin", ""]), \ + patch("eufy_sync.garmin_auth.Garmin", return_value=fake) as ctor: + _update_password(config_path) + + # The login used the new password, not the stored one. + assert ctor.call_args.args[:2] == ("g@example.com", "new-garmin") + assert _stored("garmin") == ("new-garmin", new_token) + assert _stored("zwift") == ("old-zwift", _OLD_ZWIFT_TOKEN) + + +def test_update_password_keeps_earlier_updates_when_a_later_login_fails(tmp_path, capsys): + # Each service is all-or-nothing on its own: Eufy logged in and is stored, + # Garmin did not and is left exactly as it was. + from eufy_sync.cli.maintenance import _update_password + config_path = _password_update_setup(tmp_path) + + with patch("getpass.getpass", side_effect=["new-eufy", "typo", ""]), \ + patch("eufy_sync.eufy_client.EufyClient._fresh_login"), \ + _garmin_rejecting_login(): + with pytest.raises(SystemExit): + _update_password(config_path) + + assert _stored("eufy")[0] == "new-eufy" + assert _stored("garmin") == ("old-garmin", _OLD_GARMIN_TOKEN) + assert "Already updated: Eufy" in capsys.readouterr().out diff --git a/tests/test_credentials.py b/tests/test_credentials.py index 03fd70e..0391adf 100644 --- a/tests/test_credentials.py +++ b/tests/test_credentials.py @@ -1,12 +1,15 @@ from __future__ import annotations +import hashlib import json import os import stat +import time from unittest.mock import patch import pytest +from eufy_sync import credentials from eufy_sync.credentials import ( CHUNK_LIMIT, SERVICE_NAME, @@ -328,25 +331,47 @@ def test_auto_fallback_creates_file_and_persists_token_with_no_keychain(no_keyri # --- 7. malformed vault JSON -------------------------------------------------- -def test_malformed_file_vault_json_returns_none_not_crash(no_keyring, cred_file): - from eufy_sync.credentials import get_password, get_token +def test_malformed_file_vault_raises_and_is_never_overwritten(no_keyring, cred_file): + """Reading a damaged file as empty would let the next store save an + empty vault over it. The read raises instead, and the file is untouched.""" + from eufy_sync.credentials import VaultCorruptError, get_password, get_token cred_file.parent.mkdir(parents=True, exist_ok=True) cred_file.write_text("{not valid json::") - assert get_password("default:eufy") is None - assert get_token("eufy") is None + with pytest.raises(VaultCorruptError, match="damaged"): + get_password("default:eufy") + with pytest.raises(RuntimeError): + get_token("eufy") + with pytest.raises(RuntimeError): + store_token("eufy", {"access_token": "new"}) + + assert cred_file.read_text() == "{not valid json::" -def test_malformed_keychain_vault_json_returns_none_not_crash(fake_keyring, cred_file): +def test_non_object_file_vault_raises(no_keyring, cred_file): + from eufy_sync.credentials import VaultCorruptError + + cred_file.parent.mkdir(parents=True, exist_ok=True) + cred_file.write_text("[1, 2]") + + with pytest.raises(VaultCorruptError): + get_token("eufy") + + +def test_malformed_keychain_vault_raises_and_is_never_overwritten(fake_keyring, cred_file): import keyring - from eufy_sync.credentials import SERVICE_NAME, get_password, get_token + from eufy_sync.credentials import SERVICE_NAME, VaultCorruptError, get_password keyring.set_password(SERVICE_NAME, "vault", "{not valid json::") - assert get_password("default:eufy") is None - assert get_token("eufy") is None + with pytest.raises(VaultCorruptError, match="damaged"): + get_password("default:eufy") + with pytest.raises(VaultCorruptError): + store_token("garmin", {"di_token": "new"}) + + assert fake_keyring.get_password(SERVICE_NAME, "vault") == "{not valid json::" # --- 8. backward compat: legacy config.yaml inline password ----------------- @@ -491,26 +516,84 @@ def test_marked_file_with_keyring_stays_file(fake_keyring, cred_file): assert get_password("default:eufy") == "file-pw" -def test_malformed_file_counts_as_unmarked(fake_keyring, cred_file): - from eufy_sync.credentials import _active_backend +@pytest.mark.parametrize("content", [b"{not valid json::", b"\x80\x81\xfe\xff", b"[1, 2]"]) +def test_damaged_file_raises_instead_of_counting_as_unmarked(fake_keyring, cred_file, content): + """A damaged file may be an explicit file store whose marker can no + longer be read. Picking the keychain would hide every secret in it, so + backend selection raises the corruption error instead. (Non-UTF-8 bytes + make read_text() raise UnicodeDecodeError, a ValueError.)""" + from eufy_sync.credentials import VaultCorruptError, _active_backend, get_token cred_file.parent.mkdir(parents=True, exist_ok=True) - cred_file.write_text("{not valid json::") + cred_file.write_bytes(content) - assert _active_backend() == "keychain" + with pytest.raises(VaultCorruptError, match="damaged"): + _active_backend() + with pytest.raises(VaultCorruptError): + get_token("garmin") + with pytest.raises(VaultCorruptError): + store_token("garmin", {"a": 1}) + assert cred_file.read_bytes() == content -def test_non_utf8_file_counts_as_unmarked(fake_keyring, cred_file): - """read_text() raises UnicodeDecodeError (a ValueError) on non-UTF-8 - bytes; that must count as no marker, not crash every backend lookup.""" +def test_unreadable_file_raises_instead_of_counting_as_unmarked(fake_keyring, cred_file, monkeypatch): from eufy_sync.credentials import _active_backend cred_file.parent.mkdir(parents=True, exist_ok=True) - cred_file.write_bytes(b"\x80\x81\xfe\xff") + cred_file.write_text(json.dumps({"explicit": True, "passwords": {}, "tokens": {}})) + + def denied(self, *args, **kwargs): + raise PermissionError(13, "Permission denied") + monkeypatch.setattr(type(cred_file), "read_text", denied) + with pytest.raises(RuntimeError, match="could not be read"): + _active_backend() + + +def test_parsed_unmarked_file_is_still_ignored(fake_keyring, cred_file): + from eufy_sync.credentials import _active_backend + + cred_file.parent.mkdir(parents=True, exist_ok=True) + cred_file.write_text(json.dumps({"passwords": {"a": "b"}, "tokens": {}})) assert _active_backend() == "keychain" +@pytest.mark.parametrize("section", ["passwords", "tokens"]) +@pytest.mark.parametrize("bad", [[], "x", None, 3]) +def test_wrong_type_section_in_file_raises_and_is_not_overwritten(no_keyring, cred_file, section, bad): + from eufy_sync.credentials import VaultCorruptError, store_password + + cred_file.parent.mkdir(parents=True, exist_ok=True) + original = json.dumps({"explicit": True, "passwords": {}, "tokens": {}, section: bad}) + cred_file.write_text(original) + + with pytest.raises(VaultCorruptError, match=section): + store_password("default:eufy", "pw") + assert cred_file.read_text() == original + + +@pytest.mark.parametrize("section", ["passwords", "tokens"]) +def test_wrong_type_section_in_keychain_raises_and_is_not_overwritten(fake_keyring, section): + from eufy_sync.credentials import VaultCorruptError, store_password + + original = json.dumps({"passwords": {}, "tokens": {}, section: ["recoverable"]}) + fake_keyring.set_password(SERVICE_NAME, VAULT_ACCOUNT, original) + + with pytest.raises(VaultCorruptError, match=section): + store_password("default:eufy", "pw") + assert fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT) == original + + +def test_absent_sections_still_read_as_empty(fake_keyring): + from eufy_sync.credentials import get_password, store_password + + fake_keyring.set_password(SERVICE_NAME, VAULT_ACCOUNT, json.dumps({"tokens": {"a": {"b": 1}}})) + assert get_password("default:eufy") is None + store_password("default:eufy", "pw") + assert get_password("default:eufy") == "pw" + assert get_token("a") == {"b": 1} + + # --- 11. explicit opt-in: use_file_store merge + marker ---------------------- @@ -771,31 +854,534 @@ def test_small_vault_keeps_single_entry_shape(fake_keyring): assert fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:1") is None +def _vault_accounts(store: _FakeKeyringStore) -> set[str]: + """Every vault-related entry currently stored (header and chunks).""" + return { + account for account in store.accounts_written() + if account == VAULT_ACCOUNT or account.startswith(f"{VAULT_ACCOUNT}:") + } + + +def _header(store: _FakeKeyringStore) -> dict: + return json.loads(store.get_password(SERVICE_NAME, VAULT_ACCOUNT))["__vault__"] + + +def _chunk_accounts(tag: str, count: int) -> set[str]: + return {f"{VAULT_ACCOUNT}:{tag}:{i}" for i in range(1, count + 1)} + + def test_oversized_vault_chunks_and_round_trips(fake_keyring): store_token("garmin", _big_token(3 * CHUNK_LIMIT)) - header = json.loads(fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT)) - n = header["__chunks__"] + header = _header(fake_keyring) + n = header["chunks"] assert n >= 3 - for i in range(1, n + 1): - chunk = fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{i}") + for account in _chunk_accounts(header["tag"], n): + chunk = fake_keyring.get_password(SERVICE_NAME, account) assert chunk is not None assert len(chunk) <= CHUNK_LIMIT + # Windows caps one entry at 2,560 bytes of UTF-16. + assert len(chunk.encode("utf-16-le")) <= 2560 assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + assert _vault_accounts(fake_keyring) == {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], n) + + +def test_non_ascii_vault_respects_the_windows_entry_cap(fake_keyring): + """json.dumps escapes non-ASCII, so a vault full of multi-byte characters + still splits into entries under the per-entry byte cap.""" + store_token("garmin", {"access_token": "é\U0001f600" * CHUNK_LIMIT}) + header = _header(fake_keyring) + for account in _chunk_accounts(header["tag"], header["chunks"]): + assert len(fake_keyring.get_password(SERVICE_NAME, account).encode("utf-16-le")) <= 2560 + assert get_token("garmin") == {"access_token": "é\U0001f600" * CHUNK_LIMIT} + + +def test_each_save_uses_a_new_generation_and_deletes_the_old_one(fake_keyring): + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + first = _header(fake_keyring) + store_token("garmin", _big_token(4 * CHUNK_LIMIT)) + second = _header(fake_keyring) + + assert second["gen"] == first["gen"] + 1 + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(second["tag"], second["chunks"]) + ) + assert get_token("garmin") == _big_token(4 * CHUNK_LIMIT) def test_shrinking_vault_deletes_stale_chunks(fake_keyring): store_token("garmin", _big_token(3 * CHUNK_LIMIT)) store_token("garmin", {"a": 1}) # replaces the big token; vault fits again assert get_token("garmin") == {"a": 1} - raw = fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT) - assert "__chunks__" not in json.loads(raw) - assert fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:1") is None + raw = json.loads(fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT)) + assert "__vault__" not in raw + assert raw["tokens"]["garmin"] == {"a": 1} + assert _vault_accounts(fake_keyring) == {VAULT_ACCOUNT} -def test_missing_chunk_reads_as_empty_vault(fake_keyring, caplog): +def test_missing_chunk_raises_and_is_never_overwritten(fake_keyring, cred_file): + """A header whose chunk is gone is a damaged vault. Reading it as empty + would let the next store_token save a vault holding only that token over + every stored password.""" + from eufy_sync.credentials import VaultCorruptError, store_password + + store_password("default:eufy", "pw") store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + tag = _header(fake_keyring)["tag"] + fake_keyring.delete_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{tag}:2") + before = dict(fake_keyring.data) + + with pytest.raises(VaultCorruptError, match="chunk 2 of"): + get_token("garmin") + with pytest.raises(VaultCorruptError): + store_token("strava", {"access_token": "t"}) + + assert fake_keyring.data == before + + +def test_chunks_that_do_not_match_the_header_raise(fake_keyring): + from eufy_sync.credentials import VaultCorruptError + + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + tag = _header(fake_keyring)["tag"] + account = f"{VAULT_ACCOUNT}:{tag}:1" + chunk = fake_keyring.get_password(SERVICE_NAME, account) + fake_keyring.set_password(SERVICE_NAME, account, chunk.replace("x", "y")) + + with pytest.raises(VaultCorruptError, match="do not match"): + get_token("garmin") + + +@pytest.mark.parametrize("header", [ + {"__chunks__": 0}, + {"__chunks__": True}, + {"__chunks__": 10_000}, + {"__vault__": {"gen": 1, "chunks": 2}}, + {"__vault__": "nope"}, +]) +def test_malformed_chunk_header_raises(fake_keyring, header): + from eufy_sync.credentials import VaultCorruptError + + fake_keyring.set_password(SERVICE_NAME, VAULT_ACCOUNT, json.dumps(header)) + with pytest.raises(VaultCorruptError): + get_token("garmin") + + +def test_absent_vault_reads_as_empty(fake_keyring): + assert get_token("garmin") is None + assert _vault_accounts(fake_keyring) == set() + + +# --- Crash safety ------------------------------------------------------------ +# +# A save is a sequence of keyring writes and deletes. Killing the process +# between any two of them must leave a vault that reads back as either the old +# contents or the new ones, and the next completed save must clear whatever +# the killed one left behind. + + +class _Killed(BaseException): + """Simulates the process dying: BaseException, so no `except Exception` + cleanup path inside the module can swallow it.""" + + +def _kill_after(monkeypatch, store: _FakeKeyringStore, steps: int) -> None: + """Let `steps` backend mutations through, then kill on the next one.""" + calls = {"n": 0} + + def tick(): + calls["n"] += 1 + if calls["n"] > steps: + raise _Killed() + + def set_password(service, account, password): + tick() + store.set_password(service, account, password) + + def delete_password(service, account): + tick() + store.delete_password(service, account) + + monkeypatch.setattr("keyring.set_password", set_password) + monkeypatch.setattr("keyring.delete_password", delete_password) + + +def _restore(monkeypatch, store: _FakeKeyringStore) -> None: + monkeypatch.setattr("keyring.set_password", store.set_password) + monkeypatch.setattr("keyring.delete_password", store.delete_password) + + +def _count_mutations(monkeypatch, store, action) -> int: + snapshot = dict(store.data) + counted = {"n": 0} + + def set_password(service, account, password): + counted["n"] += 1 + store.set_password(service, account, password) + + def delete_password(service, account): + counted["n"] += 1 + store.delete_password(service, account) + + monkeypatch.setattr("keyring.set_password", set_password) + monkeypatch.setattr("keyring.delete_password", delete_password) + action() + _restore(monkeypatch, store) + store.data = snapshot + return counted["n"] + + +_OLD_PW = {"default:eufy": "pw1", "default:garmin": "pw2"} + +# (description, token before, token after): big -> big, small -> big, +# big -> small, and big -> big starting from a vault already on a generation. +_TRANSITIONS = [ + ("big_to_big", _big_token(3 * CHUNK_LIMIT), _big_token(5 * CHUNK_LIMIT)), + ("big_to_smaller_big", _big_token(5 * CHUNK_LIMIT), _big_token(2 * CHUNK_LIMIT)), + ("small_to_big", {"a": 1}, _big_token(3 * CHUNK_LIMIT)), + ("big_to_small", _big_token(3 * CHUNK_LIMIT), {"a": 1}), +] + + +@pytest.mark.parametrize("name,old,new", _TRANSITIONS, ids=[t[0] for t in _TRANSITIONS]) +@pytest.mark.parametrize("presaves", [1, 2, "legacy"]) +def test_kill_at_every_step_of_a_save_leaves_a_readable_vault( + fake_keyring, cred_file, monkeypatch, name, old, new, presaves +): + """presaves "legacy" starts from the released "vault:i" chunk layout, so + the killed save is the one migrating it.""" + from eufy_sync.credentials import _load_vault, store_password + + def setup(): + fake_keyring.data.clear() + if presaves == "legacy": + _write_legacy_chunked( + fake_keyring, {"passwords": dict(_OLD_PW), "tokens": {"garmin": old}} + ) + return + for account, pw in _OLD_PW.items(): + store_password(account, pw) + for _ in range(presaves): + store_token("garmin", old) + + setup() + total = _count_mutations(monkeypatch, fake_keyring, lambda: store_token("garmin", new)) + assert total >= 2 + + for steps in range(total): + setup() + _kill_after(monkeypatch, fake_keyring, steps) + with pytest.raises(_Killed): + store_token("garmin", new) + _restore(monkeypatch, fake_keyring) + + vault = _load_vault() + assert vault["passwords"] == _OLD_PW, f"killed after {steps} of {total}" + assert vault["tokens"]["garmin"] in (old, new), f"killed after {steps} of {total}" + + # The next completed save leaves only the entries its header uses. + # Saves are serialized by the vault lock, so the killed save's + # chunks cannot belong to a save still running and go at once. + store_token("strava", {"access_token": "t"}) + vault = _load_vault() + assert vault["passwords"] == _OLD_PW + assert vault["tokens"]["strava"] == {"access_token": "t"} + raw = json.loads(fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT)) + expected = {VAULT_ACCOUNT} + if "__vault__" in raw: + expected |= _chunk_accounts(raw["__vault__"]["tag"], raw["__vault__"]["chunks"]) + assert _vault_accounts(fake_keyring) == expected, f"killed after {steps} of {total}" + + +def test_reader_retries_once_when_a_save_switches_the_header_mid_read(fake_keyring, monkeypatch): + """A save in another process can switch the header and delete the old + chunks while this process is reading them. The reader sees a missing + chunk, notices the header moved on, and reads the new vault.""" + from eufy_sync import credentials + + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + old_tag = _header(fake_keyring)["tag"] + real_get = fake_keyring.get_password + state = {"saved": False} + + def racing_get(service, account): + if account == f"{VAULT_ACCOUNT}:{old_tag}:2" and not state["saved"]: + state["saved"] = True + monkeypatch.setattr("keyring.get_password", real_get) + credentials._save_vault_to_keychain( + {"passwords": {}, "tokens": {"garmin": _big_token(4 * CHUNK_LIMIT)}} + ) + monkeypatch.setattr("keyring.get_password", racing_get) + return real_get(service, account) + + monkeypatch.setattr("keyring.get_password", racing_get) + assert get_token("garmin") == _big_token(4 * CHUNK_LIMIT) + + +# --- Upgrade from the released chunk layout ---------------------------------- + + +def _write_legacy_chunked(store: _FakeKeyringStore, vault: dict) -> int: + """Store `vault` exactly as released versions did: "vault:1".."vault:N" + under a {"__chunks__": N} header.""" + payload = json.dumps(vault) + chunks = [payload[i:i + CHUNK_LIMIT] for i in range(0, len(payload), CHUNK_LIMIT)] + for i, chunk in enumerate(chunks, start=1): + store.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{i}", chunk) + store.set_password(SERVICE_NAME, VAULT_ACCOUNT, json.dumps({"__chunks__": len(chunks)})) + return len(chunks) + + +def test_legacy_chunked_vault_reads_and_migrates_on_next_save(fake_keyring, cred_file): + from eufy_sync.credentials import get_password + + legacy = { + "passwords": {"default:eufy": "pw"}, + "tokens": {"garmin": _big_token(3 * CHUNK_LIMIT)}, + } + n = _write_legacy_chunked(fake_keyring, legacy) + assert n >= 3 + + assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + assert get_password("default:eufy") == "pw" + + store_token("strava", {"access_token": "t"}) + + header = _header(fake_keyring) + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], header["chunks"]) + ) + assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + assert get_token("strava") == {"access_token": "t"} + assert get_password("default:eufy") == "pw" + + +def test_legacy_chunked_vault_with_missing_chunk_raises(fake_keyring): + from eufy_sync.credentials import VaultCorruptError + + _write_legacy_chunked(fake_keyring, {"passwords": {}, "tokens": {"garmin": _big_token(3 * CHUNK_LIMIT)}}) fake_keyring.delete_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:2") - assert get_token("garmin") is None # malformed vault treated as empty + with pytest.raises(VaultCorruptError): + get_token("garmin") + + +def test_legacy_single_entry_vault_reads_and_stays_single(fake_keyring, cred_file): + from eufy_sync.credentials import get_password + + fake_keyring.set_password( + SERVICE_NAME, VAULT_ACCOUNT, + json.dumps({"passwords": {"default:eufy": "pw"}, "tokens": {"garmin": {"a": 1}}}), + ) + assert get_password("default:eufy") == "pw" + store_token("strava", {"b": 2}) + assert get_token("garmin") == {"a": 1} + assert get_token("strava") == {"b": 2} + assert _vault_accounts(fake_keyring) == {VAULT_ACCOUNT} + + +def test_prerelease_generation_leftovers_are_cleaned_up(fake_keyring): + """Pre-release builds named chunks by generation alone ("vault::") + and left crash leftovers at the neighbouring generations. A vault still + on that layout reads, and the first save deletes its chunks and both + neighbours.""" + vault = {"passwords": {"default:eufy": "pw"}, "tokens": {"garmin": _big_token(3 * CHUNK_LIMIT)}} + payload = json.dumps(vault) + chunks = [payload[i:i + CHUNK_LIMIT] for i in range(0, len(payload), CHUNK_LIMIT)] + for i, chunk in enumerate(chunks, start=1): + fake_keyring.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:5:{i}", chunk) + fake_keyring.set_password(SERVICE_NAME, VAULT_ACCOUNT, json.dumps({"__vault__": { + "gen": 5, "chunks": len(chunks), + "sha256": hashlib.sha256(payload.encode()).hexdigest(), + }})) + for stale in (4, 6): + for i in (1, 2, 3): + fake_keyring.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{stale}:{i}", "junk") + + assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + store_token("garmin", _big_token(2 * CHUNK_LIMIT)) + + header = _header(fake_keyring) + assert header["gen"] == 6 and header["tag"].startswith("6.") + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], header["chunks"]) + ) + assert get_token("garmin") == _big_token(2 * CHUNK_LIMIT) + + +def _kill_on_header_write(monkeypatch, store: _FakeKeyringStore) -> None: + """Let a save write its chunks, then kill it as it switches the header.""" + def set_password(service, account, password): + if account == VAULT_ACCOUNT: + raise _Killed() + store.set_password(service, account, password) + + monkeypatch.setattr("keyring.set_password", set_password) + + +def test_consecutive_interrupted_saves_leave_no_orphaned_chunks(fake_keyring, monkeypatch): + """Each killed save leaves a full set of chunks (plaintext slices of the + vault) under a tag no header names. However many pile up, the next + completed save deletes all of them, along with released-version + "vault:i" chunks.""" + credentials.store_password("default:eufy", "pw") + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + for i in (1, 2): + fake_keyring.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{i}", "legacy-junk") + + for n in range(4): + _kill_on_header_write(monkeypatch, fake_keyring) + with pytest.raises(_Killed): + store_token("garmin", _big_token((3 + n) * CHUNK_LIMIT)) + _restore(monkeypatch, fake_keyring) + assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + orphans = _vault_accounts(fake_keyring) - {VAULT_ACCOUNT, credentials.JOURNAL_ACCOUNT} + assert len(orphans) > 4 * 3 + + store_token("strava", {"access_token": "t"}) + + header = _header(fake_keyring) + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], header["chunks"]) + ) + assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + + +def test_crash_leftovers_are_deleted_by_the_very_next_save(fake_keyring, monkeypatch): + """Under the vault lock no other save can be running, so a journal tag + the header does not name is a crash leftover with nothing to wait for.""" + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + _kill_on_header_write(monkeypatch, fake_keyring) + with pytest.raises(_Killed): + store_token("garmin", _big_token(4 * CHUNK_LIMIT)) + _restore(monkeypatch, fake_keyring) + pending = set(json.loads(fake_keyring.get_password(SERVICE_NAME, credentials.JOURNAL_ACCOUNT))) + live = _header(fake_keyring)["tag"] + other = (pending - {live}).pop() + assert fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{other}:1") is not None + + store_token("strava", {"access_token": "t"}) + + assert fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{other}:1") is None + assert fake_keyring.get_password(SERVICE_NAME, credentials.JOURNAL_ACCOUNT) is None + + +def test_prerelease_timestamped_journal_is_still_swept(fake_keyring): + """Pre-release builds stored the journal as {tag: timestamp}.""" + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + for i in (1, 2): + fake_keyring.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:9.0000abcd:{i}", "junk") + fake_keyring.set_password( + SERVICE_NAME, credentials.JOURNAL_ACCOUNT, json.dumps({"9.0000abcd": time.time()}) + ) + + store_token("strava", {"access_token": "t"}) + + header = _header(fake_keyring) + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], header["chunks"]) + ) + + +def test_switching_to_the_file_store_deletes_every_known_chunk(fake_keyring, cred_file, monkeypatch): + """--use-file-store must not leave plaintext slices of the vault in the + keychain the user just opted out of: the live chunks, journal-tracked + leftovers of any age, and released-version "vault:i" chunks all go.""" + from eufy_sync.credentials import _load_vault, use_file_store + + credentials.store_password("default:eufy", "pw") + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + fake_keyring.set_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:1", "legacy-junk") + for n in range(2): + _kill_on_header_write(monkeypatch, fake_keyring) + with pytest.raises(_Killed): + store_token("garmin", _big_token((4 + n) * CHUNK_LIMIT)) + _restore(monkeypatch, fake_keyring) + + use_file_store() + + assert _vault_accounts(fake_keyring) == set() + assert _load_vault()["tokens"]["garmin"] == _big_token(3 * CHUNK_LIMIT) + + +def test_a_save_inside_another_save_waits_for_the_lock(fake_keyring, monkeypatch): + """The review's race, replayed: writer B has loaded the vault, and writer + A tries to save completely before B's header write lands. With the vault + lock A cannot start until B finishes, so A works on B's result and both + updates survive. A runs in a thread, standing in for another process.""" + import threading + + from eufy_sync.credentials import _load_vault_from_keychain, store_password + + store_token("garmin", {"blob": "o" * 3000}) + store_token("garmin", {"blob": "o" * 3000}) + real_set = fake_keyring.set_password + started = threading.Event() + other = {} + + def writer_a(): + started.set() + store_token("garmin", {"blob": "a" * 2000}) + + def racing_set(service, account, password): + if account == VAULT_ACCOUNT and "thread" not in other: + other["thread"] = threading.Thread(target=writer_a) + other["thread"].start() + started.wait() + # Give A every chance to run if it were not blocked. + other["thread"].join(timeout=0.3) + assert other["thread"].is_alive(), "writer A ran while B held the vault lock" + real_set(service, account, password) + + monkeypatch.setattr("keyring.set_password", racing_set) + store_password("u:garmin", "x") + other["thread"].join(timeout=10) + assert not other["thread"].is_alive() + + after = _load_vault_from_keychain() + assert after["passwords"]["u:garmin"] == "x" + assert after["tokens"]["garmin"] == {"blob": "a" * 2000} + header = _header(fake_keyring) + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], header["chunks"]) + ) + + +def test_concurrent_threads_storing_different_tokens_all_persist(fake_keyring): + import threading + + names = [f"service{i}" for i in range(8)] + errors = [] + + def store(name): + try: + for round_ in range(5): + store_token(name, {"blob": name * 400, "round": round_}) + except Exception as e: # pragma: no cover - reported below + errors.append(e) + + threads = [threading.Thread(target=store, args=(name,)) for name in names] + for t in threads: + t.start() + for t in threads: + t.join(timeout=30) + assert not errors + for name in names: + assert get_token(name) == {"blob": name * 400, "round": 4} + header = _header(fake_keyring) + assert _vault_accounts(fake_keyring) == ( + {VAULT_ACCOUNT} | _chunk_accounts(header["tag"], header["chunks"]) + ) + + +def test_shared_service_name_and_legacy_token_item_are_untouched(fake_keyring, cred_file): + """Another tool reads the legacy "token:garmin" item under the same + service name. Vault saves, chunked or not, must never touch it.""" + from eufy_sync.credentials import SERVICE_NAME as service + + assert service == "eufy-garmin-sync" + fake_keyring.set_password(SERVICE_NAME, "token:garmin", json.dumps({"x": 1})) + store_token("strava", _big_token(3 * CHUNK_LIMIT)) + store_token("strava", {"a": 1}) + assert fake_keyring.get_password(SERVICE_NAME, "token:garmin") == json.dumps({"x": 1}) def test_chunk_read_failure_raises_friendly_error(fake_keyring, cred_file, monkeypatch): @@ -829,15 +1415,12 @@ def test_use_file_store_deletes_chunk_entries_not_just_the_header(fake_keyring, from eufy_sync.credentials import _active_backend, get_token, use_file_store store_token("garmin", _big_token(3 * CHUNK_LIMIT)) - header = json.loads(fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT)) - n = header["__chunks__"] - assert n >= 3 + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) # move past generation 1 + assert _header(fake_keyring)["chunks"] >= 3 use_file_store() - assert fake_keyring.get_password(SERVICE_NAME, VAULT_ACCOUNT) is None - for i in range(1, n + 1): - assert fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:{i}") is None + assert _vault_accounts(fake_keyring) == set() on_disk = json.loads(cred_file.read_text()) assert on_disk["explicit"] is True @@ -887,3 +1470,275 @@ def payload_len(n: int) -> int: assert "__chunks__" not in data assert fake_keyring.get_password(SERVICE_NAME, f"{VAULT_ACCOUNT}:1") is None assert get_token("garmin") == {"access_token": "x" * n} + + +def test_doctor_fails_the_keychain_line_for_a_damaged_credentials_file(fake_keyring, cred_file): + from eufy_sync.cli import doctor + + cred_file.parent.mkdir(parents=True, exist_ok=True) + cred_file.write_text("{not json") + lines: list[tuple] = [] + doctor._check_keychain(lambda status, label, detail, fix=None: lines.append((status, detail))) + assert lines[0][0] == "FAIL" + assert "damaged" in lines[0][1] + + +# --- Vault lock ---------------------------------------------------------------- + +_FAKE_KEYRING_PRELUDE = """ +import sys, types +# No real keychain in the child: a stub module that is never used. +sys.modules["keyring"] = types.ModuleType("keyring") +from pathlib import Path +from eufy_sync import credentials +credentials.CRED_FILE = Path(sys.argv[1]) +credentials._keyring_available = lambda: False +""" + + +def _run_child(script: str, *args: str, timeout: float = 60): + import subprocess + import sys + return subprocess.Popen( + [sys.executable, "-c", _FAKE_KEYRING_PRELUDE + script, *args], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + ) + + +def test_concurrent_processes_storing_different_tokens_all_persist(no_keyring, cred_file): + """Separate processes share only the OS lock. Without it, two + read-modify-writes of the same file interleave and one token is lost.""" + script = """ +name = sys.argv[2] +for round_ in range(15): + credentials.store_token(name, {"blob": name * 200, "round": round_}) +""" + names = [f"proc{i}" for i in range(4)] + children = [_run_child(script, str(cred_file), name) for name in names] + for child in children: + _, err = child.communicate(timeout=60) + assert child.returncode == 0, err + for name in names: + assert get_token(name) == {"blob": name * 200, "round": 14} + + +def test_another_process_cannot_write_while_the_lock_is_held(no_keyring, cred_file): + from eufy_sync.credentials import vault_lock + + script = """ +credentials.VAULT_LOCK_TIMEOUT = 0.3 +try: + credentials.store_token("other", {"a": 1}) +except credentials.VaultLockError as e: + print("refused:", e) +""" + with vault_lock(): + child = _run_child(script, str(cred_file)) + out, err = child.communicate(timeout=60) + assert child.returncode == 0, err + assert "refused:" in out and "Retry" in out + assert not cred_file.exists() + + +def test_vault_lock_is_reentrant_and_released(no_keyring, cred_file): + from eufy_sync import file_lock + from eufy_sync.credentials import store_password, vault_lock, vault_lock_path + + with vault_lock(): + with vault_lock(): + store_password("default:eufy", "pw") + # Still held by this process after the inner block. + fd = file_lock.acquire(vault_lock_path()) + assert fd is None + fd = file_lock.acquire(vault_lock_path()) + assert fd is not None + file_lock.release(fd) + assert vault_lock_path().exists() + + +def test_store_refuses_when_the_lock_file_cannot_open(fake_keyring, cred_file): + from eufy_sync.credentials import VaultLockError, store_password + + with patch("eufy_sync.file_lock.os.open", side_effect=OSError(30, "Read-only file system")): + with pytest.raises(VaultLockError, match="could not be created"): + store_password("default:eufy", "pw") + assert _vault_accounts(fake_keyring) == set() + + +def test_migration_runs_under_the_lock(fake_keyring, cred_file): + from eufy_sync import credentials as creds + + fake_keyring.set_password(SERVICE_NAME, "default:eufy", "legacy-pw") + seen = [] + real_save = creds._save_vault_to_keychain.__wrapped__ + + def spy(vault): + seen.append(creds._lock_depth) + real_save(vault) + + with patch.object(creds, "_save_vault_to_keychain", spy): + assert creds.get_password("default:eufy") == "legacy-pw" + assert seen and all(depth >= 1 for depth in seen) + assert fake_keyring.get_password(SERVICE_NAME, "default:eufy") is None + + +def _before_first_lock(monkeypatch, action): + """Run action once, just before the first vault_lock() entry: the window + after an unlocked read where another process can finish a write.""" + import contextlib + + from eufy_sync import credentials as creds + + real = creds.vault_lock + pending = [action] + + @contextlib.contextmanager + def hooked(): + if pending: + pending.pop()() + with real(): + yield + + monkeypatch.setattr(creds, "vault_lock", hooked) + + +def test_password_migration_does_not_undo_a_concurrent_delete(fake_keyring, cred_file, monkeypatch): + """get_password reads the legacy item before locking. If a delete_password + finishes in that window, the migration must not save the cached value.""" + from eufy_sync.credentials import get_password + + fake_keyring.set_password(SERVICE_NAME, "default:eufy", "legacy-pw") + _before_first_lock(monkeypatch, lambda: fake_keyring.delete_password(SERVICE_NAME, "default:eufy")) + + assert get_password("default:eufy") is None + assert credentials._load_vault()["passwords"] == {} + assert fake_keyring.get_password(SERVICE_NAME, "default:eufy") is None + + +def test_token_migration_does_not_undo_a_concurrent_delete(fake_keyring, cred_file, monkeypatch): + fake_keyring.set_password(SERVICE_NAME, "token:strava", json.dumps({"t": 1})) + _before_first_lock(monkeypatch, lambda: fake_keyring.delete_password(SERVICE_NAME, "token:strava")) + + assert get_token("strava") is None + assert credentials._load_vault()["tokens"] == {} + assert fake_keyring.get_password(SERVICE_NAME, "token:strava") is None + + +def test_migration_saves_the_legacy_value_read_under_the_lock(fake_keyring, cred_file, monkeypatch): + """A legacy item rewritten in the window is migrated with its new value.""" + from eufy_sync.credentials import get_password + + fake_keyring.set_password(SERVICE_NAME, "default:eufy", "old") + _before_first_lock(monkeypatch, lambda: fake_keyring.set_password(SERVICE_NAME, "default:eufy", "new")) + + assert get_password("default:eufy") == "new" + assert credentials._load_vault()["passwords"] == {"default:eufy": "new"} + assert fake_keyring.get_password(SERVICE_NAME, "default:eufy") is None + + +def _switch_during_first_read(monkeypatch, backend, switch): + """Run switch (a store change "in another process") after the read has + chosen backend but before it loads it.""" + from eufy_sync import credentials as creds + + real = creds._load_from + pending = [switch] + + def hooked(chosen): + if pending and chosen == backend: + pending.pop()() + return real(chosen) + + monkeypatch.setattr(creds, "_load_from", hooked) + + +def test_read_that_chose_the_file_survives_a_switch_to_the_keychain(fake_keyring, cred_file, monkeypatch): + """use_keychain_store unlinks the file the read picked; the credentials + are in the keychain by then and must not read as missing.""" + from eufy_sync.credentials import get_password, use_keychain_store + + cred_file.parent.mkdir(parents=True, exist_ok=True) + cred_file.write_text(json.dumps({"explicit": True, "passwords": {"default:eufy": "pw"}, "tokens": {}})) + _switch_during_first_read(monkeypatch, "file", use_keychain_store) + + assert get_password("default:eufy", migrate=False) == "pw" + assert not cred_file.exists() + + +def test_read_that_chose_the_keychain_survives_a_switch_to_the_file(fake_keyring, cred_file, monkeypatch): + from eufy_sync.credentials import store_token, use_file_store + + store_token("garmin", {"t": 1}) + _switch_during_first_read(monkeypatch, "keychain", use_file_store) + + assert get_token("garmin") == {"t": 1} + assert _vault_accounts(fake_keyring) == set() + + +def test_empty_vault_with_no_switch_reads_the_backend_once(fake_keyring, cred_file, monkeypatch): + from eufy_sync import credentials as creds + + calls = [] + real = creds._load_from + monkeypatch.setattr(creds, "_load_from", lambda b: calls.append(b) or real(b)) + assert creds._load_vault() == {"passwords": {}, "tokens": {}} + assert calls == ["keychain"] + + +# --- Chunk cap ----------------------------------------------------------------- + + +def test_oversized_vault_is_refused_before_any_write(fake_keyring): + from eufy_sync.credentials import MAX_CHUNKS, VaultTooLargeError + + store_token("garmin", _big_token(3 * CHUNK_LIMIT)) + before = dict(fake_keyring.data) + with pytest.raises(VaultTooLargeError, match="--use-file-store"): + store_token("huge", _big_token(MAX_CHUNKS * CHUNK_LIMIT)) + assert fake_keyring.data == before + assert get_token("garmin") == _big_token(3 * CHUNK_LIMIT) + + +def test_largest_allowed_vault_round_trips(fake_keyring): + from eufy_sync.credentials import MAX_CHUNKS + + overhead = len(json.dumps({"passwords": {}, "tokens": {"garmin": _big_token(0)}})) + token = _big_token(MAX_CHUNKS * CHUNK_LIMIT - overhead) + store_token("garmin", token) + assert _header(fake_keyring)["chunks"] == MAX_CHUNKS + assert get_token("garmin") == token + + +def test_use_keychain_store_keeps_the_file_when_the_vault_is_too_large(fake_keyring, cred_file): + from eufy_sync.credentials import MAX_CHUNKS, VaultTooLargeError, use_keychain_store + + cred_file.parent.mkdir(parents=True, exist_ok=True) + original = json.dumps({ + "explicit": True, "passwords": {"default:eufy": "pw"}, + "tokens": {"huge": _big_token(MAX_CHUNKS * CHUNK_LIMIT)}, + }) + cred_file.write_text(original) + + with pytest.raises(VaultTooLargeError): + use_keychain_store() + assert cred_file.read_text() == original + assert _vault_accounts(fake_keyring) == set() + + +def test_legacy_vault_with_more_chunks_than_the_cap_still_reads_and_migrates(fake_keyring, cred_file): + """Released versions wrote any number of "vault:i" chunks.""" + from eufy_sync.credentials import MAX_CHUNKS + + legacy = {"passwords": {"default:eufy": "pw"}, "tokens": {"garmin": _big_token((MAX_CHUNKS + 5) * CHUNK_LIMIT)}} + n = _write_legacy_chunked(fake_keyring, legacy) + assert n > MAX_CHUNKS + + assert get_token("garmin") == legacy["tokens"]["garmin"] + # Too big for the new layout, so the save is refused and the legacy + # vault is left as it was rather than half-migrated. + with pytest.raises(credentials.VaultTooLargeError): + store_token("strava", {"a": 1}) + assert get_token("garmin") == legacy["tokens"]["garmin"] + # Shrinking it migrates, and every legacy chunk is deleted. + store_token("garmin", {"a": 1}) + assert _vault_accounts(fake_keyring) == {VAULT_ACCOUNT} diff --git a/tests/test_eufy_client.py b/tests/test_eufy_client.py index 0adc7dd..f7a5fc6 100644 --- a/tests/test_eufy_client.py +++ b/tests/test_eufy_client.py @@ -1,6 +1,7 @@ from datetime import datetime, timezone from unittest.mock import MagicMock, patch +import httpx import pytest from eufy_sync.config import EufyConfig @@ -395,3 +396,178 @@ def test_processed_measurement_is_not_weight_only(): c = _client() m = c._parse_record(_record("a", 800, 100)) assert m.weight_only is False + + +# --------------------------------------------------------------------------- +# Login host fallback: EufyLife 3.3.12 logs in at home-api.eufylife.com. Use it +# only when the original endpoint looks moved or gone, never after a rejected +# password or a rate limit. +# --------------------------------------------------------------------------- + +PRIMARY_LOGIN = "https://api.eufylife.com/v1/user/v2/email/login" +FALLBACK_LOGIN = "https://home-api.eufylife.com/v1/user/v2/email/login/" + + +def _login_client(): + c = EufyClient.__new__(EufyClient) + c.config = EufyConfig(email="e@example.com", password="pw") + c.access_token = None + c.user_id = None + c._client = MagicMock() + c._save_token = MagicMock() + return c + + +def _http_resp(status_code, json_body=None, *, url=PRIMARY_LOGIN): + request = httpx.Request("POST", url) + if json_body is None: + return httpx.Response(status_code, text="Not Found", request=request) + return httpx.Response(status_code, json=json_body, request=request) + + +def _login_ok(token="tok-1", user_id="uid-1", **extra): + return {"res_code": 1, "access_token": token, "user_id": user_id, **extra} + + +def _posted_urls(c): + return [call.args[0] for call in c._client.post.call_args_list] + + +def test_login_primary_success_does_not_fall_back(): + c = _login_client() + c._client.post.return_value = _http_resp(200, _login_ok(expires_in=86400)) + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN] + assert (c.access_token, c.user_id) == ("tok-1", "uid-1") + c._save_token.assert_called_once_with(86400) + + +@pytest.mark.parametrize("status", [404, 410]) +def test_login_primary_gone_falls_back_to_home_api(status): + c = _login_client() + c._client.post.side_effect = [ + _http_resp(status), + _http_resp(200, _login_ok("tok-2", "uid-2"), url=FALLBACK_LOGIN), + ] + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN, FALLBACK_LOGIN] + assert (c.access_token, c.user_id) == ("tok-2", "uid-2") + + fallback = c._client.post.call_args_list[1] + assert fallback.kwargs["headers"]["User-Agent"] == "EufyLife-Android-3.3.12" + assert fallback.kwargs["headers"]["Country"] == "US" + assert fallback.kwargs["headers"]["Category"] == "Health" + assert fallback.kwargs["json"]["ab"] == "us" + assert fallback.kwargs["json"]["email"] == "e@example.com" + + +def test_login_connection_failure_falls_back(): + c = _login_client() + c._client.post.side_effect = [ + httpx.ConnectError("Name or service not known"), + _http_resp(200, _login_ok(), url=FALLBACK_LOGIN), + ] + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN, FALLBACK_LOGIN] + + +def test_login_non_json_200_falls_back(): + c = _login_client() + c._client.post.side_effect = [ + _http_resp(200), # HTML body + _http_resp(200, _login_ok(), url=FALLBACK_LOGIN), + ] + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN, FALLBACK_LOGIN] + + +def test_login_deprecated_message_falls_back(): + c = _login_client() + c._client.post.side_effect = [ + _http_resp(200, {"res_code": 26050, "message": "This API is deprecated, please upgrade the app"}), + _http_resp(200, _login_ok(), url=FALLBACK_LOGIN), + ] + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN, FALLBACK_LOGIN] + + +def test_login_wrong_password_does_not_fall_back(): + from eufy_sync.sync import PermanentSyncError + c = _login_client() + c._client.post.return_value = _http_resp(200, {"res_code": 26006, "message": "Incorrect password"}) + with pytest.raises(PermanentSyncError): + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN] + c._save_token.assert_not_called() + + +@pytest.mark.parametrize("status", [401, 403]) +def test_login_auth_rejected_status_does_not_fall_back(status): + c = _login_client() + c._client.post.return_value = _http_resp(status, {"res_code": 0}) + with pytest.raises(httpx.HTTPStatusError): + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN] + + +def test_login_rate_limited_does_not_fall_back(): + c = _login_client() + c._client.post.return_value = _http_resp(429, {"res_code": 0, "message": "Too many requests"}) + with pytest.raises(httpx.HTTPStatusError): + c._fresh_login() + assert _posted_urls(c) == [PRIMARY_LOGIN] + + +def test_login_fallback_wrong_password_raises_permanent_error(): + from eufy_sync.sync import PermanentSyncError + c = _login_client() + c._client.post.side_effect = [ + _http_resp(404), + _http_resp(200, {"res_code": 26006, "message": "Incorrect password"}, url=FALLBACK_LOGIN), + ] + with pytest.raises(PermanentSyncError): + c._fresh_login() + assert len(_posted_urls(c)) == 2 + + +def test_home_api_response_shape_parses(): + """Field names from the EufyLife 3.3.12 login response, as read by + m4ary/eufylife-api-hacs and osjayaprakash/eufylife-scale-mcp.""" + c = _login_client() + home_api_body = { + "res_code": 1, + "message": "success", + "access_token": "home-tok", + "user_id": "home-uid", + "expires_in": 2592000, + "user_center_id": "center-id", + "user_center_token": "center-tok", + "device_id": "phone-id", + "customers": [{"id": "cust-a", "name": "A"}, {"id": "cust-b", "name": "B"}], + } + c._client.post.side_effect = [_http_resp(404), _http_resp(200, home_api_body, url=FALLBACK_LOGIN)] + c._fresh_login() + # The data endpoints take the login access_token and user_id, not user_center_token. + assert c.access_token == "home-tok" + assert c.user_id == "home-uid" + c._save_token.assert_called_once_with(2592000) + + +def test_login_response_nested_under_data_parses_and_defaults_ttl(): + c = _login_client() + c._client.post.return_value = _http_resp( + 200, {"res_code": 1, "data": {"access_token": "n-tok", "user_id": 12345}}, + ) + c._fresh_login() + assert (c.access_token, c.user_id) == ("n-tok", "12345") + c._save_token.assert_called_once_with(2592000) + + +def test_login_logs_host_without_secrets(caplog): + c = _login_client() + c._client.post.side_effect = [_http_resp(404), _http_resp(200, _login_ok("secret-tok"), url=FALLBACK_LOGIN)] + with caplog.at_level("DEBUG", logger="eufy_sync.eufy_client"): + c._fresh_login() + assert "home-api.eufylife.com" in caplog.text + assert "secret-tok" not in caplog.text + assert "pw" not in caplog.text.split() diff --git a/tests/test_garmin_auth.py b/tests/test_garmin_auth.py index 74bd388..f07d90b 100644 --- a/tests/test_garmin_auth.py +++ b/tests/test_garmin_auth.py @@ -73,17 +73,91 @@ def test_token_status_valid_with_blob(monkeypatch): assert auth.token_status()["state"] == "valid" -def test_force_reauth_clears_token_logs_in_and_saves(monkeypatch): +def test_force_reauth_logs_in_and_saves(monkeypatch): auth = _auth() calls = [] - monkeypatch.setattr(auth, "_clear_token", lambda: calls.append("clear")) monkeypatch.setattr(auth, "_save_token", lambda g: calls.append("save")) fake_garmin = MagicMock() with patch("eufy_sync.garmin_auth.Garmin", return_value=fake_garmin): result = auth.force_reauth() assert result is fake_garmin fake_garmin.login.assert_called_once() - assert calls == ["clear", "save"] # cleared before login, saved after + assert calls == ["save"] + + +NEW_BLOB = {"di_token": "new", "di_refresh_token": "new-r", "di_client_id": "cid"} + + +def _store_old_token(): + from eufy_sync.credentials import store_token + store_token("garmin", dict(BLOB)) + + +def _stored_token(): + from eufy_sync.credentials import get_token + return get_token("garmin") + + +def test_force_reauth_keeps_the_stored_token_when_login_fails(monkeypatch): + # A cancelled MFA prompt or a passing Garmin error must not cost the + # session that was stored before the attempt. + from eufy_sync.garmin_auth import GarminLoginCancelled + _store_old_token() + auth = _auth() + fake = MagicMock() + fake.login.side_effect = GarminLoginCancelled("no code") + with patch("eufy_sync.garmin_auth.Garmin", return_value=fake): + with pytest.raises(PermanentSyncError): + auth.force_reauth() + assert _stored_token() == BLOB + + +def test_force_reauth_replaces_the_stored_token_on_success(): + _store_old_token() + auth = _auth() + fake = MagicMock() + fake.client.dumps.return_value = json.dumps(NEW_BLOB) + with patch("eufy_sync.garmin_auth.Garmin", return_value=fake): + auth.force_reauth() + assert _stored_token() == NEW_BLOB + + +def test_silent_reauth_keeps_the_stored_token_when_garmin_wants_mfa(monkeypatch): + # The headless case from the field: a 403 looked like a dead session, the + # relogin hit an MFA demand, and the working token used to be gone. + from eufy_sync.garmin_auth import GarminLoginCancelled + _store_old_token() + _fail_on_browser(monkeypatch) + auth = _auth() + fake = MagicMock() + fake.login.side_effect = GarminLoginCancelled("mfa") + with patch("eufy_sync.garmin_auth.Garmin", return_value=fake): + with pytest.raises(PermanentSyncError): + auth.silent_reauth() + assert _stored_token() == BLOB + + +def test_silent_reauth_keeps_the_stored_token_on_a_transient_failure(monkeypatch): + _store_old_token() + _fail_on_browser(monkeypatch) + auth = _auth() + fake = MagicMock() + fake.login.side_effect = ConnectionError("Connection reset by peer") + with patch("eufy_sync.garmin_auth.Garmin", return_value=fake): + with pytest.raises(ConnectionError): + auth.silent_reauth() + assert _stored_token() == BLOB + + +def test_silent_reauth_replaces_the_stored_token_on_success(monkeypatch): + _store_old_token() + _fail_on_browser(monkeypatch) + auth = _auth() + fake = MagicMock() + fake.client.dumps.return_value = json.dumps(NEW_BLOB) + with patch("eufy_sync.garmin_auth.Garmin", return_value=fake): + assert auth.silent_reauth() is fake + assert _stored_token() == NEW_BLOB def test_login_falls_back_to_fresh_when_blob_unusable(monkeypatch): @@ -315,10 +389,9 @@ def test_headless_mfa_prompt_cancels_without_reading_input(monkeypatch): _headless_mfa_prompt() -def test_silent_reauth_clears_token_logs_in_and_saves(monkeypatch): +def test_silent_reauth_logs_in_and_saves(monkeypatch): auth = _auth() calls = [] - monkeypatch.setattr(auth, "_clear_token", lambda: calls.append("clear")) monkeypatch.setattr(auth, "_save_token", lambda g: calls.append("save")) _fail_on_browser(monkeypatch) fake = MagicMock() @@ -326,13 +399,12 @@ def test_silent_reauth_clears_token_logs_in_and_saves(monkeypatch): result = auth.silent_reauth() assert result is fake fake.login.assert_called_once() - assert calls == ["clear", "save"] + assert calls == ["save"] def test_silent_reauth_mfa_demand_names_reauth(monkeypatch): from eufy_sync.garmin_auth import GarminLoginCancelled auth = _auth() - monkeypatch.setattr(auth, "_clear_token", lambda: None) _fail_on_browser(monkeypatch) fake = MagicMock() fake.login.side_effect = GarminLoginCancelled("mfa") @@ -345,7 +417,6 @@ def test_silent_reauth_mfa_demand_names_reauth(monkeypatch): def test_force_reauth_falls_back_to_browser(monkeypatch): from garminconnect import GarminConnectTooManyRequestsError auth = _auth() - monkeypatch.setattr(auth, "_clear_token", lambda: None) monkeypatch.setattr(auth, "_save_token", lambda g: None) fake = MagicMock() fake.login.side_effect = GarminConnectTooManyRequestsError("429") diff --git a/tests/test_garmin_client.py b/tests/test_garmin_client.py index fd2d736..8804b22 100644 --- a/tests/test_garmin_client.py +++ b/tests/test_garmin_client.py @@ -298,6 +298,33 @@ def test_duplicate_check_fails_open_when_the_relogin_fails(): assert client.has_weight_on_date(datetime(2026, 6, 10, tzinfo=timezone.utc)) is False +def test_a_successful_relogin_is_not_repeated_when_the_api_keeps_refusing(): + # Cloudflare 403s on the API while SSO logins succeed: the first call + # relogs in, and every later call must fail with its own error instead + # of running a full login each time. + from garminconnect import GarminConnectConnectionError + + blocked = GarminConnectConnectionError("API Error 403 - ") + stale = MagicMock() + stale.get_body_composition.side_effect = blocked + fresh = MagicMock() + fresh.get_body_composition.side_effect = blocked + fresh.get_daily_weigh_ins.side_effect = blocked + fresh.add_body_composition.side_effect = blocked + client = _client_with_fake_garmin(stale) + client._allow_interactive = False + bc = GarminBodyComposition(timestamp="2026-06-10T08:00:00+00:00", weight=86.2) + + with patch.object(client._auth, "silent_reauth", return_value=fresh) as reauth: + assert client.has_weight_on_date(datetime(2026, 6, 10, tzinfo=timezone.utc)) is False + with pytest.raises(GarminConnectConnectionError, match="403"): + client.check_connection() + with pytest.raises(GarminConnectConnectionError, match="403"): + client.upload_body_composition(bc) + + reauth.assert_called_once() + + # --------------------------------------------------------------------------- # close() persists whatever the library rotated during the run # --------------------------------------------------------------------------- @@ -496,3 +523,68 @@ def test_delete_weight_entry_fails_open_when_the_relogin_fails(): with patch.object(client._auth, "silent_reauth", side_effect=PermanentSyncError("Garmin wants an MFA code")): assert client.delete_weight_entry(UPLOADED_AT, 85.0) is False + + +# --------------------------------------------------------------------------- +# One relogin per run +# --------------------------------------------------------------------------- + + +def test_failed_relogin_in_duplicate_check_is_not_retried_by_the_upload(): + # The duplicate check fails open after a relogin that wants MFA. The + # upload that follows must report that same failure, with its fix-it hint, + # rather than try a second login (another MFA demand, more 429 risk). + from garminconnect import GarminConnectConnectionError + + from eufy_sync.sync import PermanentSyncError + dead = MagicMock() + dead.get_body_composition.side_effect = GarminConnectConnectionError("API Error 403") + dead.add_body_composition.side_effect = GarminConnectConnectionError("API Error 403") + client = _client_with_fake_garmin(dead) + client._allow_interactive = False + failure = PermanentSyncError("Garmin wants an MFA code. Run: eufy-sync --reauth garmin") + bc = GarminBodyComposition(timestamp="2026-06-10T08:00:00+00:00", weight=80.0) + with patch.object(client._auth, "silent_reauth", side_effect=failure) as reauth: + assert client.has_weight_on_date(datetime(2026, 6, 10, tzinfo=timezone.utc)) is False + with pytest.raises(PermanentSyncError) as exc: + client.upload_body_composition(bc) + reauth.assert_called_once() + assert exc.value is failure + assert "--reauth garmin" in str(exc.value) + + +def test_failed_relogin_in_delete_is_not_retried_by_the_upload(): + from garminconnect import GarminConnectAuthenticationError + + from eufy_sync.sync import PermanentSyncError + dead = MagicMock() + dead.get_daily_weigh_ins.side_effect = GarminConnectAuthenticationError("dead") + dead.add_body_composition.side_effect = GarminConnectAuthenticationError("dead") + client = _client_with_fake_garmin(dead) + client._allow_interactive = True + failure = PermanentSyncError("Garmin login cancelled") + bc = GarminBodyComposition(timestamp="2026-06-10T08:00:00+00:00", weight=80.0) + with patch.object(client._auth, "force_reauth", side_effect=failure) as reauth: + assert client.delete_weight_entry(UPLOADED_AT, 85.0) is False + with pytest.raises(PermanentSyncError): + client.upload_body_composition(bc) + reauth.assert_called_once() + + +def test_old_session_still_serves_calls_after_a_failed_relogin(): + # A Cloudflare 403 reads like a dead session but may pass. After the + # relogin fails, the old session is kept, and a later call that goes + # through is not blocked by the remembered failure. + from garminconnect import GarminConnectConnectionError + + from eufy_sync.sync import PermanentSyncError + session = MagicMock() + session.get_body_composition.side_effect = GarminConnectConnectionError("API Error 403") + session.add_body_composition.return_value = {"ok": True} + client = _client_with_fake_garmin(session) + client._allow_interactive = False + bc = GarminBodyComposition(timestamp="2026-06-10T08:00:00+00:00", weight=80.0) + with patch.object(client._auth, "silent_reauth", side_effect=PermanentSyncError("mfa")): + assert client.has_weight_on_date(datetime(2026, 6, 10, tzinfo=timezone.utc)) is False + assert client.upload_body_composition(bc) == {"ok": True} + assert client._garmin is session diff --git a/tests/test_lock.py b/tests/test_lock.py index bc7c00d..706848a 100644 --- a/tests/test_lock.py +++ b/tests/test_lock.py @@ -1,6 +1,7 @@ """The single-instance lock that keeps a manual sync off the scheduled one.""" from __future__ import annotations +import sys from pathlib import Path from unittest.mock import patch @@ -55,7 +56,7 @@ def test_lock_is_released_when_the_block_raises(): def test_unusable_lock_file_runs_unlocked(): """The lock is a courtesy. A data dir we cannot write to must not become a new way for the sync to refuse to start.""" - with patch("eufy_sync.cli.lock.os.open", side_effect=OSError("read-only")): + with patch("eufy_sync.file_lock.os.open", side_effect=OSError("read-only")): with lock.single_instance() as acquired: assert acquired is True # Nothing is holding anything, so a second run is not blocked. @@ -64,7 +65,7 @@ def test_unusable_lock_file_runs_unlocked(): def test_strict_lock_refuses_to_run_when_lock_file_cannot_open(): - with patch("eufy_sync.cli.lock.os.open", side_effect=OSError("read-only")): + with patch("eufy_sync.file_lock.os.open", side_effect=OSError("read-only")): with lock.single_instance(require_lock=True) as acquired: assert acquired is False @@ -110,8 +111,10 @@ def boom(*a, **kw): out = capsys.readouterr().out assert "another eufy-sync run is in progress" in out.lower() mock_notify.assert_not_called() - # The skip happens before any other sync-path work. + # The skip happens before any other sync-path work, including the + # password migration, which writes the credential vault. mock_updates.assert_not_called() + _migrate.assert_not_called() @patch("eufy_sync.cli.status._print_summary") @@ -138,3 +141,366 @@ def test_sync_runs_and_releases_the_lock_for_the_next_run( assert exc.value.code == 0 with lock.single_instance() as acquired: assert acquired is True + + +class _DictKeyring: + def __init__(self): + self.data = {} + self.writes = [] + + def get_password(self, service, account): + return self.data.get((service, account)) + + def set_password(self, service, account, password): + self.writes.append(account) + self.data[(service, account)] = password + + def delete_password(self, service, account): + self.writes.append(account) + self.data.pop((service, account), None) + + +@pytest.mark.parametrize("flag", [["--status"], ["--history"]]) +def test_status_and_history_read_credentials_without_writing_the_vault(flag, tmp_path, monkeypatch): + """--status and --history run without the sync lock, so they must not + write the vault: a sync may be saving it at that moment. Plaintext YAML + passwords stay in the YAML and legacy per-password keychain items are + read where they are, both left for the next locked run to migrate.""" + from eufy_sync import credentials + from eufy_sync.cli.app import main + + store = _DictKeyring() + monkeypatch.setattr("keyring.get_password", store.get_password) + monkeypatch.setattr("keyring.set_password", store.set_password) + monkeypatch.setattr("keyring.delete_password", store.delete_password) + monkeypatch.setattr(credentials, "_keyring_available", lambda: True) + monkeypatch.setattr(credentials, "CRED_FILE", tmp_path / "credentials.json") + store.data[(credentials.SERVICE_NAME, "default:garmin")] = "legacy-pw" + + config_path = tmp_path / "config.yaml" + _write_config(config_path, { + "users": [{ + "name": "default", + "eufy": {"email": "e@example.com", "password": "yaml-pw"}, + "garmin": {"email": "g@example.com"}, + }], + }) + before = config_path.read_text() + argv = ["eufy-sync", "--config", str(config_path), "--db", str(tmp_path / "state.db"), *flag] + + with patch("sys.argv", argv), patch("eufy_sync.cli.setup._show_upgrade_notice"): + main() + + assert store.writes == [] + assert store.data[(credentials.SERVICE_NAME, "default:garmin")] == "legacy-pw" + assert config_path.read_text() == before + + +@patch("eufy_sync.cli.status._print_summary") +@patch("eufy_sync.platform_support.notify") +@patch("eufy_sync.cli.updater._check_for_updates") +@patch("eufy_sync.cli.setup._show_upgrade_notice") +@patch("eufy_sync.credentials._keyring_available", return_value=False) +def test_sync_runs_the_password_migration_while_holding_the_lock( + _keyring, _notice, _updates, _notify, _summary, tmp_path +): + """The migration writes the vault, so it runs inside the sync lock.""" + from eufy_sync.cli.app import main + + config_path = _write_synced_config(tmp_path) + argv = ["eufy-sync", "--config", str(config_path), "--db", str(tmp_path / "state.db"), "--headless"] + held = {} + + def migrate(path): + with lock.single_instance() as other: + held["by_us"] = other is False + + with patch("eufy_sync.cli.setup._migrate_config_passwords", side_effect=migrate), \ + patch("eufy_sync.sync.sync_user", return_value=({"garmin": 1}, {})), \ + patch("sys.argv", argv), \ + pytest.raises(SystemExit): + main() + + assert held == {"by_us": True} + + +# --------------------------------------------------------------------------- +# Commands that change credentials, tokens, or config wait for a running sync +# --------------------------------------------------------------------------- + +# (extra argv, function the command runs, flag named in the retry hint) +_CREDENTIAL_COMMANDS = [ + (["--uninstall"], "eufy_sync.cli.maintenance._uninstall", "--uninstall"), + (["--use-file-store"], "eufy_sync.credentials.use_file_store", "--use-file-store"), + (["--use-keychain"], "eufy_sync.credentials.use_keychain_store", "--use-keychain"), + (["--update"], "eufy_sync.cli.updater._self_update", "--update"), + (["--setup-strava"], "eufy_sync.cli.setup._setup_strava", "--setup-strava"), + (["--setup-zwift"], "eufy_sync.cli.setup._setup_zwift", "--setup-zwift"), + (["--disconnect-zwift"], "eufy_sync.cli.maintenance._disconnect_zwift", "--disconnect-zwift"), + (["--select-profile"], "eufy_sync.cli.profiles._select_profile", "--select-profile"), + (["--update-password"], "eufy_sync.cli.maintenance._update_password", "--update-password"), + (["--reauth"], "eufy_sync.cli.maintenance._reauth", "--reauth"), + (["--reauth", "garmin"], "eufy_sync.cli.maintenance._reauth", "--reauth"), + # No config: the first-run wizard, which stores passwords and logs in. + ([], "eufy_sync.cli.setup._first_run_setup", "eufy-sync"), +] + + +def _command_argv(tmp_path: Path, extra: list[str]) -> list[str]: + # The config path does not exist, so a bare run reaches first-run setup. + return ["eufy-sync", "--config", str(tmp_path / "missing.yaml"), *extra] + + +@pytest.mark.parametrize("extra, target, flag", _CREDENTIAL_COMMANDS) +def test_credential_command_refuses_while_a_sync_holds_the_lock(tmp_path, capsys, extra, target, flag): + from eufy_sync.cli.app import main + + with lock.single_instance() as held: + assert held is True + with patch(target) as command, \ + patch("sys.argv", _command_argv(tmp_path, extra)), \ + pytest.raises(SystemExit) as exc: + main() + + assert exc.value.code == 1 + command.assert_not_called() + assert f"Retry {flag} when it finishes" in capsys.readouterr().out + + +@pytest.mark.parametrize("extra, target, flag", _CREDENTIAL_COMMANDS) +def test_credential_command_runs_while_holding_the_lock(tmp_path, extra, target, flag): + from eufy_sync.cli.app import main + + seen = [] + + def check_lock(*args, **kwargs): + with lock.single_instance() as other: + seen.append(other) + + with patch(target, side_effect=check_lock), \ + patch("sys.argv", _command_argv(tmp_path, extra)): + try: + main() + except SystemExit: + pass # several of these commands exit on their own + + assert seen == [False] # a sync starting mid-command would have been kept out + with lock.single_instance() as acquired: + assert acquired is True # and the lock is free again afterwards + + +@pytest.mark.parametrize("extra, target, flag", _CREDENTIAL_COMMANDS) +def test_credential_command_refuses_when_the_lock_file_cannot_open(tmp_path, capsys, extra, target, flag): + from eufy_sync.cli.app import main + + with patch("eufy_sync.file_lock.os.open", side_effect=OSError("read-only")), \ + patch(target) as command, \ + patch("sys.argv", _command_argv(tmp_path, extra)), \ + pytest.raises(SystemExit) as exc: + main() + + assert exc.value.code == 1 + command.assert_not_called() + + +@patch("eufy_sync.credentials._keyring_available", return_value=False) +@patch("eufy_sync.platform_support.agent_installed", return_value=False) +@patch("eufy_sync.cli.maintenance.sys.stdin") +@patch("builtins.input", return_value="y") +def test_uninstall_under_the_lock_still_removes_the_whole_data_dir( + _input, mock_stdin, _agent, _keyring, tmp_path +): + """--uninstall holds the lock file open while it sweeps the data dir, and + Windows cannot delete an open file. The sweep skips it and the command + removes it after release, so nothing is left behind.""" + from eufy_sync.cli.app import main + + mock_stdin.isatty.return_value = True + config_path = _write_synced_config(shared.DATA_DIR) + with patch("sys.argv", ["eufy-sync", "--uninstall", "--config", str(config_path)]): + main() + + assert not config_path.exists() + assert not lock.lock_path().exists() + assert not shared.DATA_DIR.exists() + + +@patch("eufy_sync.credentials._keyring_available", return_value=False) +@patch("eufy_sync.platform_support.agent_installed", return_value=False) +@patch("eufy_sync.cli.maintenance.sys.stdin") +@patch("builtins.input", return_value="y") +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX unlink ordering") +def test_uninstall_deletes_each_lock_file_before_releasing_it(_input, mock_stdin, _agent, _keyring, tmp_path): + """Deleting a lock file after release lets another process lock the old + file while a third creates and locks a new one at the same path. On + POSIX every lock --uninstall releases must already be unlinked.""" + import os + + from eufy_sync import file_lock + from eufy_sync.cli.app import main + + mock_stdin.isatty.return_value = True + config_path = _write_synced_config(shared.DATA_DIR) + real_release = file_lock.release + links_at_release = [] + + def spy(fd): + links_at_release.append(os.fstat(fd).st_nlink) + real_release(fd) + + with patch("eufy_sync.file_lock.release", spy), \ + patch("sys.argv", ["eufy-sync", "--uninstall", "--config", str(config_path)]): + main() + + # The vault lock (released when the sweep ends) and the sync lock. + assert len(links_at_release) == 2 + assert links_at_release == [0, 0] + assert not shared.DATA_DIR.exists() + + +@patch("eufy_sync.credentials._keyring_available", return_value=True) +@patch("eufy_sync.platform_support.agent_installed", return_value=False) +@patch("eufy_sync.cli.maintenance.sys.stdin") +@patch("builtins.input", side_effect=["y", "n"]) +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX unlink ordering") +def test_uninstall_removes_the_vault_lock_last_and_holds_it_throughout( + _input, mock_stdin, _agent, _keyring, tmp_path +): + """A vault lock file deleted mid-sweep lets another process create and + lock a fresh one while uninstall is still removing credentials. Even when + the directory listing yields vault.lock first, every other removal (data + dir and keychain) must happen while it is still on disk and held, and it + must be the last thing deleted under the vault lock.""" + import shutil + + from eufy_sync import credentials + from eufy_sync.cli.app import main + from eufy_sync.credentials import store_password, store_token, vault_lock_path + + mock_stdin.isatty.return_value = True + config_path = _write_synced_config(shared.DATA_DIR) + store_password("default:eufy", "pw") + store_token("garmin", {"t": 1}) + (shared.DATA_DIR / "credentials.json").write_text('{"passwords": {}, "tokens": {}}') + (shared.DATA_DIR / "state.db").write_text("") + (shared.DATA_DIR / "cache").mkdir() + (shared.DATA_DIR / "cache" / "x").write_text("") + vault_lock_path().touch() + + events = [] + + def record(what): + events.append((what, vault_lock_path().exists(), credentials._lock_depth > 0)) + + real_unlink = Path.unlink + real_rmtree = shutil.rmtree + real_iterdir = Path.iterdir + real_delete_password = credentials.delete_password + real_delete_token = credentials.delete_token + + def unlink(self, *args, **kwargs): + if self.parent == shared.DATA_DIR: + record(self.name) + return real_unlink(self, *args, **kwargs) + + def rmtree(path, *args, **kwargs): + record(Path(path).name) + return real_rmtree(path, *args, **kwargs) + + def iterdir(self): + items = list(real_iterdir(self)) + # Worst case for the old loop: the vault lock comes first. + return iter(sorted(items, key=lambda p: p.name != "vault.lock")) + + def delete_password(account): + record(f"password:{account}") + real_delete_password(account) + + def delete_token(name): + record(f"token:{name}") + real_delete_token(name) + + with patch.object(Path, "unlink", unlink), \ + patch.object(Path, "iterdir", iterdir), \ + patch("eufy_sync.cli.maintenance.shutil.rmtree", rmtree), \ + patch("eufy_sync.credentials.delete_password", delete_password), \ + patch("eufy_sync.credentials.delete_token", delete_token), \ + patch("sys.argv", ["eufy-sync", "--uninstall", "--config", str(config_path)]): + main() + + names = [name for name, _, _ in events] + assert "vault.lock" in names + vault_index = names.index("vault.lock") + # Only the sync lock (handled by the caller) may go after the vault lock. + assert names[vault_index + 1:] == ["sync.lock"] + for name in ("credentials.json", "config.yaml", "state.db", "cache", + "password:default:eufy", "token:garmin"): + assert name in names[:vault_index] + for name, lock_on_disk, held in events[:vault_index + 1]: + assert lock_on_disk and held, name + assert not shared.DATA_DIR.exists() + + +def test_windows_uninstall_deletes_lock_files_only_after_release(tmp_path, monkeypatch): + """Windows cannot delete an open file, so the lock files are skipped + while held and deleted by _remove_lock_files afterwards.""" + from types import SimpleNamespace + + from eufy_sync.cli import maintenance + + fake_sys = SimpleNamespace(platform="win32") + monkeypatch.setattr(lock, "sys", fake_sys) + monkeypatch.setattr(maintenance, "sys", fake_sys) + shared.DATA_DIR.mkdir(parents=True) + for name in ("sync.lock", "vault.lock"): + (shared.DATA_DIR / name).write_text("") + + lock.unlink_while_held() + assert lock.lock_path().exists() + maintenance._remove_lock_files(shared.DATA_DIR) + assert not shared.DATA_DIR.exists() + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX only") +def test_posix_remove_lock_files_never_deletes_a_lock_file_after_release(tmp_path): + """On POSIX a lock file present after release may be a new one another + process just created and locked; it must be left alone.""" + from eufy_sync.cli import maintenance + + data_dir = tmp_path / "data" + data_dir.mkdir() + (data_dir / "sync.lock").write_text("") + maintenance._remove_lock_files(data_dir) + assert (data_dir / "sync.lock").exists() + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX only") +def test_lock_on_an_unlinked_file_is_retried_on_the_new_file(tmp_path): + """A process that opened the lock file before a holder unlinked it must + not treat a lock on that orphan as the lock for the path.""" + import os + + from eufy_sync import file_lock + + path = tmp_path / "x.lock" + stale = os.open(str(path), os.O_RDWR | os.O_CREAT, 0o600) + path.unlink() + real_open = file_lock._open + opened = [] + + def first_open_is_stale(p): + if not opened: + opened.append(stale) + return stale + fd = real_open(p) + opened.append(fd) + return fd + + with patch("eufy_sync.file_lock._open", first_open_is_stale): + fd = file_lock.acquire(path) + # acquire noticed the orphan and opened the path a second time. Compare + # against the file now at the path, not the stale inode number: once the + # orphan is closed, filesystems such as ext4 may hand that number out again. + assert fd is not None and len(opened) == 2 + assert os.fstat(fd).st_ino == os.stat(path).st_ino + file_lock.release(fd) diff --git a/tests/test_setup_zwift.py b/tests/test_setup_zwift.py index 906d93f..1ab4a68 100644 --- a/tests/test_setup_zwift.py +++ b/tests/test_setup_zwift.py @@ -22,15 +22,13 @@ def _config(path): })) -def test_setup_zwift_reuses_probe_credentials_and_writes_no_secret(tmp_path, capsys): +def test_setup_zwift_reuses_configured_credentials_and_writes_no_secret(tmp_path, capsys): path = tmp_path / "config.yaml" _config(path) + config = yaml.safe_load(path.read_text()) + config["users"][0]["zwift"] = {"email": "z@example.com"} + path.write_text(yaml.safe_dump(config)) credentials.store_password("default:zwift", "private-value") - credentials.store_token("zwift_probe", { - "email": "z@example.com", - "user_name": "default", - "password_account": "default:zwift", - }) client = MagicMock() with patch("eufy_sync.zwift_client.ZwiftClient", return_value=client), \ @@ -93,15 +91,11 @@ def test_disconnect_zwift_preserves_other_config_and_arbitrary_probe_account(tmp assert "zwift_probe" not in vault["tokens"] -def test_setup_zwift_ignores_probe_metadata_for_another_user(tmp_path, capsys): +def test_setup_zwift_without_saved_credentials_needs_a_terminal(tmp_path, capsys): path = tmp_path / "config.yaml" _config(path) - credentials.store_password("other:zwift", "other-password") - credentials.store_token("zwift_probe", { - "email": "other@example.com", - "user_name": "other", - "password_account": "other:zwift", - }) + # A saved password alone, with no configured Zwift email, is not enough. + credentials.store_password("default:zwift", "orphaned-password") with patch("sys.stdin.isatty", return_value=False), \ patch("builtins.input", side_effect=AssertionError("must refuse before prompting")), \ diff --git a/tests/test_strava_client.py b/tests/test_strava_client.py index 20d338c..01c8013 100644 --- a/tests/test_strava_client.py +++ b/tests/test_strava_client.py @@ -1,10 +1,18 @@ from __future__ import annotations import time +from datetime import date from unittest.mock import MagicMock, patch +import httpx +import pytest + from eufy_sync.config import StravaConfig -from eufy_sync.strava_client import StravaClient +from eufy_sync.strava_client import ( + STRAVA_API_BASE_NEW, + STRAVA_API_BASE_OLD, + StravaClient, +) def _make_config(): @@ -190,12 +198,13 @@ def test_update_weight(mock_load, mock_save): client = StravaClient(_make_config()) client.authenticate() - with patch.object(client._client, "put", return_value=mock_response) as mock_put: + with patch.object(client._client, "request", return_value=mock_response) as mock_request: result = client.update_weight(86.2) assert result == {"weight": 86.2} - call_kwargs = mock_put.call_args - assert "athlete" in call_kwargs[0][0] + method, url = mock_request.call_args[0] + assert method == "PUT" + assert url.endswith("/athlete") client.close() @@ -211,7 +220,7 @@ def test_update_weight_failure_raises(mock_load, mock_save): client = StravaClient(_make_config()) client.authenticate() - with patch.object(client._client, "put", return_value=mock_response): + with patch.object(client._client, "request", return_value=mock_response): try: client.update_weight(86.2) raise AssertionError("Should have raised") @@ -235,3 +244,148 @@ def test_auth_url_encodes_every_query_value(): assert query["scope"] == ["profile:write,profile:read_all"] assert query["state"] == ["st@te/value"] assert query["approval_prompt"] == ["force"] + + +# Strava's API base moves to api-v3.strava.com on a published schedule. + +BEFORE_NEW_HOST = date(2027, 1, 3) +NEW_HOST_DAY_ONE = date(2027, 1, 4) +LAST_OVERLAP_DAY = date(2027, 5, 31) +LONG_AFTER = date(2028, 3, 1) + + +def _client_on(day: date) -> StravaClient: + return StravaClient(_make_config(), today=lambda: day) + + +def _ok(json_body=None): + resp = MagicMock() + resp.status_code = 200 + resp.json.return_value = json_body or {"weight": 86.2} + return resp + + +def _status(code: int): + resp = MagicMock() + resp.status_code = code + resp.text = "error" + return resp + + +def _urls(mock_request) -> list[str]: + return [c[0][1] for c in mock_request.call_args_list] + + +@pytest.mark.parametrize("day, expected", [ + (date(2026, 10, 2), STRAVA_API_BASE_OLD), + (BEFORE_NEW_HOST, STRAVA_API_BASE_OLD), + (NEW_HOST_DAY_ONE, STRAVA_API_BASE_NEW), + (LAST_OVERLAP_DAY, STRAVA_API_BASE_NEW), + (LONG_AFTER, STRAVA_API_BASE_NEW), +]) +def test_api_base_follows_the_published_schedule(day, expected): + client = _client_on(day) + with patch.object(client._client, "request", return_value=_ok()) as mock_request: + client.update_weight(86.2) + assert _urls(mock_request) == [f"{expected}/athlete"] + client.close() + + +def test_new_host_is_api_v3_strava_com_without_www(): + assert STRAVA_API_BASE_NEW == "https://api-v3.strava.com" + assert STRAVA_API_BASE_OLD == "https://www.strava.com/api/v3" + + +@pytest.mark.parametrize("failure", [ + httpx.ConnectError("[Errno 8] nodename nor servname provided"), # DNS + httpx.ConnectError("[Errno 61] Connection refused"), + httpx.ConnectError("[SSL: CERTIFICATE_VERIFY_FAILED]"), # TLS + httpx.ConnectTimeout("timed out connecting"), +]) +def test_overlap_falls_back_to_old_host_when_new_host_is_unreachable(failure): + client = _client_on(NEW_HOST_DAY_ONE) + with patch.object(client._client, "request", side_effect=[failure, _ok()]) as mock_request: + assert client.update_weight(86.2) == {"weight": 86.2} + assert _urls(mock_request) == [ + f"{STRAVA_API_BASE_NEW}/athlete", + f"{STRAVA_API_BASE_OLD}/athlete", + ] + client.close() + + +def test_fallback_is_per_call_and_the_next_call_tries_the_new_host_again(): + client = _client_on(NEW_HOST_DAY_ONE) + responses = [httpx.ConnectError("refused"), _ok(), _ok()] + with patch.object(client._client, "request", side_effect=responses) as mock_request: + client.update_weight(86.2) + client.update_weight(86.0) + assert _urls(mock_request) == [ + f"{STRAVA_API_BASE_NEW}/athlete", + f"{STRAVA_API_BASE_OLD}/athlete", + f"{STRAVA_API_BASE_NEW}/athlete", + ] + client.close() + + +@pytest.mark.parametrize("code", [401, 403, 429, 500]) +def test_overlap_does_not_fall_back_on_http_errors(code): + """An HTTP status means the new host answered; retrying elsewhere would + only hide a real auth, rate-limit or server problem.""" + client = _client_on(NEW_HOST_DAY_ONE) + with patch.object(client._client, "request", return_value=_status(code)) as mock_request: + with pytest.raises(RuntimeError, match=str(code)): + client.update_weight(86.2) + assert _urls(mock_request) == [f"{STRAVA_API_BASE_NEW}/athlete"] + client.close() + + +def test_overlap_does_not_fall_back_on_read_timeout(): + """A read timeout may mean the PUT already reached Strava.""" + client = _client_on(NEW_HOST_DAY_ONE) + with patch.object(client._client, "request", side_effect=httpx.ReadTimeout("slow")) as mock_request: + with pytest.raises(httpx.ReadTimeout): + client.update_weight(86.2) + assert _urls(mock_request) == [f"{STRAVA_API_BASE_NEW}/athlete"] + client.close() + + +def test_connection_failure_before_the_switch_is_not_retried(): + client = _client_on(BEFORE_NEW_HOST) + with patch.object(client._client, "request", side_effect=httpx.ConnectError("refused")) as mock_request: + with pytest.raises(httpx.ConnectError): + client.update_weight(86.2) + assert _urls(mock_request) == [f"{STRAVA_API_BASE_OLD}/athlete"] + client.close() + + +def test_old_host_stays_as_fallback_with_no_end_date(): + client = _client_on(LONG_AFTER) + with patch.object(client._client, "request", side_effect=[httpx.ConnectError("refused"), _ok()]) as mock_request: + client.update_weight(86.2) + assert _urls(mock_request) == [f"{STRAVA_API_BASE_NEW}/athlete", f"{STRAVA_API_BASE_OLD}/athlete"] + client.close() + + +def test_both_hosts_unreachable_raises_the_old_host_error(): + client = _client_on(NEW_HOST_DAY_ONE) + failures = [httpx.ConnectError("new down"), httpx.ConnectError("old down")] + with patch.object(client._client, "request", side_effect=failures): + with pytest.raises(httpx.ConnectError, match="old down"): + client.update_weight(86.2) + client.close() + + +def test_check_connection_uses_the_same_fallback(): + client = _client_on(NEW_HOST_DAY_ONE) + responses = [httpx.ConnectError("refused"), _ok({"id": 1})] + with patch.object(client._client, "request", side_effect=responses) as mock_request: + client.check_connection() + assert [c[0][0] for c in mock_request.call_args_list] == ["GET", "GET"] + assert _urls(mock_request)[-1] == f"{STRAVA_API_BASE_OLD}/athlete" + client.close() + + +def test_oauth_urls_stay_on_www_strava_com_after_the_move(): + from eufy_sync.strava_client import STRAVA_AUTH_URL, STRAVA_TOKEN_URL + assert STRAVA_AUTH_URL == "https://www.strava.com/oauth/authorize" + assert STRAVA_TOKEN_URL == "https://www.strava.com/oauth/token" diff --git a/tests/test_summary.py b/tests/test_summary.py index f9b5501..1c78d69 100644 --- a/tests/test_summary.py +++ b/tests/test_summary.py @@ -63,7 +63,7 @@ def test_summary_no_new_measurements(capsys): output = capsys.readouterr().out.strip() assert "No new measurements" in output - assert "last sync: 5h ago" in output + assert "latest weigh-in: 5h ago" in output assert "Garmin connected" in output @@ -76,7 +76,7 @@ def test_summary_no_new_measurements_days_ago(capsys): _print_summary({}, [], state, [user]) output = capsys.readouterr().out.strip() - assert "last sync: 3d ago" in output + assert "latest weigh-in: 3d ago" in output def test_summary_synced_garmin_only(capsys): diff --git a/tests/test_sync.py b/tests/test_sync.py index 56bdad1..1b80e11 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -994,7 +994,8 @@ def test_second_full_record_same_day_does_not_delete(tmp_path: Path): def test_automatic_sync_revisits_older_weight_only_readings(tmp_path: Path): state = SyncState(tmp_path / "test.db") user = _garmin_user() - old_dt = datetime(2026, 5, 10, 12, tzinfo=timezone.utc) + # Inside UPGRADE_LOOKBACK_DAYS; older readings are no longer revisited. + old_dt = datetime.now(timezone.utc).replace(microsecond=0) - timedelta(days=6) old_raw = _raw_measurement(85.0, old_dt) newest = _full_measurement(84.0, old_dt + timedelta(days=3)) _run_garmin_sync(user, state, [old_raw, newest], has_weight_on_date_return=False) @@ -1016,6 +1017,62 @@ def test_automatic_sync_revisits_older_weight_only_readings(tmp_path: Path): state.close() +def test_stale_weight_only_reading_stops_widening_the_fetch_window(tmp_path: Path): + """A raw reading whose full record never matched must not pull every + run back to its date forever; past the lookback it is ignored.""" + from eufy_sync.sync import UPGRADE_LOOKBACK_DAYS + + state = SyncState(tmp_path / "test.db") + user = _garmin_user() + now = datetime.now(timezone.utc) + stale = now - timedelta(days=UPGRADE_LOOKBACK_DAYS + 30) + recent = now - timedelta(days=1) + state.record_sync(user.name, "cust_stale", stale.isoformat(), 85.0, + stale.isoformat(), target="garmin", weight_only=True) + state.record_sync(user.name, "cust_recent", recent.isoformat(), 84.0, + recent.isoformat(), target="garmin") + + source = MagicMock() + source.fetch_measurements.return_value = [] + with patch("eufy_sync.sync.EufyClient", return_value=source), \ + patch("eufy_sync.garmin_client.GarminClient", return_value=MagicMock()), \ + patch("eufy_sync.sync.time.sleep"): + _, errors = sync_user(user, state) + + assert errors == {} + after = source.fetch_measurements.call_args.kwargs["after_timestamp"] + assert after == int(recent.timestamp()) + # The row is kept, so a backfill that returns its full record can still upgrade it. + assert len(state.weight_only_syncs_on_date(user.name, "garmin", stale.astimezone().date())) == 1 + assert state.get_oldest_weight_only_timestamp(user.name, "garmin") == int(stale.timestamp()) + state.close() + + +def test_recent_weight_only_reading_still_widens_the_fetch_window(tmp_path: Path): + from eufy_sync.sync import UPGRADE_MAX_SECONDS + + state = SyncState(tmp_path / "test.db") + user = _garmin_user() + now = datetime.now(timezone.utc) + pending = now - timedelta(days=5) + recent = now - timedelta(days=1) + state.record_sync(user.name, "cust_pending", pending.isoformat(), 85.0, + pending.isoformat(), target="garmin", weight_only=True) + state.record_sync(user.name, "cust_recent", recent.isoformat(), 84.0, + recent.isoformat(), target="garmin") + + source = MagicMock() + source.fetch_measurements.return_value = [] + with patch("eufy_sync.sync.EufyClient", return_value=source), \ + patch("eufy_sync.garmin_client.GarminClient", return_value=MagicMock()), \ + patch("eufy_sync.sync.time.sleep"): + sync_user(user, state) + + after = source.fetch_measurements.call_args.kwargs["after_timestamp"] + assert after == int(pending.timestamp()) - UPGRADE_MAX_SECONDS + state.close() + + @pytest.mark.parametrize("mode,include_current", [ ({"backfill_days": 30}, True), ({"backfill_days": 30}, False), diff --git a/tests/test_update_check.py b/tests/test_update_check.py index 96b32bb..2533fd3 100644 --- a/tests/test_update_check.py +++ b/tests/test_update_check.py @@ -179,7 +179,7 @@ def test_self_update_uses_uv_when_installed_via_uv_tool(): patch("eufy_sync.cli.updater.subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: _self_update() - assert mock_run.call_args.args[0] == ["uv", "tool", "install", "--force", "eufy-sync==9.9.9"] + assert mock_run.call_args.args[0] == ["uv", "tool", "install", "--force", "--refresh-package", "eufy-sync", "eufy-sync==9.9.9"] def test_self_update_uses_uv_for_windows_uv_tool_path(): @@ -317,4 +317,4 @@ def test_self_update_keeps_the_browser_extra_when_installed(monkeypatch): patch("eufy_sync.cli.updater.subprocess.run", return_value=MagicMock(returncode=0)) as mock_run: _self_update() - assert mock_run.call_args.args[0] == ["uv", "tool", "install", "--force", "eufy-sync[browser]==9.9.9"] + assert mock_run.call_args.args[0] == ["uv", "tool", "install", "--force", "--refresh-package", "eufy-sync", "eufy-sync[browser]==9.9.9"] diff --git a/tests/test_zwift_client.py b/tests/test_zwift_client.py index e0f75ec..8121770 100644 --- a/tests/test_zwift_client.py +++ b/tests/test_zwift_client.py @@ -200,6 +200,21 @@ def handler(request): client.close() +@pytest.mark.parametrize("weight", [25.0, 301.0, float("nan")]) +def test_out_of_range_weight_is_skipped_without_any_request(weight): + from eufy_sync.sync import UnsupportedMeasurementError, _is_permanent + + def handler(request): + raise AssertionError("no request for a weight Zwift cannot take") + + client = _authenticated_client(handler) + with pytest.raises(UnsupportedMeasurementError) as exc: + client.update_weight(weight) + # Permanent, so _retry gives up at once instead of sleeping through retries. + assert _is_permanent(exc.value) + client.close() + + def test_204_and_wrong_readback_never_count_as_success(): def handler(request): if request.method == "PUT": diff --git a/tests/test_zwift_sync.py b/tests/test_zwift_sync.py index a993c29..5de3cf7 100644 --- a/tests/test_zwift_sync.py +++ b/tests/test_zwift_sync.py @@ -253,3 +253,31 @@ def test_invalid_target_fails_before_any_client_is_constructed(tmp_path: Path, t eufy_class.assert_not_called() state.close() + + +def test_zwift_out_of_range_weight_is_skipped_without_retry_or_error(tmp_path: Path): + from eufy_sync.sync import UnsupportedMeasurementError + + state = SyncState(tmp_path / "state.db") + user = _user(garmin=True) + # Valid for Garmin (22.7 kg floor) but under Zwift's 30 kg floor. + light = _measurement(25.0, datetime(2026, 5, 10, tzinfo=timezone.utc)) + garmin = MagicMock() + garmin.has_weight_on_date.return_value = False + garmin.upload_body_composition.return_value = {"ok": True} + zwift = MagicMock() + zwift.update_weight.side_effect = UnsupportedMeasurementError("Zwift only accepts 30 to 300 kg, got 25.0 kg") + + with patch("eufy_sync.sync.EufyClient", return_value=_source([light])), \ + patch("eufy_sync.garmin_client.GarminClient", return_value=garmin), \ + patch("eufy_sync.zwift_client.ZwiftClient", return_value=zwift), \ + patch("eufy_sync.sync.time.sleep") as sleep: + counts, errors = sync_user(user, state, backfill_days=7) + + assert errors == {} + assert counts == {"garmin": 1, "zwift": 0} + zwift.update_weight.assert_called_once_with(25.0) + assert not state.is_synced(user.name, light.measurement_id, "zwift") + # Only the inter-upload pause after Garmin; no retry backoff. + assert all(call.args[0] <= 1 for call in sleep.call_args_list) + state.close()