diff --git a/AGENTS.md b/AGENTS.md index 101e339..7a6be7c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -203,8 +203,9 @@ show only `bin/`, `src/`, `examples/`, `docs/`, `schemas/`, `package.json`, `REA - Bump with `npm version --no-git-tag-version` so `package.json` and `package-lock.json` stay in sync, then update `CITATION.cff` and `CHANGELOG.md` to the same number. The README carries no version pins; it points at badges and the changelog instead. -- `main` is protected: required status checks on all six CI jobs and **one approving review**. Push a branch - and open a pull request; never force-push, never rewrite history, never self-approve, never use admin bypass. +- `main` is protected: all six CI jobs are required, conversations must be resolved, and history stays + linear. The single maintainer cannot self-approve, so no approving review is required. Push a branch and open + a pull request; never force-push, never rewrite history, never use admin bypass. - Tag (`v`), GitHub Release, and `npm publish` happen only after the pull request is merged into `main` and CI is green. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0277971..a8342ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to this project are documented in this file. +## [2.0.1] - 2026-10-21 + +### Security + +- Live model discovery sent the provider credential to any configured `modelsUrl`, even when that + catalog lived on a different origin from the provider API. Discovery now applies the same scoping + rule as the interactive path: the key is attached only on the provider origin, or when the provider + sets an authenticated `modelsAuth` mode explicitly. Providers that rely on a cross-origin catalog + must set `modelsAuth` (for example `"bearer"`); no configuration change is required otherwise. + ## [2.0.0] - 2026-10-14 ### Added diff --git a/CITATION.cff b/CITATION.cff index 572bef5..3dfecd9 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -5,6 +5,6 @@ authors: - family-names: Gasperini given-names: Michael url: "https://github.com/TheStreamCode/copilot-byok-switcher" -version: "2.0.0" -date-released: "2026-10-14" +version: "2.0.1" +date-released: "2026-10-21" license: MIT diff --git a/package-lock.json b/package-lock.json index 7cc3e54..e8413c9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "copilot-byok-switcher", - "version": "2.0.0", + "version": "2.0.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "copilot-byok-switcher", - "version": "2.0.0", + "version": "2.0.1", "license": "MIT", "dependencies": { "cross-spawn": "^7.0.6" diff --git a/package.json b/package.json index 33f3a59..8eef186 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "copilot-byok-switcher", - "version": "2.0.0", + "version": "2.0.1", "description": "Add your own provider models to the GitHub Copilot CLI /model picker, in the same session as the GitHub ones, without storing credentials.", "keywords": [ "github-copilot", diff --git a/src/cli.mjs b/src/cli.mjs index 7f57680..35d46b6 100644 --- a/src/cli.mjs +++ b/src/cli.mjs @@ -7,6 +7,7 @@ import { parseArgs } from './args.mjs'; import { resolveCopilotBin } from './copilot-bin.mjs'; import { findProvider, loadConfig } from './config.mjs'; import { rankModels } from './model-ranking.mjs'; +import { providerModelHeaders } from './models-auth.mjs'; import { buildProviderEnvironment } from './provider-env.mjs'; import { sanitizeCopilotEnvironment } from './process-env.mjs'; import { isQueryable, resolveLiveModels, resolveOfflineModels, runCopilotWithRouter, selectActiveProviders, startRouter } from './launcher.mjs'; @@ -318,26 +319,6 @@ async function loadProviderModels(provider, io, { strict = false } = {}) { } } -function providerModelHeaders(provider) { - const headers = { ...(provider.modelsHeaders || {}) }; - if (provider.modelsAuth === false || provider.modelsAuth === 'none') return headers; - const authMode = provider.modelsAuth === true || provider.modelsAuth == null - ? 'bearer' - : provider.modelsAuth; - const token = authMode === 'bearer' - ? provider.bearerToken || provider.apiKey - : provider.apiKey || provider.bearerToken; - if (!token) return headers; - - const sameOrigin = new URL(provider.modelsUrl).origin === new URL(provider.baseUrl).origin; - const explicitAuth = provider.modelsAuth === true || typeof provider.modelsAuth === 'string'; - if (!sameOrigin && !explicitAuth) return headers; - - if (authMode === 'x-api-key') headers['x-api-key'] = token; - else if (authMode === 'api-key') headers['api-key'] = token; - else headers.Authorization = `Bearer ${token}`; - return headers; -} async function readJsonResponse(response, maxBytes) { const contentLength = Number(response.headers?.get?.('content-length')); diff --git a/src/discovery.mjs b/src/discovery.mjs index c597d3f..7c1de84 100644 --- a/src/discovery.mjs +++ b/src/discovery.mjs @@ -8,6 +8,7 @@ // falls back to the list its key returned last time — or, when nothing was ever // remembered, it stays out of the picker with the reason logged. +import { providerModelHeaders } from './models-auth.mjs'; import { rankModels } from './model-ranking.mjs'; import { recallModels, rememberModels } from './model-cache.mjs'; @@ -108,9 +109,13 @@ export async function discoverModels(provider, { } async function fetchCatalog(provider, { timeoutMs, fetchImpl }) { - const headers = { accept: 'application/json', ...(provider.headers || {}) }; - const token = provider.bearerToken || provider.apiKey; - if (token) headers.authorization = `Bearer ${token}`; + // Same scoping rule as the interactive path: the catalog receives the + // provider credential only on the provider origin, or when modelsAuth opts in. + const headers = { + accept: 'application/json', + ...(provider.headers || {}), + ...providerModelHeaders(provider), + }; const response = await fetchImpl(provider.modelsUrl, { headers, diff --git a/src/models-auth.mjs b/src/models-auth.mjs new file mode 100644 index 0000000..48764c2 --- /dev/null +++ b/src/models-auth.mjs @@ -0,0 +1,27 @@ +// Request headers for a provider's model-catalog endpoint. +// +// A provider credential is only attached when the catalog lives on the same +// origin as the provider API, or when `modelsAuth` opts in explicitly. A custom +// provider can ship any `modelsUrl`, so sending the key unconditionally would +// hand it to a third-party host. `headers` from the provider config are always +// preserved; authentication is scoped separately. +export function providerModelHeaders(provider) { + const headers = { ...(provider.modelsHeaders || {}) }; + if (provider.modelsAuth === false || provider.modelsAuth === 'none') return headers; + const authMode = provider.modelsAuth === true || provider.modelsAuth == null + ? 'bearer' + : provider.modelsAuth; + const token = authMode === 'bearer' + ? provider.bearerToken || provider.apiKey + : provider.apiKey || provider.bearerToken; + if (!token) return headers; + + const sameOrigin = new URL(provider.modelsUrl).origin === new URL(provider.baseUrl).origin; + const explicitAuth = provider.modelsAuth === true || typeof provider.modelsAuth === 'string'; + if (!sameOrigin && !explicitAuth) return headers; + + if (authMode === 'x-api-key') headers['x-api-key'] = token; + else if (authMode === 'api-key') headers['api-key'] = token; + else headers.Authorization = `Bearer ${token}`; + return headers; +} diff --git a/test/discovery.test.mjs b/test/discovery.test.mjs index 1158afa..5a31b88 100644 --- a/test/discovery.test.mjs +++ b/test/discovery.test.mjs @@ -134,7 +134,7 @@ test('the credential is sent to the catalog endpoint', async () => { let seen; await discoverModels(provider, { fetchImpl: async (url, options) => { - seen = { url, auth: options.headers.authorization }; + seen = { url, auth: options.headers.Authorization }; return respondWith({ data: [{ id: 'x' }] })(); }, }); @@ -289,3 +289,39 @@ test('a transient failure is retried sooner than a good result is refreshed', as await discoverModels(provider, { fetchImpl: failing, cache, now: () => 90_000 }); assert.equal(calls, 2, 'retried after the short failure window'); }); + +// The model catalog is fetched by discovery, not by the interactive path; both +// must apply the same origin scoping to provider credentials. +function captureRequests(payload, sink) { + return async (url, init = {}) => { + sink.push({ url, headers: init.headers || {} }); + return { + ok: true, + status: 200, + statusText: 'OK', + headers: { get: () => null }, + json: async () => payload, + }; + }; +} + +test('discovery does not send the provider key to a catalog on another origin', async () => { + const requests = []; + await discoverModels( + { ...provider, modelsUrl: 'https://catalog.other.test/models' }, + { fetchImpl: captureRequests({ data: [{ id: 'x' }] }, requests) } + ); + + const { headers } = requests[0]; + assert.equal(headers.Authorization, undefined); + assert.equal(headers.authorization, undefined); + assert.equal(headers['x-api-key'], undefined); + assert.equal(headers['api-key'], undefined); +}); + +test('discovery sends the key to a same-origin catalog', async () => { + const requests = []; + await discoverModels(provider, { fetchImpl: captureRequests({ data: [{ id: 'x' }] }, requests) }); + + assert.equal(requests[0].headers.Authorization, 'Bearer secret'); +}); \ No newline at end of file diff --git a/test/models-auth.test.mjs b/test/models-auth.test.mjs new file mode 100644 index 0000000..f55beb7 --- /dev/null +++ b/test/models-auth.test.mjs @@ -0,0 +1,77 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { providerModelHeaders } from '../src/models-auth.mjs'; + +const baseProvider = { + id: 'acme', + name: 'Acme', + baseUrl: 'https://api.acme.test/v1', + modelsUrl: 'https://api.acme.test/v1/models', + apiKey: 'secret', +}; + +test('a same-origin catalog receives the key by default', () => { + const headers = providerModelHeaders(baseProvider); + assert.equal(headers.Authorization, 'Bearer secret'); +}); + +test('a cross-origin catalog receives no key without an explicit modelsAuth', () => { + const headers = providerModelHeaders({ + ...baseProvider, + modelsUrl: 'https://catalog.other.test/models', + }); + assert.equal(headers.Authorization, undefined); + assert.equal(headers['x-api-key'], undefined); + assert.equal(headers['api-key'], undefined); +}); + +test('an explicit bearer modelsAuth allows a cross-origin catalog', () => { + const headers = providerModelHeaders({ + ...baseProvider, + modelsUrl: 'https://catalog.other.test/models', + modelsAuth: 'bearer', + }); + assert.equal(headers.Authorization, 'Bearer secret'); +}); + +test('modelsAuth false keeps even a same-origin catalog unauthenticated', () => { + const headers = providerModelHeaders({ ...baseProvider, modelsAuth: false }); + assert.equal(headers.Authorization, undefined); +}); + +test('modelsAuth none behaves like false', () => { + const headers = providerModelHeaders({ ...baseProvider, modelsAuth: 'none' }); + assert.equal(headers.Authorization, undefined); +}); + +test('modelsAuth x-api-key uses the dedicated header', () => { + const headers = providerModelHeaders({ ...baseProvider, modelsAuth: 'x-api-key' }); + assert.equal(headers['x-api-key'], 'secret'); + assert.equal(headers.Authorization, undefined); +}); + +test('modelsAuth api-key uses the dedicated header', () => { + const headers = providerModelHeaders({ ...baseProvider, modelsAuth: 'api-key' }); + assert.equal(headers['api-key'], 'secret'); + assert.equal(headers.Authorization, undefined); +}); + +test('provider modelsHeaders are preserved', () => { + const headers = providerModelHeaders({ + ...baseProvider, + modelsHeaders: { 'x-tenant': 'acme' }, + }); + assert.equal(headers['x-tenant'], 'acme'); + assert.equal(headers.Authorization, 'Bearer secret'); +}); + +test('a bearer token is preferred over the api key in bearer mode', () => { + const headers = providerModelHeaders({ ...baseProvider, bearerToken: 'token' }); + assert.equal(headers.Authorization, 'Bearer token'); +}); + +test('no credential means no auth header', () => { + const headers = providerModelHeaders({ ...baseProvider, apiKey: null }); + assert.equal(headers.Authorization, undefined); +});