feat(home-assistant): add WebSocket-only registry and automation actions - #227
Merged
l1shen merged 4 commits intoJul 30, 2026
Merged
Conversation
Provider egress that is not a fetch had no shared guard: the existing non-HTTP transports (imap-smtp, the Pushover realtime socket) all target hosts fixed in code, so nothing forced the policy for a credential-supplied host. A self-hosted provider connecting a WebSocket to a user-configured instance is the first case that needs one. Extract the per-hop check behind createGuardedFetch as assertGuardedEgressUrl so both transports enforce one policy instead of two drifting ones, and add openGuardedWebSocket on top of it. ws/wss targets are validated as their http/https equivalents, then connected over the normalized ws form. Node and workerd both expose a client WebSocket constructor, so this is a single code path rather than a Node-only capability.
Record two rules the WebSocket guard depends on: non-fetch transports reuse assertGuardedEgressUrl rather than growing their own host check, and the private-network opt-in governs what the guard permits, not what a deployment can actually route to.
The REST API serves entity states but not the registries behind them, so an agent could read light.living_room without learning which device, area, or floor it belongs to. Those registries, related-item search, per-device automation capabilities, script execution, and config validation are only reachable over the WebSocket API. Add five actions covering that gap. Each is a one-shot command, so it maps onto the existing request/response executor model without change; subscription commands are deliberately left out because the action model has no streaming and every Home Assistant subscription that carries an initial snapshot already has a one-shot equivalent. get_registries sends all five registry commands over a single connection. The handshake costs two round trips before any command can be sent, so batching matters more here than it would over HTTP.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Summary by CodeRabbit
WalkthroughThe PR centralizes SSRF egress validation and adds Sequence Diagram(s)sequenceDiagram
participant Executor
participant HomeAssistantWebSocketHandlers
participant runHomeAssistantCommands
participant HomeAssistantWebSocket
Executor->>HomeAssistantWebSocketHandlers: invoke WebSocket action
HomeAssistantWebSocketHandlers->>runHomeAssistantCommands: build commands
runHomeAssistantCommands->>HomeAssistantWebSocket: open guarded connection
HomeAssistantWebSocket-->>runHomeAssistantCommands: auth_required
runHomeAssistantCommands->>HomeAssistantWebSocket: send API key and commands
HomeAssistantWebSocket-->>runHomeAssistantCommands: command results
runHomeAssistantCommands-->>HomeAssistantWebSocketHandlers: ordered results
HomeAssistantWebSocketHandlers-->>Executor: normalized action response
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
reopenpilot
force-pushed
the
feat-home-assistant-websocket
branch
from
July 30, 2026 03:09
37a3b50 to
b442c9b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/home_assistant/runtime-ws.ts`:
- Around line 117-129: Use one shared deadline for the entire Home Assistant
WebSocket request in the surrounding request flow: derive the remaining budget
before connecting, pass that remaining duration as connectTimeoutMs, and start
the inbox timer with the same deadline rather than resetting the full timeout
after socket creation. Preserve abort handling and ensure the timer still
reports the existing 504 timeout error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00067566-3bda-4fde-a3b5-0f62d69a1fd8
📒 Files selected for processing (8)
AGENTS.mdsrc/core/guarded-fetch.tssrc/core/guarded-websocket.test.tssrc/core/guarded-websocket.tssrc/providers/home_assistant/actions.tssrc/providers/home_assistant/executors.tssrc/providers/home_assistant/runtime-ws.tssrc/providers/home_assistant/runtime.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/home_assistant/runtime-ws.ts`:
- Around line 12-16: Use one shared session deadline for the entire Home
Assistant WebSocket flow: derive the connection timeout from a deadline
established before the handshake, then pass only the remaining time to
authentication and command execution instead of starting a fresh
homeAssistantWebSocketTimeoutMs timer after the socket opens. Update the
relevant connection and command-timeout logic while preserving the documented
total budget across handshake, authentication, and all batch commands.
- Around line 130-134: Update the abort handling in the request flow around
onAbort to recheck context.signal?.aborted immediately after registering the
abort listener, invoking onAbort when it is already aborted. Preserve the
existing once-only listener and inbox failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ac342af1-a769-4c58-948c-b2a16ec02c11
📒 Files selected for processing (8)
AGENTS.mdsrc/core/guarded-fetch.tssrc/core/guarded-websocket.test.tssrc/core/guarded-websocket.tssrc/providers/home_assistant/actions.tssrc/providers/home_assistant/executors.tssrc/providers/home_assistant/runtime-ws.tssrc/providers/home_assistant/runtime.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
The handshake and the command phase each started a full timeout, so a slow instance could spend the budget twice and reach roughly sixty seconds against a documented thirty. Derive both from one deadline. Also recheck the abort signal after attaching the session listener. It can fire between the socket opening and the listener attaching, and openGuardedWebSocket already guards the same race for its own handshake listeners.
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
l1shen
pushed a commit
that referenced
this pull request
Jul 31, 2026
… calendars (#239) ## Summary - add automation, script, and scene config CRUD, reached over REST at `/api/config/<component>/config/<key>` - add history, logbook, calendar, and error log actions from Home Assistant's published REST API - fix `buildHomeAssistantUrl` dropping the instance base path Fifteen actions in total, all REST. No WebSocket code is touched. ## Problem Two separate gaps, both in the REST surface. **The editable config store was missing entirely.** Home Assistant serves automation, script, and scene configuration over REST, but at `/api/config/<component>/config/<key>` rather than under the endpoints listed in the published REST API reference. The provider could therefore run an automation but never write one. That endpoint family is absent from the reference page, which is the most likely reason it was never covered — the eight actions the provider started with map exactly onto that page and nothing outside it. **There was no time dimension.** `/api/states` answers "what is the temperature now"; nothing answered "how did it get there" or "what tripped that switch overnight". `history`, `logbook`, `calendars`, and `error_log` are all on the published reference page and were simply never implemented. ## Config CRUD `automation`, `script`, and `scene` differ only in three respects: the path segment, the input field carrying the key, and whether Home Assistant validates that key as a slug (`cv.slug` for script, `cv.string` for automation and scene). One descriptor captures those differences and parameterises shared read/write/delete handlers, rather than nine near-identical copies. The key travels in the path and Home Assistant injects it into the stored entry itself, so the request body is the bare config object. `POST` is create-or-update: writing to an unused key creates the entry, which is how the UI creates automations. Both endpoints require an admin token, and they only see entries Home Assistant manages itself — an automation defined in a separate YAML file or a package is not visible and returns not found. The action descriptions say so, because a non-admin token fails at the provider rather than at input validation. This pairs with `validate_config` from #227 to close the loop an agent needs for automation work: discover what a device can do, validate a candidate config, write it, then read the logbook to confirm it fired. `followUpActions` wires that sequence into the catalog so it is discoverable rather than something the caller has to know. ## History and diagnostics `get_history` exposes `minimalResponse` and `noAttributes` because a full-attribute history across several entities is large enough to matter for an agent's context budget. Against a real instance, one entity over 24 hours returned 7219 points; the compact form drops each row from the full state object to `{"state", "last_changed"}`. Home Assistant reads `minimal_response`, `no_attributes`, and `skip_initial_state` by presence rather than by value, so `false` omits the parameter instead of sending `0`. `significant_changes_only` is read by value and defaults to true, so it is encoded normally. `get_error_log` needs one caveat: Home Assistant registers that view only when the instance runs with file logging, so it can report not found on an otherwise healthy instance. Rather than surface a bare "Not Found", the handler maps that case to a message explaining why, and the action description repeats it. ## Base path fix `normalizeBaseUrl` accepts an instance URL with a path, so a Home Assistant behind a reverse proxy at `/ha` is a supported configuration. `buildHomeAssistantUrl` assigned the endpoint path over `url.pathname`, which dropped that prefix and sent every request to `/api/...` at the origin instead of `/ha/api/...`. It now appends to the base path. The WebSocket URL builder added in #227 already did this, so the two transports agree. This is a separate commit; it predates both PRs, and the fifteen new paths here would have multiplied it. ## Actions | Action | Endpoint | | --- | --- | | `get_automation_config` / `save_automation_config` / `delete_automation_config` | `GET`/`POST`/`DELETE /api/config/automation/config/{id}` | | `get_script_config` / `save_script_config` / `delete_script_config` | `GET`/`POST`/`DELETE /api/config/script/config/{key}` | | `get_scene_config` / `save_scene_config` / `delete_scene_config` | `GET`/`POST`/`DELETE /api/config/scene/config/{id}` | | `check_config` | `POST /api/config/core/check_config` | | `get_history` | `GET /api/history/period/{timestamp}` | | `get_logbook` | `GET /api/logbook/{timestamp}` | | `list_calendars` | `GET /api/calendars` | | `list_calendar_events` | `GET /api/calendars/{entity_id}` | | `get_error_log` | `GET /api/error_log` | Reference: https://developers.home-assistant.io/docs/api/rest ## Verification - `npm run fix-check`, `npm test` (584 passing) - read paths against a real instance: `check_config` returned valid; `get_history` returned 7219 points for one entity over 24 hours with the compact form visibly applied; `get_logbook` returned 4417 entries; `list_calendars` returned an empty list on an instance with no calendar integration; `get_error_log` returned the explanatory 404 described above - write paths against the same instance, creating and then removing an `oc_smoke_test` entry in each domain: create returned `ok`, the read-back showed the key injected into the stored entry, delete returned `ok`, and each subsequent read reported not found. A non-slug script key was rejected at the provider before any request. A following scan of 3830 entities found nothing left behind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openGuardedWebSocketinsrc/core/guarded-websocket.ts, so provider egress that is not a fetch is held to the same SSRF policycreateGuardedFetchasassertGuardedEgressUrl, and make both transports use itProblem
The Home Assistant provider exposes entity states but not the structure behind them.
/api/statesreturnslight.living_roomand nothing that says which device, area, or floor it belongs to, so "turn off the lights downstairs" is a string-matching guess rather than a query.Those registries are only reachable over Home Assistant's WebSocket API. So are related-item search, per-device automation capabilities, multi-step script execution, and trigger/condition/action validation. None of them have a REST equivalent.
Egress guard
This is the part worth review attention.
Every non-fetch egress in this repository so far targets a host fixed in code: the IMAP/SMTP runtime resolves its hosts from a per-provider allowlist (
netease_mail) or a literal (qq_mail), and the Pushover realtime socket uses a constantwss://URL. None of them needed a guard, which is whysrc/coredid not have one.A self-hosted Home Assistant is the first case where a non-fetch transport connects to a credential-supplied host, which is exactly the case AGENTS.md says the DNS resolved-address check must cover.
Rather than add a second, narrower check for WebSockets, this extracts the hop check that
createGuardedFetchalready performs —assertPublicHttpUrlon the literal, then resolved-address validation — intoassertGuardedEgressUrl, and has both transports call it.ws/wsstargets are validated as theirhttp/httpsequivalents and then connected over the normalizedwsform.allowPrivateNetworkandskipDnsValidationbehave as they do for fetch.Two limits are inherent to the transport and match what the fetch guard can offer: the constructor re-resolves the hostname itself, so low-TTL rebinding remains possible, and WebSocket handshakes have no redirects to revalidate. Both are stated in the JSDoc.
Runtime support
Node 22+ and workerd both expose a client
WebSocketconstructor, so this is one code path rather than a Node-only capability. Verified on workerd withwrangler devand the repository's compatibility settings rather than from documentation:{ "hasConstructor": "function", "constructed": true, "readyState": 0, "hasAddEventListener": "function", "hasSend": "function", "hasClose": "function", "hasNodeDns": "function" }node:dnsbeing present undernodejs_compatmeans the resolved-address check applies on Cloudflare too, consistent with the note added in #135.The provider is therefore not marked
nodeOnly; doing so would also drop the eight REST actions that work on Cloudflare today. Home Assistant authenticates in-band, with the token in a message rather than a header, so nothing here needs a constructor that can set request headers.Reachability is a separate matter from the guard: Workers cannot route to private addresses, so a LAN instance remains a Node/Docker/Fly deployment concern regardless of
OOMOL_CONNECT_ALLOW_PRIVATE_NETWORK. AGENTS.md now says so.Actions
get_registriesconfig/{entity,device,area,floor,label}_registry/listsearch_relatedsearch/relatedlist_device_automationsdevice_automation/{trigger,condition,action}/listexecute_scriptexecute_scriptvalidate_configvalidate_configAll five are one-shot commands, so they map onto the existing request/response executor model unchanged.
Subscription commands are deliberately excluded. The action model has no streaming, and every Home Assistant subscription that carries an initial snapshot already has a one-shot equivalent —
render_templateis a subscription whose first message equalsPOST /api/template, andsubscribe_entitiesstarts from what/api/statesalready returns. What remains is increment-only (subscribe_events,subscribe_trigger,logbook/event_stream), which a bounded listen window would serve badly. That needs a webhook design, not an action.get_registriessends all five registry commands over one connection. The handshake costs two round trips before any command can be sent, so batching matters more here than it would over HTTP.Reference: https://developers.home-assistant.io/docs/api/websocket
Verification
npm run fix-check,npm test(574 passing, 20 new)wrangler devget_registriesreturned 160 devices and 7 areas over a single connection with ids 1–5;search_relatedresolved a device to its area, config entry, integration, and three entities; auth failure mapped to 401; a loopback base URL stayed blocked with the private-network flag enabled