Repository navigation
feat(github_copilot): per-user GitHub OAuth connections for Copilot credentials - #45241
yassin-berriai merged 61 commits into
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
|
| if ( | ||
| isinstance(values, Mapping) | ||
| and values.get(GITHUB_COPILOT_AUTH_TYPE_KEY) == GITHUB_COPILOT_PER_USER_AUTH_TYPE | ||
| ): | ||
| names.append(credential_name_obj) |
There was a problem hiding this comment.
Provider logic crosses shared code
_per_user_credential_names_for_groups implements GitHub Copilot-specific mode checks in the proxy. The repository requires provider-specific logic to stay under llms/. Move the mode decision there and keep the proxy discovery generic before merging. The same pattern appears in the connection endpoints and response-cache check
Rule Used: What: Avoid writing provider-specific code outside of the llms/ directory. Why: This practice ensures better maintainability and reduces complexity over time. Good: ```python # Handle provider-specific logic within llms/vertex_ai/transformation.py ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
The mode decision already lives under llms/. github_copilot_auth_mode and is_github_copilot_per_user_request are in litellm/llms/github_copilot/per_user_auth.py, along with every GitHub call (device flow, seat check, exchange). The proxy only calls those helpers to decide which credential names need the caller's token, and the connection endpoints are by nature an API for that provider. Turning discovery into a provider-registry abstraction with one implementation would add indirection without removing any provider knowledge from the proxy, so I'm keeping the direct call
There was a problem hiding this comment.
That’s fair—the exchange/device-flow behavior and the authoritative github_copilot_auth_mode decision do live in llms/github_copilot. My concern is narrower: this proxy helper still independently imports Copilot constants and reads credential.credential_values to reproduce that decision, rather than calling the provider helper. That leaves two mode-resolution implementations to keep in sync and is the provider-specific logic the repo rule is intended to prevent.
A small provider-facing predicate/credential-name resolver, or a generic registry hook that invokes it, would preserve the current discovery behavior without adding meaningful indirection. If you intentionally accept this one provider-specific discovery exception, I’ll treat the comment as addressed.
a4b5b2d to
c27c316
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 9 · PR risk: 0/10 |
c27c316 to
8a39c9e
Compare
8a39c9e to
db3ba26
Compare
db3ba26 to
d7136c4
Compare
d7136c4 to
f7101c2
Compare
f7101c2 to
a72a51c
Compare
a72a51c to
e26d863
Compare
…redentials Related to LIT-9306
e26d863 to
47160b0
Compare
|
@greptileai please re-review at 55a9346. It adds the missing Final annotations to the new tests, on top of the base health endpoint merge |
…runcated comments
|
@greptileai please re-review at 7ab1853. It types the per-user OAuth JSON, DB and request boundaries so the basedpyright gate passes, with no behavior change |
|
@greptileai please re-review at c217402. It adds Final to the new boundary-typing locals and applies ruff format to three files |
…itellm_github_copilot_per_user_oauth
…itellm_github_copilot_per_user_oauth
|
@greptileai please re-review at 7fc259b. It merges the base after main landed, keeping both sides of two test conflicts |

Summary
Related to LIT-9306. Stacked on #45158 (
litellm_m365_copilot_obo).TLDR. A GitHub Copilot LLM credential can now use Auth Type "Per-user GitHub OAuth". Every request to a
github_copilot/...model on that credential uses the calling LiteLLM user's own GitHub token, exchanged for a short-lived Copilot token cached per user. Internal Users connect their own GitHub account from Models + Endpoints > LLM Credentials > Your connections. Callers still send only their LiteLLM key or JWT. "Shared device login" stays the default and behaves exactly as before.Add Model. "Per-user GitHub OAuth" is now
credentialOnlyin Add Model (commit 2aab235). Becausegithub_copilot_auth_typeis server-owned, the form never sends it inline. Instead it shows "Create credential", which opens the credential form with the provider locked and that auth type preselected, saves the credential withgithub_copilot_auth_type: per_user_oauthand attaches it throughlitellm_credential_name. Submitting the model without a credential is blocked. "Shared device login" stays inline. Before this change, an admin who picked per-user OAuth directly in Add Model got the 400 "server-owned ... parameter" errorFlow
credential_values = {"github_copilot_auth_type": "per_user_oauth"}, with no secret, and the model is attached to it throughlitellm_credential_name.POST /credentials/{name}/user_connection/startruns the GitHub device flow with the existing allowlisted Copilot client (Iv1.b507a08c87ecfe98, scoperead:user). Start returns an opaqueflow_handle, which isencrypt_value_helperover{user_id, credential_name, device_code, interval, expires_at}, so thedevice_codenever reaches the client in plaintext and no worker holds flow state. Each.../pollcall sends{"flow_handle": ...}, gets a 400 when the handle is undecryptable, expired or bound to another user or credential, and polls GitHub once. On success it seat-checks viacopilot_internal/v2/token(no seat returnsno_copilot_seatand stores nothing), reads the login fromGET /user, and storesencrypt_value_helper({"access_token", "github_login"})in the newLiteLLM_UserProviderCredentials. Disconnect isDELETE /credentials/{name}/user_connection. With Redis attached it writes the not-connected tombstone and reads it back before deleting the row, sinceRedisCacheswallows write errors, and returns 503 with the row kept when that write fails, so a retry is safe. A successful connect overwrites the Redis entry with the new connection, so a stale set-if-absent fill from an earlier read cannot leave a not-connected entry. It reads the write back and falls back to a verified delete, and returns 503 with the row kept if a stale entry survives both.GET /credentials/user_connectionslists status and never returns token values. All four routes act only onuser_api_key_dict.user_id. A missing user returns 401. An unknown or non-per-user credential returns 404.litellm_pre_call_utilschecks whether the requested model group, or one of its fallbacks, has a deployment on a per-user credential. Discovery uses the same lookup as routing:get_model_list(model_name, team_id=<caller team>)for team public model names, plusget_deployment(model_id=...)for calls by deployment ID. When discovery hits a strategy router deployment (semantic, complexity, adaptive or quality auto-router), it loads all of the caller's per-user connections in one query, since the router picks the target later. Only then does it load that caller's tokens intoSecretFields.user_provider_credentials, so other models never hit the DB.utils.wrapper/wrapper_asyncattach aGithubCopilotUserSession(validated host plus Copilot token) afterload_credentials_from_listand before the cache lookup. Chat, Responses, Anthropic Messages and embeddings all read that session. When the mode is per-user, nothing touches the sharedAuthenticatoror token file:Security
_is_response_cache_excludedalso returns true foris_github_copilot_per_user_request(kwargs), because cache keys ignore caller identity. Shared device-login models keep caching.github_copilot_auth_typejoinsserver_owned_wif_litellm_paramsasgithub_copilot_oauth_litellm_params. The existing non-admin gates therefore stop team admins from creating, patching or attaching a per-user credential or setting the mode on a model.github_copilot_auth_type,user_provider_credentialsandgithub_copilot_user_sessionare added to_BANNED_REQUEST_BODY_PARAMS.api_baseoverride keepslitellm_credential_name/github_copilot_auth_typefor per-user deployments inclientside_credential_handler, so it cannot flip the call into shared mode. The session's validated host still wins.litellm_credential_name, even empty or null, gets a 400, so the caller cannot switch the call to a shared credential.SecretFields.user_provider_credentialsis aRedactedDict, the same asraw_headers, so debug request logs never print GitHub tokens. Streaming cache writes also see the per-user marker, so per-user streamed completions are never cached.(user_id, credential_name)pair, and the connection routes take{credential_name:path}, so credential names containing/or:work. Disconnect also clears any pending device flow.litellm_credential_namebody check runs as soon as discovery finds a per-user credential, before the missinguser_idreturn, so service-account keys cannot switch a per-user deployment to shared mode. Discovery walks every fallback list the router can use, transitively and with the router's ownget_fallback_model_groupmatcher: router fallbacks, request-levelfallbacks,context_window_fallbacksandcontent_policy_fallbacks, key and teamrouter_settingsfallbacks, plus the namesmodel_alias_mapandmodel_group_aliasrewrite to. Exact-match keys use one dict lookup, with duplicate source keys unioned, and the matcher only sees wildcard keys, so discovery is a superset of what the router can pick. Discovery only runs when some credential inlitellm.credential_listuses per-user OAuth, so shared-only proxies keep the base behavior exactly. With per-user OAuth configured, request bodies whose fallback lists carry more than_FALLBACK_DISCOVERY_LIMIT(256) work units get a 400, where every mapping key costs at least one unit even with an empty or scalar value, and each wildcard entry expands only once. As a second guard that doesn't depend on discovery,Router._update_kwargs_with_deploymentraisesBadRequestErrorwhen a caller-suppliedlitellm_credential_namediffers from a per-user deployment's own credentialpin_session_authorizationdrops any callerAuthorizationheader and sets the session bearer at the last header merge in Chat, Responses, Anthropic Messages, embeddings and_complete_custom_openai, soextra_headerscannot replace the stored login. When the session is attached, the wrapper also drops caller Authorization keys fromextra_headers,headersand theextra_headersinsideoptional_paramsandlitellm_params, so the HTTP handler cannot add them backRouteChecks(_INTERNAL_USER_CREDENTIAL_CONNECTION_ROUTES): Internal Users get onlyGET /credentials/user_connections,POST .../user_connection/start,POST .../user_connection/pollandDELETE .../user_connection. Any other method on those paths falls through to the admin-only denial, so a credential whose name collides with a connection path can never reach the generic credential CRUD handlers through this allowlist.add_litellm_data_to_requestcapturesauthenticated_user_idbeforeuser_header_mappingscan overwriteuser_api_key_dict.user_id, so a caller-supplied identity header cannot select another user's connection.Files changed (excluding tests)
Backend:
litellm/llms/github_copilot/per_user_auth.py(new: session, sync/async exchange with per-key single-flight, host validation, device flow, redaction),litellm/proxy/credential_endpoints/user_provider_credentials.py(new: encrypted storage plus DualCache),litellm/proxy/credential_endpoints/endpoints.py(four routes, cascade),litellm/proxy/litellm_pre_call_utils.py,litellm/utils.py,litellm/caching/caching_handler.py,litellm/llms/github_copilot/{chat,responses,messages,embedding}/transformation.py,litellm/litellm_core_utils/get_llm_provider_logic.py,litellm/main.py,litellm/responses/main.py,litellm/router.py(passlitellm_paramstoget_llm_providerat deployment add, so a per-user deployment does not trigger the shared device login),litellm/router_utils/{clientside_credential_handler,cooldown_handlers}.py,litellm/exceptions.py,litellm/types/utils.py,litellm/proxy/_types.py,litellm/proxy/auth/auth_utils.py, the threeschema.prismacopies plus migration20261007221529_user_provider_credentials.UI:
provider_auth_types.ts(two GitHub Copilot auth types,fixedValues),UserConnectionsPanel.tsx,GithubCopilotConnectModal.tsx,github_copilot_connect_flow.ts(reducer: pending, slow_down, expired, denied, no_copilot_seat, connected),useUserConnections.ts,networking.tsx,page.tsxandLlmCredentialsPanel.tsx(role gating), andschema.d.ts(regenerated bynpm run gen:api, only the four routes).Before / after
Before: GitHub Copilot had only one device login in the proxy's token file, shared by every caller and billed to one seat. Internal Users could not open LLM Credentials.
After: an admin can choose per-user mode per credential. Each Internal User connects their own GitHub account and requests run on their own Copilot seat. Unconnected users get a 401 pointing them to the UI, never the shared login.
Test evidence
tests/unit/test_router/: 1258 passed. The 6 failures there reproduce on the base (semantic_routernot installed) or are suite-order flakes that pass standalone.make check: green (ruff, format, basedpyright vs base, LIT/TQ gates, dashboard lint budgets, API type sync).github_copilot_oauth_litellm_paramsfrom the server-owned set, reverting the deployment-add per-user branch, removing the bodylitellm_credential_nameguard, and removing the per-user cooldown exemption.47160b0720, with a real GitHub Copilot account approving the device codes. Admin created the per-user credential and attached a model (this first returned 500; the fix is in this PR). User A clicked Connect and the modal showed "Connected as @yassin-berriai". A's chat completion on the per-user model returned 200 (14 prompt + 4 completion tokens), and so did Responses (200, 18 tokens), Anthropic Messages (200, 14 in + 4 out) and embeddings (200, 1536 dims, 1 token). User B sent the same request and got 401Connect GitHub Copilot for credential 'copilot-per-user' in the LiteLLM UI (LLM Credentials)with no fallback, even with response caching on. The shared device-login model returned 200 (18 tokens). After A clicked Disconnect, A's request returned 401 with the connect message. The viewer got no tab, andGET /credentials/user_connectionsreturned 401 for them. A scan of 27 logs and curl transcripts found nogho_/ghu_/ghp_/tid=/sk-.Taxonomy audit
Audited the full diff against the litellm PR-review caveat taxonomy. Notable points: tokens never reach logs, reprs or errors (
GithubCopilotUserSession.__repr__, redacted exceptions); authz uses only the authenticated user id; the new routes are declared before the greedy/credentials/{credential_name:path}; per-user requests opt out of cache identity collisions; caller credential failures don't cool down deployments; there's no DB access for unrelated models; sync and async paths are at parity; shared mode is covered by regression tests. Known limitation, not addressed here: the batch retrieve and cancel router paths callget_llm_provider(model=...)without deployment params, so a per-user deployment there would still reach the shared authenticator. Batches are not one of the four supported surfaces.Link to Devin session: https://app.devin.ai/sessions/d08b073f89e542b984aee767e79470a6
Open in Devin Desktop: https://app.devin.ai/desktop/session/d08b073f89e542b984aee767e79470a6?variant=devin