Refuse privileged HA commands on unencrypted connections (B2)

lib/universal_proxy/esphome/entity_provider.ex

@@ -191,6 +191,64 @@ defmodule UniversalProxy.ESPHome.EntityProvider do
   @impl Espex.EntityProvider
   def handle_command(command), do: GenServer.cast(__MODULE__, {:command, command})
 
+  # Entities whose commands can reboot, wipe or reflash the device.
+  @privileged_object_ids ~w(firmware_update factory_reset reboot)
+
+  @doc """
+  Command entry point that also gets the connection's security context.
+
+  Espex prefers this over `handle_command/1` when exported. On a keyless
+  server every connection is anonymous — the ESPHome protocol answers
+  `AuthenticationRequest` unconditionally, so a Noise session is the only
+  real credential — which means anything on the LAN can drive entity
+  commands until Home Assistant provisions a PSK.
+
+  That's tolerable for the diagnostic entities, which is why the whole
+  surface isn't simply closed: the keyless window is how HA adopts the
+  device in the first place. It isn't tolerable for Factory Reset, Reboot
+  or Firmware Update, so **every** command on those three entities is
+  refused unless the connection is encrypted.
+
+  Note that includes the update entity's Check, not just Install — the
+  gate is per-entity, not per-command. That is deliberate: Check reaches
+  `FirmwareUpdate.check/0`, which makes an outbound GitHub request with no
+  debounce, so leaving it open would let an unauthenticated client drive
+  the API into its secondary rate limits. Nothing legitimate is lost:
+  the keyless window is pre-adoption, HA refreshes over an encrypted
+  connection once it has provisioned a PSK, and the 24 h poller checks
+  regardless.
+
+  Refusals return `:ok`: the protocol has no "denied" reply for a command,
+  and reporting an error would only surface as an adapter failure in the
+  server's logs. The warning is the audit trail.
+  """
+  @impl Espex.EntityProvider
+  def handle_command(command, %{encrypted?: false} = _context) do
+    case privileged_object_id(command) do
+      nil ->
+        handle_command(command)
+
+      object_id ->
+        Logger.warning(
+          "EntityProvider: refusing #{object_id} command from an unencrypted client — " <>
+            "provision API encryption in Home Assistant to enable it"
+        )
+
+        :ok
+    end
+  end
+
+  def handle_command(command, _context), do: handle_command(command)
+
+  @doc false
+  @spec privileged_object_id(struct()) :: String.t() | nil
+  def privileged_object_id(%{key: key}) do
+    object_id = Map.get(key_lookup(), key)
+    if object_id in @privileged_object_ids, do: object_id
+  end
+
+  def privileged_object_id(_command), do: nil
+
   # ── GenServer callbacks ─────────────────────────────────────────────
 
   @impl GenServer

test/universal_proxy/esphome/entity_provider_test.exs

@@ -296,6 +296,81 @@ defmodule UniversalProxy.ESPHome.EntityProviderTest do
     end
   end
 
+  describe "privileged commands on an unencrypted connection" do
+    setup do
+      start_supervised!({Registry, keys: :duplicate, name: :"ep_reg_#{System.unique_integer()}"})
+      :ok
+    end
+
+    test "firmware_update, factory_reset and reboot are identified as privileged" do
+      for object_id <- ~w(firmware_update factory_reset reboot) do
+        cmd = %Proto.ButtonCommandRequest{key: EP.key_for(object_id)}
+        assert EP.privileged_object_id(cmd) == object_id
+      end
+    end
+
+    test "diagnostic entities are not privileged" do
+      # A sensor key isn't a command target, but the guard must not
+      # over-reach onto anything that merely shares the message shape.
+      cmd = %Proto.ButtonCommandRequest{key: EP.key_for("cpu_temperature")}
+      assert EP.privileged_object_id(cmd) == nil
+      assert EP.privileged_object_id(%{no_key: true}) == nil
+    end
+
+    test "a privileged command from an unencrypted client is refused, not run" do
+      log =
+        ExUnit.CaptureLog.capture_log(fn ->
+          cmd = %Proto.UpdateCommandRequest{
+            key: EP.key_for("firmware_update"),
+            command: :UPDATE_COMMAND_UPDATE
+          }
+
+          assert EP.handle_command(cmd, %{encrypted?: false}) == :ok
+        end)
+
+      assert log =~ "refusing firmware_update command from an unencrypted client"
+    end
+
+    test "an encrypted client is not refused" do
+      # Routed through to the GenServer (unregistered here, so the cast is
+      # a no-op) — the point is that it is NOT short-circuited or logged.
+      log =
+        ExUnit.CaptureLog.capture_log(fn ->
+          cmd = %Proto.ButtonCommandRequest{key: EP.key_for("reboot")}
+          EP.handle_command(cmd, %{encrypted?: true})
+        end)
+
+      refute log =~ "refusing"
+    end
+
+    # Deliberate, not an oversight: the gate is per-entity, so the update
+    # entity's CHECK is refused along with UPDATE. CHECK reaches
+    # FirmwareUpdate.check/0, which hits GitHub with no debounce — leaving
+    # it open would let an unauthenticated client drive the API into its
+    # secondary rate limits. Pinned here so it isn't "fixed" by accident.
+    test "update CHECK is refused too, not just UPDATE" do
+      for cmd <- [:UPDATE_COMMAND_UPDATE, :UPDATE_COMMAND_CHECK] do
+        log =
+          ExUnit.CaptureLog.capture_log(fn ->
+            req = %Proto.UpdateCommandRequest{key: EP.key_for("firmware_update"), command: cmd}
+            assert EP.handle_command(req, %{encrypted?: false}) == :ok
+          end)
+
+        assert log =~ "refusing firmware_update command from an unencrypted client"
+      end
+    end
+
+    test "a non-privileged command is allowed even unencrypted" do
+      log =
+        ExUnit.CaptureLog.capture_log(fn ->
+          cmd = %Proto.ButtonCommandRequest{key: EP.key_for("cpu_temperature")}
+          EP.handle_command(cmd, %{encrypted?: false})
+        end)
+
+      refute log =~ "refusing"
+    end
+  end
+
   describe "poll during an install" do
     test "carries the cached value instead of re-reading the blocked updater" do
       reads = start_supervised!(Supervisor.child_spec({Agent, fn -> 0 end}, id: :poll_reads))