feat(backend): enforcement des permissions tools MCP au serveur (#82 lot B2)
orchestrator/mcp/server.rs applique désormais les règles de permission du domaine mcp_tool_permissions (lot B1) à l'invocation d'un tool, y compris le cas requester vide. Lot B2 du ticket #82 : ferme la boucle enforcement sur le socle B1, la parité OpenAI-compatible suit en B3 sur la même branche. QA vert (32 tests dont le nouveau cas requester vide). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@ -33,9 +33,9 @@ use domain::ids::{AgentId, ProfileId, ProjectId};
|
||||
use domain::markdown::MarkdownDoc;
|
||||
use domain::ports::{
|
||||
AgentContextStore, AgentRuntime, ContextInjectionPlan, DirEntry, EventBus, EventStream,
|
||||
ExitStatus, FileSystem, FsError, IdGenerator, OutputStream, PreparedContext, ProfileStore,
|
||||
PtyError, PtyHandle, PtyPort, RemotePath, RuntimeError, SessionPlan, SkillStore, SpawnSpec,
|
||||
StoreError,
|
||||
ExitStatus, FileSystem, FsError, IdGenerator, McpToolPermissionStore, OutputStream,
|
||||
PreparedContext, ProfileStore, PtyError, PtyHandle, PtyPort, RemotePath, RuntimeError,
|
||||
SessionPlan, SkillStore, SpawnSpec, StoreError,
|
||||
};
|
||||
use domain::profile::{
|
||||
AgentProfile, ContextInjection, McpCapability, McpConfigStrategy, McpTransport,
|
||||
@ -45,7 +45,10 @@ use domain::project::{Project, ProjectPath};
|
||||
use domain::remote::RemoteRef;
|
||||
use domain::skill::{Skill, SkillScope};
|
||||
use domain::terminal::{SessionKind, TerminalSession};
|
||||
use domain::{AgentToolPolicy, IssueRef, PtySize, SessionId};
|
||||
use domain::{
|
||||
AgentMcpToolPolicyOverride, AgentToolPolicy, IssueRef, McpToolPolicy,
|
||||
ProjectMcpToolPermissions, PtySize, SessionId,
|
||||
};
|
||||
use uuid::Uuid;
|
||||
|
||||
use application::{
|
||||
@ -53,6 +56,7 @@ use application::{
|
||||
OrchestratorService, TerminalSessions, UpdateAgentContext,
|
||||
};
|
||||
use infrastructure::orchestrator::mcp::jsonrpc::error_codes;
|
||||
use infrastructure::orchestrator::mcp::tools::classified_tool_names;
|
||||
use infrastructure::orchestrator::mcp::{TicketToolError, TicketToolProvider};
|
||||
use infrastructure::{
|
||||
InMemoryConversationRegistry, InMemoryMailbox, McpServer, MediatedInbox, MemoryTransport,
|
||||
@ -274,6 +278,38 @@ impl FileSystem for FakeFs {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct FakeMcpToolPermissionStore {
|
||||
doc: Arc<Mutex<ProjectMcpToolPermissions>>,
|
||||
}
|
||||
|
||||
impl FakeMcpToolPermissionStore {
|
||||
fn new(doc: ProjectMcpToolPermissions) -> Self {
|
||||
Self {
|
||||
doc: Arc::new(Mutex::new(doc)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl McpToolPermissionStore for FakeMcpToolPermissionStore {
|
||||
async fn load_mcp_tool_permissions(
|
||||
&self,
|
||||
_project: &Project,
|
||||
) -> Result<ProjectMcpToolPermissions, StoreError> {
|
||||
Ok(self.doc.lock().unwrap().clone())
|
||||
}
|
||||
|
||||
async fn save_mcp_tool_permissions(
|
||||
&self,
|
||||
_project: &Project,
|
||||
permissions: &ProjectMcpToolPermissions,
|
||||
) -> Result<(), StoreError> {
|
||||
*self.doc.lock().unwrap() = permissions.clone();
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct FakePty;
|
||||
#[async_trait]
|
||||
@ -433,6 +469,34 @@ fn server(service: Arc<OrchestratorService>) -> McpServer {
|
||||
McpServer::new(service, project())
|
||||
}
|
||||
|
||||
fn server_with_mcp_permissions(
|
||||
service: Arc<OrchestratorService>,
|
||||
permissions: ProjectMcpToolPermissions,
|
||||
) -> McpServer {
|
||||
McpServer::new(service, project())
|
||||
.with_mcp_tool_permissions(Arc::new(FakeMcpToolPermissionStore::new(permissions)))
|
||||
}
|
||||
|
||||
fn allow_doc(agent: AgentId, allowed_tools: &[&str]) -> ProjectMcpToolPermissions {
|
||||
let known_tools = classified_tool_names();
|
||||
ProjectMcpToolPermissions::new(
|
||||
None,
|
||||
vec![AgentMcpToolPolicyOverride::new(
|
||||
agent,
|
||||
McpToolPolicy::new(
|
||||
allowed_tools
|
||||
.iter()
|
||||
.map(|tool| (*tool).to_owned())
|
||||
.collect(),
|
||||
&known_tools,
|
||||
)
|
||||
.unwrap(),
|
||||
)],
|
||||
&known_tools,
|
||||
)
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
/// A capturing event sink (the MCP twin of the file watcher's publish closure):
|
||||
/// records every [`DomainEvent`] the server emits so a test can assert the
|
||||
/// `OrchestratorRequestProcessed` source tag. Returns the closure to wire via
|
||||
@ -507,6 +571,11 @@ impl TicketToolProvider for FakeTicketTools {
|
||||
) -> Result<Value, TicketToolError> {
|
||||
self.calls.lock().unwrap().push(name.to_owned());
|
||||
match name {
|
||||
"idea_ticket_read" => Ok(json!({
|
||||
"item": { "ref": "#7", "title": "Seeded ticket", "version": 1 }
|
||||
})),
|
||||
"idea_ticket_list" => Ok(json!({ "items": [] })),
|
||||
"idea_ticket_read_carnet" => Ok(json!({ "ref": "#7", "carnet": "" })),
|
||||
"idea_sprint_list" => Ok(json!({
|
||||
"items": self.sprints.lock().unwrap().clone()
|
||||
})),
|
||||
@ -666,10 +735,232 @@ async fn requester_without_tool_policy_keeps_the_full_tools_list() {
|
||||
assert!(names.len() > 3, "unfiltered requester got {names:?}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn requester_with_durable_store_but_no_override_sees_read_only_tools() {
|
||||
let (service, _s) = build_service(FakeContexts::new());
|
||||
let agent = AgentId::from_uuid(Uuid::from_u128(82));
|
||||
let server = server_with_mcp_permissions(service, ProjectMcpToolPermissions::default())
|
||||
.for_requester(agent.to_string());
|
||||
|
||||
let raw = serde_json::to_vec(&json!({
|
||||
"jsonrpc": "2.0", "id": 1, "method": "tools/list"
|
||||
}))
|
||||
.unwrap();
|
||||
let response = server.handle_raw(&raw).await.expect("reply owed");
|
||||
assert!(response.error.is_none(), "got error: {:?}", response.error);
|
||||
let result = response.result.expect("result");
|
||||
let names: Vec<&str> = result["tools"]
|
||||
.as_array()
|
||||
.expect("tools array")
|
||||
.iter()
|
||||
.map(|t| t["name"].as_str().unwrap())
|
||||
.collect();
|
||||
|
||||
assert!(names.contains(&"idea_memory_read"));
|
||||
assert!(names.contains(&"idea_ticket_list"));
|
||||
assert!(!names.contains(&"idea_memory_write"));
|
||||
assert!(!names.contains(&"idea_ask_agent"));
|
||||
assert!(!names.contains(&"idea_ticket_update_carnet"));
|
||||
assert!(!names.contains(&"idea_run_in_background"));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. tools/call → the right OrchestratorCommand (observed through the fakes)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
#[tokio::test]
|
||||
async fn general_agent_without_mcp_override_is_read_only_for_tools_call() {
|
||||
let contexts = FakeContexts::new();
|
||||
contexts.seed_agent("architect");
|
||||
let (service, _mailbox, _sessions) = build_service_with_mailbox(contexts);
|
||||
let ticket_tools = Arc::new(FakeTicketTools::default());
|
||||
let agent = AgentId::from_uuid(Uuid::from_u128(83));
|
||||
let server = server_with_mcp_permissions(service, ProjectMcpToolPermissions::default())
|
||||
.with_ticket_tools(ticket_tools.clone())
|
||||
.for_requester(agent.to_string());
|
||||
|
||||
for (id, tool, arguments) in [
|
||||
(101, "idea_memory_read", json!({})),
|
||||
(102, "idea_ticket_list", json!({})),
|
||||
] {
|
||||
let response = server
|
||||
.handle_raw(&tools_call(id, tool, arguments))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
assert!(
|
||||
response.error.is_none(),
|
||||
"read tool {tool} must not be rejected by MCP policy: {:?}",
|
||||
response.error
|
||||
);
|
||||
assert!(response.result.is_some(), "read tool {tool} should run");
|
||||
}
|
||||
|
||||
for (id, tool, arguments) in [
|
||||
(
|
||||
111,
|
||||
"idea_memory_write",
|
||||
json!({ "slug": "note-a", "content": "body" }),
|
||||
),
|
||||
(
|
||||
112,
|
||||
"idea_ask_agent",
|
||||
json!({ "target": "architect", "task": "do it" }),
|
||||
),
|
||||
(
|
||||
113,
|
||||
"idea_ticket_update_carnet",
|
||||
json!({ "ref": "#7", "expectedVersion": 1, "carnet": "body" }),
|
||||
),
|
||||
(
|
||||
114,
|
||||
"idea_run_in_background",
|
||||
json!({ "label": "task", "command": "echo" }),
|
||||
),
|
||||
] {
|
||||
let response = server
|
||||
.handle_raw(&tools_call(id, tool, arguments))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
let error = response.error.expect("MCP policy rejection expected");
|
||||
assert_eq!(error.code, error_codes::INVALID_PARAMS, "tool {tool}");
|
||||
assert!(
|
||||
error.message.contains("not permitted"),
|
||||
"message should be readable for {tool}; got {}",
|
||||
error.message
|
||||
);
|
||||
assert!(response.result.is_none(), "denied {tool} must not run");
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
ticket_tools.calls(),
|
||||
vec!["idea_ticket_list".to_owned()],
|
||||
"denied ticket mutation must not reach the provider"
|
||||
);
|
||||
assert_eq!(ticket_tools.mutation_attempts(), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn legacy_mcp_requester_with_durable_store_is_read_only() {
|
||||
let (service, _mailbox, _sessions) = build_service_with_mailbox(FakeContexts::new());
|
||||
let server = server_with_mcp_permissions(service, ProjectMcpToolPermissions::default())
|
||||
.for_requester("mcp");
|
||||
|
||||
let response = server
|
||||
.handle_raw(&tools_call(
|
||||
121,
|
||||
"idea_run_in_background",
|
||||
json!({ "label": "task", "command": "echo" }),
|
||||
))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
|
||||
let error = response.error.expect("legacy requester must fail closed");
|
||||
assert_eq!(error.code, error_codes::INVALID_PARAMS);
|
||||
assert!(error.message.contains("not permitted"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn empty_requester_with_durable_store_is_read_only() {
|
||||
let (service, _s) = build_service(FakeContexts::new());
|
||||
let server = server_with_mcp_permissions(service, ProjectMcpToolPermissions::default())
|
||||
.for_requester("");
|
||||
|
||||
let response = server
|
||||
.handle_raw(&tools_call(
|
||||
122,
|
||||
"idea_memory_write",
|
||||
json!({ "slug": "note-a", "content": "body" }),
|
||||
))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
|
||||
let error = response.error.expect("empty requester must fail closed");
|
||||
assert_eq!(error.code, error_codes::INVALID_PARAMS);
|
||||
assert!(error.message.contains("not permitted"));
|
||||
assert!(response.result.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn durable_agent_override_allows_an_explicit_write_tool() {
|
||||
let (service, _s) = build_service(FakeContexts::new());
|
||||
let ticket_tools = Arc::new(FakeTicketTools::default());
|
||||
let agent = AgentId::from_uuid(Uuid::from_u128(84));
|
||||
let server =
|
||||
server_with_mcp_permissions(service, allow_doc(agent, &["idea_ticket_update_carnet"]))
|
||||
.with_ticket_tools(ticket_tools.clone())
|
||||
.for_requester(agent.to_string());
|
||||
|
||||
let response = server
|
||||
.handle_raw(&tools_call(
|
||||
131,
|
||||
"idea_ticket_update_carnet",
|
||||
json!({ "ref": "#7", "expectedVersion": 1, "carnet": "body" }),
|
||||
))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
|
||||
assert!(
|
||||
response.error.is_none(),
|
||||
"durable override should pass MCP policy, got {:?}",
|
||||
response.error
|
||||
);
|
||||
let result = response.result.expect("tool result");
|
||||
assert_eq!(
|
||||
result["isError"],
|
||||
json!(true),
|
||||
"fake provider reports execution error after policy passes"
|
||||
);
|
||||
assert_eq!(ticket_tools.calls(), vec!["idea_ticket_update_carnet"]);
|
||||
assert_eq!(ticket_tools.mutation_attempts(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ticket_assistant_policy_still_bounds_ticket_with_durable_store_present() {
|
||||
let (service, _s) = build_service(FakeContexts::new());
|
||||
let registry = Arc::new(ToolPolicyRegistry::new());
|
||||
registry.set(
|
||||
"ticket-assistant:demo:7",
|
||||
AgentToolPolicy::new(
|
||||
vec!["idea_ticket_update_carnet".to_owned()],
|
||||
Some(IssueRef::from_str("#7").unwrap()),
|
||||
true,
|
||||
),
|
||||
);
|
||||
let ticket_tools = Arc::new(FakeTicketTools::default());
|
||||
let server = server_with_mcp_permissions(service, ProjectMcpToolPermissions::default())
|
||||
.with_ticket_tools(ticket_tools.clone())
|
||||
.with_tool_policies(registry)
|
||||
.for_requester("ticket-assistant:demo:7");
|
||||
|
||||
let rejected = server
|
||||
.handle_raw(&tools_call(
|
||||
141,
|
||||
"idea_ticket_update_carnet",
|
||||
json!({ "ref": "#8", "expectedVersion": 1, "carnet": "body" }),
|
||||
))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
let error = rejected.error.expect("bound ticket rejection expected");
|
||||
assert_eq!(error.code, error_codes::INVALID_PARAMS);
|
||||
assert!(error.message.contains("#8"));
|
||||
assert!(ticket_tools.calls().is_empty());
|
||||
|
||||
let allowed = server
|
||||
.handle_raw(&tools_call(
|
||||
142,
|
||||
"idea_ticket_update_carnet",
|
||||
json!({ "ref": "#7", "expectedVersion": 1, "carnet": "body" }),
|
||||
))
|
||||
.await
|
||||
.expect("reply owed");
|
||||
assert!(
|
||||
allowed.error.is_none(),
|
||||
"bound ticket should pass policy, got {:?}",
|
||||
allowed.error
|
||||
);
|
||||
assert_eq!(ticket_tools.calls(), vec!["idea_ticket_update_carnet"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn launch_agent_call_creates_and_launches_the_agent() {
|
||||
let contexts = FakeContexts::new();
|
||||
|
||||
Reference in New Issue
Block a user