Skip to content

mcp: check the Mcp-Method and Mcp-Name headers on 2026-07-28 requests (DEX-82, DEX-83) - #39694

Merged
bobbyiliev merged 3 commits into
MaterializeInc:mainfrom
bobbyiliev:mcp-header-validation
Oct 9, 2026
Merged

bobbyiliev merged 3 commits into
MaterializeInc:mainfrom
bobbyiliev:mcp-header-validation

Conversation

@bobbyiliev

@bobbyiliev bobbyiliev commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

On 2026-07-28 requests, Mcp-Method must match the body's method and Mcp-Name must match the tool name on tools/call, decoding the =?base64?...?= form. A missing, repeated, or mismatched header gets HTTP 400 with -32020 HeaderMismatch, as the transport spec requires. 2025-11-25 requests and the flag-off path are unchanged. Closes DEX-82 and DEX-83, and adds the HeaderMismatch code from DEX-78.

Tests: unit tests for the header check, and a new datadriven file run with the flag on. The harness now accepts Name: value header lines before the JSON body.

@bobbyiliev
bobbyiliev marked this pull request as ready for review October 9, 2026 11:11
@bobbyiliev
bobbyiliev requested a review from a team as a code owner October 9, 2026 11:11
@def-

def- commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- A repeated MCP-Protocol-Version whose first copy is old skips header validation entirely

src/environmentd/src/http/mcp.rs:748

The protocol, and so whether check_against runs at all, is picked from mcp_headers.protocol_version.first(). A request carrying MCP-Protocol-Version: 2025-11-25 followed by MCP-Protocol-Version: 2026-07-28 is served as 2025-11-25 with Mcp-Method/Mcp-Name never compared to the body. An intermediary that reads the last occurrence then sees a 2026-07-28 request it may trust, so the proxy-vs-server split this check exists to close is reopened.

Details

The McpHeaders doc (mcp.rs:102) says a repeated header "is rejected rather than reduced to one", and the repeated-header test in test_header_check covers it, but only with 2026-07-28 first. The reversed order never reaches header_text(&self.protocol_version, ...) (mcp.rs:122), because mirrors_body_in_headers() (mcp.rs:789) is already false.

The spec tells policy-enforcing intermediaries to trust Mcp-Name only when MCP-Protocol-Version names a validating revision. With the flag on, a client can send both version headers plus Mcp-Method: tools/call, Mcp-Name: query_system_catalog and a body calling query. A gateway that reads the last value and allows only query_system_catalog lets the request through, and environmentd runs query. The same split happens when a proxy merges the two copies into one 2025-11-25, 2026-07-28 line, since select sees a value that isn't exactly 2026-07-28 and falls back.

Fix: when the flag is on, take the modern path if any occurrence (or any comma-separated element of one) equals 2026-07-28. Then check_against rejects the repetition with -32020. Single unknown values keep the existing no-400 fallback.

Comment thread src/environmentd/src/http/mcp.rs Outdated
}

#[mz_ore::test]
fn test_header_check() {

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.

Feels like this test is a bit verbose and has a lot of overlap with the datadriven tests. Had Claude do an analysis and it found:

Only in the unit test:
- Positive tools/list with matching Mcp-Method and no Mcp-Name.
- Tools/List case mismatch is rejected.
- =?base64?not base64?= invalid Base64 is rejected.
- Mcp-Method with a non-ASCII byte is rejected. This is hard to express as a datadriven header line.
- Repeated MCP-Protocol-Version is rejected.

Maybe we can move everything but the "non-ASCII byte is rejected" test into datadriven then just make a solo test for this case? That way the test becomes easier to read! Not gonna have this block the PR however

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done in f3c86c4. The cases moved to the datadriven file, and only the non-ASCII one stays as a unit test.

@bobbyiliev

Copy link
Copy Markdown
Contributor Author

Fixed in f3c86c4. Any MCP-Protocol-Version copy that names 2026-07-28, also in a comma-joined list, now selects it, so a repeated or joined header gets 400 with -32020. A single other value still falls back to 2025-11-25. Added datadriven cases for both orders and the joined form.

@bobbyiliev
bobbyiliev merged commit 8ce3ed7 into MaterializeInc:main Oct 9, 2026
84 checks passed
@bosconi bosconi mentioned this pull request Oct 16, 2026
maheshwarip added a commit that referenced this pull request Oct 23, 2026
Release notes for the **v26.47.0** train. One commit was added per
snapshot; the `final` snapshot sets the real dates. **All six snapshots
(`rc.1`–`rc.5`, `final`) are now applied — this branch is complete and
ready for review.**

- **Content**: 2 Feature sections (`TRY_CAST`, `EXCLUDE ALL NOT NULL
CONSTRAINTS`), 7 Improvements, 13 Bug Fixes — `rc.1` contributed all of
it except the top four Bug Fixes: `rc.2` added two, and `rc.3` and
`rc.4` one each. `rc.5` added no new entries; it amended `rc.4`'s
probe-port Bug Fix instead (see below). `final` adds nothing at all —
its tracking issue holds only the unassociated `release: bump to version
v26.47.0` commit
([`bed23494d`](bed2349)),
and the `v26.47.0-rc.5` and `v26.47.0` tags are 14h28m apart with that
bump the only thing between them. Its commit is empty and exists to
record on the branch that the snapshot was applied.
- **Dates (final, no longer provisional)**: Cloud **2026-10-22**,
Self-Managed **2026-10-23**. Every RC estimated this same pair from the
`v26.47.0-rc.1` tag, and the real dates confirmed it — the five RCs
compressed into one week rather than pushing the train out — so no
published date text changed in the `final` commit. ⚠️ One thing worth a
last look before merge: **2026-10-22 is a Wednesday**, one day off the
usual Thursday Cloud date (2026-10-23 is the normal Friday Self-Managed
day). That is the supplied value and it stands, but if the release owner
expected Thursday 2026-10-23 / Friday 2026-10-24, this branch is where
to correct it.
- **Operator compatibility**: row added for `v26.47`, release date
`2026-10-23`.
- **Branch base**: `main`, whose newest section is `## v26.45.1` —
v26.46.0's section is still on its own unmerged branch (#39690). Expect
a top-of-list conflict when both merge; `v26.47.0` goes above
`v26.46.0`.

✅ **Corrections already on this branch** — a `final` verification pass
over the whole assembled section confirmed these and deliberately left
them alone:

- **`rc.1` named a syntax that does not exist.** It published "`CREATE
TABLE ... FROM SOURCE` now accepts an `ALL COLUMNS NULLABLE` option",
taking the name from
[#39680](#39680
title and body. At the `v26.47.0` tag the option is `EXCLUDE ALL NOT
NULL CONSTRAINTS`
(`TableFromSourceOptionName::ExcludeAllNotNullConstraints`), and
`NULLABLE` is not a lexer keyword at all, so the published syntax would
have been a parse error for anyone who copied it. A human editor caught
this and replaced the bullet with the
`{#v26.47-exclude-not-null-constraints}` feature section now on the
branch. The `rc.1` snapshot directory in mz-skills still carries the
wrong wording — snapshot directories are the record of what was reviewed
at the time and are left untouched.
- **`rc.5`'s cross-snapshot rewrite is applied once**, including the
"Deployments using password, SASL, or OIDC authentication have no
separate internal SQL port and continue to be probed on the external
one" clause.
- **PR citations are stripped from all entries** as a deliberate
house-style edit, matching how published sections read. PR links remain
available in the mz-skills snapshot drafts linked below.

⚠️ **Open questions for the release owner** — none of these were
resolved by `final`; this branch is the last place to act on them

- **Was any `rc.4` artifact handed to a customer or used for an upgrade
rehearsal?** This decides a published entry.
[#40036](#40036)
(published as a Bug Fix by `rc.4`) made the operator probe
`environmentd`'s internal SQL port unconditionally, which crashloops
deployments using Password, SASL, or OIDC authentication — they have no
internal SQL port.
[#40046](#40046) fixed
that roughly seven hours later, inside this same unreleased train, so no
shipped build (`v26.46.1` probes the external port unconditionally)
could have hit it. It is therefore recorded as **Borderline** rather
than published as a Bug Fix of its own, and its substance folded into
`rc.4`'s bullet as a scoping clause. If an `rc.4` build did reach
anyone, the crashloop *was* reachable and
[#40046](#40046)
should be published in its own right. Note the author wrote an explicit
positive release-note line for it, which is why this is flagged rather
than quietly dropped.
- **GA announcements — the largest judgment call in this train.** Three
docs PRs remove preview markers:
[#39831](#39831)
(source versioning, `CREATE TABLE ... FROM SOURCE`),
[#39836](#39836)
(replacement materialized views),
[#39830](#39830)
(Kafka sink headers, noted as "GA in v26.47"). None changes code, so all
three were held; drafted wording for each is in `omitted.md`. Which are
actually GA in v26.47.0 and should be announced? Note v26.46.0 published
Kafka sink message attribution as *private preview* — if the header goes
GA here, this release should say so rather than repeat the preview
framing.
- **GCS blob read throughput**
([#39906](#39906),
held in `rc.2`): restores the read throughput GCS deployments lost when
[#39351](#39351) —
already shipped — forced HTTP/2 and collapsed per-request connections
into one. It is selected by `persist_blob_http_pool`, verified to
default to `off` at the `v26.47.0-rc.2` tag, and the author notes "the
default should eventually be 'GCS-only'". **Is `persist_blob_http_pool`
being set to `gcs` for v26.47.0?** If so this is a clear win to publish
for GCP users, who carry the regression today; drafted wording is in
rc.2's `omitted.md`.
- **`SUBSCRIBE` soft assertions**
([#37611](#37611),
[#39764](#39764), both
held): drafted as Bug Fixes, then pulled back. Both failures come from
`soft_assert_or_log!`, which panics only where soft assertions are
enabled and otherwise logs; neither PR says what a release build does.
Are soft assertions on in the environments customers run? If yes, both
are publishable as drafted in `omitted.md`.
- **Console Freshness page**
([#39340](#39340),
held): a complete new Monitoring > Freshness page, gated behind
`freshness-page-CNS164`, off in both Cloud and self-managed, and the
LaunchDarkly flag is not yet created. If it is turned on for v26.47.0,
this likely deserves a Feature heading.
- **PostgreSQL sources ingesting during a snapshot**
([#39419](#39419),
[#39479](#39479),
held): adding a large table to an existing source would no longer stall
the tables already hydrated — but
`storage_persist_sink_description_lookahead` is `Duration::ZERO` at the
tag. Publish if the lookahead is being enabled.
- **Coalesced materialized-view sink batches**
([#37591](#37591),
held): measures ~67% fewer blob writes and ~54% smaller state diffs, at
~10 ms added p50 latency on small writes, but
`compute_mv_sink_shared_batch_max_bytes` is `0` at the tag. Publish
(with the latency cost stated) if the staged rollout reaches customers
in this release.
- **MCP 2026-07-28 conformance checks**
([#39694](#39694),
[#39695](#39695),
held): `enable_mcp_protocol_2026_07_28` is `false` at the tag. If the
revision is being served, these belong together as one Improvement.
- **Refresh strategy columns**
([#39597](#39597),
published as an Improvement): this renames `aligned_to` →
`aligned_to_expression` and `at` → `at_expression` and changes their
type. It is a breaking change for anyone selecting them, in a schema
documented as unstable — it may warrant more prominence than a bullet.
- **Retained input after `ALTER SINK ... SET FROM`** (from `rc.3`'s
[#37653](#37653)): the
PR records a pre-existing gap it does not fix — after `SET FROM`, the
*old* upstream relation stays held, and therefore un-compactable, until
the sink is dropped or `environmentd` restarts. It is not something this
release changed, so it is out of these notes, but it is a real footgun
for the blue/green pattern that `sql/alter-sink.md` recommends `SET
FROM` for. Should the `ALTER SINK` page or the blue/green guide call it
out?
- **Operator upgrade shows pending changes** (from `rc.4`'s
[#40036](#40036)): the
probe-port change alters the generated `environmentd` StatefulSet, so
after upgrading the operator, existing environments report pending
changes and pick up the new port on their next rollout — they are
**not** restarted on their own. That is an upgrade consequence rather
than something the release changed, so it is out of the notes, but a
Self-Managed operator who cannot account for the pending-changes state
may read it as a problem. The v26.47 upgrade guide looks like the right
venue.

Snapshot drafts (with PR links) in mz-skills:
- rc.1:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.1/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.1/omitted.md)
- rc.2:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.2/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.2/omitted.md)
- rc.3:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.3/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.3/omitted.md)
- rc.4:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.4/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.4/omitted.md)
- rc.5:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.5/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.5/omitted.md)
- final:
[release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/final/release_notes.md)
/
[omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/final/omitted.md)

**Borderline PRs omitted:** rc.1: 14 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.1/omitted.md#borderline),
rc.2: 2 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.2/omitted.md#borderline),
rc.3: 0 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.3/omitted.md#borderline),
rc.4: 0 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.4/omitted.md#borderline),
rc.5: 1 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/rc.5/omitted.md#borderline),
final: 0 —
[omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.47.0/final/omitted.md#borderline)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Valerie Chiang <[email protected]>
Co-authored-by: Pranshu Maheshwari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants