Skip to content

Commit 2e42bfb

Browse files
kingpanther13claude
andcommitted
fix(flows): report an entry position, not the caller's step id
A step id is a caller-controlled key, and these rejections reach the usage log unmasked the same way a value would. Nothing echoed a NESTED caller key before this directive existed — the walker's supplied_keys reports only top-level names — so this was new exposure rather than an existing one. Errors now name the entry's position (step_values entry #2) and its types. That still says which entry is wrong, and cannot carry anything the caller typed. Found by CodeRabbit review on #2256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr
1 parent 08e3931 commit 2e42bfb

2 files changed

Lines changed: 28 additions & 8 deletions

File tree

src/ha_mcp/tools/config_entry_flow_form.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -883,21 +883,27 @@ def validate_step_values(config: dict[str, Any]) -> None:
883883
)
884884
)
885885

886-
for step_id, entry in directive.items():
886+
# The step id is a caller-controlled key, and nothing echoed a NESTED one
887+
# before this directive existed — the walker's own supplied_keys reports
888+
# only top-level names. These errors reach the usage log unmasked (see the
889+
# note above), so report the entry's POSITION instead: it is what the
890+
# caller needs to find the entry in their own directive, and it cannot
891+
# carry a value they typed (CodeRabbit review, issue #2254).
892+
for index, entry in enumerate(directive.values(), start=1):
893+
where = f"{_PER_STEP_VALUES_KEY} entry #{index}"
887894
entries = entry if isinstance(entry, list) else [entry]
888895
if any(not isinstance(item, dict) for item in entries):
889896
raise_tool_error(
890897
create_error_response(
891898
ErrorCode.VALIDATION_INVALID_PARAMETER,
892-
f"'{_PER_STEP_VALUES_KEY}[{step_id!r}]' must be an object "
893-
"of field values, or a list of them for a step the flow "
894-
"presents more than once",
899+
f"{where} must be an object of field values, or a list "
900+
"of them for a step the flow presents more than once",
895901
suggestions=[
896902
f"Pass {_PER_STEP_VALUES_KEY}={example}, or a list of "
897903
"those objects to supply one per encounter.",
898904
],
899905
context={
900-
"step_id": step_id,
906+
"entry_index": index,
901907
"entry_types": [type(item).__name__ for item in entries],
902908
},
903909
)
@@ -911,12 +917,11 @@ def validate_step_values(config: dict[str, Any]) -> None:
911917
raise_tool_error(
912918
create_error_response(
913919
ErrorCode.VALIDATION_INVALID_PARAMETER,
914-
f"'{_PER_STEP_VALUES_KEY}[{step_id!r}]' supplies no field "
915-
"values, so it would apply nothing",
920+
f"{where} supplies no field values, so it would apply nothing",
916921
suggestions=[
917922
"Name at least one field for the step, or drop the entry.",
918923
],
919-
context={"step_id": step_id, "entry_count": len(entries)},
924+
context={"entry_index": index, "entry_count": len(entries)},
920925
)
921926
)
922927

tests/src/unit/test_flow_options_preserve.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,6 +1301,21 @@ def test_a_rejection_reports_shapes_not_submitted_values(self) -> None:
13011301
assert "entry_types" in message
13021302
assert "str" in message
13031303

1304+
def test_a_rejection_reports_a_position_not_the_step_id(self) -> None:
1305+
"""A step id is a caller-controlled key and reaches the same log.
1306+
1307+
Nothing echoed a NESTED caller key before this directive existed —
1308+
the walker's supplied_keys reports only top-level names — so the
1309+
position is reported instead. It still says WHICH entry is wrong
1310+
without carrying anything the caller typed (CodeRabbit review).
1311+
"""
1312+
secret = "sk-not-in-the-log"
1313+
with pytest.raises(ToolError) as exc_info:
1314+
validate_step_values({"step_values": {"init": {"a": 1}, secret: {}}})
1315+
message = str(exc_info.value)
1316+
assert secret not in message, "A step id reached the error payload"
1317+
assert "entry #2" in message, message
1318+
13041319
async def test_the_walker_rejects_before_driving_any_step(self) -> None:
13051320
"""It must raise before a single step is submitted."""
13061321
submit_fn = AsyncMock()

0 commit comments

Comments
 (0)