Skip to content

fix(conversation): sanitize nested tool responses - #10609

Merged
alexhancock merged 5 commits into
mainfrom
jbg/security-sanitize-tool-responses
Aug 10, 2026
Merged

fix(conversation): sanitize nested tool responses#10609
alexhancock merged 5 commits into
mainfrom
jbg/security-sanitize-tool-responses

Conversation

@jbg

@jbg jbg commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • sanitize Unicode Tags in tool-response text, resources, and error messages at construction and deserialization boundaries
  • sanitize direct Vec<MessageContentBlock> deserialization used by SQLite session reloads, preventing persisted history from bypassing the control
  • preserve annotations, provider metadata, result metadata, structured content, images, and legitimate Unicode
  • add end-to-end regressions for provider tool results and persisted raw-content reloads

Audit issues

Verification

  • cargo fmt --all
  • cargo test -p goose-provider-types — 449 passed
  • cargo build -p goose-provider-types -p goose
  • cargo clippy -p goose-provider-types -p goose --all-targets -- -D warnings
  • git diff --check

This finding was discovered by Project Loupe.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62e75d1f26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose-provider-types/src/conversation/message.rs Outdated
Comment thread crates/goose-provider-types/src/conversation/message.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02b9826742

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose-provider-types/src/conversation/message.rs
@jbg
jbg requested a review from DOsinga July 21, 2026 13:26
…-tool-responses

# Conflicts:
#	crates/goose-provider-types/src/conversation/message.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5fb507421

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/goose-provider-types/src/conversation/message.rs
@alexhancock alexhancock self-assigned this Jul 30, 2026
@alexhancock
alexhancock merged commit 69ddf55 into main Aug 10, 2026
26 checks passed
@alexhancock
alexhancock deleted the jbg/security-sanitize-tool-responses branch August 10, 2026 15:54
michaelneale added a commit that referenced this pull request Aug 10, 2026
* origin/main:
  fix(conversation): sanitize nested tool responses (#10609)
  fix(hints): bound recursive file expansion (#10546)
  fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007)
  fix(desktop): clarify compact cost display (#11093)
  Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874)
  docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858)
  fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908)
  fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015)
  fix(ui): support remote working directory for external backend (#10827)
@michaelneale

Copy link
Copy Markdown
Collaborator

🤖 (posted by micn's agent) Heads-up: this merge appears to have broken main's CI — cargo test -p goose-provider-types fails to compile: error[E0425]: cannot find function, tuple struct or tuple variant Meta in module rmcp::model at crates/goose-provider-types/src/conversation/message.rs:1583 (rmcp 3.0.0 in the lockfile has no rmcp::model::Meta tuple constructor). Main run: https://github.com/aaif-goose/goose/actions/runs/31406131429 — first red commit is 69ddf55; the prior commit 433f621 is green. Reproduces locally on a clean checkout of main.

michaelneale added a commit that referenced this pull request Aug 10, 2026
* origin/main:
  fix(mcp): prune dead notification subscribers (#11032)
  chore: remove the extension and tool count suggestion (#10869)
  feat: compaction in the GDK (#11042)
  fix(provider): retry transient errors on first stream item before ending turn (#10968)
  feat(cli): add /new to start a fresh session without restarting (#10767)
  feat(acp): title new sessions from _meta.sessionTitle (#10712)
  fix: adjust rmcp::model::Meta ref (#11107)
  Skip hook loading and lifecycle events for subagents (#10596)
  Sanitize Unicode tags in Responses output (#10745)
  fix(conversation): sanitize nested tool responses (#10609)
  fix(hints): bound recursive file expansion (#10546)
  fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007)
  fix(desktop): clarify compact cost display (#11093)
  Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874)
  docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858)
  fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908)
  fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015)
  fix(ui): support remote working directory for external backend (#10827)
lifeizhou-ap added a commit that referenced this pull request Aug 11, 2026
* main:
  fix(mcp): prune dead notification subscribers (#11032)
  chore: remove the extension and tool count suggestion (#10869)
  feat: compaction in the GDK (#11042)
  fix(provider): retry transient errors on first stream item before ending turn (#10968)
  feat(cli): add /new to start a fresh session without restarting (#10767)
  feat(acp): title new sessions from _meta.sessionTitle (#10712)
  fix: adjust rmcp::model::Meta ref (#11107)
  Skip hook loading and lifecycle events for subagents (#10596)
  Sanitize Unicode tags in Responses output (#10745)
  fix(conversation): sanitize nested tool responses (#10609)
  fix(hints): bound recursive file expansion (#10546)
  fix(providers): drop stale signed thinking blocks after a mid-conversation model switch (#10007)
  fix(desktop): clarify compact cost display (#11093)
  Index messages by (session_id, created_timestamp, id) to stop on-disk sort storms (#10874)
  docs: add tool shim guide covering when to enable, backends, and troubleshooting (#10858)
  fix(deep-link): route extension/session deep links to regular windows not standalone app windows (#10908)
  fix(ui): raise chat input z-index so slash menu appears above loading indicator (#11015)
  fix(ui): support remote working directory for external backend (#10827)
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.

3 participants