Skip to content

Conversation

@npkanaka
Copy link

@npkanaka npkanaka commented Jan 13, 2026

Problem

Agents can leak internal ReAct control text (e.g., Thought:, Action:, Action Input:, Observation:) to end users in two ways:

  1. format_answer() fails open on parse errors by returning the raw model output as AgentFinish.output.
  2. Some models output a valid Final Answer: and then append Thought/Action/... afterward; the parser currently treats all text after Final Answer: as user-visible.

This shows up heavily with models that mimic prior Thought/Action/Observation patterns in history (e.g., Gemini), but it can happen with any provider.

Changes

  • Fail closed on parse errors: crewai.utilities.agent_utils.format_answer() no longer returns raw model output on parsing failures; it raises OutputParserError (wrapped for unexpected exceptions).
  • Sanitize Final Answer outputs: crewai.agents.parser.parse() now strips trailing ReAct control blocks accidentally appended after Final Answer:
  • Includes a guard to reduce false positives: only strips when the tail resembles a real control block (e.g., Action: only stripped if Action Input: appears).
  • Does not strip markers inside <code> / <pre><code> spans.
  • Max-iterations safety: handle_max_iterations_exceeded() catches OutputParserError on the last forced final-answer call and returns a safe generic message rather than crashing or leaking raw output.

This PR enforces a strict output contract at the framework boundary (where AgentFinish.output is produced), rather than relying on app-side sanitizers. It prevents both parse-failure leaks and parse-success-but-contaminated leaks, while keeping the agent retry loop functional and avoiding crashes in the max-iterations edge case.

++ #4186


Note

Prevents leakage of internal ReAct control text to end users and hardens parsing behavior.

  • Parser: When Final Answer: is present, strips trailing Thought:/Action:/Action Input:/Observation: blocks outside code spans via _strip_trailing_react_blocks; also normalizes stray trailing backticks.
  • Utilities: format_answer() now fails closed by raising OutputParserError (no raw output fallback); unexpected errors are wrapped into OutputParserError for retries.
  • Max-iterations: handle_max_iterations_exceeded() catches OutputParserError and returns a safe generic AgentFinish message instead of crashing or exposing raw model output.

Written by Cursor Bugbot for commit 11eacca. This will update automatically on new commits. Configure here.

- Raise OutputParserError instead of returning raw output on parse failures

- Strip trailing ReAct markers appended after Final Answer
- Strip trailing ReAct markers appended after Final Answer

- Fail closed on parse errors without leaking raw text

- Avoid crash after max iterations
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

if marker.startswith("Thought:") and ("Action:" not in tail and "Action Input:" not in tail):
continue
if marker.startswith("Action:") and "Action Input:" not in tail:
continue
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case-insensitive regex with case-sensitive guard conditions

Medium Severity

The regex _TRAILING_REACT_MARKER_RE uses the (?i) flag for case-insensitive matching, so it captures "thought:", "THOUGHT:", etc. However, the guard conditions use case-sensitive operations: marker.startswith("Thought:") and "Action:" not in tail. When the LLM outputs lowercase markers (e.g., "thought:"), match.group(1) returns "thought:", but startswith("Thought:") is False, so the guard is bypassed and text is unconditionally stripped. Uppercase "Thought:" without a following action block is protected by the guard and not stripped. This creates inconsistent stripping behavior based on the case of the markers in the LLM output.

Additional Locations (1)

Fix in Cursor Fix in Web


_CODE_SPAN_RE: re.Pattern[str] = re.compile(
r"(?is)<pre><code>.*?</code></pre>|<code>.*?</code>"
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Markdown code blocks not protected from stripping logic

Medium Severity

The _CODE_SPAN_RE regex only matches HTML <code> and <pre><code> tags, but not markdown triple-backtick code blocks. When an LLM outputs a final answer containing a markdown code block with ReAct-like content (e.g., explaining how agents work with examples), the _is_in_code check returns False for markers inside that block. If the markdown code block contains Thought: followed by Action: and Action Input:, the guard conditions don't protect it, and the content from the marker onward is incorrectly stripped. This is inconsistent with the existing markdown awareness shown in the trailing triple-backtick handling.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link

@VedantMadane VedantMadane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid fix for a real security/UX issue. The "fail closed" approach is the right call here. Some observations:

What I like:

  • _strip_trailing_react_blocks is well-designed - the code span detection prevents false positives in legitimate code output
  • The guard conditions (checking for Action Input: presence) reduce over-stripping
  • Converting unexpected exceptions to OutputParserError keeps the retry loop functioning without leaking raw output

Minor suggestions:

  1. Regex precompilation - Good that _CODE_SPAN_RE and _TRAILING_REACT_MARKER_RE are module-level compiled patterns.

  2. Edge case question - What happens if the model outputs Final Answer: Here's how to use ReAct:\n\nThought: This is an example...? The current logic would strip the educational example. Might be worth documenting this known limitation, or checking if the "Thought:" appears within the first N characters after Final Answer.

  3. The max-iterations fallback message is appropriately generic - good choice not to expose any internal state.

This addresses a real problem I've seen with Gemini models in particular. Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants