Skip to content

Commit 7acf41e

Browse files
fix: reject malformed feature-flags payload instead of silent no-op (#1865)
* fix: reject malformed feature-flags payload instead of silent no-op POST /api/settings/features defaulted a missing `flags` key to `{}`, so a flat body (e.g. {"enable_lite_docstrings": true}) silently applied nothing yet returned success=true/restart_required=true — worse than a visible no-op, since the caller believed the flag changed and a restart was pending when nothing happened. Require the nested {"flags": {...}} shape: reject a body without a `flags` object (400 with the correct shape in the message) and reject an empty `flags` object. The web UI always POSTs {"flags": {[field]: value}}, so only malformed external/scripted callers are affected. Closes #1840 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: add actionable suggestions to feature-flags shape errors Address Gemini review: the two new VALIDATION_INVALID_PARAMETER responses now carry a `suggestions` entry per the styleguide's progressive-disclosure rule, guiding the caller to the correct {"flags": {...}} shape. Regression tests assert the suggestion is present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e234a6e commit 7acf41e

2 files changed

Lines changed: 83 additions & 2 deletions

File tree

src/ha_mcp/settings_ui/__init__.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1982,12 +1982,39 @@ async def _save_feature_flags(request: Request) -> JSONResponse:
19821982
),
19831983
status_code=400,
19841984
)
1985-
raw_flags = body.get("flags", {})
1985+
# Require the nested ``{"flags": {...}}`` shape. A flat body
1986+
# (e.g. ``{"enable_lite_docstrings": true}``) has no ``flags``
1987+
# key, so a lenient ``body.get("flags", {})`` default silently
1988+
# dropped every field and still returned
1989+
# ``success=True``/``restart_required=True`` — worse than a
1990+
# visible no-op, because the caller believed the flag changed
1991+
# and a restart was pending when nothing happened (#1840).
1992+
raw_flags = body.get("flags")
19861993
if not isinstance(raw_flags, dict):
19871994
return JSONResponse(
19881995
create_error_response(
19891996
ErrorCode.VALIDATION_INVALID_PARAMETER,
1990-
"'flags' must be an object mapping field names to values",
1997+
"Request body must contain a 'flags' object mapping "
1998+
'field names to values, e.g. {"flags": '
1999+
'{"enable_lite_docstrings": true}}. A flat body such '
2000+
'as {"enable_lite_docstrings": true} is not accepted.',
2001+
suggestions=[
2002+
'Wrap the flags in a "flags" object, e.g. '
2003+
'{"flags": {"enable_lite_docstrings": true}}.',
2004+
],
2005+
),
2006+
status_code=400,
2007+
)
2008+
if not raw_flags:
2009+
return JSONResponse(
2010+
create_error_response(
2011+
ErrorCode.VALIDATION_INVALID_PARAMETER,
2012+
"'flags' object is empty; include at least one "
2013+
"feature-flag field to update.",
2014+
suggestions=[
2015+
'Include at least one field inside "flags", e.g. '
2016+
'{"flags": {"enable_lite_docstrings": true}}.',
2017+
],
19912018
),
19922019
status_code=400,
19932020
)

tests/src/unit/test_settings_ui.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2174,6 +2174,60 @@ async def test_read_only_mode_save_round_trips_into_settings(
21742174
get_data_dir.cache_clear()
21752175
_reset_global_settings()
21762176

2177+
@pytest.mark.asyncio
2178+
async def test_flat_body_rejected_not_silent_noop(self, monkeypatch, tmp_path):
2179+
"""A flat body (missing the ``flags`` wrapper) must 400, not
2180+
silently no-op with ``success``/``restart_required`` (#1840).
2181+
2182+
Before the fix, ``POST {"enable_lite_docstrings": true}``
2183+
returned ``{"success": true, "applied": {}, "mode": "file",
2184+
"restart_required": true}`` — worse than a plain no-op, since a
2185+
caller believed the flag changed and a restart was pending when
2186+
nothing happened. No override file should be written either.
2187+
"""
2188+
post_handler = self._capture_post_handler(monkeypatch, tmp_path)
2189+
2190+
resp = await post_handler(self._make_request({"enable_lite_docstrings": True}))
2191+
2192+
assert resp.status_code == 400
2193+
body = json.loads(resp.body)
2194+
assert body["success"] is False
2195+
assert body["error"]["code"] == "VALIDATION_INVALID_PARAMETER"
2196+
# Progressive disclosure: the error guides the caller to the shape.
2197+
assert body["error"]["suggestion"]
2198+
# A rejected save must not persist anything.
2199+
assert not (tmp_path / "feature_flags.json").exists()
2200+
2201+
@pytest.mark.asyncio
2202+
async def test_empty_flags_object_rejected(self, monkeypatch, tmp_path):
2203+
"""An explicitly empty ``flags`` object applies nothing and must
2204+
400 rather than falsely reporting ``restart_required`` (#1840)."""
2205+
post_handler = self._capture_post_handler(monkeypatch, tmp_path)
2206+
2207+
resp = await post_handler(self._make_request({"flags": {}}))
2208+
2209+
assert resp.status_code == 400
2210+
body = json.loads(resp.body)
2211+
assert body["success"] is False
2212+
assert body["error"]["code"] == "VALIDATION_INVALID_PARAMETER"
2213+
assert body["error"]["suggestion"]
2214+
assert not (tmp_path / "feature_flags.json").exists()
2215+
2216+
@pytest.mark.asyncio
2217+
async def test_non_dict_flags_rejected(self, monkeypatch, tmp_path):
2218+
"""``flags`` present but not an object (e.g. a list) must 400."""
2219+
post_handler = self._capture_post_handler(monkeypatch, tmp_path)
2220+
2221+
resp = await post_handler(
2222+
self._make_request({"flags": ["enable_lite_docstrings"]})
2223+
)
2224+
2225+
assert resp.status_code == 400
2226+
body = json.loads(resp.body)
2227+
assert body["success"] is False
2228+
assert body["error"]["code"] == "VALIDATION_INVALID_PARAMETER"
2229+
assert not (tmp_path / "feature_flags.json").exists()
2230+
21772231

21782232
class TestSaveToolsResponseShape:
21792233
"""Pins the unified ``{success, applied, mode, restart_required}``

0 commit comments

Comments
 (0)