소스 검색

fix: trim whitespace from pasted IDs; drop the dead dist/ ZIP

Station and device IDs are copy-pasted out of the browser Network tab, so a
stray leading or trailing space is common. The upstream API treats " 4235…" as
a different, invalid value, and the user only sees a generic "cannot connect"
with nothing pointing at the real cause — a plausible contributor to some of
the "wrong Station ID" reports.

Only device_id was being stripped, and only on two of the paths that read it.
Replace that with WHITESPACE_SENSITIVE_FIELDS in const.py plus a shared
normalise_config_fields() helper, applied at every point user input enters or
a stored entry is read: all three config-flow steps, both re-auth merges, and
async_setup_entry. Defining the field list once keeps the config-flow path and
the setup path from drifting apart.

Normalising on read as well as on write means entries stored before this change
are healed at startup, with no need to set the station up again. CONF_PASSWORD
is deliberately excluded — whitespace in a password may be significant.

Also remove dist/solar-of-things-ha.zip. Nothing consumes it: hacs.json sets
zip_release false, and release.yml builds its asset fresh from
custom_components/. It was three versions stale and looked authoritative.
Its now-dangling entry in codeql-config.yml paths-ignore goes with it.

While here, validate.yml's syntax check listed source files by hand and so
would have skipped the new util.py — the same drift that already broke that
step once before. Replaced with a glob.

Verified: normalise_config_fields executed directly (util.py and const.py have
no Home Assistant imports) across 18 checks — trimming, password exclusion,
non-string pass-through, no input mutation, and edge cases. Covered by
tests/test_util.py. The config_flow/__init__ wiring is compile-checked only;
voluptuous and homeassistant are not installable in this environment.

Co-Authored-By: Claude <noreply@anthropic.com>
Conexo-Casa 1 개월 전
부모
커밋
9d61eb8115

+ 1 - 2
.github/codeql-config.yml

@@ -1,11 +1,10 @@
 name: "Solar of Things CodeQL config"
 
-# Only scan the actual integration code — ignore dist/ and any future test fixtures
+# Only scan the actual integration code — ignore tests and any future fixtures
 paths:
   - custom_components/solar_of_things
 
 paths-ignore:
-  - dist/
   - tests/
   - "**/*.md"
 

+ 3 - 3
.github/workflows/validate.yml

@@ -74,9 +74,9 @@ jobs:
       - name: Check Python syntax
         run: |
           cd custom_components/solar_of_things
-          python3 -m py_compile \
-            __init__.py api.py config_flow.py const.py \
-            sensor.py number.py select.py switch.py
+          # Glob rather than a hardcoded list: the explicit list silently went
+          # stale once already and skipped files that no longer existed.
+          python3 -m py_compile *.py
           echo "Python syntax OK"
 
       - name: Run unit tests (if present)

+ 16 - 10
custom_components/solar_of_things/__init__.py

@@ -47,6 +47,7 @@ from .const import (
     CONF_ACCESS_TOKEN_EXPIRES,
     CONF_REFRESH_TOKEN_EXPIRES,
 )
+from .util import normalise_config_fields
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -64,9 +65,14 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up Solar of Things from a config entry."""
     hass.data.setdefault(DOMAIN, {})
 
-    time_zone = entry.data.get(CONF_TIME_ZONE) or entry.options.get(CONF_TIME_ZONE)
-    user_id = entry.data.get(CONF_USER_ID)
-    password = entry.data.get(CONF_PASSWORD)
+    # Also trim on read, so entries stored before the config flow started
+    # normalising are healed without the user having to set the station up
+    # again.  A single stray space in a pasted ID otherwise fails every request.
+    data = normalise_config_fields(entry.data)
+
+    time_zone = data.get(CONF_TIME_ZONE) or entry.options.get(CONF_TIME_ZONE)
+    user_id = data.get(CONF_USER_ID)
+    password = entry.data.get(CONF_PASSWORD)  # never trimmed
 
     # Build the token-refreshed callback *before* constructing the API so the
     # callback reference is captured.
@@ -105,10 +111,10 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         api = SolarOfThingsAPI(
             user_id=user_id,
             password=password,
-            iot_token=entry.data.get(CONF_IOT_TOKEN),          # cached token (avoids login on every restart)
-            refresh_token=entry.data.get(CONF_REFRESH_TOKEN),
-            access_token_expires=entry.data.get(CONF_ACCESS_TOKEN_EXPIRES),
-            refresh_token_expires=entry.data.get(CONF_REFRESH_TOKEN_EXPIRES),
+            iot_token=data.get(CONF_IOT_TOKEN),          # cached token (avoids login on every restart)
+            refresh_token=data.get(CONF_REFRESH_TOKEN),
+            access_token_expires=data.get(CONF_ACCESS_TOKEN_EXPIRES),
+            refresh_token_expires=data.get(CONF_REFRESH_TOKEN_EXPIRES),
             time_zone=time_zone,
             on_token_refreshed=_on_token_refreshed,
         )
@@ -118,13 +124,13 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     else:
         # Legacy IOT-token mode
         api = SolarOfThingsAPI(
-            iot_token=entry.data[CONF_IOT_TOKEN],
+            iot_token=data[CONF_IOT_TOKEN],
             time_zone=time_zone,
             on_token_refreshed=_on_token_refreshed,
         )
 
-    station_id = entry.data[CONF_STATION_ID]
-    configured_device_id = (entry.data.get(CONF_DEVICE_ID) or "").strip()
+    station_id = data[CONF_STATION_ID]
+    configured_device_id = data.get(CONF_DEVICE_ID) or ""
 
     # ── Station coordinator (device list + monthly) ────────────────────────────
     station_coordinator = SolarOfThingsStationCoordinator(

+ 19 - 2
custom_components/solar_of_things/config_flow.py

@@ -36,6 +36,7 @@ from .const import (
     CONF_ACCESS_TOKEN_EXPIRES,
     CONF_REFRESH_TOKEN_EXPIRES,
 )
+from .util import normalise_config_fields
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -135,6 +136,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         errors: dict[str, str] = {}
 
         if user_input is not None:
+            # Trim before validating *and* before storing, so the value that is
+            # checked is the same one that gets persisted.
+            user_input = normalise_config_fields(user_input)
             try:
                 info = await _validate_password_auth(self.hass, user_input)
             except InvalidAuth:
@@ -184,6 +188,8 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         errors: dict[str, str] = {}
 
         if user_input is not None:
+            # The entry is created from this same dict, so trim it up front.
+            user_input = normalise_config_fields(user_input)
             try:
                 info = await _validate_token_auth(self.hass, user_input)
             except CannotConnect:
@@ -233,9 +239,16 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         )
 
         if user_input is not None:
+            # Matters most for the token path: a freshly pasted IOT token is the
+            # single most whitespace-prone value in this integration.
+            user_input = normalise_config_fields(user_input)
             if has_password:
                 # Re-authenticate with potentially updated email/password
-                merged = {**(existing_entry.data if existing_entry else {}), **user_input}
+                # Normalise the merge, not just user_input: the station/device
+                # IDs come from the stored entry, which may pre-date trimming.
+                merged = normalise_config_fields(
+                    {**(existing_entry.data if existing_entry else {}), **user_input}
+                )
                 try:
                     info = await _validate_password_auth(self.hass, merged)
                 except InvalidAuth:
@@ -261,7 +274,11 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
                     return self.async_abort(reason="reauth_successful")
             else:
                 # Legacy token mode: user provides a fresh token
-                merged = {**(existing_entry.data if existing_entry else {}), **user_input}
+                # Normalise the merge, not just user_input: the station/device
+                # IDs come from the stored entry, which may pre-date trimming.
+                merged = normalise_config_fields(
+                    {**(existing_entry.data if existing_entry else {}), **user_input}
+                )
                 try:
                     await _validate_token_auth(self.hass, merged)
                 except CannotConnect:

+ 16 - 0
custom_components/solar_of_things/const.py

@@ -12,6 +12,22 @@ CONF_TIME_ZONE = "time_zone"
 CONF_USER_ID = "user_id"       # Siseli account / user-ID login (not email)
 CONF_PASSWORD = "password"
 
+# Fields that are typed or copy-pasted by hand and routinely arrive with stray
+# leading/trailing whitespace.  The upstream API treats " 4235…" as a different
+# (invalid) value, which surfaces to the user as an unhelpful "cannot connect".
+# Consumed by normalise_config_fields() in util.py, which is applied both when
+# the config flow accepts input and when an entry is read back at setup, so a
+# single list keeps the two paths from drifting.
+# CONF_PASSWORD is deliberately absent: whitespace in a password may be
+# significant, so it is never trimmed.
+WHITESPACE_SENSITIVE_FIELDS = (
+    CONF_USER_ID,
+    CONF_STATION_ID,
+    CONF_DEVICE_ID,
+    CONF_IOT_TOKEN,
+    CONF_TIME_ZONE,
+)
+
 # Runtime-stored token state (written back to config entry)
 CONF_REFRESH_TOKEN = "refresh_token"
 CONF_ACCESS_TOKEN_EXPIRES = "access_token_expires"   # ISO-8601 string

+ 33 - 0
custom_components/solar_of_things/util.py

@@ -0,0 +1,33 @@
+"""Small helpers shared between the config flow and entry setup.
+
+Deliberately free of Home Assistant imports so the logic can be unit-tested on
+its own.
+"""
+from __future__ import annotations
+
+from typing import Any
+
+from .const import WHITESPACE_SENSITIVE_FIELDS
+
+
+def normalise_config_fields(data: dict[str, Any]) -> dict[str, Any]:
+    """Return a copy of *data* with whitespace trimmed from identifier fields.
+
+    The Station ID, Device ID, account name, IOT token and time zone are all
+    pasted or typed by hand.  A trailing space picked up while copying an ID out
+    of the browser's Network tab makes the upstream API reject the value, and the
+    user only sees "cannot connect" with no hint as to why.
+
+    Which fields are trimmed is defined once in WHITESPACE_SENSITIVE_FIELDS so
+    the config-flow path and the entry-setup path cannot drift apart.  The
+    password is never trimmed — whitespace there may be intentional.
+
+    Non-string values are passed through untouched, so this is safe to call on
+    a mapping that also carries token-expiry timestamps and similar.
+    """
+    cleaned = dict(data)
+    for key in WHITESPACE_SENSITIVE_FIELDS:
+        value = cleaned.get(key)
+        if isinstance(value, str):
+            cleaned[key] = value.strip()
+    return cleaned

BIN
dist/solar-of-things-ha.zip


+ 72 - 0
tests/test_util.py

@@ -0,0 +1,72 @@
+"""Tests for config-field normalisation.
+
+Station and device IDs are copy-pasted out of the browser's Network tab, so they
+routinely arrive with a stray leading or trailing space.  The upstream API
+treats " 4235…" as a different, invalid value and the user only sees a generic
+"cannot connect".
+"""
+from __future__ import annotations
+
+import pytest
+
+from custom_components.solar_of_things.const import WHITESPACE_SENSITIVE_FIELDS
+from custom_components.solar_of_things.util import normalise_config_fields
+
+
+def test_trims_pasted_station_and_device_ids() -> None:
+    cleaned = normalise_config_fields(
+        {"station_id": "486217907720650752 ", "device_id": " 486217907745816576"}
+    )
+    assert cleaned["station_id"] == "486217907720650752"
+    assert cleaned["device_id"] == "486217907745816576"
+
+
+def test_password_is_never_trimmed() -> None:
+    """Whitespace in a password may be intentional, so it must survive."""
+    cleaned = normalise_config_fields({"password": "  hunter2  ", "user_id": "  bob  "})
+    assert cleaned["password"] == "  hunter2  "
+    assert cleaned["user_id"] == "bob"
+
+
+def test_password_is_excluded_from_the_field_list() -> None:
+    assert "password" not in WHITESPACE_SENSITIVE_FIELDS
+
+
+def test_trims_a_pasted_iot_token() -> None:
+    """Tokens are copied out of DevTools and often carry a newline or tab."""
+    cleaned = normalise_config_fields({"iot_token": "\n eyJhbGciOi.token.value \t"})
+    assert cleaned["iot_token"] == "eyJhbGciOi.token.value"
+
+
+@pytest.mark.parametrize(
+    ("key", "value"),
+    [
+        ("access_token_expires", "2026-01-01T00:00:00+00:00"),
+        ("refresh_token", None),
+        ("some_number", 5),
+    ],
+)
+def test_unlisted_and_non_string_values_pass_through(key, value) -> None:
+    assert normalise_config_fields({key: value})[key] == value
+
+
+def test_returns_a_copy_without_mutating_the_input() -> None:
+    """entry.data is a read-only mapping in HA; never write through it."""
+    source = {"station_id": " 123 "}
+    cleaned = normalise_config_fields(source)
+
+    assert source["station_id"] == " 123 "
+    assert cleaned["station_id"] == "123"
+    assert cleaned is not source
+
+
+@pytest.mark.parametrize(
+    ("payload", "expected"),
+    [
+        ({}, {}),
+        ({"unrelated": "x"}, {"unrelated": "x"}),
+        ({"station_id": "   "}, {"station_id": ""}),
+    ],
+)
+def test_edge_cases(payload, expected) -> None:
+    assert normalise_config_fields(payload) == expected