fix(gateway): reject unusable skill_context entries at the input boundary (#8091)

* fix(gateway): reject unusable skill_context entries at the input boundary

merge_skill_context indexes entry["path"] and calls entry.get(), so a
caller-supplied non-object entry or one without a usable path raised during
the state merge and failed the run with a 500. Validate the list shape at the
run-input and thread-state boundaries, mirroring the todos and delegations
channels.

* Normalize entry by ensuring 'name' is a string

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
yetuge 2026-11-10 15:36:01 +08:00 committed by GitHub
parent 91f4fb4570
commit d49e0f3d7c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 81 additions and 0 deletions

View File

@ -676,6 +676,32 @@ def _normalize_external_todos(value: Any, *, location: str) -> list[dict[str, An
return normalized
def _normalize_external_skill_context(value: Any, *, location: str) -> list[dict[str, Any]] | None:
"""Reject skill references the durable-context reducer cannot read.
``skill_context`` is a checkpointed reducer channel a caller may Overwrite.
``merge_skill_context`` normalizes each entry with ``entry.get(...)`` and
``str(entry["path"])``, so a non-object entry or a missing path raises while
the state update merges — failing the run with an unhandled
``AttributeError``/``KeyError`` instead of a 400 at this boundary. The other
fields are optional: the reducer coerces ``name``/``description`` and
``loaded_at`` itself, so only the path needs to be usable here.
"""
if value is None:
return None
if not isinstance(value, list):
raise HTTPException(status_code=400, detail=f"{location} must be a list of skill reference objects")
normalized: list[dict[str, Any]] = []
for index, entry in enumerate(value):
if not isinstance(entry, Mapping):
raise HTTPException(status_code=400, detail=f"{location}[{index}] must be an object")
path = entry.get("path")
if not isinstance(path, str) or not path:
raise HTTPException(status_code=400, detail=f"{location}[{index}].path must be a non-empty string")
normalized.append({**entry, "name": str(entry.get("name") or "")})
return normalized
def _normalize_external_goal(value: Any, *, location: str) -> dict[str, Any] | None:
"""Require usable goal text without rebuilding caller-supplied goal state."""
if value is None:
@ -730,6 +756,9 @@ def strip_server_owned_state_metadata(values: Mapping[str, Any]) -> dict[str, An
stripped[channel] = _normalize_input_messages(value, location="values.messages")
elif channel == "delegations" and isinstance(value, list):
stripped[channel] = [_strip_external_delegation_verdict(item) for item in value]
elif channel == "skill_context":
normalized_skill_context = _normalize_external_skill_context(value, location="values.skill_context")
stripped[channel] = None if normalized_skill_context is None else [_strip_external_metadata_from_message_like(item) for item in normalized_skill_context]
elif isinstance(value, list):
stripped[channel] = [_strip_external_metadata_from_message_like(item) for item in value]
else:
@ -798,6 +827,8 @@ def normalize_input(raw_input: dict[str, Any] | None, *, trusted_internal: bool
cleaned = [_strip_external_delegation_verdict(entry) for entry in delegations]
if cleaned != delegations:
result = {**result, "delegations": cleaned}
if "skill_context" in result:
result = {**result, "skill_context": _normalize_external_skill_context(result["skill_context"], location="input.skill_context")}
# merge_goal ignores None, so only a goal value replaces the goal.
if result.get("goal") is not None:
normalized_goal = _normalize_external_goal(result["goal"], location="input.goal")

View File

@ -861,6 +861,56 @@ def test_normalize_input_rejects_malformed_message_with_400():
assert "input.messages[1]" in excinfo.value.detail
@pytest.mark.parametrize(
"skill_context",
[
"oops",
["oops"],
[{"name": "pdf"}],
[{"path": 3}],
[{"path": ""}],
],
ids=["string-channel", "non-object-entry", "missing-path", "non-string-path", "empty-path"],
)
def test_normalize_input_rejects_unusable_skill_context_with_400(skill_context):
"""A skill reference the durable-context reducer cannot read must be a 400,
not an ``AttributeError``/``KeyError`` while the state update merges.
``merge_skill_context`` normalizes entries with ``entry.get(...)`` and
``str(entry["path"])``; both crash on the shapes below.
"""
from fastapi import HTTPException
from app.gateway.services import normalize_input
with pytest.raises(HTTPException) as excinfo:
normalize_input({"skill_context": skill_context})
assert excinfo.value.status_code == 400
assert "input.skill_context" in excinfo.value.detail
def test_normalize_input_keeps_usable_skill_context_entries():
from app.gateway.services import normalize_input
entry = {"name": "pdf", "path": "/mnt/skills/pdf/SKILL.md", "description": "PDF tooling", "loaded_at": 3}
assert normalize_input({"skill_context": [entry]})["skill_context"] == [entry]
def test_state_values_reject_unusable_skill_context_with_400():
"""The thread-state update path shares the skill-reference boundary."""
from fastapi import HTTPException
from app.gateway.services import strip_server_owned_state_metadata
with pytest.raises(HTTPException) as excinfo:
strip_server_owned_state_metadata({"skill_context": ["oops"]})
assert excinfo.value.status_code == 400
assert "values.skill_context" in excinfo.value.detail
@pytest.mark.parametrize(
"message",
[