- Shipped
- September 23, 2026 at 1:50 AM UTC
- Author
- Kamo
- Commit
- 9060c7c
Three independent findings, fixed together because the first two share files and a class. 1. AiMcpController had zero right checks on create/update/delete/test — any signed-in member of the organization could plant an MCP server config, and mcp-gateway-service's McpStdioTransport runs config.command with config.envVars as a real OS process (new ProcessBuilder(...).start()), on gateway start and again on every 30s health check. That is unauthenticated RCE against mcp-gateway-service. Every mutating/test handler now requires MANAGE_AI_SETTINGS, matching AiProviderController, and STDIO is refused outright for an organization-created config — a tenant may configure SSE/HTTP only. (ai_mcp_server_configs has 0 rows; no migration needed.) 2. SSRF: **************** sent an organization's decrypted provider key to whatever host was in baseUrl and reflected the response, and OpenAiCompatibleAdapter (which serves 8 of the 12 provider types) had no host rule on any of its three outbound calls — including the real chat path every message takes, not just the manual Test button. Added OutboundUrlGuard, built on kamo-shared-library's existing PublicHostGuard (already used by SecurityService's SafeSiteFetcher and SafeImageFetcher for the same class of problem): resolves the host and refuses **************** addresses and cluster-only names (*.svc, *.cluster.local, dotless), checked at save time in AiProviderController and AiMcpController and again immediately before every call in OpenAiCompatibleAdapter, since a hostname's DNS answer at save time is not its answer forever. Refusal is a plain 400 with our own message — no raw network exception, and no connection is ever attempted to a refused destination. 3. **************** userId) queried usage records and then, unconditionally, returned true — every AiAccessPolicy request/token limit was decorative. The two real call sites (ChatOrchestrationService's streaming and sync chat paths) now resolve every role the member holds and enforce each role's mapped policy: a role with no mapped policy grants no restriction of its own, but it also cannot cancel a restrictive policy assigned through a different role a member happens to also hold. The wasted 2-arg overload is gone. Tests: AiMcpControllerAuthzTest (rights + STDIO refusal + url validation), OutboundUrlGuardTest, AiProviderKeyExposureTest (2 new SSRF cases), **************** AccessPolicyMultiRoleTest. Every case was verified red against the pre-fix code before this commit (fail-open rights, body-trust, no-op guard and always-true quota checks each reintroduced locally, one at a time, and restored after confirming the matching test failed). mcp-gateway-service gets a matching commit: an independent STDIO allowlist (defense in depth, in case this check is ever bypassed) and its own before-connect check on an SSE/HTTP config's url.
