Context
Split off from the trust-boundary review of PR #1765 (#1670). That PR adds MCP "extra" key passthrough and lands the modeled-field-name denylist (the actual exploit closure: keys named like command/url/type/env/headers can never be injected into an adapter config path). It intentionally keeps today's ergonomics: bare top-level unknown keys (e.g. oauth:) still pass through, at parity with the existing trust already given to command/url/env.
This follow-up tracks the stronger fail-closed posture the supply-chain lens recommended but which we deferred because it is a breaking authoring-contract change.
The change to design
Require MCP server extras to live under an explicit extra: block and drop + warn on bare unknown top-level keys, instead of preserving them.
- Today (and in all three current doc examples) extras are written as bare top-level keys:
mcp:
some-server:
oauth: { ... }
- Target contract:
mcp:
some-server:
extra:
oauth: { ... }
Why this is needs-design / breaking-change
- It changes a documented authoring surface -- bare
oauth: stops being honored. Existing apm.yml files break silently unless we stage it.
- Needs a deprecation path: ship one release that warns on bare unknown keys ("move under
extra:; bare passthrough will be removed in a future release") while still honoring them, then flip to drop-and-warn in a later release.
- Needs the 3 doc examples rewritten and a migration note.
Why deferred (not done in #1765)
The denylist already closes the concrete injection edge and is non-breaking, so it shipped now. Flipping the authoring contract is a separate posture decision that deserves its own ratification and a deprecation window, rather than riding inside a passthrough fix.
Also consider (from the same review, lower priority)
- Per-harness scoping of broadcast extras (clientId/callbackPort currently broadcast to all targets).
- A secret-shaped-value scan on extras (warn names only, never values -- current behavior).
to_dict collision debug-warn.
Refs #1670, PR #1765.
Context
Split off from the trust-boundary review of PR #1765 (#1670). That PR adds MCP "extra" key passthrough and lands the modeled-field-name denylist (the actual exploit closure: keys named like
command/url/type/env/headerscan never be injected into an adapter config path). It intentionally keeps today's ergonomics: bare top-level unknown keys (e.g.oauth:) still pass through, at parity with the existing trust already given tocommand/url/env.This follow-up tracks the stronger fail-closed posture the supply-chain lens recommended but which we deferred because it is a breaking authoring-contract change.
The change to design
Require MCP server extras to live under an explicit
extra:block and drop + warn on bare unknown top-level keys, instead of preserving them.Why this is
needs-design/breaking-changeoauth:stops being honored. Existingapm.ymlfiles break silently unless we stage it.extra:; bare passthrough will be removed in a future release") while still honoring them, then flip to drop-and-warn in a later release.Why deferred (not done in #1765)
The denylist already closes the concrete injection edge and is non-breaking, so it shipped now. Flipping the authoring contract is a separate posture decision that deserves its own ratification and a deprecation window, rather than riding inside a passthrough fix.
Also consider (from the same review, lower priority)
to_dictcollision debug-warn.Refs #1670, PR #1765.