fix(ui): disable /config/list fetch for non-admin users in useProxyConfig hook#24313
fix(ui): disable /config/list fetch for non-admin users in useProxyConfig hook#24313xykong wants to merge 1 commit intoBerriAI:mainfrom
Conversation
…sers The useProxyConfig hook called GET /config/list unconditionally on mount, regardless of user role. Since /config/list is admin-only (returns 400 for non-admin), every page load for internal_user sessions generated a 400 error log entry. Add isProxyAdminRole check to the query's enabled flag so the fetch is skipped entirely for non-admin users. The isProxyAdminRole utility already exists in @/utils/roles. Fixes BerriAI#24309
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes repeated
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/app/(dashboard)/hooks/proxyConfig/useProxyConfig.ts | Adds role-based gate to skip the /config/list fetch for non-admin users; works correctly for the fix scenario but relies on isProxyAdminRole matching formatted role strings ("Admin") rather than the raw backend values the function was designed for ("proxy_admin") |
Sequence Diagram
sequenceDiagram
participant C as Component (mount)
participant H as useProxyConfig hook
participant A as useAuthorized
participant RQ as React Query
participant API as GET /config/list
C->>H: useProxyConfig(configType)
H->>A: useAuthorized()
A-->>H: { accessToken, userRole (formatted) }
H->>H: isProxyAdminRole(userRole ?? "")
alt userRole === "Admin" (proxy_admin / app_admin)
H->>RQ: enabled: true
RQ->>API: GET /config/list?config_type=...
API-->>RQ: 200 OK + config data
RQ-->>C: { data: ProxyConfigResponse }
else userRole !== "Admin" (internal_user, viewer, etc.)
H->>RQ: enabled: false
RQ-->>C: { data: undefined, isFetching: false }
note over C,API: No network request made — no 400 errors
end
Last reviewed commit: "fix(ui): disable /co..."
| }), | ||
| queryFn: async () => await getProxyConfigCall(accessToken!, configType), | ||
| enabled: Boolean(accessToken), | ||
| enabled: Boolean(accessToken) && isProxyAdminRole(userRole ?? ""), |
There was a problem hiding this comment.
Implicit coupling between formatted and raw role values
useAuthorized exposes userRole as the formatted display string (e.g., "Admin") via formatUserRole(decoded?.user_role), not the raw backend role (e.g., "proxy_admin"). This means isProxyAdminRole is being called with a formatted value here.
isProxyAdminRole is defined as:
export const isProxyAdminRole = (role: string): boolean => {
return role === "proxy_admin" || role === "Admin";
};The role === "proxy_admin" branch will never match in this call because userRole is always pre-formatted. The guard works only because the function also checks for "Admin" (the formatted form). This creates a silent dependency: if formatUserRole were updated to no longer map proxy_admin → "Admin", or if isProxyAdminRole dropped the "Admin" branch, the guard would silently stop working for proxy_admin users.
Consider using useAuthorized's raw token instead, or explicitly documenting that isProxyAdminRole accepts formatted role strings:
| enabled: Boolean(accessToken) && isProxyAdminRole(userRole ?? ""), | |
| enabled: Boolean(accessToken) && isProxyAdminRole(userRole ?? ""), |
Alternatively, expose a rawUserRole from useAuthorized (e.g., decoded?.user_role) so isProxyAdminRole can be called with the raw backend role it was designed for.
Summary
Fixes
useProxyConfighook to skip theGET /config/listrequest for non-admin users, eliminating repeated400errors in proxy logs on every page load.Fixes #24309
Root Cause
useProxyConfigcallsGET /config/listunconditionally on component mount. Since/config/listis restricted toPROXY_ADMIN(returns400 Bad Requestfor other roles), every page load forinternal_usersessions triggers:Components using
useProxyConfig(ModelSettingsModal,SpendLogsSettingsModal) are rendered during initial page load, so this fires immediately and on every navigation.Change
The
isProxyAdminRoleutility already exists in@/utils/roles— no new dependencies.Testing
internal_usersessions no longer produce400errors on page loaduseProxyConfighandle theundefineddata state (they already do via optional chaining)Impact
useProxyConfig(ModelSettingsModal,SpendLogsSettingsModal, and future consumers)