diff --git a/src/server/desktop_serve.py b/src/server/desktop_serve.py index e2525e17..8b32652f 100644 --- a/src/server/desktop_serve.py +++ b/src/server/desktop_serve.py @@ -151,10 +151,12 @@ async def config(request: Request) -> Response: status_code=400) # The renderer wraps the payload as {"config": {...}} # (saveClawCodexConfig); accept the bare object too. - incoming = body.get("config") if isinstance(body.get("config"), dict) else body - return JSONResponse(_save_config_merged(incoming)) + return JSONResponse(_save_config_merged(unwrap_config_envelope(body))) - return JSONResponse(redact_secrets(load_config())) + # Strip a stale envelope on the way out too, so a config that already + # picked one up can't be round-tripped straight back by the renderer's + # whole-record autosave. + return JSONResponse(redact_secrets(strip_config_envelope(load_config()))) def _int_param(request: Request, name: str, default: int) -> int: try: @@ -510,6 +512,35 @@ async def not_found(request: Request) -> Response: return Starlette(routes=routes) +# ``config`` is not a key in the config schema — the file holds +# default_provider / providers / session / settings / env / projects / … at the +# top level. So a ``config`` key can only be a transport envelope that got +# stored by mistake, and treating it as data is what let one escape into +# ~/.clawcodex/config.json: the renderer autosaves the WHOLE record it last +# read, so a single mis-nested write is copied forward on every later save. +# It then shadows real settings — a voice/STT block written into the envelope +# is invisible to the backend, which reads the top level. + + +def unwrap_config_envelope(body: dict[str, Any]) -> dict[str, Any]: + """``{"config": {...}}`` → the record. Idempotent, and unwraps repeats. + + A double wrap is the shape that does the damage: unwrapping once leaves a + ``config`` key that then merges in as data. + """ + record = body + while isinstance(record, dict) and isinstance(record.get("config"), dict): + record = record["config"] + return record if isinstance(record, dict) else {} + + +def strip_config_envelope(record: dict[str, Any]) -> dict[str, Any]: + """Drop a stored envelope key. Returns the same object when there is none.""" + if not isinstance(record, dict) or "config" not in record: + return record + return {k: v for k, v in record.items() if k != "config"} + + def _save_config_merged(incoming: dict[str, Any]) -> dict[str, Any]: """Deep-merge the (secret-redacted) incoming config over the stored global config and persist it. @@ -525,8 +556,11 @@ def _save_config_merged(incoming: dict[str, Any]) -> dict[str, Any]: try: manager = _get_default_manager() current = manager.load_global() - merged = _deep_merge(current, incoming) - manager.save_global(merged) + merged = _deep_merge(current, strip_config_envelope(incoming)) + # `_deep_merge` only ever adds, so an envelope already on disk would + # outlive every future save. Dropping it here repairs a config that + # picked one up, on the next write, without a migration step. + manager.save_global(strip_config_envelope(merged)) manager.invalidate() return {"ok": True} except Exception as exc: # noqa: BLE001 diff --git a/tests/server/test_desktop_config_envelope.py b/tests/server/test_desktop_config_envelope.py new file mode 100644 index 00000000..aa505d17 --- /dev/null +++ b/tests/server/test_desktop_config_envelope.py @@ -0,0 +1,94 @@ +"""`PUT /api/config` must never store its own transport envelope. + +The renderer sends `{"config": {...}}` and autosaves the WHOLE record it last +read, so one mis-nested write is copied forward on every later save. A stored +`config` key then shadows real settings — a voice/STT block written inside it +is invisible to the backend, which reads the top level — and `_deep_merge` +only ever adds, so it survives every subsequent write. + +Seen in the wild: a config carrying a full stale snapshot under `config`, +with the live `stt` block stranded inside it. +""" + +from __future__ import annotations + +import json + +import pytest + +from src.server.desktop_serve import ( + _save_config_merged, + strip_config_envelope, + unwrap_config_envelope, +) + + +def test_unwraps_the_renderers_envelope() -> None: + assert unwrap_config_envelope({"config": {"stt": {"enabled": True}}}) == { + "stt": {"enabled": True} + } + + +def test_unwraps_a_double_envelope() -> None: + """The shape that caused the damage: unwrapping once leaves a `config` + key behind, which then merges in as data.""" + assert unwrap_config_envelope({"config": {"config": {"stt": {"enabled": True}}}}) == { + "stt": {"enabled": True} + } + + +def test_leaves_a_bare_record_alone() -> None: + record = {"providers": {"openai": {"base_url": "u"}}} + + assert unwrap_config_envelope(record) == record + + +def test_strip_is_a_no_op_without_an_envelope() -> None: + record = {"stt": {"enabled": True}} + + assert strip_config_envelope(record) is record + + +@pytest.fixture +def config_file(tmp_path, monkeypatch): + path = tmp_path / "config.json" + monkeypatch.setattr("src.config.get_global_config_path", lambda: path) + import src.config as config_mod + + config_mod._get_default_manager().invalidate() + monkeypatch.setattr(config_mod, "GLOBAL_CONFIG_FILE", path, raising=False) + + return path + + +def test_a_wrapped_save_lands_at_the_top_level(config_file) -> None: + config_file.write_text(json.dumps({"providers": {"openai": {"api_key": "secret"}}})) + + result = _save_config_merged( + unwrap_config_envelope({"config": {"stt": {"enabled": True, "provider": "openai"}}}) + ) + + assert result["ok"] is True + saved = json.loads(config_file.read_text()) + assert saved["stt"] == {"enabled": True, "provider": "openai"} + assert "config" not in saved + # The redacted round-trip must not cost the user their credentials. + assert saved["providers"]["openai"]["api_key"] == "secret" + + +def test_an_existing_envelope_is_repaired_on_the_next_save(config_file) -> None: + """No migration step: the next ordinary write cleans up a config that + already picked one up.""" + config_file.write_text( + json.dumps({ + "providers": {"openai": {"api_key": "secret"}}, + "config": {"stt": {"enabled": True}, "logoColor": "ocean"}, + }) + ) + + _save_config_merged(unwrap_config_envelope({"config": {"logoColor": "sunset"}})) + + saved = json.loads(config_file.read_text()) + assert "config" not in saved + assert saved["logoColor"] == "sunset" + assert saved["providers"]["openai"]["api_key"] == "secret"