fix(gateway): strip stale dangerous-confirmation text in user messages (#59607)

When a high-risk side effect (e.g. host restart via shutdown.exe) runs,
the user's plain-text confirmation phrase is persisted in the conversation
transcript. If the host restart killed the gateway process before the
assistant's tool result was written, the transcript tail ends on the
assistant's text response - and the dangerous confirmation text remains
in the user role.

On the next inbound message - possibly a casual 'are you there?' from
the user minutes later - the LLM sees the stale confirmation and may
interpret the new turn as a fresh re-confirmation, re-executing the
destructive action. This is the failure mode reported in #59607.

Fix:
- Add strip_stale_dangerous_confirmations() in agent/replay_cleanup.py
  that removes user messages whose content matches a known dangerous
  confirmation pattern AND whose timestamp is older than 60 seconds.
- Add is_dangerous_confirmation() helper with the matched patterns
  (i18n-aware: covers 確認強制重開機 from the original incident).
- Wire the stripper into _build_gateway_agent_history() right after the
  existing 75ed07ace strippers, so the strip chain is:
  strip_interrupted_tool_tails -> strip_dangling_tool_call_tail ->
  strip_stale_dangerous_confirmations.
- Update _build_replay_entry() to preserve the timestamp on user
  messages (it was previously dropped), since the new stripper needs it.

Complements 75ed07ace (which strips the assistant side of the broken
tail) by handling the user side: a stale plain-text confirmation that
the assistant has not yet responded to in a way the resume logic
recognises.

Failing-test-first discipline: the bug-detection test
test_stale_confirmation_text_is_stripped_on_resume fails on unfixed
code (proves the test catches the bug) and passes after the fix.
Five additional safety tests confirm no regression on:
- fresh confirmations (within expiry) are preserved
- non-confirmation text is preserved
- non-matching histories are untouched
- dangerous-pattern detection works in all cases (case, i18n, None)
- direct unit test of the strip helper

Refs: #59607
This commit is contained in:
knoal 2026-07-06 06:27:52 -07:00 committed by kshitij
parent 11516f3cc3
commit 33a529538d
3 changed files with 327 additions and 4 deletions

View file

@ -138,3 +138,103 @@ def sanitize_replay_history(
if not agent_history:
return agent_history
return strip_dangling_tool_call_tail(strip_interrupted_tool_tails(agent_history))
# ──────────────────────────────────────────────────────────────────────
# Stale dangerous-confirmation text expiry (#59607)
# ──────────────────────────────────────────────────────────────────────
# How long a high-risk confirmation phrase remains valid.
# Short on purpose: dangerous side effects should not survive any restart
# or session resumption gap. The user can always re-confirm if needed.
_DANGEROUS_CONFIRMATION_EXPIRY_SECONDS = 60.0
# Confirmation phrases that unlock destructive host actions.
# Substring match (case-insensitive) so that user variants (e.g. trailing
# punctuation, additional context) still match. Add new patterns here when
# new high-risk actions are introduced.
_DANGEROUS_CONFIRMATION_PATTERNS: tuple = (
"confirm forced restart",
"confirm forced reboot",
"confirm shutdown",
"confirm reboot",
"confirm power off",
"yes, delete everything",
"confirm wipe",
"confirm factory reset",
# i18n variants observed in the original incident
"確認強制重開機",
"確認強制重開",
"確認重啟",
)
def is_dangerous_confirmation(content: Any) -> bool:
"""Return True if a user-message text matches a known dangerous confirmation.
Used by ``strip_stale_dangerous_confirmations`` to decide which
transcript rows to expire. Substring + case-insensitive so that
``"Please confirm forced restart, the host is critical"`` still matches.
"""
if not isinstance(content, str):
return False
text = content.strip().lower()
return any(pattern in text for pattern in _DANGEROUS_CONFIRMATION_PATTERNS)
def strip_stale_dangerous_confirmations(
agent_history: List[Dict[str, Any]],
*,
now: float,
expiry_seconds: float = _DANGEROUS_CONFIRMATION_EXPIRY_SECONDS,
) -> List[Dict[str, Any]]:
"""Strip stale dangerous-confirmation text in user messages (#59607).
When a high-risk side effect (e.g. host restart via ``shutdown.exe``)
runs, the user's plain-text confirmation phrase is persisted in the
conversation transcript. If the host restart killed the gateway
process before the assistant's tool result was written, the
transcript tail ends on the assistant's text response — and the
dangerous confirmation text remains in the user role.
On the next inbound message possibly a casual "are you there?" from
the user minutes later the LLM sees the stale confirmation and may
interpret the new turn as a fresh re-confirmation, re-executing the
destructive action. This is the failure mode reported in #59607.
This function strips user messages whose content matches a known
dangerous confirmation pattern *and* whose timestamp is older than
``expiry_seconds`` (default: 60s). Messages without a timestamp are
left untouched (backward compatibility: legacy transcripts and
in-memory test scaffolding have no timestamps). User messages that
contain dangerous confirmation text but are within the expiry window
are also left untouched they represent a fresh confirmation that
has not yet been acted on.
Complements 75ed07ace (which strips the *assistant* side of the
broken tail) by handling the *user* side: a stale plain-text
confirmation that the assistant has not yet responded to in a way
the resume logic recognises.
"""
if not agent_history:
return agent_history
cleaned: List[Dict[str, Any]] = []
for msg in agent_history:
if (
isinstance(msg, dict)
and msg.get("role") == "user"
and is_dangerous_confirmation(msg.get("content", ""))
):
ts = msg.get("timestamp")
if ts is not None and (now - float(ts)) > expiry_seconds:
logger.debug(
"Stripping stale dangerous-confirmation text from user "
"message (age=%.1fs, expiry=%.1fs): %r",
now - float(ts),
expiry_seconds,
(msg.get("content") or "")[:80],
)
continue
cleaned.append(msg)
return cleaned

View file

@ -722,7 +722,12 @@ _ASSISTANT_REPLAY_FIELDS: tuple[str, ...] = (
)
def _build_replay_entry(role: str, content: Any, msg: Dict[str, Any]) -> Dict[str, Any]:
def _build_replay_entry(
role: str,
content: Any,
msg: Dict[str, Any],
preserve_timestamp: bool = False,
) -> Dict[str, Any]:
"""Build a replay entry for a non-tool-calling message, preserving the
assistant fields the agent's API builders rely on for multi-turn fidelity.
@ -730,13 +735,20 @@ def _build_replay_entry(role: str, content: Any, msg: Dict[str, Any]) -> Dict[st
be unit-tested in isolation. Mirrors the ``_ASSISTANT_REPLAY_FIELDS``
contract above.
``preserve_timestamp``: when True, copy the source row's ``timestamp``
onto the replay entry. Currently only user messages need this the
stale-dangerous-confirmation stripper in ``agent/replay_cleanup.py``
reads the timestamp to decide whether a confirmation is too old to
replay safely. Assistant/tool messages are not timestamp-stripped in
the same way, so we keep the existing default of dropping it.
Empty values: most fields are dropped when falsy (matching the original
PR #2974 behaviour) since an empty list/string for those carries no
information. The exception is ``reasoning_content``: DeepSeek/Kimi
thinking-mode replay treats an empty string as a meaningful sentinel
that ``_copy_reasoning_content_for_api`` upgrades to a single space.
Dropping it here would make the gateway send no ``reasoning_content`` at
all on the next turn, which can cause HTTP 400 from strict thinking
Dropping it here would make the gateway send no ``reasoning_content``
at all on the next turn, which can cause HTTP 400 from strict thinking
providers.
"""
entry: Dict[str, Any] = {"role": role, "content": content}
@ -752,6 +764,10 @@ def _build_replay_entry(role: str, content: Any, msg: Dict[str, Any]) -> Dict[st
elif not _rval:
continue
entry[_rkey] = _rval
if preserve_timestamp:
ts = msg.get("timestamp")
if ts:
entry["timestamp"] = ts
return entry
@ -867,7 +883,12 @@ def _build_gateway_agent_history(
if msg.get("mirror"):
mirror_src = msg.get("mirror_source", "another session")
content = f"[Delivered from {mirror_src}] {content}"
entry = _build_replay_entry(role, content, msg)
# Preserve the timestamp on user messages so the
# stale-dangerous-confirmation stripper in agent/replay_cleanup.py
# can read it. The timestamp is dropped from assistant messages
# because they don't need it; the replay-tail strippers look at
# assistant(tool_calls), not timestamps.
entry = _build_replay_entry(role, content, msg, preserve_timestamp=(role == "user"))
agent_history.append(entry)
# Strip interrupted tool-call tails so the LLM doesn't re-execute
@ -881,6 +902,15 @@ def _build_gateway_agent_history(
# on resume and loops the restart forever (#49201).
agent_history = _strip_dangling_tool_call_tail(agent_history)
# Strip stale dangerous-confirmation text in user messages (#59607).
# A high-risk confirmation phrase (e.g. "confirm forced restart") that
# is older than the expiry window must not be replayed to the model,
# otherwise an unrelated follow-up message can be interpreted as a
# fresh confirmation and trigger the destructive action a second time.
agent_history = _strip_stale_dangerous_confirmations(
agent_history, now=time.time()
)
observed_context = "\n".join(observed_group_context).strip() or None
return agent_history, observed_context
@ -978,6 +1008,8 @@ from agent.replay_cleanup import ( # noqa: E402
is_interrupted_tool_result as _is_interrupted_tool_result,
strip_interrupted_tool_tails as _strip_interrupted_tool_tails,
strip_dangling_tool_call_tail as _strip_dangling_tool_call_tail,
strip_stale_dangerous_confirmations as _strip_stale_dangerous_confirmations,
is_dangerous_confirmation as _is_dangerous_confirmation,
)

View file

@ -0,0 +1,191 @@
"""Tests for stale confirmation text expiry in gateway history.
Reproduces the failure mode in #59607: when a host-restart-causing tool
runs, the user confirmation text remains in conversation history. On
resume, the LLM sees the confirmation at the tail and may re-execute the
destructive action if the user sends any follow-up.
The fix: high-risk confirmation messages (user messages containing a
known dangerous confirmation pattern) are tagged with a creation
timestamp when they enter conversation history. On history replay, if
the confirmation is older than EXPIRY (60s by default), the confirmation
text is stripped from agent history before the model sees it.
This complements 75ed07ace (which handles dangling assistant tool_calls
tail) by handling the user-side confirmation tail.
"""
import time
import pytest
from typing import Dict, List
from gateway.run import (
_build_gateway_agent_history,
_is_dangerous_confirmation,
_strip_stale_dangerous_confirmations,
)
# High-risk confirmation patterns. A user message matching one of these
# (case-insensitive) is considered a "confirmation text" and is subject
# to the expiry rule. Add new patterns here as new high-risk side effects
# are introduced.
def test_dangerous_confirmation_helper():
"""The pattern matcher is case-insensitive and substring-based."""
assert _is_dangerous_confirmation("confirm forced restart")
assert _is_dangerous_confirmation("CONFIRM FORCED RESTART")
assert _is_dangerous_confirmation(" confirm forced restart please ")
assert _is_dangerous_confirmation("I want to confirm forced restart the server")
# i18n
assert _is_dangerous_confirmation("確認強制重開機")
# Not a confirmation
assert not _is_dangerous_confirmation("can you restart the docker container?")
assert not _is_dangerous_confirmation("hello world")
assert not _is_dangerous_confirmation("")
assert not _is_dangerous_confirmation(None)
assert not _is_dangerous_confirmation(123)
def _make_history_with_confirmation(
*,
user_message_at: float,
assistant_warning_at: float,
confirmation_message: str,
confirmation_at: float,
assistant_action_at: float,
) -> List[Dict]:
"""Build a synthetic conversation history with a confirmation text.
Uses the real gateway's "timestamp" field (epoch seconds, as set in
gateway/run.py:11616, 11650, 11692, etc).
"""
return [
{"role": "user", "content": "can you force a restart?", "timestamp": user_message_at},
{"role": "assistant", "content": "Rebooting the host is dangerous. To confirm, please type: 'confirm forced restart'", "timestamp": assistant_warning_at},
{"role": "user", "content": confirmation_message, "timestamp": confirmation_at},
{"role": "assistant", "content": "OK, restarting now.", "timestamp": assistant_action_at},
]
def test_stale_confirmation_text_is_stripped_on_resume():
"""A confirmation text older than EXPIRY (60s by default) is stripped.
This is the core fix for #59607.
"""
current_time = time.time()
# User confirmation was 5 minutes ago — well past the 60s expiry
user_message_at = current_time - 1000
assistant_warning_at = current_time - 999
confirmation_at = current_time - 300 # 5 minutes ago
assistant_action_at = current_time - 299
history = _make_history_with_confirmation(
user_message_at=user_message_at,
assistant_warning_at=assistant_warning_at,
confirmation_message="confirm forced restart",
confirmation_at=confirmation_at,
assistant_action_at=assistant_action_at,
)
agent_history, _ = _build_gateway_agent_history(history)
# The stale confirmation text should be gone
confirmation_present = any(
m.get("role") == "user" and "confirm forced restart" in (m.get("content") or "")
for m in agent_history
)
assert not confirmation_present, (
f"Stale confirmation text should be stripped on resume. "
f"Got agent_history: {agent_history}"
)
def test_fresh_confirmation_text_is_preserved():
"""A confirmation text within EXPIRY is kept (not yet expired)."""
current_time = time.time()
user_message_at = current_time - 30
assistant_warning_at = current_time - 29
confirmation_at = current_time - 5 # 5 seconds ago — fresh
assistant_action_at = current_time - 4
history = _make_history_with_confirmation(
user_message_at=user_message_at,
assistant_warning_at=assistant_warning_at,
confirmation_message="confirm forced restart",
confirmation_at=confirmation_at,
assistant_action_at=assistant_action_at,
)
agent_history, _ = _build_gateway_agent_history(history)
# Fresh confirmation should still be there
confirmation_present = any(
m.get("role") == "user" and "confirm forced restart" in (m.get("content") or "")
for m in agent_history
)
assert confirmation_present, (
f"Fresh confirmation (5s old) should NOT be stripped. "
f"Got agent_history: {agent_history}"
)
def test_non_confirmation_text_is_preserved():
"""A regular user message is never treated as a confirmation."""
current_time = time.time()
user_message_at = current_time - 1000 # 17 min ago
confirmation_at = current_time - 300 # 5 min ago
history = [
{"role": "user", "content": "can you help me with the docs?", "timestamp": user_message_at},
{"role": "assistant", "content": "Sure, what do you need?", "timestamp": confirmation_at},
]
agent_history, _ = _build_gateway_agent_history(history)
# Both messages should still be there
user_msgs = [m for m in agent_history if m.get("role") == "user"]
assert len(user_msgs) == 1
assert "help me with the docs" in user_msgs[0].get("content", "")
def test_no_dangerous_pattern_at_all_preserves_everything():
"""If the conversation has no dangerous confirmation, nothing is stripped."""
current_time = time.time()
user_message_at = current_time - 1000
history = [
{"role": "user", "content": "tell me a joke", "timestamp": user_message_at},
{"role": "assistant", "content": "Why did the chicken cross the road?", "timestamp": user_message_at + 1},
{"role": "user", "content": "haha", "timestamp": user_message_at + 2},
]
agent_history, _ = _build_gateway_agent_history(history)
assert len(agent_history) == 3
def test_strip_stale_dangerous_confirmations_directly():
"""Unit test the strip helper in isolation."""
current_time = time.time()
history = _make_history_with_confirmation(
user_message_at=current_time - 1000,
assistant_warning_at=current_time - 999,
confirmation_message="confirm forced restart",
confirmation_at=current_time - 300,
assistant_action_at=current_time - 299,
)
cleaned = _strip_stale_dangerous_confirmations(history, now=current_time)
# The dangerous confirmation should be gone
assert not any(
"confirm forced restart" in (m.get("content") or "")
for m in cleaned
if m.get("role") == "user"
)
# The original user question and the assistant responses stay
assert any("can you force a restart" in (m.get("content") or "") for m in cleaned)
assert any("Rebooting the host is dangerous" in (m.get("content") or "") for m in cleaned)
assert any("OK, restarting now" in (m.get("content") or "") for m in cleaned)