From d49e0f3d7ca721ce9c51e64c8248d70e262c1062 Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:36:01 +0800 Subject: [PATCH] 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 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- backend/app/gateway/services.py | 31 ++++++++++++++++ backend/tests/test_gateway_services.py | 50 ++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/backend/app/gateway/services.py b/backend/app/gateway/services.py index 671316c749..1afb8e3bf1 100644 --- a/backend/app/gateway/services.py +++ b/backend/app/gateway/services.py @@ -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") diff --git a/backend/tests/test_gateway_services.py b/backend/tests/test_gateway_services.py index ec07e3ac0e..d4ee65f5a1 100644 --- a/backend/tests/test_gateway_services.py +++ b/backend/tests/test_gateway_services.py @@ -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", [