Skip to content

fix(cli): preserve familiar agent access precedence#27

Open
BunsDev wants to merge 1 commit into
mainfrom
codex/fix-familiar-picker-shadowing-security-issue
Open

fix(cli): preserve familiar agent access precedence#27
BunsDev wants to merge 1 commit into
mainfrom
codex/fix-familiar-picker-shadowing-security-issue

Conversation

@BunsDev
Copy link
Copy Markdown
Member

@BunsDev BunsDev commented Jun 3, 2026

Motivation

  • The /agents picker returned a familiar id but runtime resolution merged project settings.json agents over familiars, allowing an untrusted project agent to shadow a user familiar and escalate tool access.

Description

  • Add default_agents_with_familiars_and_config() in src-rust/crates/core/src/coven_shared.rs to merge built-ins, settings-defined agents, and familiars while preventing project settings from shadowing familiar ids (built-ins still win where intended).
  • Use the new helper in headless --agent resolution and interactive agent-mode switching in src-rust/crates/cli/src/main.rs so the picker selection and runtime agent definition resolve to the same trusted familiar access tier.
  • Add regression tests default_agents_with_familiars_and_config_keeps_familiar_over_settings_shadow and default_agents_with_familiars_and_config_preserves_settings_builtin_override in claurst-core to prevent reintroduction of the shadowing bug.

Testing

  • Ran cargo test --package claurst-core coven_shared; the unit tests (including the two new regressions) passed successfully.
  • Attempted cargo check --workspace and cargo clippy --workspace --all-targets -- -D warnings, but both were blocked by a missing system dependency (alsa.pc required by alsa-sys) in the environment.
  • cargo fmt --all --check reported pre-existing workspace formatting diffs unrelated to this change (formatting issues already present outside the modified files).

Codex Task

Copilot AI review requested due to automatic review settings June 3, 2026 10:29
Copy link
Copy Markdown

Copilot AI left a comment

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 fixes a security precedence mismatch between the interactive /agents familiar picker and runtime agent resolution, where project settings.json agent definitions could shadow a user familiar ID and thereby change the effective tool-access tier.

Changes:

  • Introduces default_agents_with_familiars_and_config() to merge built-ins, settings-defined agents, and familiars while preventing settings from shadowing familiar IDs (built-ins still block familiar ID collisions as before).
  • Switches CLI agent resolution (both --agent and interactive agent-mode changes) to use the new merge helper so selection and runtime resolution align.
  • Adds regression tests to lock in the intended precedence rules (familiars over settings for the same ID; settings can still override built-ins).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src-rust/crates/core/src/coven_shared.rs Adds a new merge helper enforcing familiar-vs-settings precedence and adds regression tests for the shadowing scenario and builtin-override behavior.
src-rust/crates/cli/src/main.rs Updates headless and interactive agent-definition resolution to use the new helper, aligning picker and runtime behavior.

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

@BunsDev BunsDev self-assigned this Jun 3, 2026
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