# Vulnerability Summary ## Vulnerability Overview - **Vulnerability ID**: #860 - **Vulnerability Type**: Authentication Bypass + Default-All-Permissions RBAC (Role-Based Access Control) - **Severity**: High - **Description**: A 3-step vulnerability chain allowing unauthorized/privilege-escalated access to the management interface has been fixed. ## Scope of Impact - Affects all API methods that were not correctly classified. - Attackers can bypass authentication using invalid or expired tokens. - Unclassified methods defaulted to returning writable roles (admin/write/read). - Multiple sensitive methods were excluded from the allowlist and bypassed via the default-allow policy. ## Remediation 1. **Authentication Fix**: Reject invalid tokens, including those that exist but have expired. 2. **Default-Deny Policy**: Unclassified methods now default to "deny" instead of "allow". 3. **Method Classification**: Explicitly classify sensitive methods with admin/write/read permissions. 4. **Additional Hardening**: - Removed the `isWriteMethod` prefix fallback. - Used precise allowlists to prevent silent scope creep. - Added Phase-3 methods (TTS, Browser, Zalo, WhatsApp) to the admin/write/read permission classifications. - Removed empty `MethodRole` entries in the router scheduler. ## Key Code Changes ### Authentication Fix (router.go) ```go // Before fix: Invalid tokens silently passed // After fix: Invalid tokens are rejected if pe := r.server.policyEngine; pe != nil { if !pe.CanAccess(client.role, req.Method) { slog.Warn("permission denied", "method", req.Method, "role", client.role, "client", client.id) slog.Warn("security.permission_denied", "method", req.Method, "role", client.role, "required", string(required), "client", client.id) } } ``` ### Default-Deny Policy (policy.go) ```go // Before fix: Unclassified methods returned RoleNone (allowing access) // After fix: Unclassified methods return RoleNone (denying access) // RoleNone is a sentinel returned by MethodRole for methods that have no // explicit classification. The router treats it as deny-for-everyone so // newly-added RPCs are secure-by-default (fail-closed). RoleNone Role = "" ``` ### Method Classification (policy.go) ```go // Admin methods func isAdminMethod(method string) bool { adminMethods := []string{ protocol.MethodConfigApply, protocol.MethodConfigPatch, protocol.MethodConfigSchema, protocol.MethodConfigPermissionsList, protocol.MethodConfigPermissionsGrant, protocol.MethodConfigPermissionsRevoke, protocol.MethodAgentsCreate, protocol.MethodAgentsUpdate, protocol.MethodAgentsDelete, protocol.MethodAgentsLinkList, protocol.MethodAgentsLinkCreate, protocol.MethodAgentsLinkUpdate, protocol.MethodAgentsLinkDelete, protocol.MethodChannelsToggle, protocol.MethodChannelInstancesCreate, protocol.MethodChannelInstancesUpdate, protocol.MethodChannelInstancesDelete, protocol.MethodPairingApprove, protocol.MethodPairingDeny, protocol.MethodPairingList, protocol.MethodPairingRevoke, protocol.MethodTeamsList, protocol.MethodTeamsCreate, protocol.MethodTeamsDelete, protocol.MethodTeamsTaskList, protocol.MethodTeamsTaskSet, protocol.MethodTeamsTaskComments, protocol.MethodTeamsTaskEvents, protocol.MethodTeamsTaskUpdate, protocol.MethodTeamsMemberAdd, protocol.MethodTeamsMembersRemove, protocol.MethodTeamsTaskDelete, protocol.MethodTeamsTaskDeleteBulk, "tenants.create", "tenants.update", "tenants.users.add", "tenants.users.remove", protocol.MethodAPIKeysList, protocol.MethodAPIKeysCreate, protocol.MethodAPIKeysRevoke, protocol.MethodSkillsUpdate, protocol.MethodHeartbeatSet, protocol.MethodHeartbeatToggle, protocol.MethodHeartbeatTest, protocol.MethodHeartbeatCheckListSet, protocol.MethodLogsTail, protocol.MethodHooksCreate, protocol.MethodHooksUpdate, protocol.MethodHooksDelete, protocol.MethodHooksToggle, protocol.MethodVoicesRefresh, protocol.MethodTTSDisable, protocol.MethodTTSEnable, protocol.MethodTTSSetProvider, } return slices.Contains(adminMethods, method) } ``` ### Write Method Classification (policy.go) ```go func isWriteMethod(method string) bool { writePrefix := []string{ "pairing.", "device.pair.", "approvals.", "exec.approval.", } writeExact := []string{ protocol.MethodChatSend, protocol.MethodChatAbort, protocol.MethodSessionsDelete, protocol.MethodSessionsReset, protocol.MethodSessionsPatch, protocol.MethodCronCreate, protocol.MethodCronUpdate, protocol.MethodCronDelete, protocol.MethodCronToggle, protocol.MethodCronRun, protocol.MethodSend, protocol.MethodAgentsFileSet, protocol.MethodTeamsTaskApprove, protocol.MethodTeamsTaskReject, protocol.MethodTeamsTaskComment, protocol.MethodTeamsTaskCreate, protocol.MethodTeamsTaskAssign, protocol.MethodTeamsWorkspaceDelete, protocol.MethodHooksTest, protocol.MethodPairingRequest, protocol.MethodApprovalApprove, protocol.MethodApprovalDeny, protocol.MethodTTSConvert, protocol.MethodBrowserAct, } // ... other logic } ```