Let a roster entry carry request headers #786

Merged
coilysiren merged 1 commit from eng/roster-request-headers into main 2026-08-14 22:53:02 +00:00
Member

An authenticated hosted MCP had no supported way in. Remote servers already worked and expandRoster already resolved a secret into an entry, but there was no headers field, so a credential had to ride in the query string. addEndpoint returns early unless the URL carries an explicit port, and even then guards the host alone, so a key in a portless HTTPS query string is not among the values a reply is checked against. headers is expanded like env, rejected on stdio like env is rejected on HTTP, applied to both HTTP transports, and its values go through addOpaque so a credential cannot appear in a reply. One http.Client is shared by the roster, so a declaring entry gets a shallow copy.

closes #778

Why this replaces #779

#779 was opened through AGit flow, so its head is refs/pull/779/head and no branch push could update it. Its CI stayed pinned to a commit cut before the two main breakages below were fixed, and the PR had degraded to reporting zero changed files. This carries the identical change, authored by coilyco-ops as before, rebuilt on current main as a single commit. #779 is closed as superseded.

What changed since #779

Nothing in the roster change itself. The diff is byte-identical to 59c16a5. What changed is the base: the two failures that made #779 red were pre-existing on main and are now fixed there via #777.

  • test-skips was red on TestTheGateReadsTheDeclaredWorkflow, which skipped rather than failed after the lane declaration moved to AGENTS.md frontmatter and its readers were left behind. Fixed on main.
  • TestASecondJobInAThreadNeitherBindsNorFails failed intermittently on a real thread-binding race in the job store. Fixed on main.

Neither was ever caused by this branch.

Verification

build, policy-check, vet, test, test-skips and the full pre-commit suite pass locally against this exact tree, plus 500 iterations of the thread-binding tests.

I have not exercised the headers path against a live authenticated MCP. That needs a deployment holding a real credential, which is an operator step.

An authenticated hosted MCP had no supported way in. Remote servers already worked and expandRoster already resolved a secret into an entry, but there was no headers field, so a credential had to ride in the query string. addEndpoint returns early unless the URL carries an explicit port, and even then guards the host alone, so a key in a portless HTTPS query string is not among the values a reply is checked against. headers is expanded like env, rejected on stdio like env is rejected on HTTP, applied to both HTTP transports, and its values go through addOpaque so a credential cannot appear in a reply. One http.Client is shared by the roster, so a declaring entry gets a shallow copy. closes #778 ## Why this replaces #779 #779 was opened through AGit flow, so its head is `refs/pull/779/head` and no branch push could update it. Its CI stayed pinned to a commit cut before the two `main` breakages below were fixed, and the PR had degraded to reporting zero changed files. This carries the identical change, authored by coilyco-ops as before, rebuilt on current `main` as a single commit. #779 is closed as superseded. ## What changed since #779 Nothing in the roster change itself. The diff is byte-identical to `59c16a5`. What changed is the base: the two failures that made #779 red were pre-existing on `main` and are now fixed there via #777. * `test-skips` was red on `TestTheGateReadsTheDeclaredWorkflow`, which skipped rather than failed after the lane declaration moved to AGENTS.md frontmatter and its readers were left behind. Fixed on main. * `TestASecondJobInAThreadNeitherBindsNorFails` failed intermittently on a real thread-binding race in the job store. Fixed on main. Neither was ever caused by this branch. ## Verification `build`, `policy-check`, `vet`, `test`, `test-skips` and the full pre-commit suite pass locally against this exact tree, plus 500 iterations of the thread-binding tests. I have not exercised the headers path against a live authenticated MCP. That needs a deployment holding a real credential, which is an operator step.
feat(roster): let an entry carry request headers
All checks were successful
ci / test (pull_request) Successful in 35s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
ci / image-build (pull_request) Successful in 19s
4376267d7c
An authenticated hosted MCP had no supported way in. Remote servers already
worked and expandRoster already resolved a secret into an entry, but there was
no headers field, so a credential had to ride in the query string.

That is the worst available place for it. IdentifierGuard.addEndpoint returns
early unless the URL carries an explicit port, and even then it guards the host
alone, so a portless HTTPS endpoint contributes nothing to the forbidden set and
a key in its query string is not among the values a reply is checked against. A
query string is also the part of a URL most likely to reach a log or a span
attribute intact.

headers is expanded the way env already is, rejected on stdio the way env is
rejected on HTTP, and applied to both HTTP transports. One http.Client is shared
by the whole roster, so a declaring entry gets a shallow copy and the entries
that declared none keep the shared client untouched.

Header values go through addOpaque, so a credential delivered this way is
unsayable in a reply, matching the Discord token rather than an endpoint. A
short value contributes nothing, so a header naming a version or a content type
is not guarded.

An empty value fails validation against the named server. Without that an unset
variable would reach the vendor as an anonymous call and surface as the vendor's
error rather than as a roster mistake.

The roster doc sat at exactly its 80 line cap, so the rationale lands in a
companion doc rather than by trimming what was already there.

closes #778

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo!786
No description provided.