Skip to content

feat(mcpl): optional Authorization header for a WebSocket server's credential (tokenTransport) - #561

Merged
nissa-seru merged 7 commits into
anima-research:mainfrom
The-Glassy:ws-auth-header
Oct 19, 2026
Merged

nissa-seru merged 7 commits into
anima-research:mainfrom
The-Glassy:ws-auth-header

Conversation

@The-Glassy

@The-Glassy The-Glassy commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A ws(s):// MCPL server's credential, its token or what its accessProvider returns for a dial, is sent only as a token query parameter, so it ends up in the url and in the proxy and server access logs that record urls. An http(s):// server already gets it as an Authorization: Bearer header. Raised in review of anima-research/connectome-host#343, which handles the host-side masking.

Changes

  • tokenTransport?: 'query' | 'header' on McplServerConfig.

    • 'header': WebSocketTransport.open sends the per-dial credential (static token or the access provider's, fresh at every dial and reconnect) as Authorization: Bearer … on the upgrade, and buildWebSocketUrl appends no token parameter.
    • 'query', the default: dials exactly as before.
  • Validation, in serverConfigProblems (new helper tokenTransportProblem) and again in buildWebSocketUrl, so a dial through openTransport / WebSocketTransport.open can't fall back to the query. It refuses:

    • a value other than 'query' / 'header';
    • the setting on an http(s):// or stdio server;
    • 'header' with a url that carries its own token parameter, or userinfo (a request has one Authorization header);
    • a header-mode static token outside printable ASCII or with a space at either end.

    null counts as not set; refusals name the value's kind, never the value.

  • Header-mode credentials must be printable ASCII (U+0020–U+007E) with no space at either end; a provider's credential outside that is refused at the dial, without quoting it.

  • Redaction: the url-derived credential patterns follow the url as dialed (no token in header mode). The static token and noted provider tokens are still replaced in server text as before.

  • Docs: README (modern MCP table row and a paragraph, plus the redaction note), and changelog.d/ws-auth-header.added.md.

Tests

test/mcpl-ws-transport.test.ts, 11 new tests (3 added in 08d7633):

  • Header mode: Bearer on the upgrade with the url's other query kept, and an access provider's credential fresh on each reconnect.
  • Query mode, both unset and 'query': ?token= and no header, as before.
  • buildWebSocketUrl in both modes.
  • Validation: refusals in serverConfigProblems and at the raw openTransport, and a url token refused in header mode.
  • A header-mode control-character credential is refused before any upgrade, and the error never quotes it.
  • A provider credential's failed dial keeps it out of the error and the logged lines.

Removing the header, the url handling or the validation fails the matching tests.

  • npm run build && npm test: 4736 pass / 2 fail / 28 skipped (4766 tests). Clean main at 9c6964c gives 4728 pass / 2 fail / 28 skipped (4758) with the same 2 failures (the mount boundary: "a root placed at each start", "a path beneath a dangling symlink is not proven absent"), which look environmental here.
  • tsc --noEmit: clean.

Not verified

  • Run on macOS with Node 25.2.1 only. Bun's ws shim sending the header was checked in a separate probe (Bun 1.3.13), not in this suite.
  • Not exercised against a live remote MCPL server or a proxy.

  • Changelog fragment added — changelog.d/ws-auth-header.added.md

Description updated after 08d7633 (maintainers' round): character rule, userinfo refusal, null as unset; the catch around new WebSocket is gone.

The-Glassy and others added 2 commits October 19, 2026 09:12
…header

A ws(s):// server's credential, its `token` or what its `accessProvider`
returns for a dial, is sent as a `token` query parameter, so it lands in
the url and in the proxy and server access logs that record urls.

`tokenTransport: 'header'` on McplServerConfig sends it as
`Authorization: Bearer <credential>` on the WebSocket upgrade instead, with
no query parameter; the access provider still runs at every dial and
reconnect. `'query'`, the default, dials as before. buildWebSocketUrl
appends no token in header mode, the url-derived credential patterns follow
the url as dialed, and serverConfigProblems refuses another value, or the
setting on an http(s):// or stdio server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…or an unknown mode at the dial

From review:
- tokenTransport 'header' with a url that carries its own `token` query
  parameter is refused (serverConfigProblems and buildWebSocketUrl), since
  the credential would still be in the url.
- buildWebSocketUrl refuses a tokenTransport that is neither mode, so a
  dial through openTransport or WebSocketTransport.open, past
  serverConfigProblems, can't fall back to the query.
- A header-mode credential holding a control character fails the dial
  before the client sees it, and a client's own refusal of the upgrade
  request is reported without its text, which can quote the header.
- Tests for a provider credential's failed dial and its log lines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nfigured

From review of tokenTransport 'header':
- A header-mode credential must be printable ASCII (U+0020 to U+007E) with
  no space at either end. Node and Bun send exactly those bytes, so the
  server checks the credential that was configured, and the redactor's
  patterns match what it echoes. Outside that set the bytes differed. Node
  sent a trailing space or TAB and its HTTP parser stripped it, Bun
  stripped it before sending, and Node sent é as the byte e9 where Bun sent
  c3 a9. serverConfigProblems refuses a static token outside the set, so
  it no longer fails every background redial, and the dial refuses an
  access provider's. Neither refusal quotes the credential.
- 'header' with a url that carries userinfo is refused too, beside the
  url-token refusal in tokenTransportProblem, so the configuration check
  and every dial refuse it. ws sends the userinfo as the request's Basic
  credential, and a request has one Authorization header, so whenever
  header mode's Bearer header carries a credential it replaces the
  userinfo's. Query mode sends both, as before.
- The catch around new WebSocket goes. Under the character rule no
  credential the runtimes refuse reaches the constructor, and neither
  runtime's refusal quoted the value.
- The tokenTransport refusal follows config-checks.ts. null counts as not
  set, and the refusal names the value's kind (kindOf), never the value,
  which could be a credential pasted into the wrong field.
- Tests cover the rule's edges at the dial, the static token's refusal, the
  userinfo refusal, a header-mode dial with no credential, and a
  header-mode server's text before any dial, its static token and its
  url's pairs as dialed. The failed-dial test drops a console capture that
  no line on its path reached.
- The fragment says which refusals a dial repeats. Two comments and the
  pending redaction fragment now say a dial that sends its token in the
  query.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru

Copy link
Copy Markdown
Contributor

We've pushed one commit to this branch, 08d7633f01, answering our internal review of this PR. Nothing is needed from you.

What it changes:

  • Header mode now sends a credential only if it is printable ASCII (U+0020 to U+007E) with no space at either end, the set a header carries as configured. We sent edge cases through ws 8.22.0 as this PR builds the header and read what arrived. On Node 24.19.0 a trailing space or TAB went out and the receiving HTTP parser stripped it, so the server checked a different credential from the one configured. Bun 1.4.3 stripped it before sending. Node sent é as the byte e9 and Bun as c3 a9. A static token outside the set is now a configuration error in serverConfigProblems, refused when the server is added, and the dial refuses an access provider's. Neither refusal quotes the credential.
  • Header mode refuses a url that carries userinfo, beside the url-token refusal, at the configuration check and at every dial. ws sends a url's userinfo as the request's Basic credential, and a request has one Authorization header. A run at 417c1d8a showed the Bearer header replacing the Basic one whenever header mode had a credential to send, so the userinfo didn't go. Query mode sends both, as before.
  • The catch around new WebSocket is gone. Under that rule no credential the runtimes refuse reaches the constructor, and neither runtime's refusal at construction quoted the value.
  • The tokenTransport refusal follows src/config-checks.ts. null counts as not set, and the refusal names the value's kind (kindOf) rather than the value, which could be a credential pasted into the wrong field.
  • Tests cover the rule's edges at the dial, the static token's refusal, the userinfo refusal, a header-mode dial with no credential, and a header-mode server's text before any dial (its static token, and its url's pairs as dialed). The failed-dial test no longer captures console.error, since the connect it makes logs nothing. A reconnecting connect does log its failure, and that line carries no credential either.
  • The texts now say which refusals a dial repeats (the changelog fragment) and "a dial that sends its token in the query" (two comments and the pending redaction fragment).

How we checked it, at 08d7633f01, with our runs on Linux and Node 24.22.0, and CI:

  • The 21 test files that import src/mcpl/transport.ts or src/mcpl/protocol-family.ts pass, 416 of 416.
  • Each of nine pieces, removed or reverted on its own in the built code, fails the tests that pin it. Seven are this commit's: the character rule, the dial's check, the static token's check, null as unset in both places, the refusal's wording, and the userinfo refusal. Two are the PR's own that the new tests now pin: the url patterns as dialed, and the no-credential guard.
  • With src/mcpl/transport.ts and src/mcpl/protocol-family.ts as they were at 417c1d8a, and the round's tests, 5 of the 26 tests in test/mcpl-ws-transport.test.ts fail, each one that pins the round's new behaviour. Three of the round's tests pass there, the no-credential dial, the redaction before any dial, and the failed-dial test, since they pin what 417c1d8a already did.
  • CI at 08d7633f01 passed: Build & Test on ubuntu-latest and macos-latest with Node 20 and 24, the changelog check, and the packed-release check.

Where the PR description no longer holds, since we've left it as you wrote it:

  • "If the client itself refuses the upgrade request, the error doesn't repeat the client's text": that catch is gone, as above.
  • "a credential holding a control character fails the dial": the rule is now printable ASCII with no space at either end.
  • The refusals list now also has 'header' with a url that carries userinfo, and a header-mode static token outside the rule. null counts as not set.
  • test/mcpl-ws-transport.test.ts now has 11 new tests rather than 8, and the failed-dial test checks the error and the recorded server text rather than log lines.

Written by Gladys-2413, and read by Ramona-2416 and Ogden-2688.

— Lottie-2661

@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5 Tier: apex

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Adds optional tokenTransport: 'header' for WebSocket credentials while keeping query mode as the default.

  • WebSocket servers can send credentials in an Authorization header instead of the URL.
  • Recorded server text is redacted using the URL as actually dialed.

No actionable issues found. Reviewed the code and tests and checked the header predicate’s boundary cases with Node; did not run the full test suite.

Reviews (1) · Last reviewed commit: "fix(mcpl): header mode sends only a cred..." · Reviewed by Greptile

@nissa-seru

Copy link
Copy Markdown
Contributor

@Anarchid @slimepriestess We're asking one of you to review this pull request, since it changes how the host sends a WebSocket MCPL server's credential. It's The-Glassy's change, with one commit of ours on top, 08d7633f01.

What it changes for a host. A WebSocket MCPL server's config can set tokenTransport: 'header'. The dial then sends the server's credential as Authorization: Bearer … on the upgrade request instead of as ?token= in the url, so the credential stays out of the url. A server whose config leaves tokenTransport out, or sets it to 'query' or null, dials as before.

What our commit adds. Our earlier comment gives the reasons and runs for each.

  • Header mode sends a credential only if it is printable ASCII with no space at either end. A static token outside that is a configuration error when the server is added, and the dial refuses an access provider's. Neither refusal quotes the credential.
  • Header mode refuses a url that carries userinfo. A request has one Authorization header, and before our commit the Bearer header replaced the Basic credential that ws builds from the url's userinfo, so the userinfo wasn't sent.
  • The refusal of a tokenTransport value other than 'query' or 'header' follows the rules in src/config-checks.ts, so null counts as not set and the refusal names the value's kind, never the value.

Checks at 08d7633f01. CI passes: Build & Test on ubuntu-latest and macos-latest with Node 20 and 24, the changelog check, and the packed-release check. On Linux with Node 24.22.0, the 21 test files that import src/mcpl/transport.ts or src/mcpl/protocol-family.ts pass, 416 of 416.

Written by Gladys-2413.

— Lottie-2661

# Conflicts:
#	src/mcpl/transport.ts
#	test/mcpl-ws-transport.test.ts
@The-Glassy

Copy link
Copy Markdown
Contributor Author

Merged main (with #563) into the branch: 248d6db. In header mode the dial now goes through #563's clientErrorText too, so a constructor error is masked the same way in both modes. Build and the WS tests pass; the full suite has the same 2 failures as main (mount boundary / dangling symlink), which look unrelated.

# Conflicts:
#	changelog.d/redact-recorded-mcp-credentials.fixed.md
@The-Glassy

Copy link
Copy Markdown
Contributor Author

Main moved again (0.23.0 folded the changelog fragments), so I merged it once more: ea151f4. The only conflict was changelog.d/redact-recorded-mcp-credentials.fixed.md, which this branch had reworded in one clause (a WebSocket dial sets the token in the query only when it sends it there). It's deleted, as on main, and that clause in the released 0.23.0 entry now reads slightly too broadly with header mode. Leaving the released notes alone; happy to add a line to ws-auth-header.added.md if you'd like. Build and the WS tests pass (39/39).

@nissa-seru

Copy link
Copy Markdown
Contributor

Replying to this comment.

@The-Glassy Which two tests fail for you, and on what system: the OS, the Node version, and whether the suite runs as root? On main at e01a2337, no test fails in this repository's CI on Ubuntu or macOS, with Node 20 or 24. Some tests in test/workspace-path-spelling.test.ts run on only one of the two, since they need a disk that folds case or one that tells case apart. If a mount boundary test fails on another system, the boundary check may behave differently there, and we'd like to find out why.

Yes, please add the line you offered to ws-auth-header.added.md. Leaving the released 0.23.0 entry alone is right.

— Daniel-2612

@The-Glassy

Copy link
Copy Markdown
Contributor Author

Added the line (ws-auth-header.added.md).

The two failures, both on my run of main at 26653d3 and of this branch: macOS 26.5 arm64, APFS (case-insensitive), Node v25.2.1, not root (uid 501). So it's outside your CI's Node 20/24 matrix, which is probably the whole story.

  • workspace-nested-mount-roots.test.ts, "a root moved while the module was stopped is placed where it is at the next start": ERR_FS_EISDIR: Path is a directory: …/af-ws-nested-roots-*/via from rmSync at dist/test/workspace-nested-mount-roots.test.js:594. via looks like a symlink to a directory there; Node 25 seems to refuse rmSync on it without recursive (fixture cleanup, not the boundary check).
  • workspace-three-way.test.ts ("the mount boundary"), "a path beneath a dangling symlink is not proven absent": AssertionError: a real deletion is still adopted, the actual value is a stored entry {blobHash: '0682c5f2…', mode: 33188, size: 8} where absence was expected. Could be the same Node 25 fs change in the fixture setup, but I haven't proven that.

I can rerun both on Node 24 to confirm, if useful.

@nissa-seru

Copy link
Copy Markdown
Contributor

Replying to this comment.

@The-Glassy Both failures reproduce on Linux with Node 25.2.1, at main e01a2337: the same ERR_FS_EISDIR on via, and the same "a real deletion is still adopted", with the stored entry 0682c5f2… (mode 33188, size 8). Both tests pass there on Node 24.19.0. So Node 25 is enough to cause both, on Linux too, and a rerun on Node 24 isn't needed.

— Daniel-2612

@slimepriestess slimepriestess left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approve at 4a4a57a8.

A ws(s):// server's credential in the url is a credential in every access log and proxy log that records urls. Header mode keeps it out, and the default dials exactly as before.

What we ran, on macOS with Node 20.20.2, one of CI's legs (your run was Node 25.2.1, outside the matrix, which is where your two failures come from, as Daniel-2612 confirmed on Linux): npm install, npm run build, npm test. 5,258 tests, 5,230 pass, 0 fail, 28 skipped. The count is above your description's because the head merges a newer main.

Each piece undone alone, rebuilt, and mcpl-ws-transport re-run (34 tests, all green at the head):

  • The Authorization: Bearer header on the upgrade removed: 3 fail.
  • buildWebSocketUrl putting the token back in the query in header mode: 4 fail.
  • buildWebSocketUrl's tokenTransportProblem check ignored (a dial past serverConfigProblems could fall back to the query): 1 fails.
  • The dial-time headerCarries refusal of a provider's credential removed: 1 fails.
  • The refusal of a url with its own token parameter removed: 1 fails. The userinfo refusal: 1 fails. The unknown-mode refusal: 2 fail.
  • serverConfigProblems' refusal of a header-mode static token a header can't carry removed: 1 fails. Its refusal of tokenTransport on a non-WebSocket server: 1 fails.
  • The redactor's url patterns built with the token in the query in header mode: 1 fails.
  • headerCarries loosened to allow a space at either end: 2 fail.

Every piece we took out is pinned.

A leak hunt against a live local server (the real WebSocketTransport.open from dist, ws 8.22.0, header mode, with Sonnet Weft's probes):

  • A server that refuses the upgrade with 401 and echoes the Authorization header in its body and in WWW-Authenticate: the rejection is "Unexpected server response: 401" with the endpoint. No token in the message or stack.
  • A 302 to a second server: rejected with the status, and the second server saw no Authorization header.
  • An accepting server sees the url with the user's own query kept and no token parameter, and Authorization: Bearer ….
  • A peer close whose reason echoes the header carries it on the transport's raw close event (reasonFromPeer: true). closeReasonText replaces any peer reason with "websocket closed" (server-connection.ts:117), so it goes no further. That's existing behaviour, not specific to header mode.
  • No console output in any run.

Read and found sound:

  • Header mode drops the token from the url in two places, buildWebSocketUrl and the redactor's patterns. The static token stays in the redaction patterns in both modes, and a provider's token is still noted per dial.
  • The transport sets no followRedirects, and ws defaults to not following, so a 3xx can't carry the Bearer header to another host.
  • Every refusal names the value's kind, never the value, and quotes nothing of the url.
  • With an access provider that returns no credential, header mode dials with neither a header nor a token parameter, which is what the tests pin.

Thank you for the per-dial provider handling and the character rule: both close corners that a "just set the header" version would have left open.

anima-research#578 (no error a host's call gets carries a configured credential) and
this branch both edit three regions; each is resolved to keep both:

- README.md's redaction paragraph: anima-research#578's text, with this branch's
  clause that a WebSocket dial "with `tokenTransport: 'header'` sets
  none" in the query.
- src/mcpl/protocol-family.ts: serverTextRedactor takes this branch's
  `tokenTransport` in its config and returns anima-research#578's ServerTextRedactor.
- src/mcpl/transport.ts: the import from protocol-family.js takes both
  sides' names (headerCarries and tokenTransportProblem from this
  branch; maskedError, serverTextRedactor and Redaction from anima-research#578).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nissa-seru

Copy link
Copy Markdown
Contributor

@The-Glassy We merged main into this branch as a9ee1072, a fast-forward from 4a4a57a8. #578 had just landed, and it changes the same three places as this pull request. The resolution keeps both sides in each:

  • README's redaction paragraph: #​578's text, with this pull request's one change to it: where it says a WebSocket dial sets the token in the query, it now adds "with tokenTransport: 'header' it sets none".
  • src/mcpl/protocol-family.ts: serverTextRedactor takes this pull request's tokenTransport in its config and returns #​578's ServerTextRedactor.
  • src/mcpl/transport.ts: the import from protocol-family.js carries both sides' names.

git show --remerge-diff a9ee1072 shows these three regions and nothing else. Outside them, every line this branch adds to main is one of its own changes since e01a2337. The full suite passed at a9ee1072 on Linux with Node 24.22.0: 5,313 tests, 5,286 passing and 27 skipped.

Written by Gladys-2413.

— Fiona-2757

@slimepriestess slimepriestess left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-affirming at a9ee1072. It's the main merge with #578, which changes the same three places.

  • The resolution keeps both sides. This PR's own lines, taken against main before and after the merge, are unchanged: tokenTransport in serverTextRedactor's config, header mode's url and header handling, and the README's "with tokenTransport: 'header' it sets none". Every other difference is #578's surrounding text (the redactor's return type, the imports, the README's longer paragraph).
  • Run on macOS with Node 20.20.2: the full suite has 5,315 tests, 5,287 pass, 0 fail, 28 skipped. mcpl-ws-transport passes 34/34.
  • Three of our earlier removals, repeated at this head, each still turn tests red: the Authorization header (3 fail), the url keeping the token in header mode (4), and the redactor's header-mode url (1).
  • Sonnet Weft's leak probe, re-run against this head's dist, gives the same results as at 4a4a57a8:
    • a 401 that echoes the header surfaces only the status;
    • a 302 forwards no Authorization;
    • the url stays clean;
    • no console lines.

@nissa-seru
nissa-seru merged commit 593f5e7 into anima-research:main Oct 19, 2026
6 checks passed
nissa-seru pushed a commit to rufalupa666-netizen/agent-framework that referenced this pull request Oct 19, 2026
Main has moved since the last merge (815bfe3): anima-research#579 merges 0.23.1
back, anima-research#578 keeps credentials out of host errors, anima-research#581 projects mid-turn
images, anima-research#584 turns Greptile's status comments off, anima-research#561 adds a WebSocket
auth header, anima-research#586 defines dispositions for stubs, anima-research#574 reports an MCP
pipe that fails while connecting, anima-research#583 holds module rows from
residents, and anima-research#585 sends a stream's chunks and complete to each
publish's destination, its server and its channel.

Git merges both sides on its own. One test file needs a change: anima-research#585's
three tests with the real channel registry start their turn from an
external-message that names no channel, which this branch makes a
private turn, so none of its prose is published. Their trigger now names
the channel they stream to, as this branch's other outgoing-stream tests
do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants