mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-30 01:52:18 +00:00
feat(ui): write mcp tool permissions as overrides over convention defaults
Co-Authored-By: bot_apk <apk@cognition.ai>
This commit is contained in:
Devin AI
parent
766438bcc1
commit
2bb1c4bf2d
26 changed files with 812 additions and 102 deletions
|
|
@ -247,14 +247,21 @@ async def _convert_one_row(
|
|||
conversion: Final = convert_row(row, await gather_inventories(granted, manager, inventory_cache))
|
||||
if isinstance(conversion, Unavailable):
|
||||
return conversion.server_ids
|
||||
stored_fields: Final = (
|
||||
("mcp_servers", raw_row.mcp_servers),
|
||||
("mcp_access_groups", raw_row.mcp_access_groups),
|
||||
("mcp_toolsets", raw_row.mcp_toolsets),
|
||||
("mcp_tool_permissions", raw_row.mcp_tool_permissions),
|
||||
)
|
||||
updated: Final = await ObjectPermissionRepository(prisma_client).table.update_many(
|
||||
where={
|
||||
"object_permission_id": row.object_permission_id,
|
||||
"mcp_permission_version": {"in": [0, None]},
|
||||
"mcp_servers": {"equals": raw_row.mcp_servers},
|
||||
"mcp_access_groups": {"equals": raw_row.mcp_access_groups},
|
||||
"mcp_toolsets": {"equals": raw_row.mcp_toolsets},
|
||||
"mcp_tool_permissions": {"equals": raw_row.mcp_tool_permissions},
|
||||
# prisma-client-py has no DbNull/JsonNull sentinel for `equals` on a
|
||||
# Json? column, so a stored NULL field is left unguarded rather than
|
||||
# filtered with a wrong null literal; the id and version still bound
|
||||
# the CAS.
|
||||
**{field: {"equals": value} for field, value in stored_fields if value is not None},
|
||||
},
|
||||
data={
|
||||
"mcp_tool_overrides": json.dumps(dict(conversion.mcp_tool_overrides)),
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ from pydantic import BaseModel, ConfigDict, PrivateAttr, StrictInt
|
|||
from typing_extensions import ReadOnly, Required, TypedDict
|
||||
|
||||
from litellm.types.llms.base import LiteLLMPydanticObjectBase
|
||||
from litellm.types.mcp import MCPToolOverrideEntry
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from a2a.types import SendMessageResponse
|
||||
|
|
@ -174,6 +175,7 @@ class AgentObjectPermission(TypedDict, total=False):
|
|||
mcp_access_groups: list[str] | None
|
||||
mcp_toolsets: ReadOnly[Sequence[str] | None]
|
||||
mcp_tool_permissions: dict[str, list[str]] | None
|
||||
mcp_tool_overrides: dict[str, MCPToolOverrideEntry] | None
|
||||
models: list[str] | None
|
||||
agents: list[str] | None
|
||||
|
||||
|
|
|
|||
|
|
@ -143,6 +143,22 @@ async def test_runner_converts_row_with_cas_update():
|
|||
assert where["mcp_permission_version"] == {"in": [0, None]}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_omits_cas_equals_filter_for_null_fields():
|
||||
row = _row(mcp_tool_permissions=None)
|
||||
prisma = _prisma([row])
|
||||
manager = _manager(inventories={"server-a": INVENTORY})
|
||||
with patch(
|
||||
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.MCPRequestHandler._get_mcp_servers_from_access_groups",
|
||||
AsyncMock(return_value=[]),
|
||||
):
|
||||
report = await run_mcp_tool_permission_backfill(prisma, manager)
|
||||
assert report.converted == {"perm-1"}
|
||||
where = prisma.db.litellm_objectpermissiontable.update_many.await_args.kwargs["where"]
|
||||
assert "mcp_tool_permissions" not in where
|
||||
assert where["mcp_permission_version"] == {"in": [0, None]}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_runner_unavailable_server_skips_row_no_write():
|
||||
prisma = _prisma([_row()])
|
||||
|
|
|
|||
|
|
@ -1719,11 +1719,6 @@
|
|||
"count": 1
|
||||
}
|
||||
},
|
||||
"src/components/mcp_server_management/MCPToolPermissions.tsx": {
|
||||
"local/no-complex-jsx-arrow": {
|
||||
"count": 1
|
||||
}
|
||||
},
|
||||
"src/components/mcp_tools/MCPToolArgumentsForm.tsx": {
|
||||
"no-nested-ternary": {
|
||||
"count": 1
|
||||
|
|
@ -2070,6 +2065,9 @@
|
|||
"src/components/templates/key_edit_view.tsx": {
|
||||
"local/filename-pascal-case": {
|
||||
"count": 1
|
||||
},
|
||||
"max-lines": {
|
||||
"count": 1
|
||||
}
|
||||
},
|
||||
"src/components/templates/key_info_view.tsx": {
|
||||
|
|
@ -2432,4 +2430,4 @@
|
|||
"count": 1
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -93,6 +93,7 @@ export interface AgentFormValues {
|
|||
access_group_ids?: string[];
|
||||
allowed_mcp_servers_and_groups?: McpServerSelection;
|
||||
mcp_tool_permissions?: Record<string, string[]>;
|
||||
mcp_tool_overrides?: Record<string, { allow: string[]; deny: string[] }>;
|
||||
defaultInputModes?: string[];
|
||||
defaultOutputModes?: string[];
|
||||
enable_tracing?: boolean;
|
||||
|
|
|
|||
|
|
@ -112,6 +112,7 @@ const StepProgress: React.FC<{ current: number }> = ({ current }) => (
|
|||
const SHARED_INITIAL_VALUES: AgentFormValues = {
|
||||
allowed_mcp_servers_and_groups: { servers: [], accessGroups: [] },
|
||||
mcp_tool_permissions: {},
|
||||
mcp_tool_overrides: {},
|
||||
entitlement_models: [],
|
||||
entitlement_agents: [],
|
||||
access_group_ids: [],
|
||||
|
|
@ -253,6 +254,7 @@ const AddAgentForm: React.FC<AddAgentFormProps> = ({ visible, onClose, accessTok
|
|||
const watchedFormValues = useWatch({ control: form.control });
|
||||
const mcpSelection = useWatch({ control: form.control, name: "allowed_mcp_servers_and_groups" });
|
||||
const mcpToolPermissions = useWatch({ control: form.control, name: "mcp_tool_permissions" });
|
||||
const mcpToolOverrides = useWatch({ control: form.control, name: "mcp_tool_overrides" });
|
||||
|
||||
// Build the discovery plan for the proxy. Different agent runtimes publish
|
||||
// their cards at different URL shapes:
|
||||
|
|
@ -363,6 +365,7 @@ const AddAgentForm: React.FC<AddAgentFormProps> = ({ visible, onClose, accessTok
|
|||
// Build object_permission from MCP Tools step (allowed_mcp_servers_and_groups, mcp_tool_permissions)
|
||||
const mcpServersAndGroups = values.allowed_mcp_servers_and_groups ?? {};
|
||||
const toolPermissions = values.mcp_tool_permissions ?? {};
|
||||
const toolOverrides = values.mcp_tool_overrides ?? {};
|
||||
const entitlementModels = values.entitlement_models ?? [];
|
||||
const entitlementAgents = values.entitlement_agents ?? [];
|
||||
const objectPermission: Record<string, unknown> = {
|
||||
|
|
@ -370,6 +373,7 @@ const AddAgentForm: React.FC<AddAgentFormProps> = ({ visible, onClose, accessTok
|
|||
...(mcpServersAndGroups.accessGroups?.length ? { mcp_access_groups: mcpServersAndGroups.accessGroups } : {}),
|
||||
...(mcpServersAndGroups.toolsets?.length ? { mcp_toolsets: mcpServersAndGroups.toolsets } : {}),
|
||||
...(Object.keys(toolPermissions).length ? { mcp_tool_permissions: toolPermissions } : {}),
|
||||
...(Object.keys(toolOverrides).length ? { mcp_tool_overrides: toolOverrides } : {}),
|
||||
...(entitlementModels.length ? { models: entitlementModels } : {}),
|
||||
...(entitlementAgents.length ? { agents: entitlementAgents } : {}),
|
||||
};
|
||||
|
|
@ -546,6 +550,8 @@ const AddAgentForm: React.FC<AddAgentFormProps> = ({ visible, onClose, accessTok
|
|||
selectedToolsets={mcpSelection?.toolsets ?? []}
|
||||
toolPermissions={mcpToolPermissions ?? {}}
|
||||
onChange={(toolPerms: Record<string, string[]>) => form.setValue("mcp_tool_permissions", toolPerms)}
|
||||
toolOverrides={mcpToolOverrides ?? {}}
|
||||
onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -324,6 +324,7 @@ export const parseMcpPermissionsForForm = (agent: any) => ({
|
|||
toolsets: agent.object_permission?.mcp_toolsets ?? [],
|
||||
},
|
||||
mcp_tool_permissions: agent.object_permission?.mcp_tool_permissions ?? {},
|
||||
mcp_tool_overrides: agent.object_permission?.mcp_tool_overrides ?? {},
|
||||
});
|
||||
|
||||
/**
|
||||
|
|
@ -335,6 +336,7 @@ export const buildMcpObjectPermission = (values: any) => ({
|
|||
mcp_access_groups: values.allowed_mcp_servers_and_groups?.accessGroups ?? [],
|
||||
mcp_toolsets: values.allowed_mcp_servers_and_groups?.toolsets ?? [],
|
||||
mcp_tool_permissions: values.mcp_tool_permissions ?? {},
|
||||
mcp_tool_overrides: values.mcp_tool_overrides ?? {},
|
||||
});
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -163,6 +163,7 @@ const AgentInfoView: React.FC<AgentInfoViewProps> = ({ agentId, onClose, accessT
|
|||
const watchedFormValues = useWatch({ control: form.control });
|
||||
const mcpSelection = useWatch({ control: form.control, name: "allowed_mcp_servers_and_groups" });
|
||||
const mcpToolPermissions = useWatch({ control: form.control, name: "mcp_tool_permissions" });
|
||||
const mcpToolOverrides = useWatch({ control: form.control, name: "mcp_tool_overrides" });
|
||||
const { data: mcpServers = [] } = useMCPServers();
|
||||
const { data: accessGroups = [] } = useAccessGroups();
|
||||
|
||||
|
|
@ -570,6 +571,8 @@ const AgentInfoView: React.FC<AgentInfoViewProps> = ({ agentId, onClose, accessT
|
|||
onChange={(toolPerms: Record<string, string[]>) =>
|
||||
form.setValue("mcp_tool_permissions", toolPerms)
|
||||
}
|
||||
toolOverrides={mcpToolOverrides ?? {}}
|
||||
onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import { useSeededState } from "@/components/key_team_helpers/useSeededState";
|
|||
import { getModelDisplayName } from "@/components/key_team_helpers/fetch_available_models_team_key";
|
||||
import MCPServerSelector from "@/components/mcp_server_management/MCPServerSelector";
|
||||
import MCPToolPermissions from "@/components/mcp_server_management/MCPToolPermissions";
|
||||
import { normalizeMcpToolOverrides } from "@/components/mcp_server_management/effectiveMcpServers";
|
||||
import type { ObjectPermission } from "@/components/object_permission_types";
|
||||
import { MultiSelect } from "@/components/shared/MultiSelect";
|
||||
import { FieldGroup } from "@/components/ui/field";
|
||||
|
|
@ -55,6 +56,9 @@ const userEditShape = {
|
|||
metadata: z.string().nullish(),
|
||||
mcp_servers_and_groups: MCP_SELECTION_SHAPE.optional(),
|
||||
mcp_tool_permissions: z.record(z.string(), z.array(z.string())).optional(),
|
||||
mcp_tool_overrides: z
|
||||
.record(z.string(), z.object({ allow: z.array(z.string()), deny: z.array(z.string()) }))
|
||||
.optional(),
|
||||
};
|
||||
|
||||
const budgetSchema = (unlimitedBudget: boolean) =>
|
||||
|
|
@ -78,6 +82,7 @@ const buildMcpFieldValues = (objectPermission: ObjectPermission | null | undefin
|
|||
toolsets: objectPermission?.mcp_toolsets ?? [],
|
||||
},
|
||||
mcp_tool_permissions: objectPermission?.mcp_tool_permissions ?? {},
|
||||
mcp_tool_overrides: normalizeMcpToolOverrides(objectPermission?.mcp_tool_overrides),
|
||||
});
|
||||
|
||||
// antd only reported the fields that were actually mounted, so the identity and
|
||||
|
|
@ -340,6 +345,8 @@ export function UserEditView({
|
|||
selectedToolsets={form.watch("mcp_servers_and_groups")?.toolsets || []}
|
||||
toolPermissions={form.watch("mcp_tool_permissions") || {}}
|
||||
onChange={(toolPerms) => form.setValue("mcp_tool_permissions", toolPerms)}
|
||||
toolOverrides={form.watch("mcp_tool_overrides") || {}}
|
||||
onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -324,7 +324,8 @@ export default function UserInfoView({
|
|||
const mcpEntitlement = extractMcpEntitlement(formValues, allMcpServers, allMcpToolsets);
|
||||
const userFields = Object.fromEntries(
|
||||
Object.entries(formValues).filter(
|
||||
([field]) => field !== "mcp_servers_and_groups" && field !== "mcp_tool_permissions",
|
||||
([field]) =>
|
||||
field !== "mcp_servers_and_groups" && field !== "mcp_tool_permissions" && field !== "mcp_tool_overrides",
|
||||
),
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ const teamCreateFieldsSchema = z.object({
|
|||
})
|
||||
.optional(),
|
||||
mcp_tool_permissions: z.record(z.string(), z.array(z.string())).optional(),
|
||||
mcp_tool_overrides: z.record(z.string(), z.object({ allow: z.array(z.string()), deny: z.array(z.string()) })).optional(),
|
||||
allowed_agents_and_groups: z.object({ agents: z.array(z.string()), accessGroups: z.array(z.string()) }).optional(),
|
||||
object_permission_search_tools: z.array(z.string()).optional(),
|
||||
object_permission_skills: z.array(z.string()).optional(),
|
||||
|
|
@ -131,6 +132,7 @@ const EMPTY_TEAM_CREATE_VALUES: TeamCreateFormValues = {
|
|||
allowed_passthrough_routes: undefined,
|
||||
allowed_mcp_servers_and_groups: undefined,
|
||||
mcp_tool_permissions: {},
|
||||
mcp_tool_overrides: {},
|
||||
allowed_agents_and_groups: undefined,
|
||||
object_permission_search_tools: undefined,
|
||||
object_permission_skills: undefined,
|
||||
|
|
@ -256,6 +258,7 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
|
|||
const watchedOrganizationId = form.watch("organization_id");
|
||||
const watchedMcpSelection = form.watch("allowed_mcp_servers_and_groups");
|
||||
const watchedToolPermissions = form.watch("mcp_tool_permissions");
|
||||
const watchedToolOverrides = form.watch("mcp_tool_overrides");
|
||||
|
||||
const [selectedTeam, setSelectedTeam] = useState<Team | null>(null);
|
||||
const [selectedTeamId, setSelectedTeamId] = useQueryState("team", parseAsString.withOptions({ history: "push" }));
|
||||
|
|
@ -481,6 +484,10 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
|
|||
formValues.object_permission.mcp_tool_permissions = formValues.mcp_tool_permissions;
|
||||
delete formValues.mcp_tool_permissions;
|
||||
}
|
||||
if (formValues.mcp_tool_overrides && Object.keys(formValues.mcp_tool_overrides).length > 0) {
|
||||
formValues.object_permission.mcp_tool_overrides = formValues.mcp_tool_overrides;
|
||||
delete formValues.mcp_tool_overrides;
|
||||
}
|
||||
}
|
||||
|
||||
// Transform allowed_mcp_access_groups into object_permission
|
||||
|
|
@ -1139,6 +1146,8 @@ const Teams: React.FC<TeamProps> = ({ accessToken, userID, userRole, premiumUser
|
|||
selectedToolsets={watchedMcpSelection?.toolsets || []}
|
||||
toolPermissions={watchedToolPermissions || {}}
|
||||
onChange={(toolPerms) => form.setValue("mcp_tool_permissions", toolPerms)}
|
||||
toolOverrides={watchedToolOverrides || {}}
|
||||
onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</div>
|
||||
</CollapsibleContent>
|
||||
|
|
|
|||
|
|
@ -111,11 +111,12 @@ describe("MCPToolPermissions", () => {
|
|||
error: false,
|
||||
});
|
||||
|
||||
// A closed allowlist makes the server legacy-editable, so Select All writes the list.
|
||||
renderWithProviders(
|
||||
<MCPToolPermissions
|
||||
accessToken={mockAccessToken}
|
||||
selectedServers={[mockServerId]}
|
||||
toolPermissions={{}}
|
||||
toolPermissions={{ [mockServerId]: ["read_wiki_structure"] }}
|
||||
onChange={mockOnChange}
|
||||
/>,
|
||||
);
|
||||
|
|
@ -224,19 +225,20 @@ describe("MCPToolPermissions", () => {
|
|||
expect(networking.listMCPTools).toHaveBeenCalledWith(mockAccessToken, groupServer.server_id);
|
||||
});
|
||||
|
||||
it("shows every tool selected in flat view for an unrestricted access-group server", async () => {
|
||||
it("shows non-delete tools checked and the delete unchecked in flat view for an unrestricted access-group server", async () => {
|
||||
vi.mocked(networking.fetchMCPServers).mockResolvedValue([groupServer]);
|
||||
vi.mocked(networking.fetchMCPToolsets).mockResolvedValue([]);
|
||||
vi.mocked(networking.listMCPTools).mockResolvedValue({ tools: groupTools, error: false });
|
||||
|
||||
const mockOnChange = vi.fn();
|
||||
const mockOnOverridesChange = vi.fn();
|
||||
renderWithProviders(
|
||||
<MCPToolPermissions
|
||||
accessToken={mockAccessToken}
|
||||
selectedServers={[]}
|
||||
selectedAccessGroups={["production-group"]}
|
||||
toolPermissions={{}}
|
||||
onChange={mockOnChange}
|
||||
onChange={vi.fn()}
|
||||
onOverridesChange={mockOnOverridesChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
|
|
@ -245,10 +247,12 @@ describe("MCPToolPermissions", () => {
|
|||
|
||||
const [listIssues, deleteIssue] = screen.getAllByRole("checkbox");
|
||||
expect(listIssues).toBeChecked();
|
||||
expect(deleteIssue).toBeChecked();
|
||||
expect(deleteIssue).not.toBeChecked();
|
||||
|
||||
await userEvent.click(listIssues);
|
||||
expect(mockOnChange).toHaveBeenCalledWith({ [groupServer.server_id]: ["delete_issue"] });
|
||||
expect(mockOnOverridesChange).toHaveBeenCalledWith({
|
||||
[groupServer.server_id]: { allow: [], deny: ["list_issues"] },
|
||||
});
|
||||
});
|
||||
|
||||
it("marks an access-group server as inherited and leaves a directly selected one unmarked", async () => {
|
||||
|
|
@ -522,10 +526,7 @@ describe("MCPToolPermissions", () => {
|
|||
|
||||
expect(await screen.findByText("Payments")).toBeInTheDocument();
|
||||
expect(screen.queryByText("srv-collide")).not.toBeInTheDocument();
|
||||
await waitFor(() => {
|
||||
expect(mockOnChange).toHaveBeenCalledWith({ "srv-collide": ["list_issues"] });
|
||||
});
|
||||
expect(mockOnChange.mock.calls.every(([written]) => !Object.hasOwn(written, "srv-twin"))).toBe(true);
|
||||
expect(mockOnChange).not.toHaveBeenCalled();
|
||||
expect(networking.listMCPTools).not.toHaveBeenCalledWith(mockAccessToken, "srv-twin");
|
||||
});
|
||||
|
||||
|
|
@ -549,7 +550,7 @@ describe("MCPToolPermissions", () => {
|
|||
expect(mockOnChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps blocking delete tools by default for a directly selected server", async () => {
|
||||
it("blocks delete tools by default for a directly selected server without writing an allowlist", async () => {
|
||||
const directServer = { server_id: "srv-direct-1", server_name: "Direct Server", alias: "Direct Server" };
|
||||
vi.mocked(networking.fetchMCPServers).mockResolvedValue([directServer]);
|
||||
vi.mocked(networking.fetchMCPToolsets).mockResolvedValue([]);
|
||||
|
|
@ -565,9 +566,12 @@ describe("MCPToolPermissions", () => {
|
|||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockOnChange).toHaveBeenCalledWith({ [directServer.server_id]: ["list_issues"] });
|
||||
});
|
||||
await screen.findByText("Direct Server");
|
||||
await userEvent.click(screen.getByText("Flat List"));
|
||||
|
||||
expect(screen.getByRole("checkbox", { name: "list_issues" })).toBeChecked();
|
||||
expect(screen.getByRole("checkbox", { name: "delete_issue" })).not.toBeChecked();
|
||||
expect(mockOnChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows a server that only a stale tool-permission entry still entitles", async () => {
|
||||
|
|
@ -989,3 +993,149 @@ describe("MCPToolPermissions", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("convention (unrestricted) server overrides", () => {
|
||||
const mockAccessToken = "test-token";
|
||||
const mockServerId = "srv-conv-1";
|
||||
const mockServerName = "Convention Server";
|
||||
const convServer = { server_id: mockServerId, server_name: mockServerName, alias: mockServerName };
|
||||
const convTools = [
|
||||
{ name: "list_items", description: "List items" },
|
||||
{ name: "get_item", description: "Fetch one item" },
|
||||
{ name: "delete_item", description: "Destroy an item" },
|
||||
];
|
||||
|
||||
const renderConvention = ({
|
||||
toolOverrides,
|
||||
onOverridesChange = vi.fn(),
|
||||
onChange = vi.fn(),
|
||||
}: {
|
||||
toolOverrides?: Record<string, { allow: string[]; deny: string[] }>;
|
||||
onOverridesChange?: (overrides: Record<string, { allow: string[]; deny: string[] }>) => void;
|
||||
onChange?: (permissions: Record<string, string[]>) => void;
|
||||
}) =>
|
||||
renderWithProviders(
|
||||
<MCPToolPermissions
|
||||
accessToken={mockAccessToken}
|
||||
selectedServers={[mockServerId]}
|
||||
toolPermissions={{}}
|
||||
onChange={onChange}
|
||||
toolOverrides={toolOverrides ?? {}}
|
||||
onOverridesChange={onOverridesChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
testQueryClient.clear();
|
||||
vi.mocked(networking.fetchMCPAccessGroups).mockResolvedValue([]);
|
||||
vi.mocked(networking.fetchMCPServers).mockResolvedValue([convServer]);
|
||||
vi.mocked(networking.fetchMCPToolsets).mockResolvedValue([]);
|
||||
vi.mocked(networking.listMCPTools).mockResolvedValue({ tools: convTools, error: false });
|
||||
});
|
||||
|
||||
it("checks non-delete tools and leaves the delete unchecked, without writing anything", async () => {
|
||||
const onChange = vi.fn();
|
||||
renderConvention({ onChange });
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Flat List"));
|
||||
|
||||
expect(screen.getByRole("checkbox", { name: "list_items" })).toBeChecked();
|
||||
expect(screen.getByRole("checkbox", { name: "delete_item" })).not.toBeChecked();
|
||||
expect(onChange).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("honors a stored deny for a non-delete tool and a stored allow for a delete", async () => {
|
||||
renderConvention({ toolOverrides: { [mockServerId]: { allow: ["delete_item"], deny: ["list_items"] } } });
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Flat List"));
|
||||
|
||||
expect(screen.getByRole("checkbox", { name: "list_items" })).not.toBeChecked();
|
||||
expect(screen.getByRole("checkbox", { name: "delete_item" })).toBeChecked();
|
||||
});
|
||||
|
||||
it("writes a deny when a non-delete tool is unchecked and removes it when re-checked", async () => {
|
||||
const Harness = () => {
|
||||
const [overrides, setOverrides] = useState<Record<string, { allow: string[]; deny: string[] }>>({});
|
||||
return (
|
||||
<>
|
||||
<MCPToolPermissions
|
||||
accessToken={mockAccessToken}
|
||||
selectedServers={[mockServerId]}
|
||||
toolPermissions={{}}
|
||||
onChange={vi.fn()}
|
||||
toolOverrides={overrides}
|
||||
onOverridesChange={setOverrides}
|
||||
/>
|
||||
<output>{JSON.stringify(overrides)}</output>
|
||||
</>
|
||||
);
|
||||
};
|
||||
renderWithProviders(<Harness />);
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Flat List"));
|
||||
|
||||
await userEvent.click(screen.getByRole("checkbox", { name: "list_items" }));
|
||||
expect(screen.getByRole("status")).toHaveTextContent(`"${mockServerId}":{"allow":[],"deny":["list_items"]}`);
|
||||
|
||||
await userEvent.click(screen.getByRole("checkbox", { name: "list_items" }));
|
||||
expect(screen.getByRole("status")).toHaveTextContent(`"${mockServerId}":{"allow":[],"deny":[]}`);
|
||||
});
|
||||
|
||||
it("writes an allow for a delete tool and never writes it a deny", async () => {
|
||||
const onOverridesChange = vi.fn();
|
||||
renderConvention({ onOverridesChange });
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Flat List"));
|
||||
|
||||
await userEvent.click(screen.getByRole("checkbox", { name: "delete_item" }));
|
||||
expect(onOverridesChange).toHaveBeenCalledWith({ [mockServerId]: { allow: ["delete_item"], deny: [] } });
|
||||
});
|
||||
|
||||
it("leaves other servers' entries and non-displayed tools untouched on toggle", async () => {
|
||||
const onOverridesChange = vi.fn();
|
||||
const toolOverrides = {
|
||||
[mockServerId]: { allow: [], deny: ["hidden_tool"] },
|
||||
"srv-other": { allow: ["x"], deny: ["y"] },
|
||||
};
|
||||
renderConvention({ toolOverrides, onOverridesChange });
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Flat List"));
|
||||
await userEvent.click(screen.getByRole("checkbox", { name: "list_items" }));
|
||||
|
||||
const written = onOverridesChange.mock.calls.at(-1)?.[0] as Record<string, { allow: string[]; deny: string[] }>;
|
||||
expect(written["srv-other"]).toEqual({ allow: ["x"], deny: ["y"] });
|
||||
expect(written[mockServerId]).toEqual({ allow: [], deny: ["hidden_tool", "list_items"] });
|
||||
});
|
||||
|
||||
it("Select All approves the delete and clears displayed denies only", async () => {
|
||||
const onOverridesChange = vi.fn();
|
||||
const toolOverrides = { [mockServerId]: { allow: [], deny: ["list_items", "hidden_tool"] } };
|
||||
renderConvention({ toolOverrides, onOverridesChange });
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Select All"));
|
||||
|
||||
expect(onOverridesChange).toHaveBeenCalledWith({
|
||||
[mockServerId]: { allow: ["delete_item"], deny: ["hidden_tool"] },
|
||||
});
|
||||
});
|
||||
|
||||
it("Deselect All writes denies for displayed non-deletes and removes the delete's allow", async () => {
|
||||
const onOverridesChange = vi.fn();
|
||||
const toolOverrides = { [mockServerId]: { allow: ["delete_item"], deny: ["hidden_tool"] } };
|
||||
renderConvention({ toolOverrides, onOverridesChange });
|
||||
|
||||
await screen.findByText(mockServerName);
|
||||
await userEvent.click(screen.getByText("Deselect All"));
|
||||
|
||||
expect(onOverridesChange).toHaveBeenCalledWith({
|
||||
[mockServerId]: { allow: [], deny: ["hidden_tool", "list_items", "get_item"] },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useEffect, useRef, useState, useMemo } from "react";
|
||||
import React, { useEffect, useState, useMemo } from "react";
|
||||
import { listMCPTools } from "../networking";
|
||||
import { MCPTool } from "../mcp_tools/types";
|
||||
import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group";
|
||||
|
|
@ -12,9 +12,13 @@ import { NO_MCP_SERVERS_SENTINEL } from "../mcp_tools/constants";
|
|||
import {
|
||||
EffectiveMcpServer,
|
||||
McpGrantSource,
|
||||
McpToolOverrideEntry,
|
||||
applyToolOverrideWrite,
|
||||
applyToolOverrideWrites,
|
||||
applyToolPermissionWrite,
|
||||
emptyMcpAccessGroups,
|
||||
mcpAllowedToolsFor,
|
||||
isConventionServer,
|
||||
mcpToolState,
|
||||
resolveEffectiveMcpServers,
|
||||
} from "./effectiveMcpServers";
|
||||
|
||||
|
|
@ -25,6 +29,8 @@ interface MCPToolPermissionsProps {
|
|||
selectedToolsets?: readonly string[];
|
||||
toolPermissions: Record<string, string[]>;
|
||||
onChange: (toolPermissions: Record<string, string[]>) => void;
|
||||
toolOverrides?: Record<string, McpToolOverrideEntry>;
|
||||
onOverridesChange?: (toolOverrides: Record<string, { allow: string[]; deny: string[] }>) => void;
|
||||
disabled?: boolean;
|
||||
}
|
||||
|
||||
|
|
@ -55,6 +61,8 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
selectedToolsets = NO_SELECTION,
|
||||
toolPermissions,
|
||||
onChange,
|
||||
toolOverrides = {},
|
||||
onOverridesChange,
|
||||
disabled = false,
|
||||
}) => {
|
||||
const {
|
||||
|
|
@ -70,14 +78,6 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
const [toolErrors, setToolErrors] = useState<Record<string, string>>({});
|
||||
const [viewModes, setViewModes] = useState<Record<string, "crud" | "flat">>({});
|
||||
|
||||
// Keep a ref to the latest toolPermissions so async fetch callbacks always
|
||||
// read the current value and do not overwrite sibling servers' results when
|
||||
// multiple fetches complete out-of-order (stale-closure race condition).
|
||||
const toolPermissionsRef = useRef(toolPermissions);
|
||||
useEffect(() => {
|
||||
toolPermissionsRef.current = toolPermissions;
|
||||
}, [toolPermissions]);
|
||||
|
||||
// Every server this permission level reaches, not just the directly selected ones: a server
|
||||
// reached through an access group or a toolset needs its allowlist visible and editable too.
|
||||
const effectiveMcpInput = {
|
||||
|
|
@ -87,10 +87,11 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
selectedToolsets,
|
||||
toolsets,
|
||||
toolPermissions,
|
||||
toolOverrides,
|
||||
};
|
||||
const servers = useMemo(
|
||||
() => resolveEffectiveMcpServers(effectiveMcpInput),
|
||||
[allServers, selectedServers, selectedAccessGroups, selectedToolsets, toolsets, toolPermissions],
|
||||
[allServers, selectedServers, selectedAccessGroups, selectedToolsets, toolsets, toolPermissions, toolOverrides],
|
||||
);
|
||||
|
||||
// Fetch tools for a specific server; applies delete-blocked-by-default for new servers.
|
||||
|
|
@ -109,20 +110,6 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
} else {
|
||||
const fetchedTools: MCPTool[] = response.tools || [];
|
||||
setServerTools((prev) => ({ ...prev, [serverId]: fetchedTools }));
|
||||
|
||||
// Default only unrestricted direct servers to non-delete tools.
|
||||
// Read latest permissions from the ref to avoid clobbering concurrent results.
|
||||
const latestPermissions = toolPermissionsRef.current;
|
||||
const isDirect = entry.source.kind === "direct";
|
||||
const unrestricted =
|
||||
mcpAllowedToolsFor(entry.server, latestPermissions, allServers) === undefined &&
|
||||
entry.toolsetTools === undefined;
|
||||
if (isDirect && unrestricted && (selectedToolsets.length === 0 || !toolsetsFailed) && fetchedTools.length > 0) {
|
||||
const nonDeleteTools = fetchedTools
|
||||
.filter((t) => classifyToolOp(t.name, t.description || "") !== "delete")
|
||||
.map((t) => t.name);
|
||||
onChange(applyToolPermissionWrite({ toolPermissions: latestPermissions, entry, allowed: nonDeleteTools }));
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(`Error fetching tools for server ${serverId}:`, err);
|
||||
|
|
@ -147,20 +134,55 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [servers, accessToken, toolsetsLoading]);
|
||||
|
||||
// Every write goes through here so an edit is authoritative for the SERVER, not for one of the
|
||||
// equivalent keys that may name it.
|
||||
// Every allowlist write goes through here so an edit is authoritative for the SERVER, not for
|
||||
// one of the equivalent keys that may name it.
|
||||
const writeAllowedTools = (entry: EffectiveMcpServer, allowed: string[]) => {
|
||||
onChange(applyToolPermissionWrite({ toolPermissions, entry, allowed }));
|
||||
};
|
||||
|
||||
const handleSelectAll = (entry: EffectiveMcpServer) => {
|
||||
const tools = serverTools[entry.server.server_id] || [];
|
||||
writeAllowedTools(
|
||||
entry,
|
||||
tools.map((t) => t.name),
|
||||
// On a convention server there is no snapshot to edit: the checkbox flips this tool's entry in
|
||||
// the server's overrides and nothing else. A delete tool can only sit in `allow`; a non-delete
|
||||
// tool is carved out with `deny`. Anything not on the checkbox is left alone.
|
||||
const isDelete = (tool: MCPTool) => classifyToolOp(tool.name, tool.description || "") === "delete";
|
||||
|
||||
const writeToolToggle = (entry: EffectiveMcpServer, tool: MCPTool, checked: boolean) => {
|
||||
if (isConventionServer(entry)) {
|
||||
const write = {
|
||||
toolOverrides,
|
||||
permissionKey: entry.permissionKey,
|
||||
toolName: tool.name,
|
||||
isDeleteTool: isDelete(tool),
|
||||
checked,
|
||||
};
|
||||
onOverridesChange?.(applyToolOverrideWrite(write));
|
||||
return;
|
||||
}
|
||||
const current = entry.allowedTools ?? (serverTools[entry.server.server_id] || []).map((t) => t.name);
|
||||
writeAllowedTools(entry, checked ? [...current, tool.name] : current.filter((name) => name !== tool.name));
|
||||
};
|
||||
|
||||
// Writes the override edits that make every editable displayed tool match `checked`.
|
||||
const writeConventionBulk = (entry: EffectiveMcpServer, tools: readonly MCPTool[], checked: boolean) => {
|
||||
onOverridesChange?.(
|
||||
applyToolOverrideWrites({
|
||||
toolOverrides,
|
||||
permissionKey: entry.permissionKey,
|
||||
edits: tools
|
||||
.filter((tool) => !mcpToolState(entry, tool.name, isDelete(tool)).locked)
|
||||
.map((tool) => ({ toolName: tool.name, isDeleteTool: isDelete(tool), checked })),
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
const handleBulk = (entry: EffectiveMcpServer, checked: boolean) => {
|
||||
const tools = serverTools[entry.server.server_id] || [];
|
||||
if (isConventionServer(entry)) {
|
||||
writeConventionBulk(entry, tools, checked);
|
||||
return;
|
||||
}
|
||||
writeAllowedTools(entry, checked ? tools.map((t) => t.name) : []);
|
||||
};
|
||||
|
||||
// The opt-out sentinel short-circuits the backend resolver to zero servers, so nothing stored
|
||||
// here is in force and showing a tool matrix would claim otherwise.
|
||||
if (selectedServers.includes(NO_MCP_SERVERS_SENTINEL)) {
|
||||
|
|
@ -222,7 +244,9 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
const serverId = server.server_id;
|
||||
const serverName = server.server_name || server.alias || serverId;
|
||||
const tools = serverTools[serverId] || [];
|
||||
const selectedTools = entry.allowedTools ?? tools.map((t) => t.name);
|
||||
const stateFor = (tool: MCPTool) =>
|
||||
mcpToolState(entry, tool.name, classifyToolOp(tool.name, tool.description || "") === "delete");
|
||||
const selectedTools = tools.filter((tool) => stateFor(tool).checked).map((tool) => tool.name);
|
||||
const isLoading = loadingTools[serverId];
|
||||
const error = toolErrors[serverId];
|
||||
const viewMode = viewModes[serverId] ?? "crud";
|
||||
|
|
@ -282,7 +306,7 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
<button
|
||||
type="button"
|
||||
className="text-sm text-info hover:text-info/80 font-medium"
|
||||
onClick={() => handleSelectAll(entry)}
|
||||
onClick={() => handleBulk(entry, true)}
|
||||
disabled={isLoading}
|
||||
>
|
||||
Select All
|
||||
|
|
@ -290,7 +314,7 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
<button
|
||||
type="button"
|
||||
className="text-sm text-info hover:text-info/80 font-medium"
|
||||
onClick={() => writeAllowedTools(entry, [])}
|
||||
onClick={() => handleBulk(entry, false)}
|
||||
disabled={isLoading}
|
||||
>
|
||||
Deselect All
|
||||
|
|
@ -322,9 +346,27 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
{!isLoading && !error && tools.length > 0 && viewMode === "crud" && (
|
||||
<McpCrudPermissionPanel
|
||||
tools={tools}
|
||||
value={entry.allowedTools === undefined ? undefined : [...selectedTools]}
|
||||
value={[...selectedTools]}
|
||||
lockedTools={toolsetTools}
|
||||
onChange={(allowed) => writeAllowedTools(entry, allowed)}
|
||||
onChange={(allowed) =>
|
||||
isConventionServer(entry)
|
||||
? onOverridesChange?.(
|
||||
applyToolOverrideWrites({
|
||||
toolOverrides,
|
||||
permissionKey: entry.permissionKey,
|
||||
edits: tools
|
||||
.filter(
|
||||
(tool) => allowed.includes(tool.name) !== stateFor(tool).checked && !stateFor(tool).locked,
|
||||
)
|
||||
.map((tool) => ({
|
||||
toolName: tool.name,
|
||||
isDeleteTool: isDelete(tool),
|
||||
checked: allowed.includes(tool.name),
|
||||
})),
|
||||
}),
|
||||
)
|
||||
: writeAllowedTools(entry, allowed)
|
||||
}
|
||||
readOnly={disabled}
|
||||
/>
|
||||
)}
|
||||
|
|
@ -333,22 +375,18 @@ const MCPToolPermissions: React.FC<MCPToolPermissionsProps> = ({
|
|||
{!isLoading && !error && tools.length > 0 && viewMode === "flat" && (
|
||||
<div className="space-y-2">
|
||||
{tools.map((tool) => {
|
||||
const isSelected = selectedTools.includes(tool.name);
|
||||
const isLocked = toolsetTools.includes(tool.name);
|
||||
const state = stateFor(tool);
|
||||
return (
|
||||
<div key={tool.name} className="flex items-start gap-2">
|
||||
<input
|
||||
type="checkbox"
|
||||
aria-label={tool.name}
|
||||
checked={isSelected}
|
||||
checked={state.checked}
|
||||
onChange={() => {
|
||||
if (disabled || isLocked) return;
|
||||
const next = isSelected
|
||||
? selectedTools.filter((n) => n !== tool.name)
|
||||
: [...selectedTools, tool.name];
|
||||
writeAllowedTools(entry, next);
|
||||
if (disabled || state.locked) return;
|
||||
writeToolToggle(entry, tool, !state.checked);
|
||||
}}
|
||||
disabled={disabled || isLocked}
|
||||
disabled={disabled || state.locked}
|
||||
className="mt-0.5"
|
||||
/>
|
||||
<div className="flex-1 min-w-0">
|
||||
|
|
|
|||
|
|
@ -1,11 +1,16 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { MCPServer, MCPToolset } from "../mcp_tools/types";
|
||||
import {
|
||||
applyToolOverrideWrite,
|
||||
applyToolOverrideWrites,
|
||||
applyToolPermissionWrite,
|
||||
emptyMcpAccessGroups,
|
||||
isConventionServer,
|
||||
mcpAllowedToolsFor,
|
||||
mcpServersForIdentifier,
|
||||
mcpToolOverridesFor,
|
||||
mcpToolPermissionKeyFor,
|
||||
mcpToolState,
|
||||
resolveEffectiveMcpServers,
|
||||
} from "./effectiveMcpServers";
|
||||
|
||||
|
|
@ -546,3 +551,167 @@ describe("tools a selected toolset grants", () => {
|
|||
expect(untouched.allowedTools).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("convention servers and tool overrides", () => {
|
||||
const srv = server({ server_id: "srv-1", server_name: "wiki", alias: "Wiki" });
|
||||
|
||||
const resolveOne = (
|
||||
toolPermissions: Readonly<Record<string, readonly string[]>>,
|
||||
toolOverrides?: Readonly<Record<string, { allow: string[]; deny: string[] }>>,
|
||||
) => {
|
||||
const input: Parameters<typeof resolveEffectiveMcpServers>[0] = {
|
||||
...emptyInput,
|
||||
allServers: [srv],
|
||||
selectedServers: ["srv-1"],
|
||||
toolPermissions,
|
||||
toolOverrides,
|
||||
};
|
||||
return resolveEffectiveMcpServers(input)[0];
|
||||
};
|
||||
|
||||
it("checks non-delete tools and leaves deletes unchecked on a convention server", () => {
|
||||
const entry = resolveOne({}, { "srv-1": { allow: [], deny: [] } });
|
||||
|
||||
expect(isConventionServer(entry)).toBe(true);
|
||||
expect(mcpToolState(entry, "list_pages", false)).toEqual({ checked: true, locked: false });
|
||||
expect(mcpToolState(entry, "delete_page", true)).toEqual({ checked: false, locked: false });
|
||||
});
|
||||
|
||||
it("unchecks a non-delete tool a stored deny names", () => {
|
||||
const entry = resolveOne({}, { "srv-1": { allow: [], deny: ["list_pages"] } });
|
||||
|
||||
expect(mcpToolState(entry, "list_pages", false)).toEqual({ checked: false, locked: false });
|
||||
});
|
||||
|
||||
it("checks a delete tool a stored allow names", () => {
|
||||
const entry = resolveOne({}, { "srv-1": { allow: ["delete_page"], deny: [] } });
|
||||
|
||||
expect(mcpToolState(entry, "delete_page", true)).toEqual({ checked: true, locked: false });
|
||||
});
|
||||
|
||||
it("lets a deny win over an allow for the same tool", () => {
|
||||
const entry = resolveOne({}, { "srv-1": { allow: ["delete_page"], deny: ["delete_page"] } });
|
||||
|
||||
expect(mcpToolState(entry, "delete_page", true)).toEqual({ checked: false, locked: false });
|
||||
});
|
||||
|
||||
it("keeps a nonempty allowlist a closed editable list", () => {
|
||||
const entry = resolveOne({ "srv-1": ["list_pages"] });
|
||||
|
||||
expect(isConventionServer(entry)).toBe(false);
|
||||
expect(mcpToolState(entry, "list_pages", false)).toEqual({ checked: true, locked: false });
|
||||
expect(mcpToolState(entry, "delete_page", true)).toEqual({ checked: false, locked: false });
|
||||
});
|
||||
|
||||
it("keeps an empty allowlist deny-all but editable", () => {
|
||||
const entry = resolveOne({ "srv-1": [] });
|
||||
|
||||
expect(isConventionServer(entry)).toBe(false);
|
||||
expect(mcpToolState(entry, "list_pages", false)).toEqual({ checked: false, locked: false });
|
||||
});
|
||||
|
||||
it("locks toolset tools as granted", () => {
|
||||
const toolset = { toolset_id: "ts-1", toolset_name: "TS", tools: [{ server_id: "srv-1", tool_name: "list_pages" }] };
|
||||
const input: Parameters<typeof resolveEffectiveMcpServers>[0] = {
|
||||
...emptyInput,
|
||||
allServers: [srv],
|
||||
selectedServers: [],
|
||||
selectedToolsets: ["ts-1"],
|
||||
toolsets: [toolset as unknown as MCPToolset],
|
||||
toolPermissions: {},
|
||||
};
|
||||
const entry = resolveEffectiveMcpServers(input)[0];
|
||||
|
||||
expect(isConventionServer(entry)).toBe(false);
|
||||
expect(mcpToolState(entry, "list_pages", false)).toEqual({ checked: true, locked: true });
|
||||
});
|
||||
|
||||
it("merges override entries across keys that name the same server", () => {
|
||||
expect(
|
||||
mcpToolOverridesFor(
|
||||
srv,
|
||||
{ "srv-1": { allow: ["delete_page"], deny: [] }, wiki: { allow: [], deny: ["list_pages"] } },
|
||||
[srv],
|
||||
),
|
||||
).toEqual({ allow: ["delete_page"], deny: ["list_pages"] });
|
||||
});
|
||||
|
||||
it("moves a delete tool in and out of allow on toggle", () => {
|
||||
const overrides = { "srv-1": { allow: [], deny: ["list_pages"] } };
|
||||
|
||||
const grant = { toolOverrides: overrides, permissionKey: "srv-1", toolName: "delete_page", isDeleteTool: true, checked: true };
|
||||
|
||||
expect(applyToolOverrideWrite(grant)).toEqual({ "srv-1": { allow: ["delete_page"], deny: ["list_pages"] } });
|
||||
|
||||
const revoke = {
|
||||
toolOverrides: { "srv-1": { allow: ["delete_page"], deny: [] } },
|
||||
permissionKey: "srv-1",
|
||||
toolName: "delete_page",
|
||||
isDeleteTool: true,
|
||||
checked: false,
|
||||
};
|
||||
|
||||
expect(applyToolOverrideWrite(revoke)).toEqual({ "srv-1": { allow: [], deny: [] } });
|
||||
});
|
||||
|
||||
it("moves a non-delete tool in and out of deny on toggle without writing allows", () => {
|
||||
const deny = { toolOverrides: {}, permissionKey: "srv-1", toolName: "list_pages", isDeleteTool: false, checked: false };
|
||||
|
||||
expect(applyToolOverrideWrite(deny)).toEqual({ "srv-1": { allow: [], deny: ["list_pages"] } });
|
||||
|
||||
const undeny = {
|
||||
toolOverrides: { "srv-1": { allow: [], deny: ["list_pages"] } },
|
||||
permissionKey: "srv-1",
|
||||
toolName: "list_pages",
|
||||
isDeleteTool: false,
|
||||
checked: true,
|
||||
};
|
||||
|
||||
expect(applyToolOverrideWrite(undeny)).toEqual({ "srv-1": { allow: [], deny: [] } });
|
||||
});
|
||||
|
||||
it("preserves other servers' entries and tools not edited, byte for byte", () => {
|
||||
const toolOverrides = {
|
||||
"srv-1": { allow: ["drop_table"], deny: ["hidden_tool"] },
|
||||
"srv-other": { allow: ["other_delete"], deny: [] },
|
||||
};
|
||||
|
||||
const edit = {
|
||||
toolOverrides,
|
||||
permissionKey: "srv-1",
|
||||
toolName: "list_pages",
|
||||
isDeleteTool: false,
|
||||
checked: false,
|
||||
};
|
||||
const written = applyToolOverrideWrite(edit);
|
||||
|
||||
expect(written["srv-other"]).toEqual({ allow: ["other_delete"], deny: [] });
|
||||
expect(written["srv-1"]).toEqual({ allow: ["drop_table"], deny: ["hidden_tool", "list_pages"] });
|
||||
});
|
||||
|
||||
it("bulk Select All approves deletes and clears denies; Deselect All denies non-deletes", () => {
|
||||
const toolOverrides = { "srv-1": { allow: [], deny: ["list_pages"] } };
|
||||
|
||||
const selectAll = {
|
||||
toolOverrides,
|
||||
permissionKey: "srv-1",
|
||||
edits: [
|
||||
{ toolName: "list_pages", isDeleteTool: false, checked: true },
|
||||
{ toolName: "delete_page", isDeleteTool: true, checked: true },
|
||||
],
|
||||
};
|
||||
|
||||
expect(applyToolOverrideWrites(selectAll)).toEqual({ "srv-1": { allow: ["delete_page"], deny: [] } });
|
||||
|
||||
const deselectAll = {
|
||||
toolOverrides,
|
||||
permissionKey: "srv-1",
|
||||
edits: [
|
||||
{ toolName: "list_pages", isDeleteTool: false, checked: false },
|
||||
{ toolName: "delete_page", isDeleteTool: true, checked: false },
|
||||
],
|
||||
};
|
||||
|
||||
expect(applyToolOverrideWrites(deselectAll)).toEqual({ "srv-1": { allow: [], deny: ["list_pages"] } });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -9,6 +9,28 @@ export type McpGrantSource =
|
|||
| { readonly kind: "toolset"; readonly name: string }
|
||||
| { readonly kind: "toolPermission" };
|
||||
|
||||
export interface McpToolOverrideEntry {
|
||||
allow: string[];
|
||||
deny: string[];
|
||||
}
|
||||
|
||||
export interface McpToolState {
|
||||
readonly checked: boolean;
|
||||
readonly locked: boolean;
|
||||
}
|
||||
|
||||
// The OpenAPI schema types the stored entry's arrays as optional; the forms and write helpers
|
||||
// work with concrete arrays, so rows read from the API are defaulted here.
|
||||
export const normalizeMcpToolOverrides = (
|
||||
raw: Readonly<Record<string, { allow?: readonly string[]; deny?: readonly string[] }>> | null | undefined,
|
||||
): Record<string, McpToolOverrideEntry> =>
|
||||
Object.fromEntries(
|
||||
Object.entries(raw ?? {}).map(([key, entry]) => [
|
||||
key,
|
||||
{ allow: [...(entry.allow ?? [])], deny: [...(entry.deny ?? [])] },
|
||||
]),
|
||||
);
|
||||
|
||||
export interface EffectiveMcpServer {
|
||||
readonly server: MCPServer;
|
||||
// The mcp_tool_permissions key an edit writes to. The backend accepts a server id, name or
|
||||
|
|
@ -32,6 +54,10 @@ export interface EffectiveMcpServer {
|
|||
// What this level actually allows on the server, which is what the backend enforces: the keyed
|
||||
// union widened by the toolset grant. `undefined` means nothing restricts the server from here.
|
||||
readonly allowedTools: readonly string[] | undefined;
|
||||
// The merged mcp_tool_overrides entry for this server across every key naming it, or `undefined`
|
||||
// when no key holds one. Overrides are per-tool allow/deny exceptions layered on the convention:
|
||||
// on an unrestricted server a `deny` removes a non-delete tool and an `allow` re-arms a delete.
|
||||
readonly overrides: McpToolOverrideEntry | undefined;
|
||||
readonly source: McpGrantSource;
|
||||
}
|
||||
|
||||
|
|
@ -42,6 +68,7 @@ interface ResolveInput {
|
|||
readonly selectedToolsets: readonly string[];
|
||||
readonly toolsets: readonly MCPToolset[];
|
||||
readonly toolPermissions: Readonly<Record<string, readonly string[]>>;
|
||||
readonly toolOverrides?: Readonly<Record<string, McpToolOverrideEntry>>;
|
||||
}
|
||||
|
||||
// Access groups come back as plain names, but older records carry `{ name }` objects.
|
||||
|
|
@ -83,7 +110,7 @@ export const mcpServersForIdentifier = (allServers: readonly MCPServer[], identi
|
|||
// server's id, so the catalog decides membership rather than a field-by-field comparison.
|
||||
export const mcpToolPermissionKeysFor = (
|
||||
server: MCPServer,
|
||||
toolPermissions: Readonly<Record<string, readonly string[]>>,
|
||||
toolPermissions: Readonly<Record<string, unknown>>,
|
||||
allServers: readonly MCPServer[],
|
||||
): readonly string[] =>
|
||||
[server.server_id, server.server_name, server.alias].filter(
|
||||
|
|
@ -103,7 +130,7 @@ const mcpKeyNamesOneServerOnly = (allServers: readonly MCPServer[], key: string)
|
|||
// id-keyed entry rather than rewriting the shared one, which would edit the other server too.
|
||||
export const mcpToolPermissionKeyFor = (
|
||||
server: MCPServer,
|
||||
toolPermissions: Readonly<Record<string, readonly string[]>>,
|
||||
toolPermissions: Readonly<Record<string, unknown>>,
|
||||
allServers: readonly MCPServer[],
|
||||
): string =>
|
||||
mcpToolPermissionKeysFor(server, toolPermissions, allServers).find((key) =>
|
||||
|
|
@ -121,6 +148,21 @@ export const mcpAllowedToolsFor = (
|
|||
return [...new Set(keys.flatMap((key) => toolPermissions[key] ?? []))];
|
||||
};
|
||||
|
||||
// The override entry merged across every key naming this server, `undefined` when no key has one.
|
||||
// The backend unions allow/deny across equivalent keys the same way it unions allowlists.
|
||||
export const mcpToolOverridesFor = (
|
||||
server: MCPServer,
|
||||
toolOverrides: Readonly<Record<string, McpToolOverrideEntry>>,
|
||||
allServers: readonly MCPServer[],
|
||||
): McpToolOverrideEntry | undefined => {
|
||||
const entries = mcpToolPermissionKeysFor(server, toolOverrides, allServers).map((key) => toolOverrides[key]);
|
||||
if (entries.length === 0) return undefined;
|
||||
return {
|
||||
allow: [...new Set(entries.flatMap((entry) => entry.allow ?? []))],
|
||||
deny: [...new Set(entries.flatMap((entry) => entry.deny ?? []))],
|
||||
};
|
||||
};
|
||||
|
||||
// Tool names the given toolsets grant on this server, `undefined` when they grant none.
|
||||
const mcpToolsetToolsFor = (
|
||||
server: MCPServer,
|
||||
|
|
@ -174,6 +216,7 @@ export const resolveEffectiveMcpServers = ({
|
|||
selectedToolsets,
|
||||
toolsets,
|
||||
toolPermissions,
|
||||
toolOverrides,
|
||||
}: ResolveInput): readonly EffectiveMcpServer[] => {
|
||||
const entry = (server: MCPServer, source: McpGrantSource): EffectiveMcpServer => {
|
||||
const keys = mcpToolPermissionKeysFor(server, toolPermissions, allServers);
|
||||
|
|
@ -192,6 +235,7 @@ export const resolveEffectiveMcpServers = ({
|
|||
keyedTools === undefined && toolsetTools === undefined
|
||||
? undefined
|
||||
: [...new Set([...(keyedTools ?? []), ...(toolsetTools ?? [])])],
|
||||
overrides: toolOverrides === undefined ? undefined : mcpToolOverridesFor(server, toolOverrides, allServers),
|
||||
source,
|
||||
};
|
||||
};
|
||||
|
|
@ -227,3 +271,71 @@ export const resolveEffectiveMcpServers = ({
|
|||
candidates.findIndex((other) => other.server.server_id === candidate.server.server_id) === index,
|
||||
);
|
||||
};
|
||||
|
||||
// The checked/locked state a checkbox shows for one tool, mirroring the backend's
|
||||
// level_allowed_tools: a keyed allowlist stays a closed editable list, a toolset grant is
|
||||
// checked-but-locked, and an unrestricted (convention) server allows every non-delete tool unless
|
||||
// a stored deny says otherwise — an allow only re-arms a delete, and deny always beats allow.
|
||||
export const mcpToolState = (
|
||||
entry: EffectiveMcpServer,
|
||||
toolName: string,
|
||||
isDeleteTool: boolean,
|
||||
): McpToolState => {
|
||||
const denied = (entry.overrides?.deny ?? []).includes(toolName);
|
||||
if ((entry.toolsetTools ?? []).includes(toolName)) {
|
||||
return { checked: !denied, locked: true };
|
||||
}
|
||||
if (entry.keyedTools !== undefined) {
|
||||
return { checked: entry.keyedTools.includes(toolName), locked: false };
|
||||
}
|
||||
if (entry.toolsetTools !== undefined) {
|
||||
return { checked: false, locked: true };
|
||||
}
|
||||
const allowed = (entry.overrides?.allow ?? []).includes(toolName);
|
||||
return { checked: (isDeleteTool ? false : !denied) || (allowed && !denied), locked: false };
|
||||
};
|
||||
|
||||
// Whether this server is edited through overrides (convention mode) rather than a closed
|
||||
// mcp_tool_permissions allowlist. The backend treats the same condition the same way.
|
||||
export const isConventionServer = (entry: EffectiveMcpServer): boolean =>
|
||||
entry.keyedTools === undefined && entry.toolsetTools === undefined;
|
||||
|
||||
// Apply one checkbox change to the server's override entry: a delete tool can only be added to or
|
||||
// removed from `allow` (a deny would never let it back on), and a non-delete tool flips its `deny`
|
||||
// membership. Every other key in the map, and tools this edit did not touch, is preserved as-is.
|
||||
export const applyToolOverrideWrite = ({
|
||||
toolOverrides,
|
||||
permissionKey,
|
||||
toolName,
|
||||
isDeleteTool,
|
||||
checked,
|
||||
}: {
|
||||
readonly toolOverrides: Readonly<Record<string, McpToolOverrideEntry>>;
|
||||
readonly permissionKey: string;
|
||||
readonly toolName: string;
|
||||
readonly isDeleteTool: boolean;
|
||||
readonly checked: boolean;
|
||||
}): Record<string, McpToolOverrideEntry> => {
|
||||
const existing = toolOverrides[permissionKey] ?? { allow: [], deny: [] };
|
||||
const drop = (list: readonly string[]): string[] => list.filter((name) => name !== toolName);
|
||||
const written: McpToolOverrideEntry = isDeleteTool
|
||||
? { allow: checked ? [...drop(existing.allow), toolName] : drop(existing.allow), deny: [...existing.deny] }
|
||||
: { allow: [...existing.allow], deny: checked ? drop(existing.deny) : [...drop(existing.deny), toolName] };
|
||||
return { ...toolOverrides, [permissionKey]: written };
|
||||
};
|
||||
|
||||
// Fold a list of {name, isDeleteTool} toggles over the same server entry, for Select All /
|
||||
// Deselect All applied to the displayed editable tools.
|
||||
export const applyToolOverrideWrites = ({
|
||||
toolOverrides,
|
||||
permissionKey,
|
||||
edits,
|
||||
}: {
|
||||
readonly toolOverrides: Readonly<Record<string, McpToolOverrideEntry>>;
|
||||
readonly permissionKey: string;
|
||||
readonly edits: readonly { toolName: string; isDeleteTool: boolean; checked: boolean }[];
|
||||
}): Record<string, McpToolOverrideEntry> =>
|
||||
edits.reduce((overrides, edit) => {
|
||||
const write = { toolOverrides: overrides, permissionKey, toolName: edit.toolName, isDeleteTool: edit.isDeleteTool, checked: edit.checked };
|
||||
return applyToolOverrideWrite(write);
|
||||
}, { ...toolOverrides });
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ export interface McpEntitlementUpdate {
|
|||
mcp_access_groups: string[];
|
||||
mcp_toolsets: string[];
|
||||
mcp_tool_permissions: Record<string, string[]>;
|
||||
mcp_tool_overrides: Record<string, { allow: string[]; deny: string[] }>;
|
||||
}
|
||||
|
||||
const asStringArray = (value: unknown): string[] =>
|
||||
|
|
@ -18,6 +19,17 @@ const asToolPermissions = (value: unknown): Record<string, string[]> => {
|
|||
);
|
||||
};
|
||||
|
||||
const asToolOverrides = (value: unknown): Record<string, { allow: string[]; deny: string[] }> => {
|
||||
if (value === null || typeof value !== "object" || Array.isArray(value)) return {};
|
||||
return Object.fromEntries(
|
||||
Object.entries(value as Record<string, unknown>).flatMap(([serverId, entry]) => {
|
||||
if (entry === null || typeof entry !== "object" || Array.isArray(entry)) return [];
|
||||
const override = entry as Record<string, unknown>;
|
||||
return [[serverId, { allow: asStringArray(override.allow), deny: asStringArray(override.deny) }]];
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
const mcpServerMatchesIdentifier = (server: MCPServer, identifier: string): boolean =>
|
||||
server.server_id === identifier || server.server_name === identifier || server.alias === identifier;
|
||||
|
||||
|
|
@ -87,5 +99,10 @@ export const extractMcpEntitlement = (
|
|||
([permissionKey]) => grantsEveryServer || grantsServerNamedBy(permissionKey),
|
||||
),
|
||||
),
|
||||
mcp_tool_overrides: Object.fromEntries(
|
||||
Object.entries(asToolOverrides(formValues.mcp_tool_overrides)).filter(
|
||||
([permissionKey]) => grantsEveryServer || grantsServerNamedBy(permissionKey),
|
||||
),
|
||||
),
|
||||
};
|
||||
};
|
||||
|
|
|
|||
|
|
@ -81,6 +81,11 @@ const readToolPermissions = (raw: unknown): unknown | undefined => {
|
|||
return Object.keys(permissions as object).length > 0 ? permissions : undefined;
|
||||
};
|
||||
|
||||
const readToolOverrides = (raw: unknown): unknown | undefined => {
|
||||
const overrides = raw || {};
|
||||
return Object.keys(overrides as object).length > 0 ? overrides : undefined;
|
||||
};
|
||||
|
||||
const parseMetadata = (raw: unknown): unknown => {
|
||||
try {
|
||||
return JSON.parse((raw as string) || "{}");
|
||||
|
|
@ -110,6 +115,7 @@ interface PermissionSources {
|
|||
readonly vectorStores: unknown[] | undefined;
|
||||
readonly mcp: McpSelection | undefined;
|
||||
readonly toolPermissions: unknown | undefined;
|
||||
readonly toolOverrides: unknown | undefined;
|
||||
readonly extraMcpAccessGroups: unknown[] | undefined;
|
||||
readonly agents: AgentSelection | undefined;
|
||||
readonly skills: unknown[] | undefined;
|
||||
|
|
@ -119,6 +125,7 @@ const readPermissionSources = (values: Record<string, unknown>): PermissionSourc
|
|||
vectorStores: nonEmptyList(values.allowed_vector_store_ids),
|
||||
mcp: readMcpSelection(values.allowed_mcp_servers_and_groups),
|
||||
toolPermissions: readToolPermissions(values.mcp_tool_permissions),
|
||||
toolOverrides: readToolOverrides(values.mcp_tool_overrides),
|
||||
extraMcpAccessGroups: nonEmptyList(values.allowed_mcp_access_groups),
|
||||
agents: readAgentSelection(values.allowed_agents_and_groups),
|
||||
skills: nonEmptyList(values.allowed_skills),
|
||||
|
|
@ -128,6 +135,7 @@ const buildObjectPermission = ({
|
|||
vectorStores,
|
||||
mcp,
|
||||
toolPermissions,
|
||||
toolOverrides,
|
||||
extraMcpAccessGroups,
|
||||
agents,
|
||||
skills,
|
||||
|
|
@ -138,6 +146,7 @@ const buildObjectPermission = ({
|
|||
...(mcp?.accessGroups && { mcp_access_groups: mcp.accessGroups }),
|
||||
...(mcp?.toolsets && { mcp_toolsets: mcp.toolsets }),
|
||||
...(toolPermissions !== undefined && { mcp_tool_permissions: toolPermissions }),
|
||||
...(toolOverrides !== undefined && { mcp_tool_overrides: toolOverrides }),
|
||||
...(extraMcpAccessGroups && { mcp_access_groups: extraMcpAccessGroups }),
|
||||
...(agents?.agents && { agents: agents.agents }),
|
||||
...(agents?.accessGroups && { agent_access_groups: agents.accessGroups }),
|
||||
|
|
@ -152,6 +161,7 @@ const consumedSourceKeys = (
|
|||
): ReadonlySet<string> =>
|
||||
new Set<string>([
|
||||
"mcp_tool_permissions",
|
||||
"mcp_tool_overrides",
|
||||
"allowed_skills",
|
||||
...(values.disable_global_guardrails ? [] : ["disable_global_guardrails"]),
|
||||
...(vectorStores ? ["allowed_vector_store_ids"] : []),
|
||||
|
|
|
|||
|
|
@ -124,6 +124,9 @@ const McpToolPermissionsField: React.FC<McpToolPermissionsFieldProps> = ({ acces
|
|||
| { servers?: string[]; accessGroups?: string[]; toolsets?: string[] }
|
||||
| undefined;
|
||||
const toolPermissions = useWatch({ control, name: "mcp_tool_permissions" }) as Record<string, string[]> | undefined;
|
||||
const toolOverrides = useWatch({ control, name: "mcp_tool_overrides" }) as
|
||||
| Record<string, { allow: string[]; deny: string[] }>
|
||||
| undefined;
|
||||
|
||||
return (
|
||||
<div className="mt-6">
|
||||
|
|
@ -134,6 +137,8 @@ const McpToolPermissionsField: React.FC<McpToolPermissionsFieldProps> = ({ acces
|
|||
selectedToolsets={selection?.toolsets || []}
|
||||
toolPermissions={toolPermissions || {}}
|
||||
onChange={(toolPerms) => setValue("mcp_tool_permissions", toolPerms)}
|
||||
toolOverrides={toolOverrides || {}}
|
||||
onOverridesChange={(overrides) => setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
|
|
@ -235,6 +240,7 @@ const CreateKey: React.FC<CreateKeyProps> = ({ team, teams, data, addKey, autoOp
|
|||
tpm_limit_type: null,
|
||||
rpm_limit_type: null,
|
||||
mcp_tool_permissions: {},
|
||||
mcp_tool_overrides: {},
|
||||
duration: "",
|
||||
}));
|
||||
const form = useForm<MountedFormValues>({
|
||||
|
|
@ -1573,6 +1579,10 @@ const CreateKey: React.FC<CreateKeyProps> = ({ team, teams, data, addKey, autoOp
|
|||
{(control) => <input type="hidden" id={control.id} name={control.name} />}
|
||||
</MountedFormField>
|
||||
|
||||
<MountedFormField name="mcp_tool_overrides" bare>
|
||||
{(control) => <input type="hidden" id={control.id} name={control.name} />}
|
||||
</MountedFormField>
|
||||
|
||||
<McpToolPermissionsField
|
||||
accessToken={accessToken}
|
||||
control={form.control}
|
||||
|
|
|
|||
|
|
@ -2548,6 +2548,7 @@ describe("TeamInfo MCP permission retention", () => {
|
|||
keyedTools: undefined,
|
||||
toolsetTools: undefined,
|
||||
allowedTools: undefined,
|
||||
overrides: undefined,
|
||||
source: kind === "accessGroup" ? { kind, name: "ops_readonly" } : { kind },
|
||||
}) as EffectiveMcpServer;
|
||||
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ import MCPServerSelector from "../mcp_server_management/MCPServerSelector";
|
|||
import MCPToolPermissions from "../mcp_server_management/MCPToolPermissions";
|
||||
import {
|
||||
mcpServersForIdentifier,
|
||||
normalizeMcpToolOverrides,
|
||||
resolveEffectiveMcpServers,
|
||||
type EffectiveMcpServer,
|
||||
} from "../mcp_server_management/effectiveMcpServers";
|
||||
|
|
@ -398,6 +399,7 @@ const teamUpdateFieldsSchema = z.object({
|
|||
})
|
||||
.optional(),
|
||||
mcp_tool_permissions: z.record(z.string(), z.array(z.string())).optional(),
|
||||
mcp_tool_overrides: z.record(z.string(), z.object({ allow: z.array(z.string()), deny: z.array(z.string()) })).optional(),
|
||||
agents_and_groups: z.object({ agents: z.array(z.string()), accessGroups: z.array(z.string()) }).optional(),
|
||||
object_permission_search_tools: z.array(z.string()).optional(),
|
||||
object_permission_skills: z.array(z.string()).optional(),
|
||||
|
|
@ -448,6 +450,7 @@ const EMPTY_TEAM_UPDATE_VALUES: TeamUpdateFormValues = {
|
|||
allowed_passthrough_routes: [],
|
||||
mcp_servers_and_groups: { servers: [], accessGroups: [], toolsets: [] },
|
||||
mcp_tool_permissions: {},
|
||||
mcp_tool_overrides: {},
|
||||
agents_and_groups: { agents: [], accessGroups: [] },
|
||||
object_permission_search_tools: [],
|
||||
object_permission_skills: [],
|
||||
|
|
@ -513,6 +516,7 @@ const toTeamFormValues = (info: TeamInfoRecord, effectiveGuardrails: string[]):
|
|||
toolsets: info.object_permission?.mcp_toolsets || [],
|
||||
},
|
||||
mcp_tool_permissions: info.object_permission?.mcp_tool_permissions || {},
|
||||
mcp_tool_overrides: normalizeMcpToolOverrides(info.object_permission?.mcp_tool_overrides),
|
||||
agents_and_groups: {
|
||||
agents: info.object_permission?.agents || [],
|
||||
accessGroups: info.object_permission?.agent_access_groups || [],
|
||||
|
|
@ -604,6 +608,7 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
|||
const killSwitchOn = form.watch("disable_global_guardrails");
|
||||
const watchedMcpSelection = form.watch("mcp_servers_and_groups");
|
||||
const watchedToolPermissions = form.watch("mcp_tool_permissions");
|
||||
const watchedToolOverrides = form.watch("mcp_tool_overrides");
|
||||
const mcpLookupFailure =
|
||||
(
|
||||
[
|
||||
|
|
@ -1085,11 +1090,15 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
|||
if (mcpToolPermissions) {
|
||||
updateData.object_permission.mcp_tool_permissions = mcpToolPermissions;
|
||||
}
|
||||
if (values.mcp_tool_overrides && Object.keys(values.mcp_tool_overrides).length > 0) {
|
||||
updateData.object_permission.mcp_tool_overrides = values.mcp_tool_overrides;
|
||||
}
|
||||
if (toolsets) {
|
||||
updateData.object_permission.mcp_toolsets = toolsets;
|
||||
}
|
||||
delete values.mcp_servers_and_groups;
|
||||
delete values.mcp_tool_permissions;
|
||||
delete values.mcp_tool_overrides;
|
||||
|
||||
// Handle agent permissions
|
||||
const { agents, accessGroups: agentAccessGroups } = values.agents_and_groups || {
|
||||
|
|
@ -1905,6 +1914,8 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
|
|||
selectedToolsets={watchedMcpSelection?.toolsets || []}
|
||||
toolPermissions={watchedToolPermissions || {}}
|
||||
onChange={(toolPerms) => form.setValue("mcp_tool_permissions", toolPerms)}
|
||||
toolOverrides={watchedToolOverrides || {}}
|
||||
onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { extractLoggingSettings, formatMetadataForDisplay, stripTagsFromMetadata
|
|||
import { mapInternalToDisplayNames } from "../callback_info_helpers";
|
||||
import { estimateChecks, estimateFields } from "./estimatedOutputTokens";
|
||||
import { canonicalBudgetDuration } from "./keyEditFieldNormalizers";
|
||||
import { normalizeMcpToolOverrides } from "../mcp_server_management/effectiveMcpServers";
|
||||
|
||||
export interface McpServersAndGroups {
|
||||
servers: string[];
|
||||
|
|
@ -46,6 +47,7 @@ export interface KeyEditFormValues {
|
|||
vector_stores?: string[];
|
||||
mcp_servers_and_groups?: McpServersAndGroups;
|
||||
mcp_tool_permissions?: Record<string, string[]>;
|
||||
mcp_tool_overrides?: Record<string, { allow: string[]; deny: string[] }>;
|
||||
agents_and_groups?: AgentsAndGroups;
|
||||
skills?: string[];
|
||||
organization_id?: string | null;
|
||||
|
|
@ -102,6 +104,7 @@ export const toKeyEditFormValues = (keyData: KeyResponse): KeyEditFormValues =>
|
|||
toolsets: keyData.object_permission?.mcp_toolsets || [],
|
||||
},
|
||||
mcp_tool_permissions: keyData.object_permission?.mcp_tool_permissions || {},
|
||||
mcp_tool_overrides: normalizeMcpToolOverrides(keyData.object_permission?.mcp_tool_overrides),
|
||||
agents_and_groups: {
|
||||
agents: keyData.object_permission?.agents || [],
|
||||
accessGroups: keyData.object_permission?.agent_access_groups || [],
|
||||
|
|
@ -154,6 +157,7 @@ export const keyEditFormSchema = z.object({
|
|||
vector_stores: z.custom<string[] | undefined>(),
|
||||
mcp_servers_and_groups: z.custom<McpServersAndGroups | undefined>(),
|
||||
mcp_tool_permissions: z.custom<Record<string, string[]> | undefined>(),
|
||||
mcp_tool_overrides: z.custom<Record<string, { allow: string[]; deny: string[] }> | undefined>(),
|
||||
agents_and_groups: z.custom<AgentsAndGroups | undefined>(),
|
||||
skills: z.custom<string[] | undefined>(),
|
||||
organization_id: z.custom<string | null | undefined>(),
|
||||
|
|
@ -205,6 +209,7 @@ export const toSubmittedValues = (
|
|||
vector_stores: values.vector_stores,
|
||||
mcp_servers_and_groups: values.mcp_servers_and_groups,
|
||||
mcp_tool_permissions: values.mcp_tool_permissions,
|
||||
mcp_tool_overrides: values.mcp_tool_overrides,
|
||||
agents_and_groups: values.agents_and_groups,
|
||||
skills: values.skills,
|
||||
organization_id: values.organization_id,
|
||||
|
|
|
|||
|
|
@ -766,6 +766,8 @@ export function KeyEditView({
|
|||
selectedToolsets={mcpSelection?.toolsets || []}
|
||||
toolPermissions={(mcpToolPermissions as Record<string, string[]> | undefined) || {}}
|
||||
onChange={(toolPerms) => form.setValue("mcp_tool_permissions", toolPerms)}
|
||||
toolOverrides={form.watch("mcp_tool_overrides") || {}}
|
||||
onOverridesChange={(overrides) => form.setValue("mcp_tool_overrides", overrides)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -267,6 +267,7 @@ export default function KeyInfoView({
|
|||
}
|
||||
delete formValues.mcp_servers_and_groups;
|
||||
delete formValues.mcp_tool_permissions;
|
||||
delete formValues.mcp_tool_overrides;
|
||||
|
||||
// Handle agent permissions
|
||||
if (formValues.agents_and_groups !== undefined) {
|
||||
|
|
|
|||
26
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
26
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -32508,6 +32508,10 @@ export interface components {
|
|||
mcp_access_groups?: string[] | null;
|
||||
/** Mcp Servers */
|
||||
mcp_servers?: string[] | null;
|
||||
/** Mcp Tool Overrides */
|
||||
mcp_tool_overrides?: {
|
||||
[key: string]: components["schemas"]["MCPToolOverrideEntry"];
|
||||
} | null;
|
||||
/** Mcp Tool Permissions */
|
||||
mcp_tool_permissions?: {
|
||||
[key: string]: string[];
|
||||
|
|
@ -32550,15 +32554,25 @@ export interface components {
|
|||
* @default []
|
||||
*/
|
||||
mcp_access_groups: string[] | null;
|
||||
/** Mcp Permission Version */
|
||||
mcp_permission_version?: number | null;
|
||||
/**
|
||||
* Mcp Servers
|
||||
* @default []
|
||||
*/
|
||||
mcp_servers: string[] | null;
|
||||
/** Mcp Tool Overrides */
|
||||
mcp_tool_overrides?: {
|
||||
[key: string]: components["schemas"]["MCPToolOverrideEntry"];
|
||||
} | null;
|
||||
/** Mcp Tool Permissions */
|
||||
mcp_tool_permissions?: {
|
||||
[key: string]: string[];
|
||||
} | null;
|
||||
/** Mcp Tool Permissions Archive */
|
||||
mcp_tool_permissions_archive?: {
|
||||
[key: string]: string[];
|
||||
} | null;
|
||||
/** Mcp Tool Search Enabled */
|
||||
mcp_tool_search_enabled?: boolean | null;
|
||||
/** Mcp Toolsets */
|
||||
|
|
@ -35185,6 +35199,18 @@ export interface components {
|
|||
/** Total */
|
||||
total: number;
|
||||
};
|
||||
/**
|
||||
* MCPToolOverrideEntry
|
||||
* @description Per-server tool overrides stored on an object permission row's
|
||||
* ``mcp_tool_overrides``: ``allow`` re-arms names the convention denies,
|
||||
* ``deny`` disables names the convention or an allowlist would permit.
|
||||
*/
|
||||
MCPToolOverrideEntry: {
|
||||
/** Allow */
|
||||
allow?: string[];
|
||||
/** Deny */
|
||||
deny?: string[];
|
||||
};
|
||||
/**
|
||||
* MCPToolSearchSettings
|
||||
* @description `litellm_settings.mcp_tool_search`: how the native `mcp_tool_search` virtual tool ranks the caller's tools.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,31 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { classifyToolOp, groupToolsByCrud } from "./mcpToolCrudClassification";
|
||||
|
||||
interface FixtureCase {
|
||||
name: string;
|
||||
description: string | null;
|
||||
expected: string;
|
||||
}
|
||||
|
||||
const fixture: FixtureCase[] = JSON.parse(
|
||||
readFileSync(
|
||||
new URL(
|
||||
"../../../../tests/test_litellm/proxy/_experimental/mcp_server/fixtures/mcp_tool_classification_cases.json",
|
||||
import.meta.url,
|
||||
),
|
||||
"utf8",
|
||||
),
|
||||
);
|
||||
|
||||
describe("classifyToolOp", () => {
|
||||
it.each(fixture.map((fixtureCase) => [fixtureCase.name, fixtureCase.description, fixtureCase.expected] as const))(
|
||||
"matches the backend classifier for (%s, %s) -> %s",
|
||||
(name, description, expected) => {
|
||||
expect(classifyToolOp(name, description ?? "")).toBe(expected);
|
||||
},
|
||||
);
|
||||
|
||||
it("should classify read operations by name", () => {
|
||||
expect(classifyToolOp("get-users")).toBe("read");
|
||||
expect(classifyToolOp("list-items")).toBe("read");
|
||||
|
|
|
|||
|
|
@ -1,41 +1,123 @@
|
|||
export type CrudOp = "read" | "create" | "update" | "delete" | "unknown";
|
||||
|
||||
const DELETE_RE = /\b(delete|remove|destroy|purge|drop|erase|unlink)\b/i;
|
||||
const CREATE_RE = /\b(create|add|insert|new|post|submit|register|make|generate|write|upload)\b/i;
|
||||
const UPDATE_RE = /\b(update|edit|modify|change|patch|put|set|rename|move|transform)\b/i;
|
||||
const READ_RE = /\b(get|read|list|fetch|search|find|query|retrieve|show|view|check|describe|info)\b/i;
|
||||
const READ_TOKENS = new Set([
|
||||
"get",
|
||||
"read",
|
||||
"list",
|
||||
"fetch",
|
||||
"search",
|
||||
"find",
|
||||
"query",
|
||||
"retrieve",
|
||||
"show",
|
||||
"view",
|
||||
"check",
|
||||
"describe",
|
||||
"info",
|
||||
"lookup",
|
||||
"count",
|
||||
"export",
|
||||
"download",
|
||||
]);
|
||||
const DELETE_TOKENS = new Set([
|
||||
"delete",
|
||||
"remove",
|
||||
"destroy",
|
||||
"purge",
|
||||
"drop",
|
||||
"erase",
|
||||
"unlink",
|
||||
"wipe",
|
||||
"clear",
|
||||
"revoke",
|
||||
"uninstall",
|
||||
"trash",
|
||||
"truncate",
|
||||
"rm",
|
||||
"del",
|
||||
]);
|
||||
const UPDATE_TOKENS = new Set([
|
||||
"update",
|
||||
"edit",
|
||||
"modify",
|
||||
"change",
|
||||
"patch",
|
||||
"put",
|
||||
"set",
|
||||
"rename",
|
||||
"move",
|
||||
"transform",
|
||||
"toggle",
|
||||
"enable",
|
||||
"disable",
|
||||
"archive",
|
||||
"restore",
|
||||
]);
|
||||
const CREATE_TOKENS = new Set([
|
||||
"create",
|
||||
"add",
|
||||
"insert",
|
||||
"new",
|
||||
"post",
|
||||
"submit",
|
||||
"register",
|
||||
"make",
|
||||
"generate",
|
||||
"write",
|
||||
"upload",
|
||||
"send",
|
||||
"publish",
|
||||
]);
|
||||
|
||||
const SPLIT_RE = /[_\-./\s]+/;
|
||||
const CAMEL_BOUNDARY_RE = /(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])/;
|
||||
const NON_WORD_RE = /[^\w]+/;
|
||||
|
||||
export interface MCPToolEntry {
|
||||
name: string;
|
||||
description?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Classifies a tool by its name first; falls back to description only when
|
||||
* the name alone yields no match. This prevents incidental phrasing in
|
||||
* free-form descriptions (e.g. "removes noise from…") from promoting a safe
|
||||
* tool into a high-risk bucket.
|
||||
*
|
||||
* READ is checked before DELETE/UPDATE so that tools like `get_removed_entries`
|
||||
* or `list_deleted_items` — where the primary verb is a read operation — are
|
||||
* not silently blocked by the delete-by-default policy for new servers.
|
||||
*/
|
||||
const nameTokens = (name: string): string[] =>
|
||||
name
|
||||
.split(SPLIT_RE)
|
||||
.flatMap((chunk) => chunk.split(CAMEL_BOUNDARY_RE))
|
||||
.filter((token) => token.length > 0)
|
||||
.map((token) => token.toLowerCase());
|
||||
|
||||
const descriptionTokens = (description: string): string[] =>
|
||||
description
|
||||
.split(NON_WORD_RE)
|
||||
.filter((token) => token.length > 0)
|
||||
.map((token) => token.toLowerCase());
|
||||
|
||||
const stripSuffix = (token: string, suffix: string): string => (token.endsWith(suffix) ? token.slice(0, -suffix.length) : token);
|
||||
|
||||
const tokenVariants = (token: string): string[] =>
|
||||
[token, stripSuffix(token, "es"), stripSuffix(token, "s"), stripSuffix(token, "ed"), stripSuffix(token, "ing")].filter(
|
||||
(variant) => variant.length > 0,
|
||||
);
|
||||
|
||||
// Read is checked before delete/update/create so that tools like
|
||||
// `get_removed_entries` — where the primary verb is a read — are not silently
|
||||
// blocked by the delete-by-default policy for new servers. This mirrors
|
||||
// litellm/proxy/_experimental/mcp_server/tool_classification.py; the shared
|
||||
// fixture under tests/test_litellm pins parity between the two.
|
||||
const classifyTokens = (tokens: string[]): CrudOp => {
|
||||
const variants = new Set(tokens.flatMap((token) => tokenVariants(token)));
|
||||
if ([...variants].some((variant) => READ_TOKENS.has(variant))) return "read";
|
||||
if ([...variants].some((variant) => DELETE_TOKENS.has(variant))) return "delete";
|
||||
if ([...variants].some((variant) => UPDATE_TOKENS.has(variant))) return "update";
|
||||
if ([...variants].some((variant) => CREATE_TOKENS.has(variant))) return "create";
|
||||
return "unknown";
|
||||
};
|
||||
|
||||
// The name alone decides first; a misleading description cannot reclassify a
|
||||
// tool whose name already carries a recognized verb.
|
||||
export function classifyToolOp(name: string, description = ""): CrudOp {
|
||||
const nameLower = name.toLowerCase();
|
||||
if (READ_RE.test(nameLower)) return "read";
|
||||
if (DELETE_RE.test(nameLower)) return "delete";
|
||||
if (UPDATE_RE.test(nameLower)) return "update";
|
||||
if (CREATE_RE.test(nameLower)) return "create";
|
||||
|
||||
// Only consult description when the name is unrecognised.
|
||||
if (description) {
|
||||
const descLower = description.toLowerCase();
|
||||
if (READ_RE.test(descLower)) return "read";
|
||||
if (DELETE_RE.test(descLower)) return "delete";
|
||||
if (UPDATE_RE.test(descLower)) return "update";
|
||||
if (CREATE_RE.test(descLower)) return "create";
|
||||
}
|
||||
|
||||
const byName = classifyTokens(nameTokens(name));
|
||||
if (byName !== "unknown") return byName;
|
||||
if (description) return classifyTokens(descriptionTokens(description));
|
||||
return "unknown";
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue