Let a roster entry carry request headers

coilyco-ops commented

2026-08-14 07:51:51 +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. NOTE: the gate fails on test-skips for TestTheGateReadsTheDeclaredWorkflow, identically on untouched origin/main. That is pre-existing and filed separately, not from this branch. closes #778

coilyco-ops added 1 commit

2026-08-14 07:51:51 +00:00

feat(roster): let an entry carry request headers

ci / image-build (pull_request) Failing after 20s

Details

ci / test (pull_request) Failing after 30s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

59c16a5aee

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>

coilyco-ops requested review from coilysiren

2026-08-14 07:51:51 +00:00

coilyco-ops referenced this pull request

2026-08-14 22:48:53 +00:00

Let a roster entry carry request headers #786

coilyco-ops closed this pull request

2026-08-14 22:50:28 +00:00

coilyco-ops commented

2026-08-14 22:50:43 +00:00

Author

Member

Closed as superseded by #786, which carries this change unaltered.

This PR was opened through AGit flow, so its head is refs/pull/779/head and a branch push could not update it. Its CI stayed pinned to 59c16a5, cut before two pre-existing main breakages were fixed, and the PR had degraded to reporting zero changed files.

#786 has the byte-identical diff, coilyco-ops authorship preserved, rebuilt as a single commit on current main. It is green.

The two failures that made this red were never from this branch, and both are fixed on main via #777:

test-skips was red on TestTheGateReadsTheDeclaredWorkflow. It skipped rather than failed once the lane declaration moved to AGENTS.md frontmatter and its three readers were left pointing at .ward/ward.yaml, which also left the main-branch push guard dead.
TestASecondJobInAThreadNeitherBindsNorFails failed intermittently on a real thread-binding race: the bind named the state it had just read, and the runner moved the job first, so the store refused the write and the binding was silently lost.

Closed as superseded by #786, which carries this change unaltered. This PR was opened through AGit flow, so its head is `refs/pull/779/head` and a branch push could not update it. Its CI stayed pinned to `59c16a5`, cut before two pre-existing `main` breakages were fixed, and the PR had degraded to reporting zero changed files. #786 has the byte-identical diff, coilyco-ops authorship preserved, rebuilt as a single commit on current `main`. It is green. The two failures that made this red were never from this branch, and both are fixed on `main` via #777: * `test-skips` was red on `TestTheGateReadsTheDeclaredWorkflow`. It skipped rather than failed once the lane declaration moved to AGENTS.md frontmatter and its three readers were left pointing at `.ward/ward.yaml`, which also left the main-branch push guard dead. * `TestASecondJobInAThreadNeitherBindsNorFails` failed intermittently on a real thread-binding race: the bind named the state it had just read, and the runner moved the job first, so the store refused the write and the binding was silently lost.

coilyco-ops referenced this pull request

2026-08-14 22:59:21 +00:00

Role snapshot drift is only caught at Dockerfile step 26, and the message names a verb that cannot run #788

coilyco-ops referenced this pull request

2026-08-15 15:29:27 +00:00

Catch role snapshot drift beside the other gates, not at step 26 #799