Skip to content

fix(core): ignore project shell hooks - #48

Merged
BunsDev merged 1 commit into
mainfrom
codex/fix-vulnerability-in-provider-tool-calls
Jun 8, 2026
Merged

fix(core): ignore project shell hooks#48
BunsDev merged 1 commit into
mainfrom
codex/fix-vulnerability-in-provider-tool-calls

Conversation

@BunsDev

@BunsDev BunsDev commented Jun 6, 2026

Copy link
Copy Markdown
Member

Motivation

  • Project-provided shell hooks were merged into the effective settings and could be executed for provider-returned tool calls, enabling arbitrary shell execution from an attacker-controlled repository; hooks coming from untrusted project settings must not be auto-registered.
  • The goal is to keep repository-scoped, non-executable preferences while preventing repositories from injecting hooks that run in response to model/provider events.

Description

  • Clear config.hooks in Settings::sanitize_project_settings so project-provided hooks are dropped before merging into global settings.
  • Extend the existing project_settings_do_not_merge_mcp_servers unit test to include a trusted global PreToolUse hook and a project-local PreToolUse hook, then assert that only the global hook remains after merge.
  • Change is limited to src-rust/crates/core/src/lib.rs (settings sanitization and test fixture); no behavioral changes to hook execution logic.

Testing

  • Ran cargo test -p claurst-core config::tests::project_settings_do_not_merge_mcp_servers -- --exact, and the test passed.
  • Ran cargo check -p claurst-core, which succeeded.
  • cargo check --workspace was attempted but blocked by a missing system dependency (alsa.pc) required by alsa-sys in this environment.
  • Repository-wide cargo fmt --all --check and cargo clippy -p claurst-core --all-targets -- -D warnings were not completed due to unrelated repo-wide formatting and clippy findings outside the scope of this patch.

Codex Task

Copilot AI review requested due to automatic review settings June 6, 2026 23:05

Copilot AI 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.

Pull request overview

This PR hardens settings merging in claurst-core by preventing repository-scoped (project-local) settings from registering shell hooks that could execute commands in response to model/provider events, while preserving trusted global settings behavior.

Changes:

  • Drop project-provided hook registrations by clearing config.hooks during Settings::sanitize_project_settings.
  • Extend the existing settings-merge unit test to include both global and project PreToolUse hooks and assert only the global hook survives the merge.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BunsDev
BunsDev merged commit 2b914d0 into main Jun 8, 2026
1 check passed
@BunsDev
BunsDev deleted the codex/fix-vulnerability-in-provider-tool-calls branch June 11, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants