OSPP2: Add local and federated MCP resources - #7669

Review: OSPP2: Add local and federated MCP resources

Verdict: APPROVE

This is a large but well-engineered feature addition that exposes MCP resources/list, resources/templates/list and resources/read through the existing Agent Gateway entry point, with an independent, deny-by-default authorization model. I read the core new classes, traced the full request path, ran the new test classes locally, and reviewed CI.

Security model (the strongest part of this PR)

The design consistently treats the resource URI as an identifier, never a destination, and backs it with multiple independent guards:

• No caller-selected endpoints. RemoteResourceDirectory.read resolves the requested URI to a registered Entry, then calls that entry's fixed Target.reads callback (bound to the configured server). The URI is never used to open a connection or read a file. The remote server endpoint itself is constrained by ManagedRemoteMcpCatalog.allowedEndpoints + RemoteTransportPolicy.endpoint, so even the remote hop is allowlisted.
• Canonical, fixed-scheme identifiers. AgentResourceNamespace.expose builds mcp-resource://<server>/<encoded> where server is validated [A-Za-z0-9_-]{1,48} and original is percent-encoded via UriUtils.encode; original() requires a round-trip-equal canonical form, rejecting encoding tricks. AgentResourceContent further requires the URI to be URI.create(...).isAbsolute() with no userinfo and no whitespace/control chars (no user:pass@host, no file: smuggling) — and explicitly "is never dereferenced".
• Independent, intersected authorization. AgentMcpExecutionContext computes allowedResources = ruleResources ∩ resourceGrants and allowedTools = ruleTools ∩ securityGrants. AgentMcpResourceDispatcher and RemoteResourceDirectory both re-check allowedResources.contains(name) and definition.matches(uri); ambiguous matches (size != 1) are rejected with 403. Admin rejects wildcard * and non-unique/blank grant values (AgentGatewayMcpConfig.strings), and the resources capability is only advertised when ruleResources is non-empty.
• Bounded everywhere. Local registry ≤ 16 providers; remote directory ≤ 16 entries; pagination capped at 4 pages with cycle detection; contents arrays capped at 16; URI/description ≤ 4096; config ≤ 16384 bytes with queue backpressure. AgentResourceContent.rewrite also verifies each returned content's URI equals the requested original (confused-deputy protection) and transforms it to the exposed identifier.
• Strict request parsing. AgentMcpRequestParser enforces JSON-RPC 2.0 shape, strict duplicate detection, max nesting depth 64, trailing-token rejection, strict UTF-8, body byte limit, and verifies mirrored Mcp-Name/Mcp-Method/MCP-Protocol-Version headers match the body (anti-header-injection). resources/read URI is validated as an absolute identifier before dispatch.

Correctness & lifecycle

• Hot updates preserve in-flight reads: ManagedRemoteMcpCatalog uses a single owned control-plane subscription, revision fencing, and a request-owned Snapshot; closeAsync drains before exit. AgentMcpDispatcher.dispatch is request-local, no retry, and maps SecurityException/CatalogUnavailableException to structured 403/503 errors.
• Remote resource discovery is disabled by default (resourcesEnabled flows into RemoteCatalogLifecycle), exactly as documented.

Verification

• Local: mvn -pl shenyu-plugin/shenyu-plugin-agent-gateway -am test for the resource-related suites → 170 tests, 0 failures, 0 errors (AgentMcpResourceIntegrationTest, RemoteResourceDirectoryTest, RemoteResourceLifecycleTest, RemoteResourceNetworkTest, AgentResourceContentTest, AgentResourceRegistryTest, AgentMcpRequestParserTest, AgentMcpHttpHandlerTest, plus existing suites). BUILD SUCCESS.
• CI: pr_build, Analyze (java), CodeQL, list-contract, native-sql (all 5 matrix entries), sql-matrix, scope, check-license-header, rat_check, and the storage e2e matrix (h2/mysql/opengauss/postgres) all pass. The only reds are the e2e aggregate and several e2e-case *sync-compose / logging-rocketmq jobs, which are orthogonal to this PR (Agent Gateway MCP resources; remote discovery off by default) and are known-infra-flaky — the e2e-http-sync-compose job log I inspected actually shows BUILD SUCCESS. None of the failing cases exercise agent-gateway resource code.

Non-blocking suggestions

1. Minor dead call: AgentResourceRegistry.read (line ~104) calls AgentResourceContent.text(uri, null, "") and discards the result — it serves only as a URI-validation side effect. Harmless, but a named validateUri helper would read more clearly.
2. Large feature surface: at +2346/−28 this is a substantial addition; the per-class encapsulation (registry / directory / namespace / content / identity) is clean and test-covered, so I see no structural concern, only a note that ongoing hardening (e.g. further e2e coverage for the federated read path) can follow as separate PRs.

No changes requested. The implementation is careful, security-conscious, and well tested.

Re-approving after the master merge dismissed the previous approval.

I checked the update rather than re-reading the whole PR: the only new commit is a merge of master, and every file this PR introduces is byte-identical to the version I approved (compared the blob shas at 71fe4acf/d80bda94/c831b431/58f16918 against the current head). The files that did change are the ones master touched in parallel — RELEASE-NOTES.md, the other agent-gateway sources, and AbstractShenyuPluginTest's setLogged rename — i.e. the merge brought them in, it did not rewrite this PR's content.

CI is green on the new head (52 pass / 3 skipping, no failures). No further changes requested from me.

Thanks for this contribution. The MCP resources support is cleanly layered: a new resource package (AgentResourceRegistry, AgentResourceDefinition, AgentResourceInvocation, AgentResourceContent, AgentResourceNamespace, AgentResourceProvider), an AgentMcpResourceDispatcher that maps resources/* MCP methods, capability advertisement gated on ruleResources being non-empty, and the remote-catalog path also validates resources before dispatch. The dispatcher routing (validate -> resources/ branch before route) reads correctly and the SecurityException -> 403 mapping is appropriate.

Notes (non-blocking):

• This PR and #8095 both modify AgentMcpDispatcher.java (resource handling / admission). Please coordinate the merge order and rebase so the two do not collide on the dispatcher.
• Consider adding a unit test that exercises AgentMcpResourceDispatcher.dispatch with an allowed vs denied resource to lock in the 403 path.

Approving. CI is green.