-
Notifications
You must be signed in to change notification settings - Fork 478
[fix] Resolve IDOR vulnerability: add workspace/project membership check in auth middleware and fix cache-before-auth #3680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR hardens authorization in the API by preventing cross-project access (IDOR) through explicit project_id query parameters and by ensuring RBAC checks run before cache lookups, so cached responses can’t bypass permissions.
Changes:
- Add an EE-only project membership check in
verify_bearer_tokenwhenproject_idis explicitly provided. - Reorder authorization vs caching in
list_secretsandlist_app_variantsso permission checks occur before cache reads.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
api/oss/src/services/auth_service.py |
Adds EE project membership validation for explicit project_id in bearer-token auth. |
api/oss/src/routers/app_router.py |
Moves RBAC enforcement ahead of cache reads in list_app_variants. |
api/oss/src/apis/fastapi/vault/router.py |
Moves RBAC enforcement ahead of cache reads in list_secrets. |
Comments suppressed due to low confidence (1)
api/oss/src/routers/app_router.py:103
- Test coverage: add a test that ensures RBAC is enforced on cache hits for
list_app_variants(i.e., a cached response must not be returned whencheck_action_accessdenies). This change fixes an auth bypass and should be guarded by a regression test.
if is_ee():
has_permission = await check_action_access(
user_uid=request.state.user_id,
project_id=request.state.project_id,
permission=Permission.VIEW_APPLICATIONS,
)
if not has_permission:
error_msg = "You do not have access to perform this action. Please contact your organization admin."
return JSONResponse(
{"detail": error_msg},
status_code=403,
)
cache_key = {
"app_id": app_id,
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
verify_bearer_token). When a user supplies an explicitproject_idquery parameter, the middleware now verifies the user is a member of that project before proceeding. Previously it only checked the project existed.vault/router.py(list_secrets) andapp_router.py(list_app_variants) — the RBAC permission check now runs before the cache lookup, preventing cached responses from bypassing authorization.Test plan
project_idgets 401project_id(default project) still worklist_secretsandlist_app_variantsenforce permissions on cache hits