Skip to content

Commit 3cb07cf

Browse files
sergeykadSergeyclaudejulienld
authored
fix: remove internal info leaks from error responses (#517) (#586)
* fix: remove internal info leaks from error responses (#517) - ha_deep_search: replace traceback.format_exc() with standard exception_to_structured_error() pattern; wrap both paths with add_timezone_metadata() for consistency - ha_dashboard_find_card: replace ad-hoc error dict with exception_to_structured_error(); remove error_type field that exposed Python exception class names to clients Add unit tests verifying no traceback/error_type in error responses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: normalize line endings to LF Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Sergey <sergey@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Julien Larocque-Dupont <github@qc-h.net>
1 parent 99cec2f commit 3cb07cf

6 files changed

Lines changed: 405 additions & 79 deletions

File tree

src/ha_mcp/tools/helpers.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,11 @@ def exception_to_structured_error(
203203
error_response = create_auth_error(error_msg)
204204

205205
else:
206-
# Default to internal error
206+
# Default to internal error -- use generic message to avoid leaking internals
207207
error_response = create_error_response(
208208
ErrorCode.INTERNAL_ERROR,
209-
error_msg,
210-
details="An unexpected error occurred",
209+
"An unexpected error occurred",
210+
details=error_msg,
211211
context=context,
212212
)
213213

src/ha_mcp/tools/tools_config_dashboards.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
get_security_documentation,
2222
safe_execute,
2323
)
24-
from .helpers import log_tool_usage
24+
from .helpers import exception_to_structured_error, log_tool_usage
2525
from .util_helpers import parse_json_param
2626

2727
logger = logging.getLogger(__name__)
@@ -1688,14 +1688,24 @@ async def ha_dashboard_find_card(
16881688
f"error={e}",
16891689
exc_info=True,
16901690
)
1691-
return {
1692-
"success": False,
1693-
"action": "find_card",
1694-
"url_path": url_path,
1695-
"error": str(e) if str(e) else f"{type(e).__name__} (no details)",
1696-
"error_type": type(e).__name__,
1697-
"suggestions": [
1691+
error_response = exception_to_structured_error(
1692+
e,
1693+
context={
1694+
"action": "find_card",
1695+
"url_path": url_path,
1696+
"entity_id": entity_id,
1697+
"card_type": card_type,
1698+
"heading": heading,
1699+
},
1700+
)
1701+
if "error" in error_response and isinstance(error_response["error"], dict):
1702+
error_response["error"]["suggestions"] = [
16981703
"Check HA connection",
16991704
"Verify dashboard with ha_config_get_dashboard(list_only=True)",
1700-
],
1701-
}
1705+
]
1706+
else:
1707+
logger.warning(
1708+
f"Unexpected error response structure, could not add suggestions: "
1709+
f"{type(error_response.get('error'))}"
1710+
)
1711+
return error_response

src/ha_mcp/tools/tools_search.py

Lines changed: 141 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,20 @@
44
This module provides entity search, system overview, deep search, and state retrieval tools.
55
"""
66

7+
import asyncio
78
import logging
89
from typing import Annotated, Any, Literal, cast
910

1011
from pydantic import Field
1112

1213
from ..errors import create_entity_not_found_error
1314
from .helpers import exception_to_structured_error, log_tool_usage
14-
from .util_helpers import add_timezone_metadata, coerce_bool_param, coerce_int_param, parse_string_list_param
15+
from .util_helpers import (
16+
add_timezone_metadata,
17+
coerce_bool_param,
18+
coerce_int_param,
19+
parse_string_list_param,
20+
)
1521

1622
logger = logging.getLogger(__name__)
1723

@@ -56,14 +62,18 @@ async def _exact_match_search(
5662
# Check for exact substring match in entity_id or friendly_name
5763
if query_lower in entity_id.lower() or query_lower in friendly_name.lower():
5864
is_exact = query_lower == entity_id.lower() or query_lower == friendly_name.lower()
59-
results.append({
60-
"entity_id": entity_id,
61-
"friendly_name": friendly_name,
62-
"domain": domain,
63-
"state": entity.get("state", "unknown"),
64-
"score": 100 if is_exact else 80,
65-
"match_type": "exact_match",
66-
})
65+
results.append(
66+
{
67+
"entity_id": entity_id,
68+
"friendly_name": friendly_name,
69+
"domain": domain,
70+
"state": entity.get("state", "unknown"),
71+
"score": 100
72+
if is_exact
73+
else 80,
74+
"match_type": "exact_match",
75+
}
76+
)
6777

6878
# Sort by score descending
6979
results.sort(key=lambda x: x["score"], reverse=True)
@@ -98,14 +108,16 @@ async def _partial_results_search(
98108
if domain_filter and domain != domain_filter:
99109
continue
100110

101-
results.append({
102-
"entity_id": entity_id,
103-
"friendly_name": friendly_name,
104-
"domain": domain,
105-
"state": entity.get("state", "unknown"),
106-
"score": 0,
107-
"match_type": "partial_listing",
108-
})
111+
results.append(
112+
{
113+
"entity_id": entity_id,
114+
"friendly_name": friendly_name,
115+
"domain": domain,
116+
"state": entity.get("state", "unknown"),
117+
"score": 0,
118+
"match_type": "partial_listing",
119+
}
120+
)
109121

110122
paginated = results[offset:offset + limit]
111123
return {
@@ -124,7 +136,14 @@ def register_search_tools(mcp: Any, client: Any, **kwargs: Any) -> None:
124136
if not smart_tools:
125137
raise ValueError("smart_tools is required for search tools registration")
126138

127-
@mcp.tool(annotations={"idempotentHint": True, "readOnlyHint": True, "tags": ["search"], "title": "Search Entities"})
139+
@mcp.tool(
140+
annotations={
141+
"idempotentHint": True,
142+
"readOnlyHint": True,
143+
"tags": ["search"],
144+
"title": "Search Entities",
145+
}
146+
)
128147
@log_tool_usage
129148
async def ha_search_entities(
130149
query: str,
@@ -156,7 +175,10 @@ async def ha_search_entities(
156175
- 'standard': Complete picture (all entities, friendly names only) - for comprehensive tasks
157176
- 'full': Maximum detail (includes states, device types, services) - for deep analysis"""
158177
# Coerce boolean parameter that may come as string from XML-style calls
159-
group_by_domain_bool = coerce_bool_param(group_by_domain, "group_by_domain", default=False) or False
178+
group_by_domain_bool = (
179+
coerce_bool_param(group_by_domain, "group_by_domain", default=False)
180+
or False
181+
)
160182

161183
try:
162184
offset = coerce_int_param(offset, "offset", default=0, min_value=0) or 0
@@ -177,7 +199,9 @@ async def ha_search_entities(
177199
if isinstance(
178200
area_data["entities"], dict
179201
): # grouped by domain
180-
for domain_entities in area_data["entities"].values():
202+
for domain_entities in area_data[
203+
"entities"
204+
].values():
181205
all_area_entities.extend(domain_entities)
182206
else: # flat list
183207
all_area_entities.extend(area_data["entities"])
@@ -283,7 +307,8 @@ async def ha_search_entities(
283307

284308
# Filter by domain
285309
filtered_entities = [
286-
e for e in all_entities
310+
e
311+
for e in all_entities
287312
if e.get("entity_id", "").startswith(f"{domain_filter}.")
288313
]
289314

@@ -293,14 +318,16 @@ async def ha_search_entities(
293318
for entity in paginated_entities:
294319
entity_id = entity.get("entity_id", "")
295320
attributes = entity.get("attributes", {})
296-
results.append({
297-
"entity_id": entity_id,
298-
"friendly_name": attributes.get("friendly_name", entity_id),
299-
"domain": domain_filter,
300-
"state": entity.get("state", "unknown"),
301-
"score": 100, # Perfect match since we're listing by domain
302-
"match_type": "domain_listing",
303-
})
321+
results.append(
322+
{
323+
"entity_id": entity_id,
324+
"friendly_name": attributes.get("friendly_name", entity_id),
325+
"domain": domain_filter,
326+
"state": entity.get("state", "unknown"),
327+
"score": 100, # Perfect match since we're listing by domain
328+
"match_type": "domain_listing",
329+
}
330+
)
304331

305332
domain_list_data: dict[str, Any] = {
306333
"success": True,
@@ -327,30 +354,45 @@ async def ha_search_entities(
327354

328355
# Step 1: Try fuzzy search
329356
try:
330-
result = await smart_tools.smart_entity_search(query, limit, offset=offset, domain_filter=domain_filter)
357+
result = await smart_tools.smart_entity_search(
358+
query, limit, offset=offset, domain_filter=domain_filter
359+
)
331360
search_type = "fuzzy_search"
361+
except asyncio.CancelledError:
362+
raise
332363
except Exception as fuzzy_error:
333-
logger.warning(f"Fuzzy search failed, trying exact match: {fuzzy_error}")
364+
logger.warning(
365+
f"Fuzzy search failed, trying exact match: {fuzzy_error}"
366+
)
334367

335368
# Step 2: Try exact match fallback
336369
try:
337-
result = await _exact_match_search(client, query, domain_filter, limit, offset)
370+
result = await _exact_match_search(
371+
client, query, domain_filter, limit, offset
372+
)
338373
warning = "Fuzzy search unavailable, using exact match"
339374
search_type = "exact_match"
375+
except asyncio.CancelledError:
376+
raise
340377
except Exception as exact_error:
341-
logger.warning(f"Exact match failed, trying partial results: {exact_error}")
378+
logger.warning(
379+
f"Exact match failed, trying partial results: {exact_error}"
380+
)
342381

343382
# Step 3: Try partial results fallback
344383
try:
345-
result = await _partial_results_search(client, query, domain_filter, limit, offset)
384+
result = await _partial_results_search(
385+
client, query, domain_filter, limit, offset
386+
)
346387
warning = "Search degraded, returning partial results"
347388
search_type = "partial_listing"
389+
except asyncio.CancelledError:
390+
raise
348391
except Exception as partial_error:
349392
# Step 4: All methods failed - raise to outer exception handler
350393
logger.error(f"All search methods failed: {partial_error}")
351394
raise Exception(
352-
f"All search methods failed. Fuzzy: {fuzzy_error}, "
353-
f"Exact: {exact_error}, Partial: {partial_error}"
395+
"All search methods failed"
354396
) from partial_error
355397

356398
# Convert 'matches' to 'results' for backward compatibility
@@ -408,9 +450,21 @@ async def ha_search_entities(
408450
"Try simpler search terms",
409451
"Check area/domain filter spelling",
410452
]
453+
else:
454+
logger.warning(
455+
f"Unexpected error response structure, could not add suggestions: "
456+
f"{type(error_response.get('error'))}"
457+
)
411458
return await add_timezone_metadata(client, error_response)
412459

413-
@mcp.tool(annotations={"idempotentHint": True, "readOnlyHint": True, "tags": ["search"], "title": "Get System Overview"})
460+
@mcp.tool(
461+
annotations={
462+
"idempotentHint": True,
463+
"readOnlyHint": True,
464+
"tags": ["search"],
465+
"title": "Get System Overview",
466+
}
467+
)
414468
@log_tool_usage
415469
async def ha_get_overview(
416470
detail_level: Annotated[
@@ -453,11 +507,18 @@ async def ha_get_overview(
453507
Use 'standard' (default) for most queries. Optionally customize entity fields and limits.
454508
"""
455509
# Coerce boolean parameters that may come as strings from XML-style calls
456-
include_state_bool = coerce_bool_param(include_state, "include_state", default=None)
457-
include_entity_id_bool = coerce_bool_param(include_entity_id, "include_entity_id", default=None)
510+
include_state_bool = coerce_bool_param(
511+
include_state, "include_state", default=None
512+
)
513+
include_entity_id_bool = coerce_bool_param(
514+
include_entity_id, "include_entity_id", default=None
515+
)
458516

459517
result = await smart_tools.get_system_overview(
460-
detail_level, max_entities_per_domain, include_state_bool, include_entity_id_bool
518+
detail_level,
519+
max_entities_per_domain,
520+
include_state_bool,
521+
include_entity_id_bool,
461522
)
462523
result = cast(dict[str, Any], result)
463524

@@ -492,7 +553,14 @@ async def ha_get_overview(
492553

493554
return result
494555

495-
@mcp.tool(annotations={"idempotentHint": True, "readOnlyHint": True, "tags": ["search"], "title": "Deep Search"})
556+
@mcp.tool(
557+
annotations={
558+
"idempotentHint": True,
559+
"readOnlyHint": True,
560+
"tags": ["search"],
561+
"title": "Deep Search",
562+
}
563+
)
496564
@log_tool_usage
497565
async def ha_deep_search(
498566
query: str,
@@ -553,17 +621,40 @@ async def ha_deep_search(
553621
)
554622
return cast(dict[str, Any], result)
555623
except Exception as e:
556-
return {
557-
"success": False,
558-
"error": str(e),
559-
"query": query,
560-
"suggestions": [
624+
logger.error(
625+
f"Error in deep search: query={query}, "
626+
f"search_types={parsed_search_types}, limit={limit}, "
627+
f"error={e}",
628+
exc_info=True,
629+
)
630+
error_response = exception_to_structured_error(
631+
e,
632+
context={
633+
"query": query,
634+
"search_types": parsed_search_types,
635+
"limit": limit,
636+
},
637+
)
638+
if "error" in error_response and isinstance(error_response["error"], dict):
639+
error_response["error"]["suggestions"] = [
561640
"Check Home Assistant connection",
562641
"Try simpler search terms",
563-
],
564-
}
565-
566-
@mcp.tool(annotations={"idempotentHint": True, "readOnlyHint": True, "tags": ["search"], "title": "Get Entity State"})
642+
]
643+
else:
644+
logger.warning(
645+
f"Unexpected error response structure, could not add suggestions: "
646+
f"{type(error_response.get('error'))}"
647+
)
648+
return error_response
649+
650+
@mcp.tool(
651+
annotations={
652+
"idempotentHint": True,
653+
"readOnlyHint": True,
654+
"tags": ["search"],
655+
"title": "Get Entity State",
656+
}
657+
)
567658
@log_tool_usage
568659
async def ha_get_state(entity_id: str) -> dict[str, Any]:
569660
"""Get detailed state information for a Home Assistant entity with timezone metadata."""

0 commit comments

Comments
 (0)