Give entity providers the connection's security context - #23
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds connection encryption context to entity-command providers, enabling provider-specific authorization decisions.
Changes:
- Adds optional
handle_command/2with%{encrypted?: boolean()}. - Preserves
handle_command/1compatibility and adds plaintext/Noise tests. - Includes unrelated serial-proxy review artifacts that should be removed.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
lib/espex/entity_provider.ex |
Defines and documents the optional callback. |
lib/espex/connection.ex |
Dispatches commands with encryption context. |
test/support/fake_adapters.ex |
Adds context-aware test providers and probe. |
test/espex/integration_test.exs |
Tests plaintext context and fallback behavior. |
test/espex/encrypted_integration_test.exs |
Tests established Noise-session context. |
.claude/.../testing.md |
Unrelated serial-proxy review artifact. |
.claude/.../serial-proxy-lazy-open-review.md |
Unrelated serial-proxy review summary. |
.claude/.../security.md |
Unrelated serial-proxy security audit. |
.claude/.../requirements.md |
Unrelated serial-proxy requirements report. |
.claude/.../elixir.md |
Unrelated serial-proxy code review. |
.claude/.../.requirements-input.md |
Unrelated serial-proxy requirements input. |
.claude/.../.diff.txt |
Unrelated generated serial-proxy diff. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The ESPHome protocol offers no authentication a server can rely on:
AuthenticationRequest carries a password field, but espex answers every
one with invalid_password: false. A Noise session is the only real
credential, so on a keyless server any host that can open a TCP socket
can issue entity commands.
That is usually fine — the keyless window exists so Home Assistant can
adopt the device and provision a PSK, and most entity commands are
harmless. It is not fine for commands that reboot, wipe or reflash. Espex
can't tell those apart, because which entity is dangerous is the
provider's knowledge, not the protocol's.
So espex now supplies the context and lets the provider decide: an
optional handle_command/2 receives %{encrypted?: boolean()} and is
preferred when exported. Providers that don't export it are unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Callbacks table and return-value summary still described commands as arriving only through handle_command/1, so anyone scanning the behaviour docs would miss the security hook entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bbangert
force-pushed
the
feat/entity-command-security-context
branch
from
August 15, 2026 15:08
58c3b55 to
95b2536
Compare
Merged
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
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.
Adds an optional
handle_command/2toEspex.EntityProvider, carrying%{encrypted?: boolean()}. Found while security-reviewing a downstream consumer (universal_proxy#161).Why
The ESPHome protocol gives a server no authentication it can act on.
AuthenticationRequestcarries a password field, but espex answers every one withinvalid_password: false(dispatch.ex:105-107) — there is no password check to fail. And entity commands are routed on adapter presence alone (dispatch.ex:425-431), with no auth, encryption or hello gate.A Noise session is therefore the only real credential. Once a PSK is set espex already enforces it properly — a plaintext client gets a handshake rejection so aioesphomeapi raises
RequiresEncryptionAPIError. But while the server is keyless, every connection is anonymous by construction, and any host on the LAN can issue entity commands.That window is usually benign: it exists so Home Assistant can adopt the device and provision a PSK, and most entity commands are harmless. It stops being benign when a provider exposes commands that reboot, factory-reset or reflash the device — which is exactly what prompted this (universal_proxy added firmware Install as an HA
updateentity).Why not fix it in espex
espex can't make this call. Whether an entity is dangerous is the provider's knowledge, not the protocol's — a
ButtonCommandRequestfor "factory reset" and one for "toggle a lamp" are the same message type. Any policy espex hardcoded would be either too blunt (refuse all commands keyless, breaking benign control) or wrong for someone.So espex supplies the context and the provider decides:
Compatibility
Fully backward compatible.
handle_command/2is in@optional_callbacks; espex calls it only whenfunction_exported?/3says so, otherwisehandle_command/1as before. Existing providers need no change.encrypted?is true only for{:active, _, _}— an established Noise session. Mid-handshake states are false, which is the safe direction.Verification
mix compile --warnings-as-errors— PASSmix test— 318 passed (4 doctests), up from 315mix format --check-formatted— cleanThree new tests: a provider without
/2still receives/1;/2is preferred and reportsencrypted?: falseon a plaintext connection; andencrypted?: trueafter a real Noise handshake (using the existing encrypted-integration harness, which I extended to accept anadapterstag). Mutation-tested — forcing the/1path fails both/2assertions.🤖 Generated with Claude Code