🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

1. 🟡 Ollama stream exposes thinking/reasoning fields as user-visible content (chain-of-thought leakage)

Description

The Ollama streaming adapter now falls back to thinking/reasoning and assigns it to message.content when content is empty.

This can unintentionally disclose internal model traces (chain-of-thought) or other non-user-facing content if an Ollama model/server emits it in thinking/reasoning (common for “reasoning” models):

• Input (untrusted): chunk.message.thinking / chunk.message.reasoning received from the Ollama server during streaming
• Transformation: accumulated into fallbackContent
• Sink (user-visible): written into finalResponse.message.content and then converted into a TextContent block in buildAssistantMessage(), emitted to downstream consumers via the done event

Vulnerable code (stream fallback into final content):

} else if (!sawContent && chunk.message?.thinking) {
  fallbackContent += chunk.message.thinking;
} else if (!sawContent && chunk.message?.reasoning) {
  fallbackContent += chunk.message.reasoning;
}
...
finalResponse.message.content = accumulatedContent || fallbackContent;

Even though the adapter prefers content once it appears, any conversation where the model returns an empty content and populates thinking/reasoning will surface that internal text to the user.

This is security-relevant because internal traces may contain sensitive context (e.g., parts of system prompts, policy text, tool output excerpts) that the application did not intend to display.

Recommendation

Do not automatically surface thinking/reasoning as message.content unless the caller explicitly opts in.

Options:

1. Default-safe: ignore thinking/reasoning for user-visible output:

​// Only show actual assistant content
finalResponse.message.content = accumulatedContent;

2. Opt-in exposure: gate fallback behind a flag (e.g., options?.includeThinking === true) and otherwise keep it out of the visible message:

const includeThinking = options?.includeThinking === true;
finalResponse.message.content = accumulatedContent || (includeThinking ? fallbackContent : "");

3. Keep separate metadata: store thinking/reasoning in a non-displayed field on the emitted event/message (if supported), rather than mapping it into TextContent.

Also consider stripping thinking/reasoning when tool_calls are present to avoid leaking tool planning text.

Analyzed PR: #39292 at commit 075bc83

_{Last updated on: 2026-03-08T01:24:34Z}

Greptile Summary

This PR fixes two independent but related bugs in the Ollama integration: (1) native Ollama responses from thinking-capable models (e.g. Qwen3) that populate message.thinking instead of message.content were being silently dropped; (2) an explicit lower contextWindow/maxTokens override in user config was being overwritten by a larger catalog default on every merge-mode refresh.

Changes

• src/agents/ollama-stream.ts: Adds thinking?: string to the OllamaChatResponse type and inserts a thinking fallback before reasoning in both buildAssistantMessage (non-streaming path) and the stream accumulation loop (streaming path). The priority order is now content → thinking → reasoning.
• src/agents/models-config.ts: Replaces the old max(explicit, implicit) token-limit resolution with resolvePreferredTokenLimit, which now checks whether the key is explicitly present in the user's config. A valid explicit value (positive finite number) is always honoured regardless of whether it is smaller than the catalog default. Invalid explicit values (0, negative, non-numeric) fall back to the implicit catalog default. When neither value is valid the field is omitted entirely instead of being set to undefined.
• Tests are correctly updated to reflect both the new thinking priority and the preserved-lower-limit semantics, with a new test covering the invalid-explicit-value fallback path.

Potential edge case (pre-existing): The stream accumulation loop uses an else if chain, so if a model streams thinking tokens in early chunks and then actual content in later chunks, both phases accumulate into the same buffer and are surfaced as a single text block. This behaviour predates this PR (it existed for reasoning) and is acceptable for models that use thinking exclusively in place of content, but it is worth documenting as an assumption.

Confidence Score: 4/5

• Safe to merge — targeted, well-tested bug fixes with clear scope boundaries and backward-compatible behavior.
• Both fixes are narrow and correct for their stated purpose. The thinking fallback follows the same proven pattern as the existing reasoning fallback, and the new resolvePreferredTokenLimit logic is thoroughly covered by updated tests including the invalid-value edge case. No regressions to existing behavior are expected. Score is 4 rather than 5 because the streaming accumulation still carries a pre-existing design assumption (thinking-only models never mix thinking and content phases in the same stream) that is not explicitly documented or guarded.
• No files require special attention. The comment on src/agents/ollama-stream.ts lines 475–483 is the only flagged concern, and it is a pre-existing style/documentation gap rather than a newly introduced bug.

_{Last reviewed commit: 3468b12}