mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-04 06:12:10 +03:00
fix(oauth): GitHub Copilot token refresh sends the public client_id (#4320)
GitHub Copilot is a public device-flow OAuth client (client_id, no client_secret), but the github provider config never populated clientId. The standalone refresh path omitted client_id (buildFormParams drops undefined) and the executor path sent the literal "client_id=undefined&client_secret=undefined" — both rejected by GitHub, so a Copilot connection got stuck once its short-lived token expired and the long-lived refresh path was needed. Populate the provider clientId from the embedded public cred (resolvePublicCred, never a literal) and only send client_secret when one exists. The prior github refresh test patched a fake clientId/clientSecret onto PROVIDERS.github, masking the broken real config — it now exercises the real config. Co-authored-by: Manuel B. <1494154+baslr@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
5193a595bf
commit
032387401a
@@ -25,6 +25,7 @@ _In development — bullets added per PR; finalized at release._
|
||||
- **fix(api): cache-HIT `X-OmniRoute-Response-Cost` now reports the incremental cost (≈0), not the original** — on a semantic-cache HIT the gateway serves the stored response **without** an upstream call, but `X-OmniRoute-Response-Cost` was reporting the original call's full cost (recomputed from the cached `usage`). A consumer summing `response-cost` for billing was therefore charging for responses that cost ≈$0 to serve (and stale entries could inflate it). Cache hits now bill `X-OmniRoute-Response-Cost: 0.0000000000` (the real incremental cost), and the avoided cost is surfaced in a new **`X-OmniRoute-Cost-Saved`** header for cache analytics — mirroring the existing `tokens_saved` concept. The MISS path is unchanged. (PRD-2026-06-19-cache-hit-cost-reporting)
|
||||
- **fix(models): imported vision-capable models keep their vision capability** — after importing a provider key, vision-capable models (e.g. OpenRouter models whose `architecture` declares image input, and other synced providers) were listed as text-only in `/v1/models` and the dashboard — even though image requests actually worked. Synced model records never captured the vision flag, and the catalog's OpenRouter live-enrichment (which derives vision from `architecture.input_modalities`) is skipped once a provider has synced models. Discovery now captures `supportsVision` at sync time (from `architecture.input_modalities`, the string `architecture.modality`, or a top-level `input_modalities`), mirroring the existing `supportsThinking` capture, and the catalog surfaces `capabilities.vision` for synced models. ([#4264](https://github.com/diegosouzapw/OmniRoute/issues/4264) — thanks @FerLuisxd)
|
||||
- **fix(providers): Cloudflare Workers AI model discovery shows model names, not UUIDs** — importing a Cloudflare Workers AI key listed models with internal UUID identifiers (e.g. `429b9e8b-d99e-…`) instead of their usable slugs (`@cf/meta/llama-3.1-8b-instruct`). Cloudflare's `/ai/models/search` returns `{ id: "<uuid>", name: "@cf/…" }`, and discovery was passing the raw objects through — so the UUID `id` became the callable model id. The `cloudflare-ai` discovery now maps each result's `name` → id, surfacing the real `@cf/…` model ids. ([#4259](https://github.com/diegosouzapw/OmniRoute/issues/4259) — thanks @FerLuisxd)
|
||||
- **fix(oauth): GitHub Copilot token refresh now sends the public client_id** — the `github` provider config never carried a `clientId`, so GitHub OAuth `refresh_token` exchanges either omitted `client_id` or sent the literal string `undefined` (and a bogus `client_secret=undefined`), which GitHub rejects — leaving a Copilot connection stuck once its short-lived token expired and the long-lived refresh path was needed. The provider now resolves its public device-flow `client_id` from the embedded public credential and omits `client_secret` entirely (GitHub's Copilot app is a public client with no secret). (thanks @baslr)
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import type { RegistryEntry } from "../../shared.ts";
|
||||
import { GPT_5_5_CODEX_CAPABILITIES, getGitHubCopilotChatHeaders } from "../../shared.ts";
|
||||
import {
|
||||
GPT_5_5_CODEX_CAPABILITIES,
|
||||
getGitHubCopilotChatHeaders,
|
||||
resolvePublicCred,
|
||||
} from "../../shared.ts";
|
||||
|
||||
export const githubProvider: RegistryEntry = {
|
||||
id: "github",
|
||||
@@ -10,6 +14,14 @@ export const githubProvider: RegistryEntry = {
|
||||
responsesBaseUrl: "https://api.githubcopilot.com/responses",
|
||||
authType: "oauth",
|
||||
authHeader: "bearer",
|
||||
// GitHub Copilot is a public device-flow OAuth client: it has a public client_id but
|
||||
// NO client_secret. Populate clientId so token refresh carries it (9router#442) — without
|
||||
// it, refresh requests omit/garble client_id and GitHub rejects them. Embedded via
|
||||
// resolvePublicCred per Hard Rule #11 (never a string literal).
|
||||
oauth: {
|
||||
clientIdEnv: "GITHUB_OAUTH_CLIENT_ID",
|
||||
clientIdDefault: resolvePublicCred("github_copilot_id"),
|
||||
},
|
||||
defaultContextLength: 128000,
|
||||
headers: getGitHubCopilotChatHeaders(),
|
||||
models: [
|
||||
|
||||
@@ -172,18 +172,24 @@ export class GithubExecutor extends BaseExecutor {
|
||||
|
||||
async refreshGitHubToken(refreshToken, log) {
|
||||
try {
|
||||
// GitHub Copilot is a public device-flow client: send the public client_id, and
|
||||
// only attach client_secret when one is actually configured — never the literal
|
||||
// "undefined" that new URLSearchParams produces for a missing value (9router#442).
|
||||
const params = new URLSearchParams({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: refreshToken,
|
||||
client_id: this.config.clientId,
|
||||
});
|
||||
if (this.config.clientSecret) {
|
||||
params.set("client_secret", this.config.clientSecret);
|
||||
}
|
||||
const response = await fetch(OAUTH_ENDPOINTS.github.token, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
Accept: "application/json",
|
||||
},
|
||||
body: new URLSearchParams({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: refreshToken,
|
||||
client_id: this.config.clientId,
|
||||
client_secret: this.config.clientSecret,
|
||||
}),
|
||||
body: params,
|
||||
});
|
||||
if (!response.ok) return null;
|
||||
const tokens = await response.json();
|
||||
|
||||
@@ -8,6 +8,46 @@ function registerModel(provider, model) {
|
||||
PROVIDER_MODELS[provider] = [...(PROVIDER_MODELS[provider] || []), model];
|
||||
}
|
||||
|
||||
test("GithubExecutor.refreshGitHubToken sends the public client_id and omits client_secret (port from 9router#442)", async () => {
|
||||
// GitHub Copilot is a public device-flow OAuth client (client_id, no client_secret).
|
||||
// The previous code sent client_id/client_secret straight from this.config via
|
||||
// new URLSearchParams, so an undefined config produced the literal
|
||||
// "client_id=undefined&client_secret=undefined". The fix populates the real client_id
|
||||
// and only sends client_secret when one actually exists.
|
||||
const executor = new GithubExecutor();
|
||||
const calls: any[] = [];
|
||||
const originalFetch = globalThis.fetch;
|
||||
globalThis.fetch = (async (url: any, options: any = {}) => {
|
||||
calls.push({ url: String(url), options });
|
||||
return {
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
access_token: "gh-access",
|
||||
refresh_token: "gh-next",
|
||||
expires_in: 3600,
|
||||
}),
|
||||
} as any;
|
||||
}) as any;
|
||||
|
||||
try {
|
||||
const result = await executor.refreshGitHubToken("gh-refresh", { info() {}, error() {} });
|
||||
assert.deepEqual(result, {
|
||||
accessToken: "gh-access",
|
||||
refreshToken: "gh-next",
|
||||
expiresIn: 3600,
|
||||
});
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
|
||||
const body = String(calls[0].options.body);
|
||||
assert.match(body, /client_id=Iv1\./, "the real public github client_id must be sent");
|
||||
assert.ok(
|
||||
!body.includes("client_secret="),
|
||||
"client_secret must be omitted (never the literal 'undefined')"
|
||||
);
|
||||
});
|
||||
|
||||
test("GithubExecutor.buildUrl routes response-format models to /responses", () => {
|
||||
const originalModels = [...(PROVIDER_MODELS.gh || [])];
|
||||
registerModel("gh", {
|
||||
|
||||
@@ -689,40 +689,40 @@ test("refreshQoderToken uses basic auth once qoder oauth settings are configured
|
||||
assert.match(calls[0].options.headers.Authorization, /^Basic /);
|
||||
});
|
||||
|
||||
test("refreshGitHubToken exchanges the refresh token with github oauth", async () => {
|
||||
test("refreshGitHubToken sends the real public github client_id and no client_secret (port from 9router#442)", async () => {
|
||||
// GitHub Copilot's OAuth app is a public device-flow client: it has a client_id but
|
||||
// NO client_secret. PROVIDERS.github.clientId must be populated from the embedded public
|
||||
// cred so the refresh request actually carries a client_id — a missing one makes GitHub
|
||||
// reject the refresh. The previous test patched a fake clientId/clientSecret onto
|
||||
// PROVIDERS.github, masking the fact that the real config had neither. This uses the real
|
||||
// config and asserts the real client_id is sent and no client_secret leaks out.
|
||||
const log = createLog();
|
||||
const calls: any[] = [];
|
||||
|
||||
await withPatchedProperties(
|
||||
PROVIDERS.github,
|
||||
{
|
||||
clientId: "github-client",
|
||||
clientSecret: "github-secret",
|
||||
await withMockedFetch(
|
||||
async (url, options = {}) => {
|
||||
calls.push({ url, options });
|
||||
return jsonResponse({
|
||||
access_token: "github-access",
|
||||
refresh_token: "github-refresh-next",
|
||||
expires_in: 3600,
|
||||
});
|
||||
},
|
||||
async () => {
|
||||
await withMockedFetch(
|
||||
async (url, options = {}) => {
|
||||
calls.push({ url, options });
|
||||
return jsonResponse({
|
||||
access_token: "github-access",
|
||||
refresh_token: "github-refresh-next",
|
||||
expires_in: 3600,
|
||||
});
|
||||
},
|
||||
async () => {
|
||||
const result = await refreshGitHubToken("github-refresh", log);
|
||||
assert.deepEqual(result, {
|
||||
accessToken: "github-access",
|
||||
refreshToken: "github-refresh-next",
|
||||
expiresIn: 3600,
|
||||
});
|
||||
}
|
||||
);
|
||||
const result = await refreshGitHubToken("github-refresh", log);
|
||||
assert.deepEqual(result, {
|
||||
accessToken: "github-access",
|
||||
refreshToken: "github-refresh-next",
|
||||
expiresIn: 3600,
|
||||
});
|
||||
}
|
||||
);
|
||||
|
||||
const body = bodyToString(calls[0].options.body);
|
||||
assert.equal(calls[0].url, OAUTH_ENDPOINTS.github.token);
|
||||
assert.match(bodyToString(calls[0].options.body), /client_id=github-client/);
|
||||
assert.ok(PROVIDERS.github.clientId, "PROVIDERS.github.clientId must be populated from the public cred");
|
||||
assert.match(body, /client_id=Iv1\./, "the real public github client_id must be sent on refresh");
|
||||
assert.ok(!body.includes("client_secret="), "no client_secret for the public github client");
|
||||
});
|
||||
|
||||
test("refreshCopilotToken returns the short-lived copilot token", async () => {
|
||||
|
||||
Reference in New Issue
Block a user