diff --git a/containers/api-proxy/server.js b/containers/api-proxy/server.js index 010ba5f1..829b632d 100644 --- a/containers/api-proxy/server.js +++ b/containers/api-proxy/server.js @@ -149,24 +149,44 @@ function normalizeBasePath(rawPath) { /** * Build the full upstream path by joining basePath, reqUrl's pathname, and query string. + * Applies provider-safe defaults and avoids duplicate prefixing when the incoming + * path already includes the configured base path. * * Examples: - * buildUpstreamPath('/v1/chat/completions', 'api.openai.com', '') - * → '/v1/chat/completions' + * buildUpstreamPath('/responses', 'api.openai.com', '') + * → '/v1/responses' * buildUpstreamPath('/v1/chat/completions', 'host.databricks.com', '/serving-endpoints') * → '/serving-endpoints/v1/chat/completions' * buildUpstreamPath('/v1/messages?stream=true', 'host.com', '/anthropic') * → '/anthropic/v1/messages?stream=true' * - * @param {string} reqUrl - The incoming request URL (must start with '/') + * @param {string} reqUrl - The incoming request URL (must start with '/' and not '//') * @param {string} targetHost - The upstream hostname (used only to parse the URL) * @param {string} basePath - Normalized base path prefix (e.g. '/serving-endpoints' or '') * @returns {string} Full upstream path including query string */ function buildUpstreamPath(reqUrl, targetHost, basePath) { + if (typeof reqUrl !== 'string' || !reqUrl.startsWith('/') || reqUrl.startsWith('//')) { + throw new Error('URL must be a relative origin-form path'); + } + const targetUrl = new URL(reqUrl, `https://${targetHost}`); - const prefix = basePath === '/' ? '' : basePath; - return prefix + targetUrl.pathname + targetUrl.search; + const pathname = targetUrl.pathname; + let prefix = basePath === '/' ? '' : basePath; + + // OpenAI's canonical API paths are versioned under /v1, while some newer + // clients (for example Codex CLI with OPENAI_BASE_URL pointing at the sidecar) + // send unversioned paths like /responses. Add /v1 only for the default + // OpenAI host when no explicit base path is configured. + if (!prefix && targetUrl.hostname === 'api.openai.com') { + prefix = '/v1'; + } + + if (prefix && (pathname === prefix || pathname.startsWith(`${prefix}/`))) { + return pathname + targetUrl.search; + } + + return prefix + pathname + targetUrl.search; } /** @@ -418,7 +438,7 @@ function proxyRequest(req, res, targetHost, injectHeaders, provider, basePath = }); // Validate that req.url is a relative path (prevent open-redirect / SSRF) - if (!req.url || !req.url.startsWith('/')) { + if (!req.url || !req.url.startsWith('/') || req.url.startsWith('//')) { const duration = Date.now() - startTime; metrics.gaugeDec('active_requests', { provider }); metrics.increment('requests_total', { provider, method: req.method, status_class: '4xx' }); @@ -687,7 +707,7 @@ function proxyWebSocket(req, socket, head, targetHost, injectHeaders, provider, } // ── Validate: relative path only (prevent SSRF) ──────────────────────── - if (!req.url || !req.url.startsWith('/')) { + if (!req.url || !req.url.startsWith('/') || req.url.startsWith('//')) { logRequest('warn', 'websocket_upgrade_rejected', { request_id: requestId, provider, diff --git a/containers/api-proxy/server.test.js b/containers/api-proxy/server.test.js index a83617a4..60b08865 100644 --- a/containers/api-proxy/server.test.js +++ b/containers/api-proxy/server.test.js @@ -347,6 +347,11 @@ describe('buildUpstreamPath', () => { it('should handle root path with no base path', () => { expect(buildUpstreamPath('/', HOST, '')).toBe('/'); }); + + it('should reject protocol-relative URLs to prevent host override', () => { + expect(() => buildUpstreamPath('//evil.com/v1/chat/completions', HOST, '')) + .toThrow('URL must be a relative origin-form path'); + }); }); describe('Databricks serving-endpoints (single-segment base path)', () => { @@ -406,6 +411,21 @@ describe('buildUpstreamPath', () => { .toBe('/v1/chat/completions'); }); + it('should map unversioned /responses to /v1/responses for api.openai.com', () => { + expect(buildUpstreamPath('/responses', 'api.openai.com', '')) + .toBe('/v1/responses'); + }); + + it('should preserve already-versioned OpenAI responses path', () => { + expect(buildUpstreamPath('/v1/responses', 'api.openai.com', '')) + .toBe('/v1/responses'); + }); + + it('should map unversioned /responses to /v1/responses when OpenAI host includes port', () => { + expect(buildUpstreamPath('/responses', 'api.openai.com:443', '')) + .toBe('/v1/responses'); + }); + it('should preserve /v1/messages exactly (Anthropic standard path)', () => { expect(buildUpstreamPath('/v1/messages', 'api.anthropic.com', '')) .toBe('/v1/messages'); @@ -438,6 +458,12 @@ describe('buildUpstreamPath', () => { .toBe('/v1/messages'); }); + it('should not force /v1 for non-OpenAI custom targets', () => { + const target = 'my-gateway.example.com'; + expect(buildUpstreamPath('/responses', target, '')) + .toBe('/responses'); + }); + it('should produce wrong hostname if scheme is NOT stripped (demonstrating the bug)', () => { // Without normalizeApiTarget, the scheme-prefixed value causes // new URL() to parse 'https' as the hostname instead of the real host @@ -575,6 +601,13 @@ describe('proxyWebSocket', () => { expect(socket.destroy).toHaveBeenCalled(); }); + it('rejects a protocol-relative URL with 400 (SSRF prevention)', () => { + const socket = makeMockSocket(); + proxyWebSocket(makeUpgradeReq({ url: '//evil.com/v1/responses' }), socket, Buffer.alloc(0), 'api.openai.com', {}, 'openai'); + expect(socket.write).toHaveBeenCalledWith(expect.stringContaining('HTTP/1.1 400 Bad Request')); + expect(socket.destroy).toHaveBeenCalled(); + }); + it('rejects a null URL with 400', () => { const socket = makeMockSocket(); proxyWebSocket(makeUpgradeReq({ url: null }), socket, Buffer.alloc(0), 'api.openai.com', {}, 'openai'); @@ -946,4 +979,3 @@ describe('resolveOpenCodeRoute', () => { expect(route.headers['x-api-key']).toBeUndefined(); }); }); -