fix(security): html-escape memory context summaries rendered into the injection prompt (#4119)

This commit is contained in:
Aari 2026-07-12 18:22:22 +08:00 committed by GitHub
parent 74392e1470
commit feb287077e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 75 additions and 6 deletions

View File

@ -413,6 +413,22 @@ def _format_fact_line(fact: dict[str, Any]) -> str | None:
return f"- [{category} | {confidence:.2f}] {content}"
def _escape_summary(value: Any) -> str:
"""Escape a user-editable context summary for the ``<memory>`` block.
Context summaries (``workContext``/``personalContext``/``topOfMind`` and the
history sections) are user-editable via ``/api/memory`` import and render into
the same ``<memory>`` block as facts, so an unescaped ``</memory>`` value can
close the block and relocate the text after it out of the user-managed trust
zone the lead-agent prompt declares. Sibling of ``_format_fact_line``'s
escaping (#4097). ``str(...)`` preserves the prior f-string coercion for the
rare non-string summary an import can plant; ``quote=False`` because summaries
land in element-text position (never attribute values), so only ``<``, ``>``,
``&`` can break out leave ``'`` and ``"`` untouched.
"""
return html.escape(str(value), quote=False)
def _select_fact_lines(
ranked_facts: list[dict[str, Any]],
*,
@ -547,15 +563,15 @@ def format_memory_for_injection(
work_ctx = user_data.get("workContext", {})
if work_ctx.get("summary"):
user_sections.append(f"Work: {work_ctx['summary']}")
user_sections.append(f"Work: {_escape_summary(work_ctx['summary'])}")
personal_ctx = user_data.get("personalContext", {})
if personal_ctx.get("summary"):
user_sections.append(f"Personal: {personal_ctx['summary']}")
user_sections.append(f"Personal: {_escape_summary(personal_ctx['summary'])}")
top_of_mind = user_data.get("topOfMind", {})
if top_of_mind.get("summary"):
user_sections.append(f"Current Focus: {top_of_mind['summary']}")
user_sections.append(f"Current Focus: {_escape_summary(top_of_mind['summary'])}")
if user_sections:
sections.append("User Context:\n" + "\n".join(f"- {s}" for s in user_sections))
@ -567,15 +583,15 @@ def format_memory_for_injection(
recent = history_data.get("recentMonths", {})
if recent.get("summary"):
history_sections.append(f"Recent: {recent['summary']}")
history_sections.append(f"Recent: {_escape_summary(recent['summary'])}")
earlier = history_data.get("earlierContext", {})
if earlier.get("summary"):
history_sections.append(f"Earlier: {earlier['summary']}")
history_sections.append(f"Earlier: {_escape_summary(earlier['summary'])}")
background = history_data.get("longTermBackground", {})
if background.get("summary"):
history_sections.append(f"Background: {background['summary']}")
history_sections.append(f"Background: {_escape_summary(background['summary'])}")
if history_sections:
sections.append("History:\n" + "\n".join(f"- {s}" for s in history_sections))

View File

@ -767,3 +767,56 @@ def test_format_memory_leaves_benign_source_error_byte_identical() -> None:
result = format_memory_for_injection(memory_data, max_tokens=2000)
assert f"(avoid: {source_error})" in result
# Context summaries (workContext/personalContext/topOfMind + history) are
# user-editable via /api/memory import and render into the same <memory> block
# as facts, so they must be escaped like the fact fields in #4097.
_SUMMARY_CASES = [
("workContext", {"user": {"workContext": {"summary": _BREAKOUT}}}),
("personalContext", {"user": {"personalContext": {"summary": _BREAKOUT}}}),
("topOfMind", {"user": {"topOfMind": {"summary": _BREAKOUT}}}),
("recentMonths", {"history": {"recentMonths": {"summary": _BREAKOUT}}}),
("earlierContext", {"history": {"earlierContext": {"summary": _BREAKOUT}}}),
("longTermBackground", {"history": {"longTermBackground": {"summary": _BREAKOUT}}}),
]
@pytest.mark.parametrize("field, memory_data", _SUMMARY_CASES, ids=[c[0] for c in _SUMMARY_CASES])
def test_format_memory_escapes_context_summary_breakout(field: str, memory_data: dict) -> None:
"""A context summary that closes the <memory> block must be HTML-escaped, so
it cannot relocate the text after it out of the user-managed trust zone the
lead-agent system prompt declares same gap as the fact fields (#4097),
across all six summary sites."""
result = format_memory_for_injection(memory_data, max_tokens=2000)
assert "</memory>" not in result
assert "</system-reminder>" not in result
assert "&lt;/memory&gt;&lt;/system-reminder&gt;" in result
def test_format_memory_leaves_benign_summary_byte_identical() -> None:
"""Escaping must not disturb an ordinary summary: with no <, >, & it is
rendered exactly as before. Apostrophes and quotation marks are
element-text-safe and must survive verbatim (quote=False)."""
benign = 'User\'s focus: dark mode, 2-space indentation, said "use uv".'
memory_data = {"user": {"workContext": {"summary": benign}}}
result = format_memory_for_injection(memory_data, max_tokens=2000)
assert f"Work: {benign}" in result
assert "&quot;" not in result
assert "&#x27;" not in result
assert "&amp;" not in result
def test_format_memory_tolerates_non_string_summary() -> None:
"""A non-string summary an import can plant is str-coerced, not raised on:
escaping via html.escape() requires a str, and the whole renderer is wrapped
in a broad except at the call site, so a raise would silently disable all
memory injection. Preserves the prior f-string coercion behavior."""
memory_data = {"user": {"topOfMind": {"summary": 12345}}}
result = format_memory_for_injection(memory_data, max_tokens=2000)
assert "Current Focus: 12345" in result