fix: use deepcopy for DEFAULT_SETTINGS and narrow exception catch in settings module
- Replace DEFAULT_SETTINGS.copy() with copy.deepcopy(DEFAULT_SETTINGS) in load_settings() and save_settings() to prevent mutation of module-level mutable defaults (hidden_sessions list was shared via shallow copy) - Replace except (json.JSONDecodeError, Exception) with except (FileNotFoundError, json.JSONDecodeError) to only suppress expected errors and let unexpected exceptions (PermissionError etc.) propagate - Add two regression tests: test_load_does_not_mutate_default_settings: verifies deepcopy isolation test_load_propagates_non_json_errors: verifies narrow exception handling
This commit is contained in:
+4
-5
@@ -4,6 +4,7 @@ Server-side settings management for muxplex.
|
|||||||
Settings are stored at ~/.config/muxplex/settings.json.
|
Settings are stored at ~/.config/muxplex/settings.json.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import copy
|
||||||
import json
|
import json
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -25,16 +26,14 @@ def load_settings() -> dict:
|
|||||||
Returns DEFAULT_SETTINGS if the file does not exist or contains corrupt JSON.
|
Returns DEFAULT_SETTINGS if the file does not exist or contains corrupt JSON.
|
||||||
Unknown keys in the file are ignored.
|
Unknown keys in the file are ignored.
|
||||||
"""
|
"""
|
||||||
result = DEFAULT_SETTINGS.copy()
|
result = copy.deepcopy(DEFAULT_SETTINGS)
|
||||||
try:
|
try:
|
||||||
text = SETTINGS_PATH.read_text()
|
text = SETTINGS_PATH.read_text()
|
||||||
data = json.loads(text)
|
data = json.loads(text)
|
||||||
for key in DEFAULT_SETTINGS:
|
for key in DEFAULT_SETTINGS:
|
||||||
if key in data:
|
if key in data:
|
||||||
result[key] = data[key]
|
result[key] = data[key]
|
||||||
except FileNotFoundError:
|
except (FileNotFoundError, json.JSONDecodeError):
|
||||||
pass
|
|
||||||
except (json.JSONDecodeError, Exception):
|
|
||||||
pass
|
pass
|
||||||
return result
|
return result
|
||||||
|
|
||||||
@@ -45,7 +44,7 @@ def save_settings(data: dict) -> None:
|
|||||||
Creates parent directories as needed. Writes JSON with indent=2 and a
|
Creates parent directories as needed. Writes JSON with indent=2 and a
|
||||||
trailing newline.
|
trailing newline.
|
||||||
"""
|
"""
|
||||||
merged = DEFAULT_SETTINGS.copy()
|
merged = copy.deepcopy(DEFAULT_SETTINGS)
|
||||||
for key in DEFAULT_SETTINGS:
|
for key in DEFAULT_SETTINGS:
|
||||||
if key in data:
|
if key in data:
|
||||||
merged[key] = data[key]
|
merged[key] = data[key]
|
||||||
|
|||||||
@@ -116,3 +116,29 @@ def test_patch_settings_ignores_unknown_keys(tmp_path, monkeypatch):
|
|||||||
|
|
||||||
assert "unknown_key" not in result
|
assert "unknown_key" not in result
|
||||||
assert result["sort_order"] == "alpha"
|
assert result["sort_order"] == "alpha"
|
||||||
|
|
||||||
|
|
||||||
|
def test_load_does_not_mutate_default_settings():
|
||||||
|
"""Mutating the list returned by load_settings() must not corrupt DEFAULT_SETTINGS."""
|
||||||
|
result = load_settings()
|
||||||
|
# Mutate the returned hidden_sessions list
|
||||||
|
result["hidden_sessions"].append("leaked_session")
|
||||||
|
|
||||||
|
# DEFAULT_SETTINGS must be unchanged
|
||||||
|
assert DEFAULT_SETTINGS["hidden_sessions"] == []
|
||||||
|
|
||||||
|
# A second load must still return the clean default
|
||||||
|
result2 = load_settings()
|
||||||
|
assert result2["hidden_sessions"] == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_load_propagates_non_json_errors(monkeypatch):
|
||||||
|
"""load_settings() must not swallow unexpected errors (e.g. PermissionError)."""
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
mock_path = MagicMock()
|
||||||
|
mock_path.read_text.side_effect = PermissionError("no read permission")
|
||||||
|
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", mock_path)
|
||||||
|
|
||||||
|
with pytest.raises(PermissionError):
|
||||||
|
load_settings()
|
||||||
|
|||||||
Reference in New Issue
Block a user