fix(llm): enforce persona restrictions on public LLM providers#8846
fix(llm): enforce persona restrictions on public LLM providers#8846
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes two critical access control bugs that allowed unauthorized persona access to LLM providers: Backend fix ( Frontend fix ( Both fixes are well-documented with clear comments explaining the decision matrix and comprehensive regression tests covering the public provider with persona restrictions scenario. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([User requests LLM provider access]) --> CheckPersonas{Persona restrictions set?}
CheckPersonas -->|Yes| PersonaWhitelist{Persona in whitelist?}
CheckPersonas -->|No| CheckPublic{is_public = True?}
PersonaWhitelist -->|No| Deny1[❌ Deny Access]
PersonaWhitelist -->|Yes| CheckPublic
CheckPublic -->|Yes| Allow1[✅ Allow Access]
CheckPublic -->|No| CheckGroups{Groups set?}
CheckGroups -->|Yes| UserInGroup{User in group OR admin?}
CheckGroups -->|No| CheckPersonasOnly{Personas set?}
UserInGroup -->|Yes| Allow2[✅ Allow Access]
UserInGroup -->|No| Deny2[❌ Deny Access]
CheckPersonasOnly -->|Yes| Allow3[✅ Allow Access<br/>via persona whitelist]
CheckPersonasOnly -->|No| AdminCheck{Is admin?}
AdminCheck -->|Yes| Allow4[✅ Allow Access<br/>admin-only mode]
AdminCheck -->|No| Deny3[❌ Deny Access<br/>locked provider]
style Deny1 fill:#ffcccc
style Deny2 fill:#ffcccc
style Deny3 fill:#ffcccc
style Allow1 fill:#ccffcc
style Allow2 fill:#ccffcc
style Allow3 fill:#ccffcc
style Allow4 fill:#ccffcc
Last reviewed commit: c0ed7fb |
|
Preview Deployment
|
🖼️ Visual Regression Report
|
Public providers (`is_public=True`) with persona whitelists were bypassing persona checks entirely — the early return on `is_public` short-circuited before persona restrictions were evaluated. This meant any persona could use a public provider even if it was explicitly restricted to specific personas. Additionally, `hasAnyProvider` in the frontend `useLlmManager` hook used the global provider list (no persona context), causing chat input to be disabled for all personas when the default provider was non-public — even if the selected persona had its own provider assigned. Backend: check persona restrictions first, then apply is_public as a group-bypass only. Frontend: derive hasAnyProvider from the persona-aware provider list instead of the global one.
nmgarza5
force-pushed
the
nikg/fix-llm-rbac
branch
from
c0ed7fb to
39db27c
Compare
| 6. is_public=False, neither set → admin-only (locked) | ||
| """ | ||
| # Public override - everyone has access | ||
| if provider.is_public: |
There was a problem hiding this comment.
Hmm, if it's the case that there are restriction on being public, it might be worth renaming this sometime soon. I can imagine mistakes arising down the line
| if provider.is_public: | ||
| return True | ||
|
|
||
| has_groups = bool(provider_group_ids) |
There was a problem hiding this comment.
Imo I think this sort of function is a bit cleaner when it follows a, define all data, then perform conditionals sort of approach.
Currently finding it a bit non-trivial to follow, but I think if we did something along these lines it could be a bit nicer:
provider_group_ids = {g.id for g in (provider.groups or [])}
provider_persona_ids = {p.id for p in (provider.personas or [])}
has_groups = bool(provider_group_ids)
has_personas = bool(provider_persona_ids)
# personas always enforced
if has_personas and not (persona and persona.id in provider_persona_ids):
return False
if provider.is_public:
return True
if has_groups:
return is_admin or bool(user_group_ids & provider_group_ids)
# no groups: either persona-whitelisted (already passed) or admin-only if locked
return has_personas or is_admin
Just a rough sketch out so not sure if that is correct or if it's the best way to go, but a small suggestion
2 participants
Description
Fixes two compounding bugs in LLM provider access control:
Backend (
can_user_access_llm_provider):is_public=Trueshort-circuited the entire function, bypassing persona whitelist checks. A public provider withpersonas=[0, 1]was accessible to all personas. Fix: enforce persona restrictions first, then applyis_publicas a user/group bypass only.Frontend (
useLlmManager.hasAnyProvider): Derived from the global/api/llm/providerendpoint (no persona context), so when the default provider was non-public, chat input was disabled for all personas — even those with their own assigned provider. Fix: derivehasAnyProviderfrom the persona-aware provider list (llmProviders) instead ofallUserProviders.How Has This Been Tested?
Additional Options
Summary by cubic
Enforces persona restrictions on public LLM providers and fixes chat provider detection. Global lists exclude public providers that restrict personas; persona-aware lists include them only when allowed.
Written for commit 39db27c. Summary will update on new commits.