Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions containers/api-proxy/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines 168 to +174
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildUpstreamPath() currently accepts protocol-relative paths (e.g. //host/path). Because new URL() treats //... as a host override, this can drop the first path segment and produce surprising upstream paths. Consider rejecting reqUrl values that start with // (or normalizing them) before calling new URL() to ensure only origin-form paths are forwarded.

See below for a potential fix:

 * @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 (reqUrl.startsWith('//')) {
    throw new Error('Protocol-relative request URLs are not supported');
  }

Copilot uses AI. Check for mistakes.
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;
}

/**
Expand Down Expand Up @@ -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' });
Expand Down Expand Up @@ -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,
Expand Down
34 changes: 33 additions & 1 deletion containers/api-proxy/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)', () => {
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -946,4 +979,3 @@ describe('resolveOpenCodeRoute', () => {
expect(route.headers['x-api-key']).toBeUndefined();
});
});

Loading