From 273f44a04bc3755ee5946607f1231783261c8366 Mon Sep 17 00:00:00 2001 From: rajpreetcodes Date: Wed, 28 Oct 2026 13:01:09 +0530 Subject: [PATCH] fix: distinguish browser database locks from OS permission errors Fixes #16. Previously, _locked_note() mapped any PermissionError or errno 13 to a message claiming that the browser was running and its database was locked. This produced misleading diagnostics when: 1. macOS TCC / terminal privacy permission denials were reported as running browsers with locked databases. 2. Profile directory enumeration failures on uninstalled or restricted browser remnants (such as Edge or Firefox) were falsely reported as the browser being installed and running. 3. On macOS, Firefox paths under ~/Library/Application Support were reported generically as 'the browser' because the marker search only looked for 'mozilla'. This commit: - Introduces _is_db_locked() to isolate real database locks (SQLite locks or Windows WinError 32 sharing violations) from OS permission denials. - Differentiates file and directory permission failures, reporting access denials truthfully while preserving honest coverage gaps. - Adds actionable guidance on macOS suggesting terminal permissions in System Settings > Privacy & Security (Full Disk Access). - Adds 'firefox' and 'chromium' markers so macOS Firefox and Chromium roots are properly identified. - Adds test coverage in test_browser_launch.py verifying diagnostic notes and gap recording. --- scan/adint_scan/modules/browser.py | 112 +++++++++++++++++++++++------ scan/tests/test_browser_launch.py | 89 +++++++++++++++++++++++ 2 files changed, 178 insertions(+), 23 deletions(-) diff --git a/scan/adint_scan/modules/browser.py b/scan/adint_scan/modules/browser.py index 935e820..a56a023 100644 --- a/scan/adint_scan/modules/browser.py +++ b/scan/adint_scan/modules/browser.py @@ -347,33 +347,97 @@ def _fmt_time(conv, raw): return "" -def _locked_note(path, exc): - """A locked database is the most common non-error a user will see, so say - what it means and what to do rather than printing a raw errno. - - A running browser causes this two ways: the OS refuses the snapshot with a - sharing violation (PermissionError / errno 13 on Windows), or SQLite reports - "database is locked" when the copy is opened. Both mean the same thing to the - person reading the report: close the browser and run again.""" +def _is_db_locked(exc): + """True if exc represents an SQLite database locked by a running process.""" + msg = str(exc).lower() + if "database is locked" in msg or "database is busy" in msg: + return True + if sys.platform == "win32": + if getattr(exc, "winerror", None) in (32, 33): + return True + if "used by another process" in msg or "sharing violation" in msg: + return True + # Simulated locks in tests may raise PermissionError("locked") + if "locked" in msg and not ("permission denied" in msg or "operation not permitted" in msg or "access is denied" in msg): + return True + return False + + +def _is_permission_denied(exc): + """True if exc represents an OS permission or access control denial.""" + if isinstance(exc, PermissionError): + return True + if getattr(exc, "errno", None) in (1, 13): + return True + if getattr(exc, "winerror", None) == 5: + return True + msg = str(exc).lower() + return ("permission denied" in msg or "operation not permitted" in msg + or "access is denied" in msg) + + +def _locked_note(path, exc, browser_name=None, is_dir=None): + """A locked database is a common non-error when a browser is open, so say + what it means rather than printing a raw errno. When access is denied by OS + permissions (such as macOS privacy settings), report that distinction so the + user knows to check permissions rather than closing an unopen browser. + + A running browser causes a lock when: + 1. SQLite reports "database is locked" or "database is busy". + 2. Windows refuses the snapshot with a sharing violation (WinError 32). + + Access denied on directories or files without a sharing lock indicates OS + permission restrictions. On macOS, suggest checking terminal permissions.""" leftover = getattr(exc, "snapshot_leftover", "") cleanup_warning = ( " WARNING: temporary snapshot cleanup failed; a copy of browser data " "may remain at {}. Remove this temporary directory when its files can " "be released.".format(leftover) if leftover else "") - browser_name = "the browser" - lowered = path.lower() - for marker, label in (("bravesoftware", "Brave"), ("vivaldi", "Vivaldi"), - ("google", "Chrome"), ("microsoft", "Edge"), - ("mozilla", "Firefox")): - if marker in lowered: - browser_name = label - break - if (isinstance(exc, PermissionError) or getattr(exc, "errno", None) == 13 - or "locked" in str(exc).lower()): + + if not browser_name: + browser_name = "the browser" + lowered = path.lower() + for marker, label in (("bravesoftware", "Brave"), ("vivaldi", "Vivaldi"), + ("google", "Chrome"), ("microsoft", "Edge"), + ("mozilla", "Firefox"), ("firefox", "Firefox"), + ("chromium", "Chromium")): + if marker in lowered: + browser_name = label + break + + if is_dir is None: + try: + is_dir = os.path.isdir(path) + except OSError: + is_dir = False + + # A directory is never an SQLite database lock. + if is_dir: + if _is_permission_denied(exc): + if sys.platform == "darwin": + return ("access denied reading directory {}: {}. On macOS, ensure " + "the launching terminal app has permission in System Settings " + "> Privacy & Security (Full Disk Access)." + .format(path, exc) + cleanup_warning) + return ("access denied reading directory {}: {}" + .format(path, exc) + cleanup_warning) + return "could not read directory {}: {}".format(path, exc) + cleanup_warning + + # For files: distinguish actual database locks from permission denials. + if _is_db_locked(exc): return ("{} was running, so one of its databases was locked and could " "not be checked ({}). Close {} and run again to cover it." .format(browser_name, os.path.basename(path), browser_name) + cleanup_warning) + + if _is_permission_denied(exc): + if sys.platform == "darwin": + return ("access denied reading {}: {}. On macOS, ensure the launching " + "terminal app has permission in System Settings > Privacy & " + "Security (Full Disk Access).".format(path, exc) + + cleanup_warning) + return "access denied reading {}: {}".format(path, exc) + cleanup_warning + return "could not read {}: {}".format(path, exc) + cleanup_warning @@ -895,8 +959,9 @@ def scan_blobs(label, root, evidence): return # artifact absent for this profile — not a gap except OSError as exc: r.skipped += 1 - r.note(_locked_note(root, exc)) - mark_fail(label, kind, _locked_note(root, exc)) + note = _locked_note(root, exc, is_dir=True) + r.note(note) + mark_fail(label, kind, note) return failures = [] @@ -1173,7 +1238,7 @@ def scan_firefox_hsts(label, path): profs = chromium_profiles(root) except OSError as exc: # One bad ACL on the user-data root must not abort the whole module. - r.could_not_run(label + ":profiles", _locked_note(root, exc)) + r.could_not_run(label + ":profiles", _locked_note(root, exc, is_dir=True)) continue names = _chromium_profile_names(root) ctx.emit("[*] {} ({} profile(s))".format(root, len(profs))) @@ -1232,12 +1297,13 @@ def scan_firefox_hsts(label, path): folders.update(os.listdir(profiles_dir)) except OSError as exc: r.could_not_run("firefox:profiles", - _locked_note(profiles_dir, exc)) + _locked_note(profiles_dir, exc, is_dir=True)) if cache_is_dir: try: folders.update(os.listdir(cache_dir)) except OSError as exc: - r.could_not_run("firefox:cache-dir", _locked_note(cache_dir, exc)) + r.could_not_run("firefox:cache-dir", + _locked_note(cache_dir, exc, is_dir=True)) for name in sorted(folders): prof = os.path.join(profiles_dir, name) diff --git a/scan/tests/test_browser_launch.py b/scan/tests/test_browser_launch.py index 2f63079..3ba6fdc 100644 --- a/scan/tests/test_browser_launch.py +++ b/scan/tests/test_browser_launch.py @@ -36,6 +36,7 @@ import sys import tempfile import unittest +from unittest.mock import patch sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) @@ -321,5 +322,93 @@ def test_hsts_file_with_no_indicator_is_clean_not_a_hit(self): self.assertEqual(r.status, CLEAN) +class TestBrowserPermissionDiagnostics(BrowserLaunchBase): + def test_locked_note_distinguishes_db_lock_from_permission_denial(self): + # Actual database lock + lock_exc = sqlite3.OperationalError("database is locked") + note = browser._locked_note(r"C:\fake\Google\Chrome\Default\History", lock_exc) + self.assertIn("Chrome was running", note) + self.assertIn("databases was locked", note) + + # OS permission denial (non-macOS) + perm_exc = PermissionError(13, "Permission denied") + with patch.object(browser.sys, "platform", "linux"): + note = browser._locked_note("/home/u/.config/google-chrome/Default/History", perm_exc) + self.assertIn("access denied reading", note) + self.assertNotIn("was running", note) + self.assertNotIn("databases was locked", note) + + # OS permission denial on macOS includes Privacy & Security guidance + with patch.object(browser.sys, "platform", "darwin"): + note = browser._locked_note("/Users/u/Library/Application Support/Google/Chrome/Default/History", perm_exc) + self.assertIn("access denied reading", note) + self.assertIn("Privacy & Security", note) + self.assertIn("Full Disk Access", note) + self.assertNotIn("was running", note) + + def test_directory_access_denied_is_never_reported_as_running_browser(self): + dir_path = os.path.join(self.tmp, "fake_chrome_dir") + os.makedirs(dir_path, exist_ok=True) + perm_exc = PermissionError(13, "Permission denied") + + with patch.object(browser.sys, "platform", "darwin"): + note = browser._locked_note(dir_path, perm_exc, is_dir=True) + self.assertIn("access denied reading directory", note) + self.assertIn("Full Disk Access", note) + self.assertNotIn("was running", note) + self.assertNotIn("databases was locked", note) + + def test_chromium_profile_enumeration_permission_denied_records_gap_without_running_claim(self): + user_data = os.path.join(self.tmp, "User Data") + os.makedirs(user_data, exist_ok=True) + + def mock_profiles(_root): + raise PermissionError(13, "Permission denied") + + with patch.object(browser, "chromium_profiles", side_effect=mock_profiles): + r = self.run_chromium(user_data) + self.assertEqual(r.hits, []) + gaps = [c for c in r.checks if not c.ran] + self.assertTrue(gaps, "unreadable profile directory must be recorded as a gap") + prof_gap = next((c for c in gaps if c.name == "test:profiles"), None) + self.assertIsNotNone(prof_gap) + self.assertIn("access denied reading directory", prof_gap.reason) + self.assertNotIn("was running", prof_gap.reason) + self.assertNotIn("databases was locked", prof_gap.reason) + + def test_firefox_profiles_dir_permission_denied_records_gap_without_running_claim(self): + ff_dir = os.path.join(self.tmp, "Firefox_Profiles") + os.makedirs(ff_dir, exist_ok=True) + browser.FIREFOX_ROOTS = [(ff_dir, self.empty)] + + orig_listdir = os.listdir + + def mock_listdir(path): + if path == ff_dir: + raise PermissionError(13, "Permission denied") + return orig_listdir(path) + + with patch("os.listdir", side_effect=mock_listdir): + r = browser.run(self.ctx) + gaps = [c for c in r.checks if not c.ran] + prof_gap = next((c for c in gaps if c.name == "firefox:profiles"), None) + self.assertIsNotNone(prof_gap) + self.assertIn("access denied reading directory", prof_gap.reason) + self.assertNotIn("was running", prof_gap.reason) + + def test_browser_name_recognition_for_macos_firefox_and_chromium(self): + lock_exc = sqlite3.OperationalError("database is locked") + + # macOS Firefox path (which lacks "mozilla" marker) + mac_ff = "/Users/test/Library/Application Support/Firefox/Profiles/abc.default/places.sqlite" + note = browser._locked_note(mac_ff, lock_exc) + self.assertIn("Firefox was running", note) + + # Chromium path + chromium_path = "/home/test/.config/chromium/Default/History" + note = browser._locked_note(chromium_path, lock_exc) + self.assertIn("Chromium was running", note) + + if __name__ == "__main__": unittest.main(verbosity=2)