ruvnet / ruvnet/ruflo

Browser requests erase credentials of env-configured MCP servers

Open
#2,901 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
72.7k
Forks
8.6k
Avg merge
2d 23h
Merged PRs (30d)
83

Description

# Browser requests erase the credentials of env-configured MCP servers

**Component:** `ruflo/src/ruvocal` (RuVocal / chat-ui fork)
**Severity:** Authenticated `MCP_SERVERS` entries cannot be used from the chat UI
**Version tested:** `4ac1ab9ff3ee8f0406cfa97fe463944d9b110e9a`

## Summary

An MCP server configured via `MCP_SERVERS` with an `Authorization` header works when
called server-side, but loses its credentials on every request initiated from the chat
UI. Tool listing then fails with 401, the error is swallowed, and the model silently
answers without tools.

Each individual component behaves correctly. The bug is in how they compose.

## Root cause

Three correct behaviours combine into a broken one:

1. **`src/routes/api/mcp/servers/+server.ts`** deliberately withholds credentials from the
browser — correctly, this is what stops a server-side token reaching client code:

```ts
// headers intentionally omitted
```

2. **The client echoes servers back** in `selectedMcpServers` on each chat request. Since
it never received the headers, it sends the entry with none.

3. **`src/lib/server/textGeneration/mcp/runMcpFlow.ts`** merges request-provided servers
over base servers, request-wins:

```ts
for (const s of servers) byName.set(s.name, s);
for (const s of custom) byName.set(s.name, s); // <- replaces base entry wholesale
```

The headerless client copy replaces the base entry **including its server-side
`Authorization` header**.

## Reproduction

1. Configure an MCP server requiring authentication via `MCP_SERVERS`, e.g. GitHub's
hosted server:

```
MCP_SERVERS=[{"name":"GitHub","url":"https://api.githubcopilot.com/mcp/readonly",
"headers":{"Authorization":"Bearer "}}]
```

2. Enable it in the chat UI's MCP pill and ask a question requiring a tool.

Server log:

```
[mcp] merged request-provided servers
customProvidedCount=2
mergedServers=[{"name":"GitHub","hasAuth":false}, ...] <- was true before the merge
[mcp] openai tool defs built toolCount=0 toolNames=[]
[mcp] zero tools available after listing; skipping MCP flow
```

Sending only `selectedMcpServerNames` — no `selectedMcpServers` — works correctly, because
the merge branch is never entered. That asymmetry makes this hard to reproduce with
server-side testing: the API path succeeds while the browser path fails.

## Impact

Any authenticated `MCP_SERVERS` entry is unusable from the UI. The user sees a server that
is configured, enabled, and reports as connected, while every request silently proceeds
without tools.

In our case the model went on to produce two confident, mutually contradictory, entirely
fabricated descriptions of a private repository — one citing Supabase, the next NextAuth
with a Prisma schema — including invented file paths. Nothing in either response indicated
that the tools it had been asked to use were never loaded.

## Suggested fix

Preserve base headers unless the request genuinely supplies its own:

```ts
for (const s of servers) byName.set(s.name, s);
for (const s of custom) {
const base = byName.get(s.name);
const hasOwnHeaders = !!s.headers && Object.keys(s.headers).length > 0;
byName.set(s.name, hasOwnHeaders || !base?.headers ? s : { ...s, headers: base.headers });
}
```

This keeps request-supplied credentials winning where they exist, so a user overriding a
base server's auth in the UI still takes precedence.

## Secondary issue: the failure is invisible

`getMcpToolDefinitions` runs `listServerTools` under `Promise.allSettled` and discards
rejections. A server that fails to list contributes zero tools with no log line naming it
or the reason; the only symptom is the aggregate `zero tools available after listing`.

The per-server listing log that does exist is at `logger.debug`, below the default level.

Surfacing the rejection reason — and whether the server carried an `Authorization` header
— turns a day of guesswork into a single log line. Suggested:

```ts
results.forEach((r, idx) => {
if (r.status === "rejected") {
logger.warn({
server: servers[idx]?.name,
hasAuth: !!servers[idx]?.headers?.Authorization,
reason: r.reason instanceof Error ? r.reason.message : String(r.reason),
}, "[mcp] tool listing failed for server");
}
});
```

## Related

`/api/mcp/health` has the same class of problem: it forwards only headers supplied in the
request body, so a base server's health check runs unauthenticated and reports the server
as broken even when tool calls would have worked. Mentioned here rather than filed
separately since the fix is the same idea — consult the server-side registry for
credentials the client cannot have.

Contributor guide

Open the contributing guide

Research direction

Read src/routes/api/mcp/servers/+server.ts and src/lib/server/textGeneration/mcp/runMcpFlow.ts, then reproduce the authenticated MCP_SERVERS browser flow using the supplied configuration. Verify that client-provided server entries no longer discard base credentials, request headers still take precedence, and tool-listing failures identify the affected server and reason; inspect /api/mcp/health for the related credential-handling path.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, authentication, backend, frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.