Skip to content

Commit 6af8b15

Browse files
authored
Handle unversioned OpenAI /responses paths in API proxy sidecar (#2080)
* Initial plan * fix(api-proxy): route unversioned openai responses via /v1 Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/10848884-d3b5-42d4-8299-a1a5c2cc46d8 * fix(api-proxy): normalize openai host before default /v1 mapping Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/10848884-d3b5-42d4-8299-a1a5c2cc46d8 * fix(api-proxy): reject protocol-relative request urls Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/eb1cf84f-e932-421b-89f7-5950554b86cc --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 4686dbe commit 6af8b15

File tree

2 files changed

+60
-8
lines changed

2 files changed

+60
-8
lines changed

containers/api-proxy/server.js

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,24 +149,44 @@ function normalizeBasePath(rawPath) {
149149

150150
/**
151151
* Build the full upstream path by joining basePath, reqUrl's pathname, and query string.
152+
* Applies provider-safe defaults and avoids duplicate prefixing when the incoming
153+
* path already includes the configured base path.
152154
*
153155
* Examples:
154-
* buildUpstreamPath('/v1/chat/completions', 'api.openai.com', '')
155-
* → '/v1/chat/completions'
156+
* buildUpstreamPath('/responses', 'api.openai.com', '')
157+
* → '/v1/responses'
156158
* buildUpstreamPath('/v1/chat/completions', 'host.databricks.com', '/serving-endpoints')
157159
* → '/serving-endpoints/v1/chat/completions'
158160
* buildUpstreamPath('/v1/messages?stream=true', 'host.com', '/anthropic')
159161
* → '/anthropic/v1/messages?stream=true'
160162
*
161-
* @param {string} reqUrl - The incoming request URL (must start with '/')
163+
* @param {string} reqUrl - The incoming request URL (must start with '/' and not '//')
162164
* @param {string} targetHost - The upstream hostname (used only to parse the URL)
163165
* @param {string} basePath - Normalized base path prefix (e.g. '/serving-endpoints' or '')
164166
* @returns {string} Full upstream path including query string
165167
*/
166168
function buildUpstreamPath(reqUrl, targetHost, basePath) {
169+
if (typeof reqUrl !== 'string' || !reqUrl.startsWith('/') || reqUrl.startsWith('//')) {
170+
throw new Error('URL must be a relative origin-form path');
171+
}
172+
167173
const targetUrl = new URL(reqUrl, `https://${targetHost}`);
168-
const prefix = basePath === '/' ? '' : basePath;
169-
return prefix + targetUrl.pathname + targetUrl.search;
174+
const pathname = targetUrl.pathname;
175+
let prefix = basePath === '/' ? '' : basePath;
176+
177+
// OpenAI's canonical API paths are versioned under /v1, while some newer
178+
// clients (for example Codex CLI with OPENAI_BASE_URL pointing at the sidecar)
179+
// send unversioned paths like /responses. Add /v1 only for the default
180+
// OpenAI host when no explicit base path is configured.
181+
if (!prefix && targetUrl.hostname === 'api.openai.com') {
182+
prefix = '/v1';
183+
}
184+
185+
if (prefix && (pathname === prefix || pathname.startsWith(`${prefix}/`))) {
186+
return pathname + targetUrl.search;
187+
}
188+
189+
return prefix + pathname + targetUrl.search;
170190
}
171191

172192
/**
@@ -418,7 +438,7 @@ function proxyRequest(req, res, targetHost, injectHeaders, provider, basePath =
418438
});
419439

420440
// Validate that req.url is a relative path (prevent open-redirect / SSRF)
421-
if (!req.url || !req.url.startsWith('/')) {
441+
if (!req.url || !req.url.startsWith('/') || req.url.startsWith('//')) {
422442
const duration = Date.now() - startTime;
423443
metrics.gaugeDec('active_requests', { provider });
424444
metrics.increment('requests_total', { provider, method: req.method, status_class: '4xx' });
@@ -687,7 +707,7 @@ function proxyWebSocket(req, socket, head, targetHost, injectHeaders, provider,
687707
}
688708

689709
// ── Validate: relative path only (prevent SSRF) ────────────────────────
690-
if (!req.url || !req.url.startsWith('/')) {
710+
if (!req.url || !req.url.startsWith('/') || req.url.startsWith('//')) {
691711
logRequest('warn', 'websocket_upgrade_rejected', {
692712
request_id: requestId,
693713
provider,

containers/api-proxy/server.test.js

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,11 @@ describe('buildUpstreamPath', () => {
347347
it('should handle root path with no base path', () => {
348348
expect(buildUpstreamPath('/', HOST, '')).toBe('/');
349349
});
350+
351+
it('should reject protocol-relative URLs to prevent host override', () => {
352+
expect(() => buildUpstreamPath('//evil.com/v1/chat/completions', HOST, ''))
353+
.toThrow('URL must be a relative origin-form path');
354+
});
350355
});
351356

352357
describe('Databricks serving-endpoints (single-segment base path)', () => {
@@ -406,6 +411,21 @@ describe('buildUpstreamPath', () => {
406411
.toBe('/v1/chat/completions');
407412
});
408413

414+
it('should map unversioned /responses to /v1/responses for api.openai.com', () => {
415+
expect(buildUpstreamPath('/responses', 'api.openai.com', ''))
416+
.toBe('/v1/responses');
417+
});
418+
419+
it('should preserve already-versioned OpenAI responses path', () => {
420+
expect(buildUpstreamPath('/v1/responses', 'api.openai.com', ''))
421+
.toBe('/v1/responses');
422+
});
423+
424+
it('should map unversioned /responses to /v1/responses when OpenAI host includes port', () => {
425+
expect(buildUpstreamPath('/responses', 'api.openai.com:443', ''))
426+
.toBe('/v1/responses');
427+
});
428+
409429
it('should preserve /v1/messages exactly (Anthropic standard path)', () => {
410430
expect(buildUpstreamPath('/v1/messages', 'api.anthropic.com', ''))
411431
.toBe('/v1/messages');
@@ -438,6 +458,12 @@ describe('buildUpstreamPath', () => {
438458
.toBe('/v1/messages');
439459
});
440460

461+
it('should not force /v1 for non-OpenAI custom targets', () => {
462+
const target = 'my-gateway.example.com';
463+
expect(buildUpstreamPath('/responses', target, ''))
464+
.toBe('/responses');
465+
});
466+
441467
it('should produce wrong hostname if scheme is NOT stripped (demonstrating the bug)', () => {
442468
// Without normalizeApiTarget, the scheme-prefixed value causes
443469
// new URL() to parse 'https' as the hostname instead of the real host
@@ -575,6 +601,13 @@ describe('proxyWebSocket', () => {
575601
expect(socket.destroy).toHaveBeenCalled();
576602
});
577603

604+
it('rejects a protocol-relative URL with 400 (SSRF prevention)', () => {
605+
const socket = makeMockSocket();
606+
proxyWebSocket(makeUpgradeReq({ url: '//evil.com/v1/responses' }), socket, Buffer.alloc(0), 'api.openai.com', {}, 'openai');
607+
expect(socket.write).toHaveBeenCalledWith(expect.stringContaining('HTTP/1.1 400 Bad Request'));
608+
expect(socket.destroy).toHaveBeenCalled();
609+
});
610+
578611
it('rejects a null URL with 400', () => {
579612
const socket = makeMockSocket();
580613
proxyWebSocket(makeUpgradeReq({ url: null }), socket, Buffer.alloc(0), 'api.openai.com', {}, 'openai');
@@ -946,4 +979,3 @@ describe('resolveOpenCodeRoute', () => {
946979
expect(route.headers['x-api-key']).toBeUndefined();
947980
});
948981
});
949-

0 commit comments

Comments
 (0)