Ignore malformed mDNS service names in browser callbacks

[bdraco]

What does this implement/fix?

A third party device on the LAN (an IRIS alarm module) advertises a _http._tcp service whose instance name contains ASCII control characters. The zeroconf browser hands us the raw wire name, but ServiceInfo rejects it with BadTypeInNameException, so every announce raised inside our browser callback and flooded the log with tracebacks.

Adds valid_mdns_service_name, a cached wrapper over zeroconf's own service_type_name validation, and checks it at the top of each browser callback: the device state monitor's shared dispatch (which also covers the upstream DashboardImportDiscovery we forward to), the remote build peer browser, and the discover CLI. A name that fails validation is dropped with a debug log; it never created any state, so Removed is skipped too.

Related issue or feature (if applicable):

• fixes [Bug] Malformed _http._tcp mDNS record from third-party device crashes Device Builder zeroconf listener #2620

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: esphome/device-builder-frontend#

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.

[bdraco]

@esphbot review

[codspeed-hq]

Merging this PR will not alter performance

✅ 28 untouched benchmarks
⏩ 1 skipped benchmark¹

---

_{Comparing mdns-bad-service-name (b556694) with main (76761d1)}

Footnotes

1. 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]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.72%. Comparing base (76761d1) to head (b556694).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2621   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files         275      275           
  Lines       23364    23384   +20     
=======================================
+ Hits        23300    23320   +20     
  Misses         64       64           

Flag	Coverage Δ
py3.12	99.68% <100.00%> (+<0.01%)	⬆️
py3.14	99.72% <100.00%> (+<0.01%)	⬆️

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

Files with missing lines	Coverage Δ
..._builder/controllers/_device_state_monitor/mdns.py	99.62% <100.00%> (+<0.01%)	⬆️
...vice_builder/controllers/remote_build/discovery.py	100.00% <100.00%> (ø)
esphome_device_builder/discover.py	98.63% <100.00%> (+0.05%)	⬆️
esphome_device_builder/helpers/hostname.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.

[Copilot]

Pull request overview

This PR hardens the project’s zeroconf/mDNS browsing callbacks against malformed service instance names that can cause ServiceInfo/AsyncServiceInfo construction to raise, preventing log spam and callback crashes when third-party devices advertise invalid _http._tcp (and related) records on the LAN.

Changes:

• Add a shared valid_mdns_service_name() helper and apply it as an early guard in mDNS browser callbacks (device state monitor, remote-build peer discovery) and the discover CLI.
• Update/extend tests to assert malformed instance names are dropped (no state created, no CLI output) rather than raising.
• Add a focused test module covering the shared browser dispatch guard behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
esphome_device_builder/helpers/hostname.py	Introduces valid_mdns_service_name() utility for validating raw wire service names before constructing ServiceInfo.
esphome_device_builder/controllers/_device_state_monitor/mdns.py	Adds early validation guard in the shared browser dispatch to skip malformed names before invoking handlers.
esphome_device_builder/controllers/remote_build/discovery.py	Adds early validation guard in the remote-build peer browser callback to ignore malformed names.
esphome_device_builder/discover.py	Adds early validation guard in the CLI browse callback so malformed names produce no output and don’t raise.
tests/test_remote_build_controller.py	Adds regression coverage ensuring malformed remote-build service names are ignored without mutating peer state.
tests/test_discover.py	Updates the hostile-name test to assert the entire row is dropped when the instance name is invalid.
tests/test_mdns_malformed_names.py	New test module validating valid_mdns_service_name() and asserting dispatch drops malformed names before any handler runs.

---

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

[esphbot]

PR Review — Ignore malformed mDNS service names in browser callbacks

Correct, well-placed fix — the guard sits on all three browser registrations and nothing is left uncovered. Merge-ready; two non-blocking follow-ups.

Specific things done well:

• Guard coverage is complete for the callback surface. grep for AsyncServiceBrowser(/handlers=[ finds exactly three registrations — mdns.py:129, remote_build/discovery.py:75, discover.py:135 — and this PR guards all three. Placing it in _on_browser_event rather than per-handler also covers upstream DashboardImportDiscovery.browser_callback, which I confirmed builds AsyncServiceInfo(service_type, name) unconditionally on Added/Updated — that is the actual [Bug] Malformed _http._tcp mDNS record from third-party device crashes Device Builder zeroconf listener #2620 crash site for the _http._tcp IRIS module (our own _on_http_service_state_change returns early on the empty name bucket and never reaches its constructor).
• service_type_name(name, strict=False) is the right predicate, not an approximation. Verified empirically against zeroconf 0.150.0: it accepts what ServiceInfo.__init__ accepts (including non-strict service names like _my_service) and rejects on the same [\x00-\x1f\x7f] regex, the 63-byte label cap, and the 256-byte full-name cap. Skipping Removed is sound — validity is deterministic per name, so a dropped name never created state.
• The lru_cache earns its keep. service_type_name is already @lru_cached, but lru_cache doesn't cache exceptions, so the wrapper is what actually collapses the repeated raise into one cached False — and incidentally makes the debug log fire once per unique bad name instead of per announce, which is the reported symptom.
• The test_discover.py rewrite fixes a test that was lying. The old version patched out AsyncServiceInfo — the very constructor that raises — so it asserted sanitization against a code path that would have crashed in production. The new test drives the real constructor.
• Defense in depth preserved in discover.py: _safe_label still runs, and the replacement comment correctly narrows its remaining job to non-ASCII non-printables.

What's worth a follow-up:

• probe_device and three sibling sites still build AsyncServiceInfo from a configured esphome.name with no guard; a >63-byte name aborts _do_scan mid-batch and hides the remaining devices until restart. Pre-existing, but this PR adds the helper that fixes it.
• valid_mdns_service_name sits in helpers/hostname.py, whose docstring scopes it to DNS-cache / ping-sweep / OTA hostname strings. Relocate or widen the docstring. (@Copilot's lazy-import suggestion on the same line doesn't apply — device_builder already loads zeroconf, and helper_cli.py doesn't import hostname at all.)

---

---

Checklist

• Guard applied to every mDNS browser callback registration
• Validation predicate matches ServiceInfo's actual acceptance (verified against zeroconf 0.150.0)
• Removed-event skip is safe (no state created by a dropped name)
• No legitimate device silently dropped (advertisers validate at registration)
• Peer-controlled input sanitized before reaching the terminal
• Unbounded growth from attacker-controlled cache keys
• No unused imports left after the test rewrite (patch still used at 8 sites)
• Test construction follows established repo patterns (DeviceStateMonitor.__new__)
• Diff matches PR description (no scope creep)

---

Silent Failure Analysis

🟠 **1. HIGH** — guard validates the wrong predicate (crash class still reachable)
esphome_device_builder/helpers/hostname.py:11-25

Risk: ServiceInfo.__init__ validates the pair — if not type_.endswith(service_type_name(name, strict=False)): raise BadTypeInNameException (zeroconf/_services/info.py:219) — so a wire name whose type suffix differs from the browsed service_type in case or spelling passes this guard and still raises at construction; verified empirically: valid_mdns_service_name("evil._HTTP._TCP.local.") is True while ServiceInfo("_http._tcp.local.", "evil._HTTP._TCP.local.") raises, and the browser hands callbacks pointer.alias verbatim from the wire (browser.py:693) while type_ stays as the app spelled it, so the #2620 crash remains reachable from a peer-controlled PTR.

@lru_cache(maxsize=256)
def valid_mdns_service_name(name: str) -> bool:
    try:
        service_type_name(name, strict=False)
    except BadTypeInNameException as err:
        return False

Fix: Take the browsed type as a second argument — valid_mdns_service_name(service_type, name) returning service_type.endswith(service_type_name(name, strict=False)) inside the same try — and pass service_type at all three call sites.

🟡 **2. MEDIUM** — silent no-op in a diagnostic tool
esphome_device_builder/discover.py:202-206

Risk: discover exists to show the operator what is on the network, and this replaces a printed (sanitized) row with no output at all — the only trace is a logging.debug in hostname.py, below the CLI's default INFO level, and lru_cache means even at --verbose it logs once per distinct name rather than per announcement, so an operator debugging "my device doesn't show up" sees nothing to distinguish a rejected broadcaster from a silent network.

if not valid_mdns_service_name(name):
    return
# Control characters never get past the guard above, but the instance
# label is still peer-controlled Unicode; strip the rest ...

Fix: In discover.py specifically, print a row (or a one-line logging.warning) marking the instance as malformed instead of returning, so the diagnostic tool reports what it dropped.

---

Automated review by Kōan (Claude) HEAD=b556694 6 min 55s

[esphbot]

Tip

No blocking issues found — ready to merge.