Refuse a header value that cannot be sent, instead of reporting an unreachable agent

[kevin9327]

Paste a key into an agent's key box with a line break somewhere in the middle — which is what a long
key copied out of a wrapped terminal line brings with it — and press Test connection against an
agent that is running perfectly well. What comes back is:

This server could not reach that address. If your agent runs on your own machine, it needs to be
reachable from here, a tunnel, or somewhere this server can dial.

Measured, against a real Bun.serve agent on loopback and a header value of Bearer abc\ndef: the
agent counted zero requests. It was never dialled. The person is now looking at their tunnel and
their firewall over a value in the box they had just filled in.

Save the same key instead of testing it and it is quieter and worse. The form says saved, the value
is encrypted and stored, and every turn that Bot takes afterwards fails on it.

Why

new Headers() refuses a line break, a NUL, and any code point above U+00FF, and it refuses them by
throwing a TypeError from inside fetch — not by anything this deployment decides. Measured on Bun
1.3.14, the same probe that produced the numbers below:

newline: THREW TypeError        tab: OK
cr:      THREW TypeError        inner space: OK
nul:     THREW TypeError        latin1 e-acute: OK
cjk:     THREW TypeError        0x80: OK
emoji:   THREW TypeError        del 0x7f: OK

Neither of the two places that take a header value looked at one:

• parseConnectionHeaders (server/src/agents/routes.ts) checks the name against
  /^[A-Za-z0-9-]+$/, refuses __proto__, caps the map at 32 entries and the value at 4096
  characters — and then accepts any string. Its own docstring says it exists because "an array
  value, a nested object, or a __proto__ key travelled into the network call and threw a
  TypeError 500". A value that throws the same TypeError was the case it did not cover.
• parseAgentInput, a few lines above it, checks the header name the same way and stores the
  value as it arrives.

On the connection test the throw travels out of createAgentFetch, into testAgentConnection's
catch — which exists for a dead host and a typo'd address — and is answered 200 with the sentence
above. On the stored key nothing catches it at all until the next run.

The other spelling that does this is not a line break: a hyphen copied out of a document has often
been turned into an en dash, which is U+2013, and in a password box it looks like a hyphen.

The fix

One helper, used by both. It returns what kind of character is in the way and never the value —
this is asked of a credential on one of the two paths, and one character of a secret in an error
message is one character too many.

Refused rather than stripped. A key with a line break in it is not a key with a line break removed,
and silently sending a different value than was pasted is how somebody spends an afternoon on a 401.

Measured

bun test server/tests/agent-test-connection-headers.test.ts

• Against origin/main with only the new tests applied: 20 pass, 6 fail.
  • rejects a newline in the value, rejects a carriage return in the value,
    rejects a NUL in the value, rejects a character above Latin-1 — all four return ok: true.
  • a key that cannot be sent as a header is refused — parseAgentInput returns ok: true.
  • is refused before anything is dialled — the route answers 200, not 400, carrying the
    "could not reach that address" reason.
• With the change: 26 pass, 0 fail.

Four of the new cases are the guard against over-correcting, and pass before and after: a tab, an
inner space, a Latin-1 accent, and a value made of punctuation are all things new Headers()
accepts, so refusing any of them would be taking away a header somebody's agent really wants. So are
the eleven cases that were already there.

Also run, all green:

• bun test server/tests/agent-routes.test.ts server/tests/agent-endpoint.test.ts server/tests/agent-connection-live.test.ts — 106 pass, 0 fail
• bun run --filter server typecheck — exit 0
• bunx biome check on both changed files — clean

Note on the CHANGELOG

A deployment behaves differently afterwards, so there is an entry. It is inserted at the top of
## Unreleased, which is where every entry goes, so it will conflict on that one line with any other
PR open against the same anchor. Happy to rebase whenever it suits you.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.