Skip to content

Fix crash when Authorization header is missing for /v1/responses requests#29

Open
hsperker wants to merge 3 commits intohuggingface:mainfrom
hsperker:fix/missing-auth-crash
Open

Fix crash when Authorization header is missing for /v1/responses requests#29
hsperker wants to merge 3 commits intohuggingface:mainfrom
hsperker:fix/missing-auth-crash

Conversation

@hsperker
Copy link
Copy Markdown

Fixes: #28

This PR fixes a bug where POST /v1/responses could crash the server when the Authorization header is missing—especially in streaming mode where response headers may already be sent.

What changed

  • Added regression coverage to ensure both streaming (Accept: text/event-stream) and non-streaming requests return 401 when Authorization is missing (instead of crashing).
  • Fail early in postCreateResponse before emitting any SSE bytes, preventing Cannot set headers after they are sent to the client.
  • Pass the extracted bearer token down explicitly into the streaming execution path (so deeper code doesn’t try to write to the HTTP response).
  • Centralized bearer-token extraction in a small helper for consistent parsing.

Why

Once streaming starts, the server cannot safely change status/headers. Previously, missing auth could be handled too late (inside the streaming logic), leading to attempts to set headers after data was already
written and causing instability/crashes.

How to test

  • Start the server pnpm dev
  • pnpm test -- --grep "Authorization" (or run the specific missing-auth test file)
  • Manual repro:
    • Streaming request without Authorization should respond 401 (no SSE stream started)
 curl -N -i \
 -X POST http://localhost:3000/v1/responses \
 -H 'Content-Type: application/json' \
 -H 'Accept: text/event-stream' \
 -d '{"model":"gpt-4.1-mini","input":"hi","stream":true}'
  • Non-stream request without Authorization should respond 401 and the server should remain running
curl -i \
 -X POST http://localhost:3000/v1/responses \
 -H 'Content-Type: application/json' \
 -d '{"model":"gpt-4.1-mini","input":"hi","stream":false}'

Comment on lines +74 to +83
// 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;
}
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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing Auth Header Crashes Server

2 participants