fix(security): fail closed for unknown agent access tiers#14
Merged
Conversation
The tool-filter pipeline in `filter_tools_for_agent` matched on the
`access` string with a permissive default arm:
_ => tools, // "full" — allow all tools unchanged
so any unrecognized value (typo, case mismatch, whitespace) silently
granted the full tool set — write, exec, everything. That directly
contradicts the opt-in semantics of `DEFAULT_FAMILIAR_ACCESS = "read-only"`
and meant a typo in `~/.coven/familiars.toml` or `settings.json` could
quietly hand a familiar `Bash`/`Edit`/`Write` access.
This commit:
- Adds `coven_shared::canonicalize_access_tier` to normalize case and
surrounding whitespace silently (so `"READ-ONLY"`, `" full "`, etc.
map to the canonical tier without complaint).
- Adds `coven_shared::resolve_access_tier` as the single chokepoint
that returns one of the three canonical tiers, emitting a one-line
stderr warning when it falls back to the default for a truly unknown
value.
- Rewrites `filter_tools_for_agent` to route through `resolve_access_tier`
and split the read-only / search-only branches into named helpers,
with an `unreachable!()` arm that documents the canonicalization
contract.
- Updates `CovenFamiliar::resolved_access` to return `&'static str` and
canonicalize through the same helper, so unknown values from
`~/.coven/familiars.toml` are normalized at load.
- Updates `docs/familiars.md` to describe the silent case/whitespace
normalization vs. the loud fail-closed for unknown values, and
corrects the `search-only` tier description to reflect what the code
actually allows (`Grep`, `Glob`, `Read`, `WebSearch`, `WebFetch`)
instead of the inaccurate "no filesystem access".
Tests added in both crates:
- `claurst_core::coven_shared::tests` — canonicalize accepts canonical
forms, normalizes case/whitespace, rejects unknown strings; resolve
falls back to default on unknown, passes canonical through.
- `claurst::tests` (main.rs) — full passes through unchanged, read-only
excludes Bash/Edit/Write/NotebookEdit, search-only stays within the
documented whitelist, case/whitespace canonicalize silently, and
genuinely unknown strings (`"readonly"`, `"i-am-evil"`, `""`,
`"writeable"`, `"full-access"`) fail closed to read-only.
Fixes #13.
Co-Authored-By: OpenCoven <noreply@opencoven.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the fail-open default arm in
filter_tools_for_agentthat #13 flagged. Unknownaccessstrings now fall back to"read-only"with a stderr warning instead of silently passing the full tool list through.What changed
coven_shared::canonicalize_access_tier(input)— trims + lowercases, returnsSome(&'static str)for canonical tiers,Noneotherwise. Silent.coven_shared::resolve_access_tier(input)— returns&'static str, prints a one-line stderr warning when it falls back toDEFAULT_FAMILIAR_ACCESSon unknown input. This is the single chokepoint the CLI tool-filter routes through.CovenFamiliar::resolved_accessnow returns&'static strand canonicalizes through the helper, so unknown values from~/.coven/familiars.tomlare normalized at load time too.filter_tools_for_agentrewritten: split the read-only and search-only branches intofilter_read_only_toolsandfilter_search_only_toolshelpers, route throughresolve_access_tier, and add anunreachable!()arm that documents the canonicalization contract.docs/familiars.mdupdated:search-onlydescription corrected to match the code (Grep,Glob,Read,WebSearch,WebFetch) — the previous "no filesystem access" was wrong.Behavior matrix
"full""read-only""search-only""READ-ONLY","Read-Only"," full ""readonly","i-am-evil","", anything elseTest plan
cargo test -p claurst-core --lib access_tier— 6 tests pass (canonicalize accepts canonical, normalizes case/whitespace, rejects unknowns; resolve falls back on unknown, passes canonical through; familiar_to_agent_definition still threads the tier).cargo test -p claurst --bin coven-code filter— 5 tests pass (full unchanged, read-only excludes Bash/Edit/Write/NotebookEdit, search-only stays within whitelist, case/whitespace canonicalize silently, genuinely-unknown values fail closed to read-only).cargo check -p claurst— clean.Fixes #13.