Feat: Per-user inference routes with ownership scoping#21
Conversation
pdettori
force-pushed
the
feat/per-user-inference-routes
branch
from
cc0dbad to
95b262b
Compare
Allow non-admin users to set their own inference routes. Routes are stored with owner-scoped names (inference.local/<subject-hash>) so each user gets isolated config while admin-created global routes serve as fallbacks. Changes: - Relax set_cluster_inference role from admin to user - Scope route names by owner for non-admin callers - Stamp ownership label on new routes - Verify provider ownership before referencing in a route - Verify route ownership before updating - Resolve per-user routes first, fall back to global - Sandbox bundle resolution looks up sandbox owner Closes: kagenti/kagenti#1995 Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
pdettori
force-pushed
the
feat/per-user-inference-routes
branch
from
95b262b to
8277d6f
Compare
pdettori
left a comment
pdettori
left a comment
There was a problem hiding this comment.
Self-review (cannot self-approve)
Clean implementation of per-user inference route scoping. The authorization model is sound: ownership checks gate provider references, route updates, and route creation. CAS writes prevent TOCTOU issues. The main gap is test coverage for the new ownership paths — all existing tests pass None, "" for principal/admin_role.
Areas reviewed: Rust (authorization, ownership, error handling)
CI: DCO ✓, Rust ✓, lint ✓
Verdict: Would APPROVE if this weren't my own PR.
| )) | ||
| } | ||
|
|
||
| /// Resolve a route with owner-scoping for sandbox bundle: try per-user first, |
There was a problem hiding this comment.
nit: unreachable!() — consider returning Status::internal("unexpected principal variant") for defense-in-depth if authorize_inference_bundle invariants ever change. Not blocking.
| store: &Store, | ||
| route_name: &str, | ||
| provider_name: &str, | ||
| model_id: &str, |
There was a problem hiding this comment.
nit: #[allow(clippy::too_many_arguments)] — a UpsertRequest struct would clean up the 8-param signature. Fine as-is for now.
| request: Request<SetClusterInferenceRequest>, | ||
| ) -> Result<Response<SetClusterInferenceResponse>, Status> { | ||
| let principal = request | ||
| .extensions() |
There was a problem hiding this comment.
suggestion: All existing tests pass None, "" for the new principal/admin_role params, leaving the ownership logic untested at the unit level. Consider adding tests for:
- Non-admin creates a scoped route (name =
base/<subject>) - Non-admin rejected when referencing another user's provider
- Ownership check blocks route update by non-owner
resolve_get_routefallback from per-user → globalresolve_route_for_ownerwith sandbox owner
- Replace unreachable!() with Status::internal() for defense-in-depth in get_inference_bundle when principal variant is unexpected. - Add unit tests covering per-user ownership scoping logic: - scoped_route_name: non-admin, admin, anonymous, none, sandbox cases - upsert: non-admin creates scoped route - upsert: non-owner blocked from updating another user's route - resolve_get_route: falls back to global when no per-user route - resolve_get_route: prefers per-user route over global - resolve_route_for_owner: sandbox owner resolves per-user route - resolve_route_for_owner: falls back without per-user route Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 participant
Summary
inference.local/<owner-hash>) so users get isolated configinference.local) serve as fallbacks when no per-user route existsChanges
set_cluster_inferencerole fromadmintouserscoped_route_namehelper that encodes ownership in the store nameopenshell.ai/ownerlabel on new routes viastamp_ownerget_cluster_inferenceresolves per-user route first, falls back to globalget_inference_bundlelooks up sandbox owner and resolves their routesTest plan
None, ""for backward compat)openshell inference set --provider <own-provider> --model <model> --no-verifyopenshell inference getand see their own routeinference setstill creates global routeCloses kagenti/kagenti#1995
Assisted-By: Claude Code