Read the encryption key from the esphome OTA platform and rename the command by bdraco · Pull Request #2679 · esphome/device-builder

bdraco · 2026-09-10T12:09:24Z

What does this implement/fix?

esphome 2026.9 uses one Noise key for both the native API and the esphome OTA platform: an explicit OTA key must equal the api key, a bare ota: encryption: inherits it, and a device with no api: block can still carry the key under ota:. The key lookup now resolves api: encryption: key first and falls back to the esphome OTA item's own key, Device gains ota_encryption_required so the dashboard can offer the key for an OTA only device, and devices/get_api_key is renamed to devices/get_encryption_key; the reply shape is unchanged.

Related issue or feature (if applicable):

fixes

Types of changes

Bugfix (non-breaking change which fixes an issue) — bugfix
New feature (non-breaking change which adds functionality) — new-feature
Enhancement to an existing feature — enhancement
Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
Refactor (no behaviour change) — refactor
Documentation only — docs
Maintenance / chore — maintenance
CI / workflow change — ci
Dependencies bump — dependencies

Frontend coordination

No frontend change needed
Companion frontend PR: Show the encryption key for OTA keyed devices device-builder-frontend#1722

Checklist

The code change is tested and works locally.
Pre-commit hooks pass (ruff, codespell, yaml/json/python checks).
Tests have been added or updated under tests/ where applicable.
components.index.json / definitions/components/*.json have not been hand-edited (regenerate via script/sync_components.py if a sync is needed).
Architecture-level changes are reflected in docs/ARCHITECTURE.md and/or docs/API.md.

…command esphome 2026.9 shares one Noise key between the native API and the esphome OTA platform, so the key can live under either block. Resolve it from api: first and fall back to the OTA item's own key, expose whether the OTA item declares encryption as Device.ota_encryption_required, and rename devices/get_api_key to devices/get_encryption_key.

bdraco · 2026-09-10T12:09:39Z

@esphbot review

codspeed-hq · 2026-09-10T12:11:25Z

Merging this PR will not alter performance

✅ 28 untouched benchmarks
⏩ 1 skipped benchmark¹

_{Comparing encryption-key-display (fe19d2d) with main (64cdaa4)}

1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

codecov · 2026-09-10T12:13:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.73%. Comparing base (5705bb0) to head (fe19d2d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2679   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files         282      282           
  Lines       24067    24073    +6     
=======================================
+ Hits        24003    24010    +7     
+ Misses         64       63    -1

Flag	Coverage Δ
py3.12	`99.70% <100.00%> (+<0.01%)`	⬆️
py3.14	`99.57% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ome_device_builder/controllers/devices/__init__.py	`100.00% <ø> (ø)`
...e_device_builder/controllers/devices/controller.py	`100.00% <100.00%> (+0.23%)`	⬆️
...ilder/controllers/devices/encryption_key_lookup.py	`100.00% <100.00%> (ø)`
...e_device_builder/controllers/devices/importable.py	`100.00% <100.00%> (ø)`
...ome_device_builder/helpers/device_yaml/__init__.py	`100.00% <ø> (ø)`
...ome_device_builder/helpers/device_yaml/_loading.py	`100.00% <ø> (ø)`
...ome_device_builder/helpers/device_yaml/_parsing.py	`100.00% <100.00%> (ø)`
esphome_device_builder/models/devices.py	`100.00% <100.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bdraco · 2026-09-10T12:14:47Z

@esphbot review

Copilot

🟡 Changes recommended

ota_encryption_required can be incorrectly set because the raw-YAML OTA heuristic is applied even when a parsed config is available, potentially flagging non-esphome OTA platform entries.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the device encryption-key resolution to match ESPHome 2026.9’s “single Noise key” behavior by (1) falling back to the ESPHome OTA platform’s encryption key when api.encryption.key is absent and (2) renaming the WS command from devices/get_api_key to devices/get_encryption_key, while also exposing an ota_encryption_required flag on Device for UI gating.

Changes:

Add OTA-encryption parsing + resolution helpers and a Device.ota_encryption_required flag (raw-YAML heuristic + resolved-config path).
Rename the devices-controller WS command to devices/get_encryption_key and route it through the new “api-else-ota” resolver.
Update tests and API documentation to reflect the new command name and OTA fallback behavior.

File summaries

File	Description
tests/test_run_esphome_config.py	Updates test module docs to reference `devices/get_encryption_key` fallback.
tests/test_api_key.py	Adds helper/unit tests for OTA encryption extraction and resolved “one key” behavior; adds scanner tests for `ota_encryption_required`.
tests/controllers/devices/test_branches_coverage.py	Renames controller wiring tests to `get_encryption_key` and adds coverage for OTA-only configs.
esphome_device_builder/models/devices.py	Adds `Device.ota_encryption_required` field for UI gating of “Show encryption key”.
esphome_device_builder/helpers/device_yaml/_parsing.py	Adds raw-YAML OTA encryption heuristic and OTA encryption extraction/resolution helpers.
esphome_device_builder/helpers/device_yaml/_loading.py	Populates `ota_encryption_required` during device scan/load.
esphome_device_builder/helpers/device_yaml/init.py	Re-exports the new OTA/encryption-key helpers.
esphome_device_builder/controllers/devices/controller.py	Renames the WS command handler to `devices/get_encryption_key`.
esphome_device_builder/controllers/devices/api_key.py	Updates controller helper to resolve “api key else OTA key”, including subprocess fallback.
esphome_device_builder/controllers/devices/init.py	Updates module docs to reflect broader responsibilities of `api_key`.
docs/API.md	Documents new `devices/get_encryption_key` command and `Device.ota_encryption_required`.

Review details

Files reviewed: 11/11 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

bdraco · 2026-09-10T12:41:41Z

@esphbot review

Copilot

🔵 Needs a closer look

The new get_resolved_encryption_key() precedence can incorrectly fall back to the OTA key even when an api.encryption block exists but its key is unresolved/invalid, which contradicts the documented “api first, else OTA” rule.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

esphome_device_builder/helpers/device_yaml/_parsing.py:895

get_resolved_encryption_key() currently falls back to the OTA key whenever the resolved API key is empty, which also happens when an api.encryption block exists but its key is unresolved/invalid (e.g. ${...} still present). That can make the OTA key “win” even though the API encryption block is present and should be authoritative per the documented precedence (api key first, else OTA key). Consider only falling back to the OTA key when the api.encryption block is absent, not merely when the API key resolves to an empty string.

Files reviewed: 11/11 changed files
Comments generated: 0 new
Review effort level: Lite

bdraco · 2026-09-10T12:46:54Z

On the get_resolved_encryption_key precedence: the OTA fallback stays deliberate. esphome's _resolve_encryption_key rejects any config whose explicit OTA key differs from the api key, so for a config that compiles the OTA key is the same key even when the api value only resolves at compile time. When the api block is bare (runtime provisioned) the OTA key is the only key in the YAML, and showing it beats "not found".

esphbot · 2026-09-10T13:04:05Z

@bdraco: On the get_resolved_encryption_key precedence: the OTA fallback stays deliberate. esphome's `_reso...

Agreed — precedence reasoning holds, and preferring api with an OTA fallback matches esphome's own equality constraint.

One adjacent gap, separate from precedence: bare ota: encryption: parses to None, not a dict. get_ota_encryption_block requires isinstance(encryption, dict), so ota_encryption_required comes back False on the resolved path. Raw scan only runs when load_device_yaml fails (_loading.py:328).

Benign in practice — a bare block only compiles alongside an api key, which already sets api_encrypted. But models/devices.py:269 and docs/API.md:164 both promise bare coverage, and tests/test_api_key.py:68 pins {}, a shape YAML never yields. Either presence-check "encryption" in entry like importable.py:336, or narrow the wording.

esphbot · 2026-09-10T13:04:07Z

~~Previous review~~ — superseded by a newer review below.

esphbot

Tip

No blocking issues found — ready to merge.

bdraco · 2026-09-10T13:06:38Z

Addressed in e4d51d5: ota_encryption_required is now a presence check on the esphome OTA entry (a bare block parses to None), the own-key helper scans every esphome entry again, and the module and test file are renamed to encryption_key_lookup.py / test_encryption_key_extraction.py.

bdraco · 2026-09-10T13:06:39Z

@esphbot review

A bare encryption: parses to None, so the flag is a presence check on the esphome OTA entry; the own-key helper scans every esphome entry again.

bdraco · 2026-09-10T13:07:48Z

@esphbot review

bdraco · 2026-09-10T13:14:18Z

@esphbot review

…ex factory Drop the callerless block getter and the own-key alias, share the raw regex factory with the api heuristic, route the partition-access walker through the same entry helper, and fold the resolved-or-raw choice into a helper shaped like its mdns and mac-suffix siblings.

bdraco · 2026-09-10T14:16:45Z

@esphbot review

esphbot · 2026-09-10T14:22:45Z

@bdraco: Addressed in e4d51d5: ota_encryption_required is now a presence c...

Confirmed, all three resolved.

resolved_ota_has_encryption uses "encryption" in entry — bare block counts. Test pins None, not {}. get_ota_encryption_key walks every esphome entry again. Files renamed; git grep get_api_key returns nothing.

97f0357 also worth noting. One walker plus one regex factory removes real duplication. _resolve_key's "$" not in key short-circuit stays behaviour-preserving — _UNRESOLVED_SUBSTITUTION_RE requires $.

One remaining note: get_api_connection still api-only. Details inline.

esphbot · 2026-09-10T14:22:48Z

~~Previous review~~ — superseded by a newer review below.

esphbot

Tip

No blocking issues found — ready to merge.

bdraco · 2026-09-10T14:23:51Z

On get_api_connection staying api-only: that is deliberate. It feeds the Native API probe's noise_psk, and an OTA-side key never encrypts the Native API (an OTA-only device runs a plaintext API, and a runtime-provisioned api key can differ from an explicit OTA key). fe19d2d states it in the contract.

bdraco · 2026-09-10T14:23:53Z

@esphbot review

esphbot · 2026-09-10T14:39:49Z

@bdraco: On get_api_connection staying api-only: that is deliberate. It feeds the Native API probe's `noise...

Agreed, dropping that note. Verified encryption_key_lookup.py:38 states it: "The key is the api one only: an OTA-side key never encrypts the Native API." Contract now explains the asymmetry at the call site.

esphbot · 2026-09-10T14:39:52Z

PR Review — Read the encryption key from the esphome OTA platform and rename the command

Merge-ready. Every prior finding — mine and Copilot's — is genuinely fixed, and the follow-up commits shrank the change rather than patching over it. One non-blocking merge-ordering note.

Specific things done well:

ce00b2a fixes the Copilot finding at the right layer: ota_encryption_required now routes through ota_encryption_declared (_parsing.py:328), which gates on resolved_config is not None first and only falls back to raw text on a draft that fails to load — the exact shape name_add_mac_suffix_enabled / mdns_disabled_enabled / has_top_level_block already use, so the flag joins an existing family instead of inventing a fourth pattern. The platform-blind raw regex can now only fire on YAML that already fails to parse.
c1f7af4a beat the rename Copilot asked for: the _resolve_api_key_via_esphome_config wrapper is deleted outright rather than renamed, and the helper is module-private _resolve_via_esphome_config with get_encryption_key as its only caller. One fewer public surface, not a relabelled one.
97f0357 is a real deletion of complexity: three near-identical ota: entry walkers collapse into one _ota_esphome_entries generator (_parsing.py:665) and two raw regexes into _nested_key_re, which preserves the exclusive [ \t][^\n]*\n | \n alternation the CodeQL comment documents.
e4d51d5's resolved_ota_has_encryption is a presence check ("encryption" in entry), so a bare ota: encryption: — which parses to None, not {} — counts; the test at test_encryption_key_extraction.py:96 pins the None shape a real parse produces.
Rename is complete and the new field is safe: git grep get_api_key resolved_ota_has_own_key is empty across code, tests, docs, scripts and workflows, and Device serialises through mashumaro over all dataclass fields, so ota_encryption_required reaches the wire without a to_dict allowlist edit.
Merge ordering: the WS rename drops devices/get_api_key with no alias while pyproject.toml pins the frontend exactly at 0.1.326 — land the companion bump alongside, or keep a one-release alias.
My prior note on get_api_connection staying api-only is dropped per @bdraco: "an OTA-side key never encrypts the Native API… fe19d2d60 states it in the contract." Confirmed in the docstring at encryption_key_lookup.py:38.

Checklist

Automated review by Kōan (Claude) HEAD=fe19d2d 3 min 18s

esphbot

Tip

No blocking issues found — ready to merge.

github-actions Bot added the enhancement Improvement to an existing feature label Sep 10, 2026

bdraco mentioned this pull request Sep 10, 2026

Show the encryption key for OTA keyed devices esphome/device-builder-frontend#1722

Merged

13 tasks

Use a regex heuristic for the raw-text OTA encryption scan

2c57cec

bdraco marked this pull request as ready for review September 10, 2026 12:30

Copilot AI lite review requested due to automatic review settings September 10, 2026 12:30

Copilot started reviewing on behalf of bdraco September 10, 2026 12:31 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread esphome_device_builder/helpers/device_yaml/_loading.py Outdated

Comment thread esphome_device_builder/controllers/devices/api_key.py Outdated

Comment thread esphome_device_builder/controllers/devices/controller.py Outdated

Use the raw OTA encryption heuristic only when the YAML fails to load

ce00b2a

bdraco requested a lite review from Copilot September 10, 2026 12:41

Copilot started reviewing on behalf of bdraco September 10, 2026 12:42 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

esphbot approved these changes Sep 10, 2026

View reviewed changes

Count a bare ota encryption block and rename the key lookup module

e4d51d5

A bare encryption: parses to None, so the flag is a presence check on the esphome OTA entry; the own-key helper scans every esphome entry again.

bdraco force-pushed the encryption-key-display branch from 3f836a5 to e4d51d5 Compare September 10, 2026 13:07

Pin the legacy mapping form ota block in the encryption key tests

6e51df4

Drop the callerless esphome config key wrapper

c1f7af4

esphbot approved these changes Sep 10, 2026

View reviewed changes

State that the API probe key is api-only

fe19d2d

esphbot approved these changes Sep 10, 2026

View reviewed changes

bdraco merged commit b2a8c84 into main Sep 10, 2026
23 checks passed

bdraco deleted the encryption-key-display branch September 10, 2026 14:54

Uh oh!

Uh oh!

Conversation

bdraco commented Sep 10, 2026

What does this implement/fix?

Types of changes

Frontend coordination

Checklist

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

codspeed-hq Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

codecov Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (1)

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

esphbot commented Sep 10, 2026

Uh oh!

esphbot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esphbot left a comment

Choose a reason for hiding this comment

Uh oh!

bdraco commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

esphbot commented Sep 10, 2026

Uh oh!

esphbot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

esphbot left a comment

Choose a reason for hiding this comment

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

bdraco commented Sep 10, 2026

Uh oh!

esphbot commented Sep 10, 2026

Uh oh!

esphbot commented Sep 10, 2026

PR Review — Read the encryption key from the esphome OTA platform and rename the command

Checklist

Uh oh!

esphbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

codspeed-hq Bot commented Sep 10, 2026 •

edited

Loading

codecov Bot commented Sep 10, 2026 •

edited

Loading

esphbot commented Sep 10, 2026 •

edited

Loading

bdraco commented Sep 10, 2026 •

edited

Loading

esphbot commented Sep 10, 2026 •

edited

Loading