Commit 96bbb3d
feat: let operators extend the YAML write allowlist behind a deny floor (#1997)
* feat: let operators extend the YAML write allowlist behind a deny floor
ha_config_set_yaml could only write the hardcoded ALLOWED_YAML_KEYS, so a
YAML-first integration that is valid on one install (alert2 in the filed
case) was unreachable unless its key was added upstream for everyone.
Adds an operator setting, "Extra YAML write keys" (HA_MCP_EXTRA_YAML_KEYS),
holding a comma-separated key list that widens the allowlist for that
install only. Because the setting hands allowlist control to the operator,
it ships together with the floor that bounds it: YAML_KEY_DENYLIST
(homeassistant, http, frontend) is checked before every single-key accept
branch and is not operator-extendable. Those three redefine Home Assistant's
own trust boundary rather than merely being powerful: auth_providers /
auth_mfa_modules and the packages root under homeassistant:, trusted_proxies
/ cors_allowed_origins / ip_ban_enabled under http:, and extra_module_url
under frontend:, which loads JavaScript into the authenticated dashboard.
Keys such as command_line and shell_command stay allowed - that surface is
already accepted today.
Enforcement stays in the custom component, the layer that authorizes the
write; the server passes the operator's set on the wire as extra_allowed_keys
and does not mirror the denylist, so there is one copy to keep correct.
Notes on the edges:
- extra_allowed_keys is only sent when the operator configured keys, so the
strict service schema of an older component is never handed an unknown
field under default configuration. When keys ARE configured and the
component predates them, a feature-scoped guard returns an actionable
update prompt. MIN_COMPONENT_VERSION is deliberately not bumped: an
operator who does not use this must not be forced to update.
- The backup restore path passes the same set. A write allowed only by the
extra keys is auto-snapshotted, so omitting them there would have made
that snapshot unrestorable.
- The parser lives in config.py rather than beside the YAML tool so
backup_manager does not gain an import edge into tools_*.
- The setting is registered in ADVANCED_SETTINGS_FIELDS (new beta_yamlkeys
section) and renders as a text row nested under "Enable YAML config
editing", reusing the code-mode sub-row renderer, which is generalized
here rather than duplicated.
Component 1.2.3 -> 1.2.4 for the new schema field.
Closes #1887
Adversarial review pass folded in before first push:
- the version gate now reads the component version from the REST bootstrap
cache instead of the WebSocket capability handshake, which returns None for
a transport blip as well as for an old component and would have reported a
momentary socket drop as "component too old" while blocking writes of
ordinary built-in keys
- the restore path carries the same gate, so an old component fails there with
the actionable prompt rather than an opaque schema rejection
- lovelace joins the deny floor: its resources option loads JS modules into
the authenticated dashboard under resource_mode yaml, the same primitive
frontend is denied for. Only the bare key; lovelace.dashboards.<url_path>
is a separate validated shape and stays available
- the settings read drops its getattr default so a future rename raises
instead of silently reporting "operator configured nothing"
Test-coverage pass folded in after that: three links in the chain were
reachable-but-unpinned - deleting the version-cache write, the handler's
forwarding of the caller's extra keys, or the schema entry itself each left
the whole suite green while the feature was dead or the call rejected
wholesale. Each now has a test that goes red when it is removed, the version
gate via the real REST bootstrap rather than a hand-seeded cache.
Documents the setting in docs/beta.md, where the tool safety model already
lives.
* fix: heal the extra-YAML-keys gate after a component update
Three follow-ups on the first round of review and CI.
The version gate could latch a stale answer. The caller-token cache is keyed
by the long-lived REST client and survives a Home Assistant restart, so once
this process had bootstrapped against an older component, updating that
component never refreshed the reported version: every extra-key write kept
failing, and the remediation the error itself prints ("update, then restart
Home Assistant") could not take effect without also restarting ha-mcp. The
gate now re-bootstraps once before blocking, so the update heals it on the
next call. Only on the failure path, so a satisfied gate costs nothing.
The e2e security test asserted the generic allowlist message for
`homeassistant`, which now takes the deny-floor branch instead. The probe
moves to a genuinely unknown key so it keeps testing the generic path, and a
new case covers the floor's own message for all four denied keys. This was
the single cause behind every red E2E lane, HAOS included.
The new suggestion string was two implicitly concatenated literals inside a
list, which reads as a missing comma and tripped the CodeQL quality gate.
Parenthesised so the intent is explicit.
* fix: keep packages-only keys out of configuration.yaml under extra keys
The extra-key setting is documented as additive on top of ALLOWED_YAML_KEYS,
but its branch sat above the packages-only rejection, so it was additive on
top of ALLOWED_YAML_KEYS | PACKAGES_ONLY_YAML_KEYS instead. Listing
`automation` in the setting made it writable in configuration.yaml, where it
has never been writable.
That routes around two things at once. The storage-mode/YAML-mode collision
guarantee only holds while automation, script and scene stay confined to
packages/*.yaml. And the per-key toggles that govern them are checked only
for package targets, on both the wrapper and the component side, so a
configuration.yaml write was not covered by the toggle an operator would
expect to be in charge.
Nobody designed that behaviour and the comment above the constant described
the opposite, so this restores what the comment already claimed rather than
narrowing anything: those keys still reach packages/*.yaml through their own
branch and their own toggle, and configuration.yaml still answers with the
storage-mode advisory.
Pinned in both directions at unit level and, since the earlier round showed
the e2e surface can drift unnoticed, in the e2e security suite too.
* docs: point two stale test references at the tests that actually exist
Both comments name a test as the thing that pins an invariant, and both send
the reader somewhere the test is not:
- tools_yaml_config.py cited the flag-map parity test in
test_yaml_config_tool.py; it lives in test_yaml_dashboards.py.
- const.py cited a TestManifestVersionParity class; the manifest/constant
parity assertion is TestInfo::test_manifest_version_parity in
test_component_ws_search.py.
Comment-only. Both files are already touched by this PR, and a pointer that
resolves to nothing is worth less than no pointer at all.
* fix: floor extra-YAML-keys at pending component 1.2.3 instead of opening 1.2.4
1.2.2 is the released stable and 1.2.3 is the unshipped pending version that
becomes the next stable, so the first 1.2.3 build anyone receives already
carries the extra_allowed_keys service field. Flooring the feature at 1.2.3
is therefore safe, and it keeps the component in step with the release cycle
instead of skipping 1.2.3 as a stable number.
Reverts the manifest/COMPONENT_VERSION bump to 1.2.3, sets
MIN_COMPONENT_VERSION_EXTRA_YAML_KEYS = "1.2.3", and realigns everything that
rides that number: the manifest-parity and component-version asserts, the gate
tests' old/current pairs (old build 1.2.2, current 1.2.3, floor 1.2.3 in the
error), docs/beta.md, the three locales, and the settings.js help text. Also
renames a stale renderCodeModeSubRows comment reference in settings.js.
* test: cover a successful operator-extra-key write against the real component
Every rejection path is e2e-verified, but the feature's actual capability, a
non-built-in key writing successfully, was asserted only against a stubbed
dispatch client with voluptuous mocked, plus a source guard on the schema line.
A wire or schema interaction bug on the success path would pass the whole suite
and surface only on a live install.
Adds test_extra_key_write_succeeds_against_real_component, mirroring
test_add_knx_to_package_file: it writes a key (alert2) that is reachable only
through the operator extra-keys setting and asserts success plus
post_action=restart_required. To give the write a key, HA_MCP_EXTRA_YAML_KEYS
is wired into the in-process/container server's boot env and the embedded
server's feature_flags.json override (read by _apply_advanced_overrides). The
inaddon HAOS backend has no Supervisor option for this setting, which is a
web-UI plus env-var setting by design, so it boots without the key and the
test skips there rather than assert a capability it was never given.
* docs: correct the extra-keys denylist-drop rationale and a stale JS fn ref
_caller_extra_allowed_keys said a caller sending a denied key gets the same
"not in the allowed list" answer as before. That is inaccurate: a direct write
to a denied key always takes the categorical floor message, because
_parse_and_validate_yaml_path checks the denylist first, before any allow-set.
The drop's real job is keeping denied keys out of the allowed listing shown in
the generic rejection for some other invalid key. Rewords the docstring to
match the behavior its own test (test_denylist_key_rejected_even_when_extra_allowed)
already proves.
Also updates two comments that still named renderCodeModeSubRows after it was
renamed to renderAdvancedSubRows.
* fix: restore the extra-YAML-keys floor to 1.2.4 now that 1.2.3 has shipped
1.2.3 released as a stable component on release day carrying no
extra_allowed_keys field, since this PR is still unmerged, and 1.2.3-dev
builds were already public without it. Flooring the feature at 1.2.3
would repeat the #1946 trap: an operator on a shipped 1.2.3 who sets
extra keys would clear the version gate and then hit the component's
opaque schema rejection. Bump the component to 1.2.4 and floor the
feature there, so 1.2.4 is the first version that actually carries the
field.
Restores manifest / COMPONENT_VERSION to 1.2.4,
MIN_COMPONENT_VERSION_EXTRA_YAML_KEYS to 1.2.4, and the numbers that ride
it: the manifest-parity and component-version asserts, the gate tests'
old/current pairs (old build 1.2.3, current 1.2.4, floor 1.2.4 in the
error message), docs/beta.md, the three locales, and the settings.js
help text. Keeps the renderAdvancedSubRows comment rename.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: store extra YAML write keys in the component, enforced there (#1887)
Add a component-owned extra-keys store (ha_mcp_tools_extra_yaml_keys),
mirroring the allowed-paths store: loaded into hass.data at setup, updated
live by a new set_extra_yaml_keys service, and read at enforcement. The
edit_yaml_config handler now unions the per-call wire keys with the stored
keys, so a key configured on the component takes effect without the server
having to send it. A matching get_extra_yaml_keys service exposes the store
(admin + caller-token gated) so the ha-mcp server and the integration's own
options flow can read it.
The store can never widen the deny floor: YAML_KEY_DENYLIST members are
stripped on save and re-validated on load, and _parse_and_validate_yaml_path
re-checks the floor at enforcement regardless. Removing the store's denylist
filter turns TestNormalizeExtraYamlKeys and TestLoadExtraYamlKeys red
(verified by inject-and-revert).
This is the component half of surfacing the setting in the integration UI;
the options-flow editor and the server-side union follow.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: edit the file dirs and extra YAML keys from the integration UI (#1887)
The tools-entry options flow ("HA-MCP File & YAML Tools") is no longer a
"nothing to configure here yet" placeholder: it now edits the extra
file-access directories and the extra YAML write keys directly, so both are
reachable from the integration UI and not only the ha-mcp server's own
settings. Both fields read and write the component's own .storage, the same
source of truth the server settings UI edits, applied live with no restart.
Persistence goes through shared _apply_allowed_paths / _apply_extra_yaml_keys
helpers, which the set_allowed_paths / set_extra_yaml_keys services now call
too, so the validated normalize-and-hot-swap path has a single copy. The deny
floor is unchanged: traversal / out-of-config directories and denylisted keys
are dropped on save.
Updates strings.json and the en/de translations for the new form fields.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: union the component extra-keys store into the server allowlist (#1887)
The operator can now set extra YAML write keys in the integration UI, where
they live in the component's own store. The server's pre-dispatch allowlist
check would otherwise reject such a key before the write reached the
component, so effective_extra_yaml_write_keys reads that store (via the
version-gated get_extra_yaml_keys service) and unions it with the server's
own HA_MCP_EXTRA_YAML_KEYS. Both the write path (ha_config_set_yaml) and the
backup restore path use the union, and the version gate and the wire send
follow the effective set.
On a component too old to expose the service, or any read failure, the union
falls back to the server setting alone and assert_extra_yaml_keys_supported
turns a real mismatch into an actionable prompt. The store is read per write,
not cached, since the options flow changes it at runtime. Removing the store
union turns the new TestEffectiveExtraYamlWriteKeys union cases red (verified
by inject-and-revert).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(beta): note extra YAML write keys are editable in the integration UI (#1887)
The setting can now be configured from the "HA-MCP File & YAML Tools"
integration options in addition to the server settings UI and the env var;
the two sets are unioned. Documented alongside the existing deny-floor note.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: e2e coverage for extra YAML keys set via the component store (#1887)
The store path (integration-UI setting via set_extra_yaml_keys) had only
mocked-Store unit tests plus source-string asserts; the sole real-component
e2e success test seeds the key via HA_MCP_EXTRA_YAML_KEYS, never through the
store. Add a cross-lane e2e that configures a key solely through the real
set_extra_yaml_keys service (env absent), proves it writes live, and proves
the deny floor drops a denylisted key at save and still refuses its write.
---------
Co-authored-by: kingpanther13 <25392815+kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>1 parent 9548581 commit 96bbb3d
26 files changed
Lines changed: 2021 additions & 80 deletions
File tree
- custom_components/ha_mcp_tools
- translations
- docs
- src/ha_mcp
- settings_ui
- locales
- tools
- tests/src
- e2e
- workflows/filesystem
- unit
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
241 | 241 | | |
242 | 242 | | |
243 | 243 | | |
244 | | - | |
| 244 | + | |
245 | 245 | | |
246 | | - | |
247 | | - | |
248 | | - | |
249 | | - | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
250 | 254 | | |
251 | 255 | | |
252 | 256 | | |
253 | | - | |
254 | | - | |
| 257 | + | |
| 258 | + | |
255 | 259 | | |
256 | 260 | | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
257 | 288 | | |
258 | 289 | | |
259 | 290 | | |
260 | | - | |
261 | | - | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
262 | 295 | | |
263 | 296 | | |
264 | 297 | | |
265 | 298 | | |
266 | | - | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
267 | 308 | | |
268 | 309 | | |
269 | 310 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
24 | | - | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
25 | 26 | | |
26 | 27 | | |
27 | 28 | | |
| |||
96 | 97 | | |
97 | 98 | | |
98 | 99 | | |
99 | | - | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
100 | 105 | | |
101 | 106 | | |
102 | 107 | | |
| |||
143 | 148 | | |
144 | 149 | | |
145 | 150 | | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
146 | 196 | | |
147 | 197 | | |
148 | 198 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
210 | 210 | | |
211 | 211 | | |
212 | 212 | | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
31 | 39 | | |
32 | 40 | | |
33 | 41 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
31 | 39 | | |
32 | 40 | | |
33 | 41 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
31 | 39 | | |
32 | 40 | | |
33 | 41 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
| 64 | + | |
| 65 | + | |
64 | 66 | | |
65 | 67 | | |
66 | 68 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2128 | 2128 | | |
2129 | 2129 | | |
2130 | 2130 | | |
| 2131 | + | |
| 2132 | + | |
| 2133 | + | |
| 2134 | + | |
| 2135 | + | |
| 2136 | + | |
2131 | 2137 | | |
2132 | | - | |
| 2138 | + | |
| 2139 | + | |
| 2140 | + | |
| 2141 | + | |
| 2142 | + | |
| 2143 | + | |
2133 | 2144 | | |
2134 | 2145 | | |
2135 | 2146 | | |
2136 | 2147 | | |
2137 | 2148 | | |
2138 | 2149 | | |
2139 | 2150 | | |
| 2151 | + | |
| 2152 | + | |
| 2153 | + | |
| 2154 | + | |
| 2155 | + | |
| 2156 | + | |
| 2157 | + | |
| 2158 | + | |
| 2159 | + | |
| 2160 | + | |
| 2161 | + | |
| 2162 | + | |
| 2163 | + | |
2140 | 2164 | | |
2141 | 2165 | | |
2142 | 2166 | | |
2143 | | - | |
2144 | | - | |
2145 | | - | |
2146 | | - | |
2147 | | - | |
2148 | | - | |
| 2167 | + | |
2149 | 2168 | | |
2150 | 2169 | | |
2151 | 2170 | | |
| |||
0 commit comments