Thread user_id through conversation_last_used() for defense-in-depth authorization #3075

What happened

On PR #3037, the review agent (run 40856246000, $3.93) flagged a medium-severity auth-bypass: conversation_last_used() in src/providers/inference/resolve.py calls retrieve_conversation(conversation_id) without user_id scoping. The underlying retrieve_conversation() in src/utils/endpoints.py queries session.query(UserConversation).filter_by(id=conversation_id).first() with no user_id filter. A caller who supplies another user's conversation_id can influence model selection via the soft-fallback chain.

The author replied: "The ownership of the conversation will be checked elsewhere." The human reviewer tisnik approved with "LGTM" without engaging with the finding. The PR merged on Oct 30 with the same commit that was reviewed — no code changes were made.

Codebase verification confirms: the sibling function can_access_conversation() in the same file properly checks user_id ownership, but retrieve_conversation() is an unguarded helper. The resolve_model_instance() method has no production callers yet (only test callers), so the risk is currently latent.

What could go better

The author's defense ("ownership checked elsewhere") is plausible — the codebase pattern is to check authorization at the endpoint layer. However, the new conversation_last_used() function bypasses the guarded can_access_conversation() path and uses the unguarded retrieve_conversation() directly. When resolve_model_instance() gains production callers, each caller must independently remember to pre-validate conversation ownership. This is a defense-in-depth gap.

Confidence: high that the code gap exists (verified by reading retrieve_conversation(), can_access_conversation(), and conversation_last_used()). Medium confidence that this will surface as a real bug — it depends on how callers are wired up. The data leaked would be limited to last_used_provider and last_used_model strings (not conversation content), but it could enable cross-tenant model-selection influence.

Proposed change

Add a user_id parameter to conversation_last_used() in src/providers/inference/resolve.py and use the existing can_access_conversation() check (or add a .filter_by(user_id=user_id) predicate to the query) before reading last_used_provider/last_used_model. Update resolve_model_instance() in src/providers/inference/registry.py to accept and thread user_id through to the fallback chain. Update the corresponding unit tests in tests/unit/providers/inference/test_resolve.py and tests/unit/providers/inference/test_registry.py.

Validation criteria

After the fix: (1) conversation_last_used(conversation_id, user_id) returns None when conversation_id belongs to a different user. (2) A unit test verifies the cross-user case. (3) resolve_model_instance() signature includes user_id so future callers cannot accidentally omit it.

Related issues checked

Searched: conversation_last_used user_id; cross-tenant conversation authorization
No overlapping issues found.

Generated by retro agent from #3037