fix(agents): prevent exec tool errors from leaking to channels (#9651)#10261
fix(agents): prevent exec tool errors from leaking to channels (#9651)#10261nu-gui wants to merge 2 commits intoopenclaw:mainfrom
Conversation
When exec tool returns a non-zero exit code, stderr/stdout content was forwarded as a visible message to the active messaging channel, leaking internal details. Skip emitting tool output when the tool result is an error so sensitive process output stays in the agent context only.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| import { describe, expect, it, vi } from "vitest"; | ||
| import type { EmbeddedPiSubscribeContext } from "./pi-embedded-subscribe.handlers.types.js"; | ||
| import { handleToolExecutionEnd } from "./pi-embedded-subscribe.handlers.tools.js"; |
There was a problem hiding this comment.
Missing coverage for leak
The new tests assert that emitToolOutput isn’t called on error, but they don’t cover the actual leak vector described in the PR (“forwarded as a visible message to the active messaging channel”). Since handleToolExecutionEnd still emits emitAgentEvent(... result: sanitizedResult) on errors, a regression test should assert that error results are not emitted (or are redacted) on whichever callback/path is responsible for external channel forwarding, otherwise this could reintroduce the leak via a different emitter.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.handlers.tools.test.ts
Line: 1:3
Comment:
**Missing coverage for leak**
The new tests assert that `emitToolOutput` isn’t called on error, but they don’t cover the actual leak vector described in the PR (“forwarded as a visible message to the active messaging channel”). Since `handleToolExecutionEnd` still emits `emitAgentEvent(... result: sanitizedResult)` on errors, a regression test should assert that error results are not emitted (or are redacted) on whichever callback/path is responsible for external channel forwarding, otherwise this could reintroduce the leak via a different emitter.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
This change only suppresses Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.handlers.tools.ts
Line: 161:163
Comment:
**Errors still forwarded**
This change only suppresses `emitToolOutput(...)`, but `handleToolExecutionEnd` still always emits the full `sanitizedResult` on the `emitAgentEvent({ stream: "tool", phase: "result", ... result: sanitizedResult })` path even when `isToolError` is true (see around `isToolError = isError || isToolResultError(result)`). If any consumer bridges these tool events into external channels (or logs accessible to users), stderr/stdout can still leak. If the goal is “never forwarded to channels,” consider omitting/redacting `result` for error tool events (or ensuring downstream tool-event consumers never surface `result` to channels).
How can I resolve this? If you propose a fix, please make it concise. |
Redact the tool result payload from emitAgentEvent when isToolError is true, preventing stderr/stdout from leaking through event consumers that may bridge to external channels. Add tests verifying the agent event path omits results for errors while including them for successes.
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing as AI-assisted stale-fix triage. Linked issue #9651 ("[Bug]: Exec tool error output (non-zero exit code) sent as message to active WhatsApp/Telegram channel") is currently CLOSED and was closed on 2026-02-23T01:47:22Z with state reason COMPLETED. If the underlying bug is still reproducible on current main, please reopen this PR (or open a new focused fix PR) and reference both #9651 and #10261 for fast re-triage. |
|
Closed after AI-assisted stale-fix triage (closed issue duplicate/stale fix). |
2 participants
Summary
emitToolOutput()whenisToolErroris true inhandleToolExecutionEnd(), so error content stays in the agent context only and is never forwarded to channelsFixes #9651
Test plan
handleToolExecutionEnd(): successful output emission, error output suppression (isError flag), error output suppression (isToolResultError detection), lastToolError recordingpnpm checkpasses (0 warnings, 0 errors)Greptile Overview
Greptile Summary
handleToolExecutionEndto skipemitToolOutput()when the tool execution is considered an error (isErrororisToolResultError).isError, suppression viaisToolResultError, andlastToolErrorrecording.Confidence Score: 3/5
emitToolOutput()behind!isToolErrorand adds tests for that behavior, butemitAgentEventstill includesresult: sanitizedResulteven on error; if any downstream consumer forwards tool result events to external channels, the original leak may persist. Confidence is reduced until the full external-forwarding path is confirmed/redacted.