diff --git a/api/config.py b/api/config.py index c82fcf7..04f6c2d 100644 --- a/api/config.py +++ b/api/config.py @@ -1232,7 +1232,14 @@ _SETTINGS_LEGACY_DROP_KEYS = {"assistant_language"} def load_settings() -> dict: """Load settings from disk, merging with defaults for any missing keys.""" settings = dict(_SETTINGS_DEFAULTS) - if SETTINGS_FILE.exists(): + try: + settings_exists = SETTINGS_FILE.exists() + except OSError: + # PermissionError or other OS-level error (e.g. UID mismatch in Docker) + # Treat as missing — start with defaults rather than crashing. + logger.debug("Cannot stat settings file %s (inaccessible?)", SETTINGS_FILE) + settings_exists = False + if settings_exists: try: stored = json.loads(SETTINGS_FILE.read_text(encoding="utf-8")) if isinstance(stored, dict): @@ -1316,7 +1323,11 @@ def save_settings(settings: dict) -> dict: # config must never shadow an explicit env-var override (Docker deployments # rely on this — otherwise deleting settings.json is the only escape). _startup_settings = load_settings() -if SETTINGS_FILE.exists(): +try: + _settings_file_exists = SETTINGS_FILE.exists() +except OSError: + _settings_file_exists = False +if _settings_file_exists: if _startup_settings.get("default_model"): DEFAULT_MODEL = _startup_settings["default_model"] if not os.getenv("HERMES_WEBUI_DEFAULT_WORKSPACE"): diff --git a/tests/test_issue570_permission.py b/tests/test_issue570_permission.py new file mode 100644 index 0000000..541036c --- /dev/null +++ b/tests/test_issue570_permission.py @@ -0,0 +1,56 @@ +""" +Regression tests for GitHub issue #570 follow-up: +PermissionError from SETTINGS_FILE.exists() in Docker UID-mismatch scenarios. + +When ~/.hermes is owned by a different UID than the container user (common in +Docker setups), Path.exists() raises PermissionError instead of returning False. +load_settings() must treat that as "file not accessible = use defaults" rather +than propagating the exception up to crash the request handler. +""" +import stat +import pytest +import api.config as config + + +def test_load_settings_returns_defaults_when_settings_file_unreadable(monkeypatch, tmp_path): + """PermissionError from SETTINGS_FILE.exists() must not propagate — return defaults instead. + + Regression for issue #570 comment: Docker UID mismatch caused every request + to 500 because load_settings() called SETTINGS_FILE.exists() without catching OSError. + """ + state_dir = tmp_path / "state" + state_dir.mkdir() + settings_file = state_dir / "settings.json" + # Create the file then make the parent unreadable so .exists() raises PermissionError + settings_file.write_text('{"send_key": "ctrl+enter"}', encoding="utf-8") + state_dir.chmod(stat.S_IWUSR) # write-only: stat() on children will fail + + monkeypatch.setattr(config, "SETTINGS_FILE", settings_file) + + try: + result = config.load_settings() + # Must not raise; must return a dict with default values + assert isinstance(result, dict) + assert "send_key" in result + # The corrupted/inaccessible value should NOT appear — defaults win + assert result["send_key"] == config._SETTINGS_DEFAULTS["send_key"] + finally: + state_dir.chmod(stat.S_IRWXU) # restore for cleanup + + +def test_load_settings_returns_defaults_when_exists_raises_permission_error(monkeypatch, tmp_path): + """Direct simulation: monkeypatch SETTINGS_FILE.exists to raise PermissionError.""" + from unittest import mock + + state_dir = tmp_path / "state" + state_dir.mkdir() + settings_file = state_dir / "settings.json" + + monkeypatch.setattr(config, "SETTINGS_FILE", settings_file) + + with mock.patch.object(type(settings_file), "exists", + side_effect=PermissionError("Permission denied")): + result = config.load_settings() + + assert isinstance(result, dict) + assert result["send_key"] == config._SETTINGS_DEFAULTS["send_key"]