Skip to content

fix(agent): readonly scope must deny join_links resource#450

Merged
intendednull merged 1 commit into
auto-fix/batch-2026-04-28-002530from
auto-fix/issue-436-readonly-deny-join-links
Apr 28, 2026
Merged

fix(agent): readonly scope must deny join_links resource#450
intendednull merged 1 commit into
auto-fix/batch-2026-04-28-002530from
auto-fix/issue-436-readonly-deny-join-links

Conversation

@intendednull
Copy link
Copy Markdown
Owner

Bug

TokenScope::allows_resource returned true for every URI. ReadOnly + Messaging tokens could read willow://server/join-links and pull link_id. link_id = single-step bearer credential for WireMessage::JoinRequest. Audit finding AUD-2.

Fix

Per-scope match in crates/agent/src/scopes.rs:

match self {
    Self::Full | Self::Admin => true,
    Self::ReadOnly | Self::Messaging => !uri.starts_with("willow://server/join-links"),
    Self::Custom(set) => set.contains(uri),
}

list_resources already runs .filter(|r| self.scope.allows_resource(...)) so denial cascades to listing for free.

Doc on TokenScope updated. Custom(set) now gates resources too (not tools only) — runner-up was leaving Custom resource-permissive, rejected because issue's preferred option (a) makes Custom symmetric for tools + resources, safer default for least-privilege tokens.

Complexity gate: skipped brainstorming/spec — single-file logic + suggested patch in issue. Decision noted here.

Tests added

Rust unit (crates/agent/src/scopes.rs):

  • readonly_scope_denies_join_links
  • messaging_scope_denies_join_links
  • full_and_admin_allow_join_links
  • custom_resource_allowlist_gates_join_links

Agent integration (crates/agent/tests/e2e.rs):

  • readonly_list_resources_omits_join_links — replicates WillowMcpServer::list_resources filter pipeline
  • readonly_scope_rejects_join_links_read — replaces prior stub denied_uri_rejects_with_invalid_request, now drives gate with real WillowMcpServer<ReadOnly>
  • readonly_token_hides_tools updated: every URI visible except join-links

Lowest tier covering behaviour per CLAUDE.md test-tier guidance. RequestContext<RoleServer> not externally constructible so we replicate the exact gate the handler runs (matches existing readonly_scope_rejects_send_message pattern).

Verification

  • cargo fmt --check clean
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo test --workspace all green (incl. 35 willow-agent unit + 33 e2e tests)
  • cargo check --target wasm32-unknown-unknown -p ... clean (agent excluded — binary crate)
test result: ok. 35 passed; 0 failed; 0 ignored ... willow-agent
test result: ok. 33 passed; 0 failed; 0 ignored ... e2e

Refs #436


Generated by Claude Code

TokenScope::allows_resource returned true for every URI, so ReadOnly
and Messaging tokens could read willow://server/join-links and harvest
link_id values. link_id is a single-step bearer credential for
WireMessage::JoinRequest, so any client with a low-privilege token
could join a server unattended (audit finding AUD-2).

Per-scope match now: Full/Admin = all URIs, ReadOnly/Messaging = all
except willow://server/join-links, Custom(set) = explicit allowlist
(now also gates resources, not just tools — doc updated).

Tests added at lowest tier (Rust unit + agent integration):
- scopes::tests::readonly_scope_denies_join_links
- scopes::tests::messaging_scope_denies_join_links
- scopes::tests::full_and_admin_allow_join_links
- scopes::tests::custom_resource_allowlist_gates_join_links
- e2e::readonly_list_resources_omits_join_links
- e2e::readonly_scope_rejects_join_links_read (replaces the prior
  stub-closure denied_uri_rejects_with_invalid_request — now drives
  the gate with a real WillowMcpServer<ReadOnly>)
- readonly_token_hides_tools updated to expect join-links denied while
  every other URI stays visible.

Tradeoff: option (a) from the issue tightens Custom(set) to also gate
resources, not tools only. Runner-up was leaving Custom unchanged and
only filtering join-links in ReadOnly/Messaging — rejected because the
issue's preferred patch makes Custom an explicit allowlist for both
surfaces, which is the safer default for least-privilege tokens.

Refs #436
@intendednull intendednull merged commit 6846ff7 into auto-fix/batch-2026-04-28-002530 Apr 28, 2026
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.

2 participants