Hardening browsers.json config load

changedetectionio/model/browser_config.py

@@ -29,7 +29,7 @@ from os import path
 from typing import List, Optional
 
 from loguru import logger
-from pydantic import BaseModel, Field, field_validator
+from pydantic import BaseModel, Field, ValidationError, field_validator
 
 try:
     import orjson
@@ -172,7 +172,15 @@ class BrowserConfigStore:
 
     # ---- low level load/save ----
     def all(self):
-        """Raw dict {id: entry-dict}. Empty dict when the file is absent. mtime-cached."""
+        """Validated dict {id: entry-dict}. Empty dict when the file is absent. mtime-cached.
+
+        This is the single read-side validation gate: browsers.json can be hand-edited, restored
+        from an older/newer version, or corrupted, so every entry is coerced through
+        BrowserConfigEntry here rather than trusted raw. A malformed entry is dropped (with a
+        warning) instead of crashing every consumer downstream - callers get clean, normalized
+        dicts with the expected shape. Unknown extra keys inside browser_config are still tolerated
+        (FetcherConfig has no extra='forbid') for cross-version compatibility.
+        """
         try:
             mtime = os.path.getmtime(self._path)
         except OSError:
@@ -191,8 +199,19 @@ class BrowserConfigStore:
         except Exception as e:
             logger.error(f"Could not load browsers.json: {e}")
             return {}
-        self._cache, self._cache_mtime = data, mtime
-        return data
+        # Top level must be an id->entry mapping; anything else (a list, a scalar) is corrupt.
+        if not isinstance(data, dict):
+            logger.error(f"browsers.json is not a JSON object (got {type(data).__name__}) - ignoring")
+            self._cache, self._cache_mtime = {}, mtime
+            return {}
+        clean = {}
+        for cid, raw in data.items():
+            try:
+                clean[cid] = BrowserConfigEntry(**raw).model_dump()
+            except (ValidationError, TypeError) as e:
+                logger.warning(f"Dropping malformed browsers.json entry '{cid}': {e}")
+        self._cache, self._cache_mtime = clean, mtime
+        return clean
 
     def _save(self, configs):
         # Deferred import avoids a model -> store import cycle at module load.

changedetectionio/tests/test_browser_config.py

@@ -571,3 +571,48 @@ def test_group_override_with_builtin_browser(client, live_server, measure_memory
     uuid = datastore.add_watch(url="https://example.com", tag_uuids=[tag_uuid])
     override = resolve_browser_config_override(datastore.data['watching'][uuid], datastore)
     assert override is not None and override['config_id'] == 'html_webdriver'
+
+
+def test_browsers_json_corruption_is_tolerated(tmp_path):
+    """browsers.json can be hand-edited / restored / corrupted, so BrowserConfigStore.all() is the
+    single read-side validation gate: it never lets a malformed file take down its consumers.
+    Good entries survive (normalized), bad entries are dropped, extra keys are tolerated."""
+    import json
+    from changedetectionio.model.browser_config import BrowserConfigStore
+
+    def store_with(text):
+        p = tmp_path / "browsers.json"
+        p.write_text(text)
+        return BrowserConfigStore(str(tmp_path), lock=None)
+
+    # Syntactically broken JSON -> empty, never raises
+    assert store_with("{ not valid json ,,,").all() == {}
+
+    # Top level isn't an id->entry object (a list) -> empty, never raises
+    assert store_with(json.dumps([1, 2, 3])).all() == {}
+
+    # An entry that isn't even a dict -> dropped, never raises
+    assert store_with(json.dumps({"x": "i am a string"})).all() == {}
+
+    # An entry with a bad-typed inner field -> dropped
+    assert store_with(json.dumps({
+        "x": {"label": "L", "base_fetcher": "html_requests",
+              "browser_config": {"viewport_width": "not-a-number"}}
+    })).all() == {}
+
+    # Unknown extra keys (top level AND inside browser_config) are tolerated for version skew
+    good = store_with(json.dumps({
+        "keep": {"label": "Good", "base_fetcher": "html_requests", "is_default": True,
+                 "browser_config": {"timeout": 5, "future_field": "ignored"}}
+    })).all()
+    assert list(good) == ["keep"]
+    assert good["keep"]["label"] == "Good"
+    assert good["keep"]["browser_config"]["timeout"] == 5
+    assert "future_field" not in good["keep"]["browser_config"]  # unknown inner key stripped
+
+    # Mixed file: the good entry survives, only the bad one is dropped
+    mixed = store_with(json.dumps({
+        "good": {"label": "G", "base_fetcher": "html_requests", "browser_config": {}},
+        "bad": "nope",
+    })).all()
+    assert list(mixed) == ["good"]