Sigenergy: redact accessToken from the MQTT publish log line

[springfall2008]

This is an automated draft PR generated from issue #4920 — a maintainer should review it before merging.

Fixes #4920

Summary

SigenergyAPI._publish_mqtt() logged the full command payload dict, which both callers (set_operating_mode() and send_battery_command()) populate with the live accessToken — the same token used as the MQTT broker password. Since MQTT commands fire on every mode switch and battery command, the token landed in the app log in plaintext, repeatedly, for as long as it was valid.

This adds a SigenergyAPI.redact() static method and a SIGENERGY_LOG_REDACT_KEYS tuple, mirroring the existing DeyeAPI.redact() / SunsynkAPI.redact() pattern the report pointed at, and applies it to the one leaking log line. Redaction is log-only — the real token still goes to the broker on the wire. redact() recurses into dicts and lists because the battery command payload nests its per-system commands one level down inside a list.

Scope is deliberately limited to that log line: the other token paths were checked and are clean today — the subscription publishes (sigenergy.py:1465) log only a count, get_access_token() logs only code/msg/expiresIn, and _request() sends the token in the Authorization header rather than in the params/json_data it logs.

Testing

• tools/triage_test.sh sigenergy — all Sigenergy tests pass, including two new ones:
  • test_sigenergy_redact — masking at top level and nested inside a list, every key in the redact list, and scalar/list/None passthrough.
  • test_sigenergy_publish_mqtt_redacts_token — the broker still receives the real token while the log line contains <redacted> and not the token, non-credential payload content (systemId) is still logged, and the caller's payload dict is not mutated.
• Confirmed the new test is not vacuous: with the redaction call reverted it fails with Token must not appear in the log, and passes again once restored.
• coverage/run_pre_commit — all hooks pass (ruff, black, cspell, markdownlint) and the quick test suite is green.

[springfall2008]

Same bug class on the inbound path — the topic embeds app_key and the listener logs it raw.

The listener logs the full topic_str on every received message (sigenergy.py:1521) and in the non-JSON warning (sigenergy.py:1507), and those topics embed app_key (SIGENERGY_MQTT_TOPIC_CHANGE/PERIOD/ALARM, sigenergy.py:148-150) — a key this very list classifies as credential-bearing (it is the MQTT broker username and half of the base64 login key). So a user pasting a debug log still leaks it on every inbound line; the fix covers only the outbound publish line.

Worth either redact()-ing the topic (or the format args) in those two listener lines as part of this PR, or an explicit follow-up. The CLI tools (test_sigenergy_api, test_mqtt_connection) deliberately print only a 10-char app_key prefix, but the listener lines print the full topic.

Separately: the key/token catch-alls here will mask benign diagnostic fields in what is often the only log line for debugging a broker-side rejection (an alarm/instruction entry carrying a literal key id renders as <redacted>), while the match is case-exact, so a snake_case access_token variant would slip through. Sibling lists cover both directions (Deye adds tokenHash; Sunsynk covers snake_case plus Authorization/sign). Worth a conscious decision on both edges.

[springfall2008]

redact() now guards exactly one log line. _request() still logs params/json_data verbatim (sigenergy.py:474) and the full parsed response body (sigenergy.py:530) — both clean today (no current caller passes a credential, no current body carries one, and the token goes in the Authorization header which is never logged), but any future endpoint that accepts or echoes a token-bearing body re-opens the leak through a site the new invariant doesn't cover.

Applying redact() at those two lines too (or routing all payload/body logging through one helper) would make the guarantee hold by construction rather than by call site — this is the same bandaid pattern that regressed once already in a sibling integration.

[springfall2008]

Minor latent edge: this branch recurses list only, but json.dumps() serialises tuples as JSON arrays too — so a future payload with {"commands": ({...},)} would be published as an array yet logged unmasked, silently breaking the "redaction is log-only, content-equal" assumption. Cheap fix: accept any non-str/bytes sequence.

Also worth noting this is now the fourth hand-rolled redact copy (deye, sunsynk, alphaess — the last non-recursive) with four independently-drifting key tuples; a single parameterised helper in utils.py next to mask_secret_args would make the next integration correct by default. Not a blocker for this PR, which deliberately mirrors the existing pattern.

[springfall2008]

Two test-strength gaps:

1. This loop re-hardcodes the 7-key tuple instead of importing SIGENERGY_LOG_REDACT_KEYS, so a key added to the constant later is silently untested — the "every documented credential key is covered" guarantee only holds for today's snapshot. Iterating the constant closes that.

2. The nesting case covered is dict-in-list; dict-in-dict ({"outer": {"accessToken": "x"}}) never exercises the dict-value recursion branch, so deleting that recursion would leave the suite green. One added case covers it.

[springfall2008]

The nested commands[0] here carries no credential key, so both tests would stay green if _publish_mqtt regressed to a top-level-only redaction — yet the exact shape of the #4920 bug (a credential nested inside commands) is never asserted through the publish path; test_sigenergy_redact covers nesting but tests the function directly, not the wiring. Adding e.g. a "password" to commands[0] and asserting it is masked in the published log line closes that gap.

Minor: topic is unpacked from mock_client.publishes[0] but never asserted (the sibling publish_mqtt_success test does assert it), so a topic swap would also pass this test.