Skip to content

Security: agent tool-filter fails open for unknown access strings #13

@BunsDev

Description

@BunsDev

Summary

The agent tool filter at src-rust/crates/cli/src/main.rs:1277-1302 falls back to full tool access for any access string that isn't exactly "read-only" or "search-only". Typos, case mismatches, or stray whitespace silently grant write/exec privileges — the opposite of the documented opt-in semantics that DEFAULT_FAMILIAR_ACCESS = "read-only" aims for.

This is a pre-existing issue, but PR #12 expanded the input surface by adding ~/.coven/familiars.toml as a new source of access strings, so a typo there now grants a Coven familiar full power without any warning.

Repro

# ~/.coven/familiars.toml
[[familiar]]
id = "rogue"
display_name = "Rogue"
role = "Research"
access = "READ-ONLY"   # uppercase — looks correct, behaves as "full"

coven-code --agent rogue → the familiar can call Bash, Edit, Write etc. despite the user's clear intent to lock it down.

Cause

src-rust/crates/cli/src/main.rs:1277-1302:

match access {
    "read-only"   => { /* filter to ReadOnly/None + AskUserQuestion */ }
    "search-only" => { /* filter to Grep, Glob, Read, WebSearch, WebFetch */ }
    _             => tools,  // ← fail-open default
}

Any unknown value lands in the _ arm and the unrestricted tool list passes through.

Recommended fix

  1. Fail closed in the default arm — unknown access strings should be treated as read-only, matching DEFAULT_FAMILIAR_ACCESS:
    match access {
        "full"        => tools,
        "read-only"   => { /* … */ }
        "search-only" => { /* … */ }
        _             => { /* same as read-only, plus a warning to stderr */ }
    }
  2. Validate at parse timeCovenFamiliar::resolved_access() (and the equivalent on AgentDefinition) should reject/normalize unknown values, surfacing the typo to the user rather than silently downgrading.
  3. Fix docs/familiars.md — the table says search-only means "no filesystem access," but the code allows Read/Grep/Glob. Update to "read+search, no writes or shell."

Related cleanup (optional, lower priority)

  • src-rust/crates/tui/src/agents_view.rs in PR feat(familiars): add access tier so build-tier familiars can use git/shell #12 included ~150 lines of CRLF / trailing-whitespace churn around slugify_agent_name, daemon_status_cache, and the existing mod tests block. git diff --ignore-all-space main..bd6ff57 -- src-rust/crates/tui/src/agents_view.rs shows the real change is small. Worth a one-line whitespace-normalization commit to keep future diffs in this file readable.

Severity

Medium. Local config file under the user's own control, but a footgun that silently violates the principle-of-least-privilege the access-tier system was designed to enforce.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions