Skip to content
Open
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
45 changes: 32 additions & 13 deletions src/routes/responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,38 @@ const NOT_FORWARDED_HEADERS = new Set([
"upgrade",
]);

function getBearerToken(authorizationHeader: unknown): string | undefined {
if (typeof authorizationHeader !== "string") {
return undefined;
}

// We don't validate the token - just extract it in a safe, predictable way.
const match = authorizationHeader.match(/^Bearer\s+(.+)$/i);
if (!match) {
return undefined;
}

const token = match[1].trim();
return token.length > 0 ? token : undefined;
}

export const postCreateResponse = async (
req: ValidatedRequest<CreateResponseParams>,
res: ExpressResponse
): Promise<void> => {
// This service doesn't validate the token, but it cannot proceed without one.
// Fail early before we start streaming (once we write SSE bytes, we can't return a 401).
const apiKey = getBearerToken(req.headers.authorization);
if (!apiKey) {
res.status(401).json({
success: false,
error: "Unauthorized",
});
return;
}
Comment on lines +74 to +83
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we should raise a 401 not authorized if bearer token does not exist. responses.js is meant to be a lightweight proxy, meaning it does not handle authentication. One use case is to place the responses.js proxy in front of a local llama.cpp server. In this case, it doesn't make any sense to have authorizations at all.

I think the only thing that the PR should solve is that if the authorization header is not present, and the backend server needs one, then the proxy should not crash (which was the initial issue)

Does that make sense to you?


// To avoid duplicated code, we run all requests as stream.
const events = runCreateResponseStream(req, res);
const events = runCreateResponseStream(req, res, apiKey);

// Then we return in the correct format depending on the user 'stream' flag.
if (req.body.stream) {
Expand Down Expand Up @@ -88,7 +114,8 @@ export const postCreateResponse = async (
*/
async function* runCreateResponseStream(
req: ValidatedRequest<CreateResponseParams>,
res: ExpressResponse
res: ExpressResponse,
apiKey: string
): AsyncGenerator<PatchedResponseStreamEvent> {
let sequenceNumber = 0;
// Prepare response object that will be iteratively populated
Expand Down Expand Up @@ -134,7 +161,7 @@ async function* runCreateResponseStream(

// Any events (LLM call, MCP call, list tools, etc.)
try {
for await (const event of innerRunStream(req, res, responseObject)) {
for await (const event of innerRunStream(req, res, responseObject, apiKey)) {
yield { ...event, sequence_number: sequenceNumber++ };
}
} catch (error) {
Expand Down Expand Up @@ -174,17 +201,9 @@ async function* runCreateResponseStream(
async function* innerRunStream(
req: ValidatedRequest<CreateResponseParams>,
res: ExpressResponse,
responseObject: IncompleteResponse
responseObject: IncompleteResponse,
apiKey: string
): AsyncGenerator<PatchedResponseStreamEvent> {
// Retrieve API key from headers
const apiKey = req.headers.authorization?.split(" ")[1];
if (!apiKey) {
res.status(401).json({
success: false,
error: "Unauthorized",
});
return;
}

// Forward headers (except authorization handled separately)
const defaultHeaders = Object.fromEntries(
Expand Down
51 changes: 51 additions & 0 deletions tests/00-missing-auth-header.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { strict as assert } from "assert";

const BASE_URL = process.env.RESPONSES_BASE_URL ?? "http://localhost:3000";

describe("missing Authorization header", function () {
// Matches the existing test suite behavior: tests expect a running dev server.
before(async function () {
try {
const response = await fetch(`${BASE_URL}/`);
if (response.status !== 200) {
throw new Error(`Server returned status ${response.status}`);
}
} catch (error) {
console.error("❌ Server is not running. Please start the server with 'pnpm dev' before running the tests.");
throw error;
}
});

it("streaming request returns 401 (does not start an SSE stream)", async function () {
const res = await fetch(`${BASE_URL}/v1/responses`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Accept: "text/event-stream",
},
body: JSON.stringify({ model: "gpt-4.1-mini", input: "hi", stream: true }),
});

assert.equal(res.status, 401);

const contentType = res.headers.get("content-type") ?? "";
assert.ok(!contentType.includes("text/event-stream"), `Expected non-SSE response, got content-type: ${contentType}`);
});

it("non-streaming request returns 401 and server remains healthy", async function () {
const res = await fetch(`${BASE_URL}/v1/responses`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ model: "gpt-4.1-mini", input: "hi" }),
});

assert.equal(res.status, 401);

// Regression: the handler must not crash the whole process after responding.
// (The bug was triggering "Cannot set headers after they are sent to the client".)
const healthRes = await fetch(`${BASE_URL}/health`);
assert.equal(healthRes.status, 200);
});
});
Loading