From 864e9dccfc8ed32a94403fc31ebfe97fef0db6cd Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:58:37 -0700 Subject: [PATCH] fix(browser): real-profile snapshot names the locked auth databases and the reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macOS/Linux a running Chrome lets `Cookies` back up but holds `Login Data`, `Login Data For Account` and `Web Data` with a hot write lock, so their SQLite online backup misses the five-second deadline. The launch failed closed with a bare "3 database(s) unavailable" count, and the docs still promised that macOS and Linux "can copy the profile while the browser is running" (#111647). - `_copy_auth_file` returns the failure reason (None on success) instead of a bool; the backup deadline is the named constant `_AUTH_BACKUP_DEADLINE_S` and raises `_AUTH_DB_LOCKED`, so the caller can tell "browser holds a write lock" from "file is not a database". - `_mirror_profile_auth` returns `{db name: reason}`; the new `_unavailable_auth_dbs_error` renders "chrome is running and holds the profile's Login Data, Login Data For Account, Web Data with a write lock … Fully quit chrome and retry" when every failure is the deadline, and the per-database SQLite error otherwise. No raw-copy fallback is added: the no-fallback stance from 58d6d5223af14 (WAL consistency) is unchanged. - Docs: the Real-profile note no longer says macOS/Linux copy live; it states which databases need the browser quit and what the error looks like. The `real_profile_autoclose` comment stops claiming copy-while-running works on POSIX. --- hermes_cli/browser_connect.py | 58 ++++++++++---- hermes_cli/config_defaults.py | 5 +- ...er_connect_real_profile_snapshot_errors.py | 76 +++++++++++++++++++ tests/tools/test_browser_real_profile.py | 12 +-- website/docs/user-guide/features/browser.md | 28 ++++--- 5 files changed, 146 insertions(+), 33 deletions(-) create mode 100644 tests/hermes_cli/test_browser_connect_real_profile_snapshot_errors.py diff --git a/hermes_cli/browser_connect.py b/hermes_cli/browser_connect.py index d07bd2a10d..925bed9342 100644 --- a/hermes_cli/browser_connect.py +++ b/hermes_cli/browser_connect.py @@ -391,18 +391,25 @@ def _secure_snapshot(path: str, *, contents: bool = False) -> None: # exclusive lock, so a raw copy raises WinError 32 and a best-effort skip leaves the copy # signed-out. They are copied via SQLite's online-backup API instead. Matched by basename. _SQLITE_AUTH_DBS = frozenset({"Cookies", "Login Data", "Login Data For Account", "Web Data"}) +# Budget for one auth DB's online backup. A running browser holds Login Data / Login Data For +# Account / Web Data with a hot write lock, so backup makes no progress and this deadline is +# what fails — the reason users see, so it is a named constant rather than a bare string. +_AUTH_BACKUP_DEADLINE_S = 5.0 +_AUTH_DB_LOCKED = ("SQLite backup made no progress within five seconds — " + "a running browser holds a write lock on it") -def _copy_auth_file(src_file: str, dst_file: str) -> bool: - """Copy auth state; refuse a DB that cannot be snapshotted consistently within five seconds.""" +def _copy_auth_file(src_file: str, dst_file: str) -> str | None: + """Copy auth state; returns None on success, else WHY the file could not be snapshotted. + A DB that cannot be backed up consistently within the deadline is refused, never raw-copied.""" os.makedirs(os.path.dirname(dst_file), exist_ok=True) try: if os.path.basename(src_file) in _SQLITE_AUTH_DBS: - deadline = time.monotonic() + 5.0 + deadline = time.monotonic() + _AUTH_BACKUP_DEADLINE_S def check_deadline(_status: int, _remaining: int, _total: int) -> None: if _status != sqlite3.SQLITE_DONE and time.monotonic() >= deadline: - raise TimeoutError("auth database backup exceeded five seconds") + raise TimeoutError(_AUTH_DB_LOCKED) # SQLite must coordinate both ends: immutable ignores committed source WAL, # while replacing only the destination file can replay its abandoned WAL. @@ -413,22 +420,42 @@ def _copy_auth_file(src_file: str, dst_file: str) -> bool: source.backup(out, pages=256, progress=check_deadline, sleep=0.1) else: shutil.copy2(src_file, dst_file) - return True + return None except (OSError, sqlite3.Error) as e: # A raw DB copy can lose committed WAL or overwrite a locked destination. logger.debug("real-profile: could not copy %s: %s", src_file, e) - return False + return str(e) or type(e).__name__ -def _mirror_profile_auth(src: str, dst: str, source_profile: str) -> int: +def _mirror_profile_auth(src: str, dst: str, source_profile: str) -> dict[str, str]: """Mirror ``source_profile``'s auth files into the copy's ``Default`` (agent-browser opens it); - returns the number of DB auth files that could NOT be copied (0 = clean).""" - failed_dbs = 0 + returns ``{relative name: reason}`` for the DB auth files that could NOT be copied ({} = clean).""" + failed: dict[str, str] = {} for rel in _AUTH_REFRESH_PROFILE_FILES: s = os.path.join(src, source_profile, rel) - if os.path.isfile(s) and not _copy_auth_file(s, os.path.join(dst, "Default", rel)): - failed_dbs += os.path.basename(rel) in _SQLITE_AUTH_DBS - return failed_dbs + if not os.path.isfile(s): + continue + reason = _copy_auth_file(s, os.path.join(dst, "Default", rel)) + if reason and os.path.basename(rel) in _SQLITE_AUTH_DBS: + failed[rel] = reason + return failed + + +def _unavailable_auth_dbs_error(browser: str, failed: dict[str, str]) -> str: + """Fail-closed message naming WHICH auth databases could not be snapshotted and WHY. On + macOS/Linux a running browser typically lets Cookies back up but holds Login Data / Login Data + For Account / Web Data with a write lock, so the message must not read as "close the browser" + when the real cause is an unreadable file (and vice versa).""" + names = ", ".join(failed) + if all(reason == _AUTH_DB_LOCKED for reason in failed.values()): + return (f"{browser} is running and holds the profile's {names} with a write lock, so their " + "SQLite backup made no progress within five seconds. Hermes does not fall back to a " + "raw file copy (it could lose committed logins). Fully quit " + f"{browser} (including any background instance) and retry, or turn " + "browser.use_real_profile off.") + details = "; ".join(f"{name}: {reason}" for name, reason in failed.items()) + return (f"could not read the '{browser}' profile's login data ({details}). " + f"Close {browser} and retry, or turn browser.use_real_profile off.") _SNAPSHOT_DONE_MARKER = ".hermes-snapshot-complete" @@ -642,7 +669,8 @@ def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | N return None, resolve_err dst = real_profile_copy_dir(browser) # Fast lock probe BEFORE any copy: a blocking file op on a Windows-locked cookie DB can - # hang the launch for minutes. Never trips on POSIX, so copy-while-running still works. + # hang the launch for minutes. Never trips on POSIX; there a running browser surfaces later as + # auth DB backups that miss their deadline (``_unavailable_auth_dbs_error``). if _profile_is_locked(src, source_profile): return None, _locked_profile_error(browser) marker = os.path.join(dst, _SNAPSHOT_DONE_MARKER) @@ -661,9 +689,7 @@ def snapshot_real_profile(browser: str, src: str | None = None) -> tuple[str | N # Both paths: lock-aware auth DB copy into Default — also the per-launch re-sync. failed_dbs = _mirror_profile_auth(src, dst, source_profile) if failed_dbs: # even online-backup failed: never launch a silently signed-out session - return None, (f"could not read the '{browser}' profile's login data ({failed_dbs} " - f"database(s) unavailable). Close {browser} and retry, or turn " - "browser.use_real_profile off.") + return None, _unavailable_auth_dbs_error(browser, failed_dbs) # Never carry live-instance leftovers into the copy. for leftover in ("SingletonLock", "SingletonSocket", "SingletonCookie"): with contextlib.suppress(OSError): diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index 3acddad8fe..1d92dd6170 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -411,8 +411,9 @@ DEFAULT_CONFIG = { # Windows only: a running Chrome/Edge/Brave locks its cookie DB, so the profile can't be # copied. When on, a locked profile still blocks and the agent ASKS first; on approval it # runs `hermes browser close-profile` (kills that profile's browser tree, unsaved tabs lost) - # and retries once; still locked -> stays blocked, no auto-kill. No effect on macOS/Linux - # (copy-while-running works). + # and retries once; still locked -> stays blocked, no auto-kill. No effect on macOS/Linux, + # where a running browser instead makes the Login Data / Web Data SQLite backups miss + # their deadline; quit the browser by hand there. "real_profile_autoclose": False, # Pin WHICH source profile directory is snapshotted for real-profile browsing (e.g. "Profile # 2"). Empty = browser's last-used profile, which on multi-profile machines can hand the diff --git a/tests/hermes_cli/test_browser_connect_real_profile_snapshot_errors.py b/tests/hermes_cli/test_browser_connect_real_profile_snapshot_errors.py new file mode 100644 index 0000000000..b7aa74a942 --- /dev/null +++ b/tests/hermes_cli/test_browser_connect_real_profile_snapshot_errors.py @@ -0,0 +1,76 @@ +"""Real-profile snapshot error names the auth databases it could not read and why. + +A running Chrome on macOS/Linux lets ``Cookies`` back up but holds ``Login Data`` / +``Login Data For Account`` / ``Web Data`` with a write lock, so their SQLite online backup misses +its deadline. The launch must fail closed with the database NAMES and the lock reason, never a +bare "3 database(s) unavailable" count (#111647). +""" +import json +import os +import sqlite3 + +import pytest + +import hermes_cli.browser_connect as bc + +_LOCKED = ("Login Data", "Login Data For Account", "Web Data") + + +def _fake_profile(root): + (root / "Default" / "Network").mkdir(parents=True) + (root / "Local State").write_text(json.dumps({"profile": {"last_used": "Default"}})) + (root / "Default" / "Preferences").write_text("{}") + for name in ("Cookies", *_LOCKED): + con = sqlite3.connect(root / "Default" / name) + con.execute("create table t(x)") + con.execute("insert into t values(1)") + con.commit() + con.close() + + +def test_mirror_profile_auth_reports_locked_dbs_by_name(tmp_path, monkeypatch): + """Exclusive writers on the three login/autofill DBs → each is reported with the lock reason; + the unlocked Cookies DB still lands in the snapshot (no all-or-nothing regression).""" + root, dst = tmp_path / "real", tmp_path / "copy" + _fake_profile(root) + monkeypatch.setattr(bc, "_AUTH_BACKUP_DEADLINE_S", 0.3) # keep the test fast; 5 s in prod + holders = [] + for name in _LOCKED: + h = sqlite3.connect(root / "Default" / name) + h.execute("begin exclusive") + holders.append(h) + try: + failed = bc._mirror_profile_auth(str(root), str(dst), "Default") + finally: + for h in holders: + h.rollback() + h.close() + assert failed == {name: bc._AUTH_DB_LOCKED for name in _LOCKED} + assert os.path.isfile(dst / "Default" / "Cookies") + # Locks released → clean re-sync. + assert bc._mirror_profile_auth(str(root), str(dst), "Default") == {} + + +@pytest.mark.parametrize("reason, expect", [ + (None, ("is running and holds the profile's Login Data, Login Data For Account, Web Data " + "with a write lock", "Fully quit chrome")), + ("file is not a database", ("Login Data: file is not a database", "Close chrome")), +]) +def test_snapshot_error_names_databases_and_reason(tmp_path, monkeypatch, reason, expect): + """The user-facing snapshot error carries the failed DB names plus the reason: the lock + wording when every failure is the backup deadline, the SQLite error otherwise.""" + root = tmp_path / "real" + _fake_profile(root) + monkeypatch.setattr(bc, "get_hermes_home", lambda: tmp_path / "hh") + locked_reason = reason or bc._AUTH_DB_LOCKED + + def fake_copy(src, dst_file): + return locked_reason if os.path.basename(src) in _LOCKED else None + + monkeypatch.setattr(bc, "_copy_auth_file", fake_copy) + dst, err = bc.snapshot_real_profile("chrome", src=str(root)) + assert dst is None and err + for fragment in expect: + assert fragment in err, err + assert "database(s) unavailable" not in err + assert "Cookies" not in err # only the databases that actually failed are named diff --git a/tests/tools/test_browser_real_profile.py b/tests/tools/test_browser_real_profile.py index 2fde6b6803..ace36127ed 100644 --- a/tests/tools/test_browser_real_profile.py +++ b/tests/tools/test_browser_real_profile.py @@ -1066,7 +1066,7 @@ class TestWindowsLockedProfileCopy: src = str(tmp_path / "Cookies") con = sqlite3.connect(src); con.execute("create table cookies(x)"); con.execute("insert into cookies values(1)"); con.commit(); con.close() dst = str(tmp_path / "out" / "Cookies") - assert bc._copy_auth_file(src, dst) is True + assert bc._copy_auth_file(src, dst) is None assert sqlite3.connect(dst).execute("select count(*) from cookies").fetchone()[0] == 1 @pytest.mark.parametrize("locked", ["source", "destination"]) @@ -1093,14 +1093,14 @@ class TestWindowsLockedProfileCopy: str(src), str(dst)], capture_output=True, text=True, timeout=15, stdin=subprocess.DEVNULL) assert result.returncode == 0, result.stderr - assert result.stdout.strip() == "False" + assert result.stdout.strip() == bc._AUTH_DB_LOCKED finally: holder.rollback() holder.close() with sqlite3.connect(dst) as conn: assert conn.execute("select x from cookies").fetchall() == [(99,)] conn.close() - assert bc._copy_auth_file(str(src), str(dst)) is True + assert bc._copy_auth_file(str(src), str(dst)) is None with sqlite3.connect(dst) as conn: assert conn.execute("select x from cookies").fetchall() == [(7,)] conn.close() @@ -1129,7 +1129,7 @@ class TestWindowsLockedProfileCopy: str(dst)], check=True, timeout=15, stdin=subprocess.DEVNULL) assert os.path.exists(str(dst) + "-wal") try: - assert bc._copy_auth_file(str(src), str(dst)) is True + assert bc._copy_auth_file(str(src), str(dst)) is None with sqlite3.connect(dst) as conn: assert conn.execute("select x from cookies").fetchall() == [(8,)] conn.close() @@ -1140,7 +1140,7 @@ class TestWindowsLockedProfileCopy: import hermes_cli.browser_connect as bc src = str(tmp_path / "Preferences"); open(src, "w").write('{"k":1}') dst = str(tmp_path / "out" / "Preferences") - assert bc._copy_auth_file(src, dst) is True + assert bc._copy_auth_file(src, dst) is None assert open(dst).read() == '{"k":1}' def test_fail_closed_when_db_unreadable(self, tmp_path, monkeypatch): @@ -1157,7 +1157,7 @@ class TestWindowsLockedProfileCopy: monkeypatch.setattr(bc, "get_hermes_home", lambda: home) # Force both sqlite-backup and raw copy to fail for the DB. monkeypatch.setattr(bc, "_copy_auth_file", - lambda s, d: False if os.path.basename(s) in bc._SQLITE_AUTH_DBS else True) + lambda s, d: "file is not a database" if os.path.basename(s) in bc._SQLITE_AUTH_DBS else None) dst, err = bc.snapshot_real_profile("chrome", src=str(root)) assert dst is None assert err and "login data" in err.lower() and "close" in err.lower() diff --git a/website/docs/user-guide/features/browser.md b/website/docs/user-guide/features/browser.md index 2506a74ba3..1383392f20 100644 --- a/website/docs/user-guide/features/browser.md +++ b/website/docs/user-guide/features/browser.md @@ -217,23 +217,33 @@ When you turn the toggle back off, Hermes deletes the snapshot store (`~/.hermes/browser-profile/`) on the next browser use, so the copied credentials don't linger after you revoke consent. -:::note Windows: the browser must be fully closed +:::note A running browser can block the login/autofill databases On Windows a running Chrome/Edge/Brave holds its cookie and login databases with an exclusive (deny-all) lock, so Hermes cannot copy them while the browser is open — it fails fast with a "fully quit the browser and retry" message rather than hang or produce a signed-out session. Real-profile browsing on Windows therefore requires the browser **fully quit**, including any background/tray instance (Chrome's "continue running background apps when closed" keeps a -`chrome.exe` alive after you close the window). macOS and Linux can usually copy -the profile while the browser is running. On every platform, each authentication -database backup has a five-second retry budget. If the source or snapshot database -stays locked, Hermes stops the launch and asks you to close the browser and retry. -It preserves committed WAL data through SQLite rather than falling back to a raw -file copy, which could silently lose recent logins. Unreadable or corrupt databases -also stop the launch. +`chrome.exe` alive after you close the window). + +On macOS and Linux the profile is not file-locked, but each authentication +database is snapshotted through SQLite's online backup with a five-second +budget, and a running Chrome typically holds `Login Data`, `Login Data For +Account` and `Web Data` with a hot write lock that never yields within that +budget (`Cookies` usually snapshots fine). When that happens Hermes stops the +launch and names the databases it could not read — for example "chrome is +running and holds the profile's Login Data, Login Data For Account, Web Data +with a write lock" — so in practice **quit the browser before launching a +real-profile session on macOS/Linux too**. You can reopen it once the session is +up (the snapshot is a separate directory), but the auth files are re-synced on +every fresh session launch, so a browser left running then hits the same lock. +Hermes preserves committed +WAL data through SQLite rather than falling back to a raw file copy, which could +silently lose recent logins. Unreadable or corrupt databases also stop the +launch, with the SQLite error named in the message. Set `browser.real_profile_autoclose: true` to let Hermes **offer to close the -browser for you** when it's holding the profile. Even with this on, Hermes never +browser for you** when it's holding the profile lock (the Windows case). Even with this on, Hermes never closes it automatically — when the profile is locked it always stops and the agent asks you first; only on your approval does it run `hermes browser close-profile` (terminates the browser process tree bound to that profile,