fix: distinguish browser database locks from OS permission errors

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)

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)