From 8e9bd7b4ef1d592e98958924d4171fd783764561 Mon Sep 17 00:00:00 2001 From: Sudhir Verma <9924513+sudhirverma@users.noreply.github.com> Date: Fri, 17 Jul 2026 13:06:29 +0530 Subject: [PATCH 1/3] fix(daemon): stop leaking API keys via URL query params (H2/H3) Send Gemini keys via x-goog-api-key and accept discover-models keys only from X-Api-Key/body; reject ?apiKey= and add a CI regression check. --- .agents/skills/lean-ci/SKILL.md | 1 + .github/workflows/ci.yml | 3 + docs/ARCHITECTURE.md | 2 + docs/CONFIGURATION.md | 20 ++ docs/DEVELOPMENT.md | 1 + docs/TESTING.md | 2 + docs/decisions.md | 17 ++ package.json | 1 + packages/daemon/src/core/engines.test.ts | 47 +++- packages/daemon/src/core/engines.ts | 12 +- packages/daemon/src/daemon/web/server.ts | 62 ++++-- packages/daemon/static/index.html | 27 ++- .../integration/discover-models-auth.test.ts | 206 ++++++++++++++++++ scripts/check-no-query-secrets.js | 109 +++++++++ 14 files changed, 481 insertions(+), 29 deletions(-) create mode 100644 packages/daemon/tests/integration/discover-models-auth.test.ts create mode 100644 scripts/check-no-query-secrets.js diff --git a/.agents/skills/lean-ci/SKILL.md b/.agents/skills/lean-ci/SKILL.md index fd95f68..95849d8 100644 --- a/.agents/skills/lean-ci/SKILL.md +++ b/.agents/skills/lean-ci/SKILL.md @@ -36,6 +36,7 @@ locally-runnable scripts; CI just calls them. | npm script | What it does | When to use in CI | |------------|-------------|-------------------| | `npm run lint` | Lint all workspace packages | Quality gate | +| `npm run check:no-query-secrets` | Ban query-secret URL patterns in sources (DR-035) | Quality gate | | `npm test` | Test all workspace packages | Quality gate | | `npm run ci:build` | Full build with `--skip-proto` (SEA + VSIX + tar.gz) | Build artifacts | | `npm run ci:package-python` | Build Python wheel via `uvx hatch build` | Python artifact | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cb9c85d..9607180 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,9 @@ jobs: - name: Lint run: npm run lint + - name: Check no query-string secrets + run: npm run check:no-query-secrets + - name: Disable GPU acceleration run: | mkdir -p ~/.vscode diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 83ad9d6..f930d1d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -456,6 +456,8 @@ internal MCP tool for cross-session retrieval. 3. Frontend makes API calls to Express routes: - GET /api/providers → state.listProviders() - GET /api/models → state.listModels() + - GET|POST /api/discover-models/:engineId → state.discoverModels() + (provider API key via `X-Api-Key` / JSON body only — never `?apiKey=`; DR-035) - GET /api/config → loadConfig() - POST /api/config → saveConfig() - POST /api/secrets → state.secretStore.set() diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 03f60a1..afd1b83 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -102,6 +102,26 @@ Call APIs with: curl -H "Authorization: Bearer $ABBENAY_API_TOKEN" http://127.0.0.1:8787/api/health ``` +Provider API keys for model discovery must never appear in query strings +(DR-035). Use `X-Api-Key` or a JSON body (daemon Bearer auth stays on +`Authorization`): + +```bash +# Header +curl -H "Authorization: Bearer $ABBENAY_API_TOKEN" \ + -H "X-Api-Key: $GOOGLE_API_KEY" \ + http://127.0.0.1:8787/api/discover-models/gemini + +# Body +curl -X POST -H "Authorization: Bearer $ABBENAY_API_TOKEN" \ + -H "Content-Type: application/json" \ + -d "{\"apiKey\":\"$GOOGLE_API_KEY\"}" \ + http://127.0.0.1:8787/api/discover-models/gemini +``` + +`GET/POST ...?apiKey=` is rejected (HTTP 400). Outbound Gemini calls use the +`x-goog-api-key` header, not `?key=` in the URL. + The dashboard uses SameSite=Strict cookies plus a CSRF token for browser session auth. Open `http://127.0.0.1:8787/login` (or `POST /login` with the token in the body) to establish a session — prefer that over putting the diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index e1e864c..cb474a6 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -109,6 +109,7 @@ The build handles macOS-specific SEA requirements automatically: it passes `--ma | `build:dev` | `node build.js --skip-zip --code-install` | Build + install VSIX, no zip | | `build:proto` | `node build.js --proto-only` | Regenerate proto stubs only | | `lint` | `npm run lint --workspaces --if-present` | Lint all packages | +| `check:no-query-secrets` | `npm run check:no-query-secrets` | Ban query-string API key patterns in sources (DR-035) | | `test` | `npm run test --workspaces --if-present` | Test all packages | | `ci:build` | `node build.js --skip-proto` | Full build, skip proto (stubs committed) | | `ci:package-python` | `cd packages/python && uvx hatch build` | Build Python wheel | diff --git a/docs/TESTING.md b/docs/TESTING.md index 8f89fd6..1b3e8a9 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -90,6 +90,8 @@ Tests that start real servers, make real HTTP/gRPC calls. | `tests/integration/openai-compat.test.ts` | OpenAI-compatible API: /v1/models, streaming, non-streaming, errors, tool calls | | `tests/integration/sessions.test.ts` | Session REST API: CRUD endpoints, session chat SSE streaming + persistence | | `tests/integration/mcp-http-policy.test.ts` | `/mcp` auth + connection consent + tool_policy E2E | +| `tests/integration/http-security.test.ts` | HTTP auth, CORS, bind defaults, dashboard login | +| `tests/integration/discover-models-auth.test.ts` | discover-models: reject `?apiKey=`; accept `X-Api-Key` / body | ### Mock Engine diff --git a/docs/decisions.md b/docs/decisions.md index 13ce895..6f335f4 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -539,3 +539,20 @@ consent closes the remaining C3 recommendation and stops token-bearing callers from silently opening an MCP session. API-token holders can both call `/mcp` and approve via `/api/mcp/connections`, so consent is interactive friction, not a second principal against a stolen/shared token. + +--- + +## DR-035: No API keys in URL query strings + +**Date:** 2026-07-17 +**Decision:** Never place provider API keys in request URLs. Gemini outbound +calls use the `x-goog-api-key` header (same as `@ai-sdk/google`). The +`/api/discover-models/:engineId` endpoint accepts provider keys only via the +`X-Api-Key` header or JSON body (`POST`); `?apiKey=` is rejected with HTTP 400. +Non-secret params (`baseUrl`, `providerId`) may remain on the query string for +GET. CI runs `npm run check:no-query-secrets` to block regressions of common +query-secret patterns (`?key=`, `query.apiKey` / `query['apiKey']`, +`params.set/append('apiKey'`, `searchParams.set('key'`) in production sources. +**Rationale:** Query-string secrets leak into access logs, reverse proxies, +browser history, and `Referer` headers. Header/body transport matches other +engines (Bearer / `x-api-key`) and closes findings H2/H3. diff --git a/package.json b/package.json index 2b7a6a3..9dee422 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "lint": "npm run lint --workspaces --if-present", "test": "npm run test --workspaces --if-present", "audit:check": "node scripts/audit-check.js", + "check:no-query-secrets": "node scripts/check-no-query-secrets.js", "ci:build": "node build.js --skip-proto", "ci:package-python": "cd packages/python && uvx hatch build", "ci:publish-vscode": "node scripts/publish-vscode.js" diff --git a/packages/daemon/src/core/engines.test.ts b/packages/daemon/src/core/engines.test.ts index 560f75b..5384bde 100644 --- a/packages/daemon/src/core/engines.test.ts +++ b/packages/daemon/src/core/engines.test.ts @@ -9,10 +9,10 @@ * JSON→SSE conversion). */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import { sanitizeVertexRequestBody, convertAnthropicJsonToSse } from './engines.js'; +import { sanitizeVertexRequestBody, convertAnthropicJsonToSse, fetchModels } from './engines.js'; const ENGINES_SRC = fs.readFileSync( path.join(__dirname, 'engines.ts'), @@ -221,3 +221,46 @@ describe('convertAnthropicJsonToSse', () => { expect(deltaData.delta.stop_reason).toBe('end_turn'); }); }); + +describe('Gemini model discovery auth (no query-string keys)', () => { + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('should send API key via x-goog-api-key and never put key material in the URL', async () => { + const apiKey = 'test-gemini-secret-key-xyz'; + let capturedUrl = ''; + let capturedHeaders: HeadersInit | undefined; + + vi.stubGlobal('fetch', vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + capturedUrl = String(input); + capturedHeaders = init?.headers; + return new Response(JSON.stringify({ + models: [{ + name: 'models/gemini-2.0-flash', + supportedGenerationMethods: ['generateContent'], + inputTokenLimit: 1048576, + }], + }), { status: 200, headers: { 'Content-Type': 'application/json' } }); + })); + + const models = await fetchModels('gemini', apiKey); + expect(models).toHaveLength(1); + expect(models[0].id).toBe('gemini-2.0-flash'); + + expect(capturedUrl).toContain('/v1beta/models'); + expect(capturedUrl).not.toContain('key='); + expect(capturedUrl).not.toContain(apiKey); + + const headers = new Headers(capturedHeaders); + expect(headers.get('x-goog-api-key')).toBe(apiKey); + }); + + it('should not construct Gemini URLs with query-string API keys', () => { + // Ban executable patterns; comments documenting the ban may mention key=. + expect(ENGINES_SRC).not.toMatch(/`\$\{[^`]*\}\?key=\$\{/); + expect(ENGINES_SRC).not.toMatch(/['"`]\?key=\$\{/); + expect(ENGINES_SRC).toMatch(/x-goog-api-key/); + }); +}); diff --git a/packages/daemon/src/core/engines.ts b/packages/daemon/src/core/engines.ts index 8595569..f460b1f 100644 --- a/packages/daemon/src/core/engines.ts +++ b/packages/daemon/src/core/engines.ts @@ -805,9 +805,17 @@ async function fetchGeminiModels( baseUrl?: string, ): Promise { const base = baseUrl || 'https://generativelanguage.googleapis.com'; - const url = `${base}/v1beta/models${apiKey ? `?key=${apiKey}` : ''}`; + // Never put API keys in the URL (?key= leaks via logs, proxies, history, Referer). + // Match @ai-sdk/google: send the key via x-goog-api-key header. + const url = `${base}/v1beta/models`; + const headers: Record = { + Accept: 'application/json', + }; + if (apiKey) { + headers['x-goog-api-key'] = apiKey; + } - const resp = await fetch(url, { signal: AbortSignal.timeout(15000) }); + const resp = await fetch(url, { headers, signal: AbortSignal.timeout(15000) }); if (!resp.ok) { throw new Error(`Gemini HTTP ${resp.status}`); } diff --git a/packages/daemon/src/daemon/web/server.ts b/packages/daemon/src/daemon/web/server.ts index 981bdea..46a7a12 100644 --- a/packages/daemon/src/daemon/web/server.ts +++ b/packages/daemon/src/daemon/web/server.ts @@ -523,27 +523,58 @@ export function createWebApp(state: DaemonState, options?: WebSecurityOptions): }); /** - * GET /api/discover-models/:providerId - Discover all models a provider offers - * Ignores config — for browsing/selection UI. Does NOT trigger notifications. - */ - /** - * GET /api/discover-models/:engineId - Discover all models an engine offers - * Operates on the actual layer (engine, not virtual provider). - * Query params: ?apiKey=xxx&baseUrl=xxx (optional, for authenticated discovery) + * Discover models for an engine (actual layer). Does NOT trigger notifications. + * + * API keys MUST NOT be passed as query params (leaks via logs, proxies, history, + * Referer). Accept provider keys only via: + * - Header: X-Api-Key + * - JSON body: { apiKey } (POST) + * Non-secret params (baseUrl, providerId) may be query (GET) or body (POST). + * Query ?apiKey= is rejected with 400. */ - app.get('/api/discover-models/:engineId', async (req, res) => { + const handleDiscoverModels = async (req: Request, res: Response): Promise => { try { - let apiKey = req.query.apiKey as string | undefined; - let baseUrl = req.query.baseUrl as string | undefined; - const providerId = req.query.providerId as string | undefined; - + // Reject query-string secrets without naming req.query.apiKey (CI ban pattern). + if (Object.prototype.hasOwnProperty.call(req.query, 'apiKey')) { + res.status(400).json({ + error: + 'apiKey must not be passed as a query parameter. ' + + 'Send it via the X-Api-Key header or JSON body (POST).', + }); + return; + } + + const body = (req.body && typeof req.body === 'object' ? req.body : {}) as { + apiKey?: unknown; + baseUrl?: unknown; + providerId?: unknown; + }; + + const headerKey = req.headers['x-api-key']; + const headerKeyStr = Array.isArray(headerKey) ? headerKey[0] : headerKey; + let apiKey: string | undefined; + if (typeof headerKeyStr === 'string' && headerKeyStr.length > 0) { + apiKey = headerKeyStr; + } else if (typeof body.apiKey === 'string' && body.apiKey.length > 0) { + apiKey = body.apiKey; + } + + let baseUrl = + (typeof body.baseUrl === 'string' && body.baseUrl) || + (typeof req.query.baseUrl === 'string' ? req.query.baseUrl : undefined) || + undefined; + const providerId = + (typeof body.providerId === 'string' && body.providerId) || + (typeof req.query.providerId === 'string' ? req.query.providerId : undefined) || + undefined; + // If providerId is given (edit mode), resolve API key and base URL from config if (providerId && !apiKey) { const resolved = await state.resolveProviderCredentials(providerId); if (resolved.apiKey) apiKey = resolved.apiKey; if (resolved.baseUrl && !baseUrl) baseUrl = resolved.baseUrl; } - + const models = await state.discoverModels(req.params.engineId, apiKey, baseUrl); res.json({ models: models.map((m) => ({ @@ -561,7 +592,10 @@ export function createWebApp(state: DaemonState, options?: WebSecurityOptions): console.error('[Web] /api/discover-models error:', msg); res.status(500).json({ error: msg }); } - }); + }; + + app.get('/api/discover-models/:engineId', handleDiscoverModels); + app.post('/api/discover-models/:engineId', handleDiscoverModels); /** * GET /api/models - List models from configured providers diff --git a/packages/daemon/static/index.html b/packages/daemon/static/index.html index 8068047..8acc299 100644 --- a/packages/daemon/static/index.html +++ b/packages/daemon/static/index.html @@ -1723,10 +1723,11 @@

Save config to

this.wizardFromModelsTab = true; // Discover models so the dual-list has data if user clicks "Back to models" try { - const params = new URLSearchParams(); - params.set('providerId', model.provider); - const url = `/api/discover-models/${encodeURIComponent(model.engine)}?${params}`; - const resp = await apiFetch(url); + const resp = await apiFetch(`/api/discover-models/${encodeURIComponent(model.engine)}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ providerId: model.provider }), + }); const data = await resp.json(); this.wizardDiscoveredModels = data.models || []; } catch (e) { console.error('Discovery failed:', e); } @@ -1746,17 +1747,21 @@

Save config to

this.wizardStep = 2; } else if (this.wizardStep === 2) { if (!this.wizardName) return; - // Discover models for Step 3 + // Discover models for Step 3 — API keys go in header/body only (never query) try { - const params = new URLSearchParams(); - if (this.wizardApiKey) params.set('apiKey', this.wizardApiKey); - if (this.wizardBaseUrl) params.set('baseUrl', this.wizardBaseUrl); + const body = {}; + if (this.wizardBaseUrl) body.baseUrl = this.wizardBaseUrl; // In edit mode without a new key, tell backend to resolve from config if (this.wizardEditMode && !this.wizardApiKey) { - params.set('providerId', this.wizardEditId); + body.providerId = this.wizardEditId; } - const url = `/api/discover-models/${encodeURIComponent(this.wizardEngine)}?${params}`; - const resp = await apiFetch(url); + const headers = { 'Content-Type': 'application/json' }; + if (this.wizardApiKey) headers['X-Api-Key'] = this.wizardApiKey; + const resp = await apiFetch(`/api/discover-models/${encodeURIComponent(this.wizardEngine)}`, { + method: 'POST', + headers, + body: JSON.stringify(body), + }); const data = await resp.json(); this.wizardDiscoveredModels = data.models || []; } catch (e) { console.error('Discovery failed:', e); } diff --git a/packages/daemon/tests/integration/discover-models-auth.test.ts b/packages/daemon/tests/integration/discover-models-auth.test.ts new file mode 100644 index 0000000..b795b26 --- /dev/null +++ b/packages/daemon/tests/integration/discover-models-auth.test.ts @@ -0,0 +1,206 @@ +/** + * Integration tests: discover-models API key transport + * + * - Rejects ?apiKey= query param (400) + * - Accepts X-Api-Key header + * - Accepts JSON body apiKey (POST) + * - Does not use query apiKey when only that is supplied + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as http from 'node:http'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { createWebApp } from '../../src/daemon/web/server.js'; +import type { DaemonState } from '../../src/daemon/state.js'; +import type { SecretStore } from '../../src/core/secrets.js'; +import { SessionStore } from '../../src/core/session-store.js'; + +const TEST_TOKEN = 'test-discover-models-auth-token'; + +const mockSecretStore: SecretStore = { + async get() { return null; }, + async set() {}, + async delete() { return true; }, + async has() { return false; }, +}; + +let sessionsDir: string; +let httpServer: http.Server; +let baseUrl: string; +let lastDiscoverArgs: { engineId: string; apiKey?: string; baseUrl?: string } | null = null; + +function createMockState(): DaemonState { + return { + version: '0.1.0-test', + startedAt: new Date(), + secretStore: mockSecretStore, + sessionStore: new SessionStore(sessionsDir), + async listProviders() { return []; }, + async listModels() { return []; }, + async discoverModels(engineId: string, apiKey?: string, baseUrl?: string) { + lastDiscoverArgs = { engineId, apiKey, baseUrl }; + return [{ + id: 'gemini-2.0-flash', + engine: engineId, + contextWindow: 1048576, + capabilities: { supportsTools: true, supportsVision: true }, + }]; + }, + async resolveProviderCredentials() { return {}; }, + async chat() { return (async function* () { yield { type: 'done' as const, finishReason: 'stop' }; })(); }, + getStatus() { + return { version: '0.1.0-test', uptime: 0, providers: 0, models: 0 }; + }, + getConnectedClients() { return []; }, + mcpClientPool: { + getAllStatus() { return []; }, + async reconnect() {}, + }, + toolRegistry: { + listTools() { return []; }, + }, + mcpServer: { + isRunning() { return false; }, + async start() {}, + async stop() {}, + }, + } as unknown as DaemonState; +} + +function httpRequest( + method: string, + urlPath: string, + opts?: { + body?: unknown; + headers?: Record; + }, +): Promise<{ statusCode: number; body: any }> { + return new Promise((resolve, reject) => { + const urlObj = new URL(urlPath, baseUrl); + const headers: Record = { + Connection: 'close', + Authorization: `Bearer ${TEST_TOKEN}`, + ...(opts?.headers || {}), + }; + + let postData: string | undefined; + if (opts?.body !== undefined) { + postData = JSON.stringify(opts.body); + headers['Content-Type'] = 'application/json'; + headers['Content-Length'] = String(Buffer.byteLength(postData)); + } + + const req = http.request( + { + hostname: urlObj.hostname, + port: urlObj.port, + path: urlObj.pathname + urlObj.search, + method, + headers, + }, + (res) => { + let data = ''; + res.setEncoding('utf-8'); + res.on('data', (c) => { data += c; }); + res.on('end', () => { + let body: any = data; + try { body = JSON.parse(data); } catch { /* raw */ } + resolve({ statusCode: res.statusCode || 0, body }); + }); + }, + ); + req.on('error', reject); + if (postData) req.write(postData); + req.end(); + }); +} + +beforeAll(async () => { + sessionsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'abbenay-discover-auth-')); + const state = createMockState(); + const app = createWebApp(state, { + apiToken: TEST_TOKEN, + skipConfig: true, + host: '127.0.0.1', + }); + + await new Promise((resolve) => { + httpServer = app.listen(0, '127.0.0.1', () => { + const addr = httpServer.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + baseUrl = `http://127.0.0.1:${port}`; + resolve(); + }); + }); +}); + +afterAll(async () => { + if (httpServer) { + await new Promise((resolve) => httpServer.close(() => resolve())); + } + fs.rmSync(sessionsDir, { recursive: true, force: true }); +}); + +describe('discover-models API key transport', () => { + it('rejects apiKey query parameter with 400', async () => { + lastDiscoverArgs = null; + const res = await httpRequest( + 'GET', + '/api/discover-models/gemini?apiKey=should-not-be-used', + ); + expect(res.statusCode).toBe(400); + expect(res.body.error).toMatch(/must not be passed as a query parameter/i); + expect(lastDiscoverArgs).toBeNull(); + }); + + it('accepts API key via X-Api-Key header on GET', async () => { + lastDiscoverArgs = null; + const res = await httpRequest('GET', '/api/discover-models/gemini', { + headers: { 'X-Api-Key': 'header-secret-key' }, + }); + expect(res.statusCode).toBe(200); + expect(res.body.models).toHaveLength(1); + expect(lastDiscoverArgs).toEqual({ + engineId: 'gemini', + apiKey: 'header-secret-key', + baseUrl: undefined, + }); + }); + + it('accepts API key via JSON body on POST', async () => { + lastDiscoverArgs = null; + const res = await httpRequest('POST', '/api/discover-models/gemini', { + body: { apiKey: 'body-secret-key', baseUrl: 'https://example.invalid' }, + }); + expect(res.statusCode).toBe(200); + expect(res.body.models).toHaveLength(1); + expect(lastDiscoverArgs).toEqual({ + engineId: 'gemini', + apiKey: 'body-secret-key', + baseUrl: 'https://example.invalid', + }); + }); + + it('prefers X-Api-Key header over body apiKey', async () => { + lastDiscoverArgs = null; + const res = await httpRequest('POST', '/api/discover-models/gemini', { + headers: { 'X-Api-Key': 'from-header' }, + body: { apiKey: 'from-body' }, + }); + expect(res.statusCode).toBe(200); + expect(lastDiscoverArgs?.apiKey).toBe('from-header'); + }); + + it('does not accept key material from query when only ?apiKey= is supplied', async () => { + lastDiscoverArgs = null; + const res = await httpRequest( + 'POST', + '/api/discover-models/gemini?apiKey=query-only-secret', + { body: {} }, + ); + expect(res.statusCode).toBe(400); + expect(lastDiscoverArgs).toBeNull(); + }); +}); diff --git a/scripts/check-no-query-secrets.js b/scripts/check-no-query-secrets.js new file mode 100644 index 0000000..311cfb1 --- /dev/null +++ b/scripts/check-no-query-secrets.js @@ -0,0 +1,109 @@ +#!/usr/bin/env node + +/** + * Regression check: API keys must not be passed via URL query strings (DR-035). + * + * Scans production source for patterns that historically leaked secrets: + * - ?key= / `?key=${...}` (Gemini-style query API keys) + * - query.apiKey / query['apiKey'] (Express discover-models query param) + * - params.set/append('apiKey' (dashboard URLSearchParams) + * - searchParams.set('key' (URLSearchParams Gemini-style) + * + * Usage: node scripts/check-no-query-secrets.js + * npm run check:no-query-secrets + */ + +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, relative, extname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = join(fileURLToPath(new URL('.', import.meta.url)), '..'); + +const SCAN_ROOTS = [ + 'packages/daemon/src', + 'packages/daemon/static', + 'packages/vscode/src', +]; + +const EXTENSIONS = new Set(['.ts', '.tsx', '.js', '.mjs', '.cjs', '.html']); + +/** Paths (relative to repo root) that may mention these patterns for documentation/tests of the ban. */ +const ALLOWLIST = new Set([ + 'scripts/check-no-query-secrets.js', + 'packages/daemon/src/core/engines.test.ts', + 'packages/daemon/tests/integration/discover-models-auth.test.ts', +]); + +const PATTERNS = [ + { name: '?key= query API key', re: /\?key=/ }, + { name: 'req.query.apiKey', re: /query\.apiKey\b/ }, + { name: "query['apiKey']", re: /query\[\s*['"]apiKey['"]\s*\]/ }, + { name: "URLSearchParams set apiKey", re: /params\.set\(\s*['"]apiKey['"]/ }, + { name: "URLSearchParams append apiKey", re: /params\.append\(\s*['"]apiKey['"]/ }, + { name: "searchParams.set('key'", re: /searchParams\.set\(\s*['"]key['"]/ }, +]; + +function* walk(dir) { + let entries; + try { + entries = readdirSync(dir); + } catch { + return; + } + for (const name of entries) { + if (name === 'node_modules' || name === 'dist' || name === 'out') continue; + const full = join(dir, name); + const st = statSync(full); + if (st.isDirectory()) { + // Skip test trees under vscode (they may assert against the ban) + if (name === 'test' || name === 'tests' || name === '__tests__') continue; + yield* walk(full); + } else if (EXTENSIONS.has(extname(name))) { + yield full; + } + } +} + +const violations = []; + +for (const scanRoot of SCAN_ROOTS) { + const absRoot = join(root, scanRoot); + for (const file of walk(absRoot)) { + const rel = relative(root, file).split('\\').join('/'); + if (ALLOWLIST.has(rel)) continue; + // Unit/integration tests under daemon are outside SCAN_ROOTS except src/; + // still skip any *.test.* under src if present. + if (/\.test\.(ts|tsx|js)$/.test(rel)) continue; + + const content = readFileSync(file, 'utf8'); + for (const { name, re } of PATTERNS) { + if (re.test(content)) { + // Allow explanatory comments that document the ban (must include "must not" / "Never") + const lines = content.split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (!re.test(line)) continue; + const trimmed = line.trim(); + if ( + trimmed.startsWith('//') || + trimmed.startsWith('*') || + trimmed.startsWith('/*') + ) { + // Comment-only mention of the anti-pattern is OK + continue; + } + violations.push(`${rel}:${i + 1}: ${name}: ${trimmed.slice(0, 120)}`); + } + } + } + } +} + +if (violations.length > 0) { + console.error('Query-string secret patterns found (use header/body instead):\n'); + for (const v of violations) console.error(` ${v}`); + console.error(`\n${violations.length} violation(s). See docs/decisions.md DR-035.`); + process.exit(1); +} + +console.log('check-no-query-secrets: OK (no query-secret patterns in scanned sources)'); From 730f2449b39983ea66101dd201617a4e4d08eac5 Mon Sep 17 00:00:00 2001 From: Sudhir Verma <9924513+sudhirverma@users.noreply.github.com> Date: Fri, 17 Jul 2026 13:30:20 +0530 Subject: [PATCH 2/3] fix(daemon): remove node scripts/check-no-query-secrets.js" --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 9dee422..2b7a6a3 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,6 @@ "lint": "npm run lint --workspaces --if-present", "test": "npm run test --workspaces --if-present", "audit:check": "node scripts/audit-check.js", - "check:no-query-secrets": "node scripts/check-no-query-secrets.js", "ci:build": "node build.js --skip-proto", "ci:package-python": "cd packages/python && uvx hatch build", "ci:publish-vscode": "node scripts/publish-vscode.js" From 3e62bda686766edaf31d531a025b1a2c5dfc380a Mon Sep 17 00:00:00 2001 From: Sudhir Verma <9924513+sudhirverma@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:27:48 +0530 Subject: [PATCH 3/3] fix(ci): restore check:no-query-secrets npm script CI still invokes the gate; removing only the package.json entry broke lint-and-test. --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 2b7a6a3..9dee422 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "lint": "npm run lint --workspaces --if-present", "test": "npm run test --workspaces --if-present", "audit:check": "node scripts/audit-check.js", + "check:no-query-secrets": "node scripts/check-no-query-secrets.js", "ci:build": "node build.js --skip-proto", "ci:package-python": "cd packages/python && uvx hatch build", "ci:publish-vscode": "node scripts/publish-vscode.js"