Commit 6abb289
feat: Add ha_manage_security_policy tool behind Policies-tab toggle (#2155)
* refactor(internal): Extract shared tool-security-policy edit helpers
Move the policy get/set read-modify-write path out of DevTools into
policy/editing.py so a second tool can drive the same tool_policy.json
without duplicating the lock choreography, the optimistic-concurrency
check, or the approval remember-cache invalidation.
Only the action names inside user-facing messages vary per caller, and
those come from a PolicyCaller passed in, so ha_dev_manage_settings'
get_policy / set_policy responses and error strings are unchanged.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Add ha_manage_security_policy tool behind Policies-tab toggle
Adds a regular (non-dev) MCP tool that reads and rewrites the tool
security policy, gated by the new enable_security_policy_tool flag.
Off by default; a restart registers or unregisters it. The tool exposes
get and set only and never reaches the approval queue — listing,
approving, and denying pending requests stay developer-mode. It is
policy-gated like any other tool, so a rule targeting
ha_manage_security_policy itself keeps a human in the loop.
The toggle renders on the Tool Security Policies tab beside the rules it
governs, with a warning callout, instead of in the generic Server
Settings feature list: the flag ships no features.* locale keys, so it
never enters the generated FEATURE_META while /api/settings/features
still serves and persists it. It is declared in both add-on flavors'
config.yaml because a non-beta flag's web-UI save routes through
Supervisor in add-on mode, which rejects undeclared options.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(addon): Export ENABLE_SECURITY_POLICY_TOOL from the add-on start script
start.py maps each add-on option to its env var one at a time, and the
new option was declared in both flavors' config.yaml without a matching
export. In add-on mode that export is the ONLY channel that reaches the
server: get_feature_flag_origin reports "addon" for every non-beta flag,
so the override-file applier skips it. The Policies-tab toggle would
have saved through Supervisor and then done nothing on either flavor.
Both Dockerfiles copy this one start.py, so the single edit covers the
stable and dev add-ons. Adds the source-level wiring test the
read_only_mode flag already has, and extends the two non-beta option
parity tests with the new key.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Harden security-policy writes against silent rule loss and dead gates
Four failure modes the review found in the shared set path, all of which
returned plain success:
- A payload without "rules" validated (the model defaults it to [] and
Policy ignores unknown keys) and deleted every approval gate. It is
now rejected, with the message naming "rules": [] as the deliberate
way to clear them. Removing rules that WERE sent still succeeds but
warns with the count and the tool names it dropped.
- Rules written while the engine is enabled but not running (restart
pending, or the middleware failed to load) looked live. That window
now warns; the existing warning only covered the flag being off.
- Everything after save_policy could turn a committed write into an
INTERNAL_ERROR the caller retries into a spurious version mismatch.
Settings-dependent warnings are now computed before the write, and a
failing remember-cache clear degrades to a warning. A rules change
with no live queue logs a WARNING instead of passing silently.
- register_security_policy_tools read kwargs.get("server"), so a call
site that forgot it would silently lose the liveness report; it now
uses kwargs["server"] and the editing helpers require the argument.
Also: the version-mismatch message names the calling tool's own read
action, ENABLE_SECURITY_POLICY_TOOL joins the empty-string-means-false
validator (an empty env value crashed startup), and the duplicated
_with_file_lock helper moves to utils.config_write_lock as
run_with_file_lock.
The missing-rules rejection applies to ha_dev_manage_settings too —
intended: the same payload wiped the same gates there.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings-ui): Lock the sidecar policy PUT against cross-process writes
The sidecar's policy PUT did load_policy -> version check -> save_policy
with no locks. In stdio mode it runs in a SEPARATE process from the MCP
server, so its optimistic-concurrency check was not a compare-and-swap:
it could read version N, lose the race to a policy tool write, and
overwrite that commit without the version moving.
Wraps the load-check-save in config_write_guard(), the same one-liner
the main-server handler already uses. Response shapes are unchanged.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Allow reading the security policy in Read Only Mode
Adds ha_manage_security_policy to READ_ONLY_EXEMPT_TOOLS with a
predicate that permits action='get' and blocks everything else, so an
operator can still inspect what is gated while the mode is on. It fits
the table's contract: the read has no pure-read duplicate outside
dev mode (ha_dev_manage_settings('get_policy')), and a missing action
fails closed rather than reading as a get.
Editing the policy stays blocked — it is exactly the kind of write the
mode exists for.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(settings-ui): Let the policy tool be gated before it is enabled
The warning told operators to gate ha_manage_security_policy with a
rule, but that was impossible before enabling it: with no stub the tool
had no Tools-tab row, and the gate switch was disabled for any row that
was not enabled. The first enable+restart therefore exposed the tool
ungated — the one window it must not have.
- Adds the FEATURE_GATED_TOOLS stub so the row exists while the flag is
off, and keeps the security-gate switch live on gated rows whenever
policies are on. Gate rules are plain tool-name strings in the policy
document, so authoring one for an unregistered tool needs no backend
change (Rule.tool_name only rejects empty).
- Feature-gated no longer implies beta. Stubs carry disabled_by_beta,
the row renders the matching "how to enable this" copy instead of the
hardcoded beta/docs/beta.md text, and the LLM-API exposure default
reads the same classification (falling back to the flag registry for
metadata-cache rows written before this field existed).
- Both Policies-tab switches now render unknown state as indeterminate +
disabled behind a notice, instead of a confident "off" for a server
whose flags could not be read — the treatment read-only mode already
has on the Tools tab.
- The warning now says to gate the tool BEFORE enabling it, and that
rules enforce only once the master switch is on and the engine is live.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: Pin the policy tool's self-gating claim end to end
Adds an e2e that installs a rule naming ha_manage_security_policy
through the tool itself, then asserts the next call — a read, and a
write that would remove the gate — blocks with USER_APPROVAL_REQUIRED.
The middleware re-reads the policy per call and exempts only the proxy
meta-tools and the dev queue actions, so this is the security claim
behind shipping the tool at all.
Also switches the success-path calls to call_tool_success inside
MCPAssertions per tests/AGENTS.md, keeps safe_call_tool for the expected
failures, covers the missing-rules rejection, and fixes an
`if original:` restore that would not restore an empty env value.
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: Drop stale rendered-vs-parsed key count from docstring
The feature-gated stub added for ha_manage_security_policy changes the
number of keys that differ between the two English renderings, so the
pinned count in the prose went stale. Describe the mechanism instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Repair round-2 CI failures (format, harness kwargs, e2e shape)
- start.py: collapse the ENABLE_SECURITY_POLICY_TOOL export to one line
(ruff format checks files outside src/ and tests/ too).
- Container-coercion harness: pass server= like the real registry does,
matching register_security_policy_tools's strict kwargs read.
- Self-gating e2e: create_error_response spreads context at the top
level of the body, not under error.context; assert there. Consolidate
the ha_mcp.config import to one style (CodeQL import-and-import-from).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings-ui): Stop the flag switches from asserting unverified state
Three review findings on the Policies-tab switches, each of which the
pre-existing Read Only Mode switch shared — fixed together so the three
hand-written flag switches behave identically:
- A save that succeeded but whose confirming re-read failed reverted the
checkbox to its pre-flip value, presenting a state the server no longer
has as fact. Both save paths echo `applied`, so saveFeatureFlag now
returns the parsed body and the handlers prefer that echo; when neither
the re-read nor the echo can say, the switch goes to the unknown
treatment instead of a confident wrong answer.
- The two unknown-state notices were revealed by a class alone, so a
screen reader never learned the switches had gone indeterminate. Both
now carry role="alert" + aria-live="assertive", the pattern
#visibility-load-error already uses for a failure region.
- An env-pinned flag reports editable:false and every save of it is
rejected server-side, but the switches only read `value` — so they
rendered usable and silently failed. They now lock and render the same
env-locked note the generated Server Settings rows show, via one shared
applyFlagToggle painter that owns all three states (unknown, locked,
editable).
Refs #2148
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings-ui): Derive per-flag known state from payload presence
A successful /api/settings/features response marked every hand-written
switch known even when the payload omitted that flag's entry (older
server build, or an overlay dropping it), rendering the switch
off-and-editable so a save could overwrite an enabled server value.
Known now derives from the presence of each flag's entry, and the
Policies-tab unknown notice shows when either switch is unknown.
jsdom regression test: a 200 omitting one flag leaves that switch
indeterminate and disabled while the present one renders normally.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Address review nitpicks and outside-diff findings
- _js_harness: reject non-finite HA_MCP_JS_HARNESS_TIMEOUT overrides.
- policy/editing: read the policy file off the event loop in get_policy,
matching set_policy's to_thread pattern.
- test_readonly_mode: success path through MCPAssertions/call_tool_success.
- jsdom POST-body tests: initialize both policy toggles to a known-off
state and assert editability before dispatching the synthetic change
(an absent flag entry now renders the switch disabled, so the old
setup exercised a click no user could make); revert test additionally
asserts the switch stays editable after a failed save.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Close the two review gaps around enabling the policy tool
- Dev-tools leash: enable_security_policy_tool joins the unconditional
arm of _guard_security_policy_setting (web UI / env var only), with
the matching row lock in the list matrix. Flipping it registers
ha_manage_security_policy after a restart — the same end state as
unbuckling the leash, so it gets the same treatment as the access
toggle itself, regardless of dev_tools_security_policy_access.
- LLM API: ha_manage_security_policy joins LLM_API_DEFAULT_OFF_TOOLS so
the one tool that can remove approval gates is not exposed to
conversation agents by default; the per-tool override stays.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: Open pending component version 1.3.3
The Component Version Gate fails on any PR whose merge ref carries the
component at 1.3.2 now that 1.3.2 shipped as the mirror's stable: master
gained a Dutch component translation after the cut without a bump, so
its content drifted onto the released version. Level with stable means
one patch bump opens the next pending version (manifest + const +
parity-test literal, per AGENTS.md 'Version bumps ride the stable
release cycle'). No server-gated services added, so
MIN_COMPONENT_VERSION stays.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Revert "chore: Open pending component version 1.3.3"
This reverts commit c6e6791.
* chore: Regenerate nl add-on projections for the new option key
The Dutch locale merged from master was generated before this branch
added addon.enable_security_policy_tool to both schemas; regenerate so
the derived-catalog check stays in sync.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Correct the approval-queue claim in the toggle help text
A policy-gated set on ha_manage_security_policy CAN enter the pending
approval queue (the self-gating e2e proves it); what the tool never
exposes are the queue operations. Say that instead. Also route the
rule-installing call in that e2e through MCPAssertions per the test
guidelines.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: Add enable_security_policy_tool to the dev-mode guard enumerations
Both lists in docs/dev-mode.md predate the flag joining the
unconditional guard arm: the locked_reason matrix rows and the
refused-even-with-access settings.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: State each guarded toggle's activation timing separately
dev_tools_security_policy_access applies live; enable_security_policy_tool
takes effect on restart. The shared paragraph implied live for both.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>1 parent cf0b883 commit 6abb289
46 files changed
Lines changed: 2769 additions & 247 deletions
File tree
- docs
- homeassistant-addon-dev
- translations
- homeassistant-addon
- translations
- src/ha_mcp
- policy
- settings_ui
- locales
- tools
- utils
- tests
- addon
- src
- e2e
- policy
- tools
- unit
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
97 | 102 | | |
98 | 103 | | |
99 | 104 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
28 | 33 | | |
29 | 34 | | |
30 | 35 | | |
| |||
72 | 77 | | |
73 | 78 | | |
74 | 79 | | |
| 80 | + | |
75 | 81 | | |
76 | 82 | | |
77 | 83 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
57 | 66 | | |
58 | 67 | | |
59 | 68 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
51 | 60 | | |
52 | 61 | | |
53 | 62 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
59 | 68 | | |
60 | 69 | | |
61 | 70 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
58 | 67 | | |
59 | 68 | | |
60 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
54 | 63 | | |
55 | 64 | | |
56 | 65 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
51 | 60 | | |
52 | 61 | | |
53 | 62 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
57 | 66 | | |
58 | 67 | | |
59 | 68 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
28 | 37 | | |
29 | 38 | | |
30 | 39 | | |
| |||
0 commit comments