Skip to content

fix(codex): reject socket-backed MCP extensions - #11304

Merged
jbg merged 2 commits into
mainfrom
jbg/security-codex-socket-downgrade
Aug 20, 2026
Merged

jbg merged 2 commits into
mainfrom
jbg/security-codex-socket-downgrade

Conversation

@jbg

@jbg jbg commented Aug 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • reject socket-backed Streamable HTTP extensions when building Codex MCP overrides
  • preserve existing stdio and ordinary Streamable HTTP configuration behavior
  • return a clear configuration error instead of silently changing transports

Verification

  • cargo fmt
  • cargo test -p goose providers::codex::tests -- --test-threads=1
  • cargo build -p goose
  • cargo clippy -p goose --all-targets -- -D warnings

This finding was discovered by Project Loupe

Signed-off-by: Jasper Hugo <jasper@spiral.xyz>

@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: 4406de21bb

ℹ️ 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/src/providers/codex.rs Outdated
@DOsinga

DOsinga commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

This should be expanded beyond the Codex CLI provider. The same transport downgrade appears possible anywhere we translate ExtensionConfig::StreamableHttp into a configuration that carries uri and headers but not socket.

In particular:

  • extension_configs_to_mcp_servers drops socket, affecting the Codex, Claude, Copilot, Amp, and Pi ACP providers.
  • claude_mcp_config_json also drops socket for the Claude Code CLI provider.

Please apply the rejection to all CLI and ACP providers that cannot preserve socket-backed transport, preferably at a shared boundary. If the issue is not relevant to a particular provider, please document why—for example, because that provider preserves the Unix-socket transport or never receives socket-backed extension configurations—and add coverage for that distinction.

Otherwise, this PR protects only the deprecated Codex path while the same configuration can still silently become direct HTTP, including forwarding its headers, through the other external-provider paths.

@jbg
jbg added this pull request to the merge queue Aug 20, 2026
Merged via the queue into main with commit c250daa Aug 20, 2026
27 of 31 checks passed
@jbg
jbg deleted the jbg/security-codex-socket-downgrade branch August 20, 2026 11:59
alexhancock added a commit that referenced this pull request Aug 20, 2026
…bined

* origin/main: (85 commits)
  feat(desktop): sort configured providers to the top of the provider list (#11409)
  fix(cli): refuse symlink diagnostics outputs (#11398)
  test(plugins): isolate GOOSE_PATH_ROOT in discovery tests (#11407)
  fix(config): serialize secret mutations (#11388)
  fix: decouple source file and tool response limits (#11391)
  chore(deps): bump pctx_code_mode from 0.4.1 to 0.5.0 (#11245)
  fix(security): suppress sensitive OTLP traces (#11381)
  feat(openrouter): forward session_id and add app category header (#10868)
  feat(acp): derive and forward thinking effort from the ACP harness (#10949)
  fix(aws_bedrock): replace flat model list with routing table, add Gemma 4 Mantle support (#10297)
  Add GPT-5.6 follow-up support for Codex and Responses API (#10460)
  fix(update): fetch attestation bundles from bundle_url (#10557)
  fix(security): fail closed on invalid default GCP credentials (#11363)
  fix(codex): reject socket-backed MCP extensions (#11304)
  fix: pass complete response to stop hooks (#11366)
  fix: contain and bound skill supporting file reads (#11342)
  chore(deps): bump the ui-minor-and-patch group across 1 directory with 53 updates (#11386)
  fix(security): bound call graph traversal (#11193)
  fix: pin arrayref to known-good commit (#11389)
  feat(providers): add SayGM as declarative OpenAI-compatible provider (#11267)
  ...

# Conflicts:
#	crates/goose/src/agents/agent.rs
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