Skip to content

Commit 03313f7

Browse files
kingpanther13claude
andcommitted
fix: redact credentialed URLs in diagnostics and drop the unconditional reinstall claim
Two CodeRabbit findings on the pinner sentence: - Requirement strings reach warnings and repair issues - text users paste into public bug reports - and a direct reference may embed tokens; URL userinfo and query parameters are now redacted textually in both the violation and pinner sentences. - Home Assistant reinstalls a manifest requirement only when setup finds it unsatisfied; the sentence now says exactly that instead of claiming a reinstall on every setup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QiuyTYBQ2opBVPp8xzMq3r
1 parent df48beb commit 03313f7

2 files changed

Lines changed: 51 additions & 8 deletions

File tree

custom_components/ha_mcp_tools/dependency_diagnostics.py

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
from __future__ import annotations
3737

3838
import json
39+
import re
3940
from collections import deque
4041
from collections.abc import Iterator
4142
from dataclasses import dataclass
@@ -546,31 +547,48 @@ def _exception_text(exc: BaseException) -> str:
546547

547548
def _violation_sentence(violation: DependencyViolation) -> str:
548549
"""One sentence naming an unsatisfied requirement and who declared it."""
550+
requirement = _redact_requirement(violation.requirement)
549551
if violation.installed is None:
550552
if violation.required_by == REQUESTED_BY:
551553
return f"Package {violation.package} is not installed."
552554
return (
553555
f"Package {violation.package} is not installed, but "
554-
f"'{violation.requirement}' is required by {violation.required_by}."
556+
f"'{requirement}' is required by {violation.required_by}."
555557
)
556558
return (
557559
f"Installed {violation.package} {violation.installed} does not satisfy "
558-
f"'{violation.requirement}' required by {violation.required_by}."
560+
f"'{requirement}' required by {violation.required_by}."
559561
)
560562

561563

564+
def _redact_requirement(raw: str) -> str:
565+
"""``raw`` with URL credentials and query parameters removed.
566+
567+
Requirement strings reach user-facing warnings and repair issues —
568+
text users paste into public bug reports — and a direct reference may
569+
embed tokens (``mcp @ https://user:token@host/pkg.whl?sig=...``).
570+
Textual redaction rather than URL parsing keeps this total: an input
571+
no parser accepts still comes back with its userinfo and query gone.
572+
"""
573+
if "://" not in raw:
574+
return raw
575+
redacted = re.sub(r"://[^/@\s]*@", "://", raw)
576+
return re.sub(r"\?\S*", "", redacted)
577+
578+
562579
def _pinner_sentence(pinner: PinningIntegration) -> str:
563580
"""One sentence naming a pinning integration and why the pin keeps coming back.
564581
565-
"Whenever ... is set up" rather than "at every startup": the manifest
566-
scan is directory-wide, and a dormant integration's requirement is only
567-
enforced once something sets it up — the softer claim is truthful for
568-
both the configured culprit and a leftover directory.
582+
The enforcement claim is conditional on purpose: the manifest scan is
583+
directory-wide, a dormant integration's requirement is only processed
584+
once something sets it up, and even then Home Assistant reinstalls it
585+
only when the installed version does not satisfy it.
569586
"""
570587
return (
571588
f"The custom integration '{pinner.name}' ({pinner.domain}) pins "
572-
f"'{pinner.requirement}' in its manifest, and Home Assistant "
573-
f"reinstalls that requirement whenever the integration is set up."
589+
f"'{_redact_requirement(pinner.requirement)}' in its manifest, and "
590+
f"Home Assistant may reinstall that requirement when the "
591+
f"integration's setup finds it unsatisfied."
574592
)
575593

576594

tests/src/unit/test_dependency_diagnostics.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,31 @@ def test_message_less_exception_falls_back_to_its_type(self):
656656

657657
assert message.startswith("The underlying failure is: ImportError.")
658658

659+
def test_credentialed_url_requirements_are_redacted(self):
660+
"""CodeRabbit on #2245: repair details get pasted into public issues."""
661+
message = describe_dependency_failure(
662+
None,
663+
[
664+
DependencyViolation(
665+
package="mcp",
666+
installed=_PINNED_MCP,
667+
requirement="mcp@ https://user:vsecret@host.invalid/m.whl?vsig=1",
668+
required_by="something 1.0",
669+
)
670+
],
671+
[
672+
PinningIntegration(
673+
domain="p",
674+
name="P",
675+
requirement="mcp @ https://user:psecret@host.invalid/m.whl?ptok=2",
676+
)
677+
],
678+
)
679+
680+
for leaked in ("vsecret", "psecret", "vsig=1", "ptok=2", "user:"):
681+
assert leaked not in message
682+
assert "host.invalid" in message
683+
659684
def test_nothing_found_says_so(self):
660685
assert (
661686
describe_dependency_failure(None, [], [])

0 commit comments

Comments
 (0)