Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions api/envoy/extensions/filters/http/mcp/v3/mcp.proto
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@ message Mcp {
// If unset (default), do not extract or inject baggage.
BaggagePropagationConfig propagate_baggage = 7;

// When true, reject requests that contain duplicate JSON keys at any
// nesting level. RFC 8259 Section 4 states that names within an object SHOULD be
// unique. Defaults to false (last-key-wins / last-win).
// When true, disallow requests that contain duplicate JSON keys at any
// nesting level. RFC 8259 Section 4 states that names within an object SHOULD
// be unique. Defaults to false (last-key-wins / last-win). If traffic mode is
// REJECT_NO_MCP, reject these requests; if PASS_THROUGH, log an error status
// but don't reject the request.
google.protobuf.BoolValue reject_duplicate_keys = 8;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``McpFilter`` no longer rejects any requests in ``PASS_THROUGH`` mode. Previously, the filter could reject requests in some cases when it failed to parse the request as an MCP request. Parse failures are now recorded in the filter's dynamic metadata, under ``passthrough_reason``.
1 change: 1 addition & 0 deletions source/extensions/filters/common/mcp/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ constexpr absl::string_view DEFAULT_SERVER_VERSION = "1.0.0";
constexpr absl::string_view IS_MCP_REQUEST = "is_mcp_request";
constexpr absl::string_view IS_EXCEEDING_LIMIT = "is_exceeding_limit";
constexpr absl::string_view STATUS = "status";
constexpr absl::string_view PASSTHROUGH_REASON = "passthrough_reason";

namespace StatusValues {
constexpr absl::string_view OK = "mcp_ok";
Expand Down
40 changes: 30 additions & 10 deletions source/extensions/filters/http/mcp/mcp_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,13 @@ Http::FilterDataStatus McpFilter::decodeData(Buffer::Instance& data, bool end_st

if (!status.ok()) {
config_->stats().invalid_json_.inc();
sendErrorReply("not a valid JSON", Filters::Common::Mcp::Status::NotJsonRpc);
return Http::FilterDataStatus::StopIterationNoBuffer;
if (shouldRejectRequest()) {
sendErrorReply("not a valid JSON", Filters::Common::Mcp::Status::NotJsonRpc);
return Http::FilterDataStatus::StopIterationNoBuffer;
} else {
passthrough_reason_ = Filters::Common::Mcp::Status::NotJsonRpc;
return completeParsing();
}
}

if (parser_->isParsingComplete()) {
Expand All @@ -398,9 +403,14 @@ Http::FilterDataStatus McpFilter::decodeData(Buffer::Instance& data, bool end_st
}
auto final_status = parser_->finishParse();
if (!final_status.ok()) {
if (truncated_by_limit && !shouldRejectRequest()) {
// PASS_THROUGH mode: size limit caused truncation, allow through.
ENVOY_LOG(debug, "size limit hit in PASS_THROUGH mode; proceeding with partial parse");
if (!shouldRejectRequest()) {
if (truncated_by_limit) {
ENVOY_LOG(debug, "size limit hit in PASS_THROUGH mode; proceeding with partial parse");
passthrough_reason_ = Filters::Common::Mcp::Status::BodyTooLarge;
} else {
ENVOY_LOG(debug, "parse error in PASS_THROUGH mode; proceeding");
passthrough_reason_ = Filters::Common::Mcp::Status::ParseError;
}
return completeParsing();
}
Filters::Common::Mcp::Status status = Filters::Common::Mcp::Status::ParseError;
Expand Down Expand Up @@ -450,11 +460,15 @@ Http::FilterDataStatus McpFilter::completeParsing() {

ENVOY_LOG(debug, "parsing complete: is_mcp={}, bytes_parsed={}", is_mcp_request_, bytes_parsed_);

// Check for duplicate keys — reject if configured.
// Check for duplicate keys — reject if configured and we are in reject mode.
if (parser_->hasDuplicateKeys() && rejectDuplicateKeys()) {
config_->stats().duplicate_keys_rejected_.inc();
sendErrorReply("duplicate JSON keys detected", Filters::Common::Mcp::Status::DuplicateKeys);
return Http::FilterDataStatus::StopIterationNoBuffer;
if (shouldRejectRequest()) {
config_->stats().duplicate_keys_rejected_.inc();
sendErrorReply("duplicate JSON keys detected", Filters::Common::Mcp::Status::DuplicateKeys);
return Http::FilterDataStatus::StopIterationNoBuffer;
} else if (!passthrough_reason_.has_value()) {
passthrough_reason_ = Filters::Common::Mcp::Status::DuplicateKeys;
}
}

if (!is_mcp_request_ && shouldRejectRequest()) {
Expand Down Expand Up @@ -497,7 +511,9 @@ Http::FilterDataStatus McpFilter::completeParsing() {
}

const bool has_metadata = !metadata.fields().empty();
const bool should_store_metadata = has_metadata || is_exceeding_limit_;
const bool should_store_metadata = has_metadata || is_exceeding_limit_ ||
status_ != Filters::Common::Mcp::Status::Ok ||
passthrough_reason_.has_value();

if (should_store_metadata) {
if (shouldStoreToFilterState()) {
Expand Down Expand Up @@ -532,6 +548,10 @@ void McpFilter::setDynamicMetadataStatus(Protobuf::Struct metadata) {
(*metadata.mutable_fields())[Filters::Common::Mcp::McpConstants::IS_EXCEEDING_LIMIT]
.set_bool_value(true);
}
if (passthrough_reason_.has_value()) {
(*metadata.mutable_fields())[Filters::Common::Mcp::McpConstants::PASSTHROUGH_REASON]
.set_string_value(std::string(statusToString(passthrough_reason_.value())));
}
decoder_callbacks_->streamInfo().setDynamicMetadata(config_->metadataNamespace(), metadata);
ENVOY_STREAM_LOG(debug, "MCP filter set dynamic metadata: {}", *decoder_callbacks_,
metadata.DebugString());
Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/http/mcp/mcp_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class McpFilter : public Http::PassThroughFilter, public Logger::Loggable<Logger
bool is_mcp_request_{false};
bool is_json_post_request_{false};
Filters::Common::Mcp::Status status_{Filters::Common::Mcp::Status::Ok};
std::optional<Filters::Common::Mcp::Status> passthrough_reason_;
};

} // namespace Mcp
Expand Down
42 changes: 36 additions & 6 deletions test/extensions/filters/http/mcp/mcp_filter_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ TEST_P(McpFilterIntegrationTest, ValidJsonRpcPostRequest) {
EXPECT_TRUE(metadata_verified);
}

// Test that an MCP request with malformed JSON is rejected with a 400.
TEST_P(McpFilterIntegrationTest, InvalidJsonBodyRejected) {
// Test that an MCP request with malformed JSON is passed through in PASS_THROUGH mode.
TEST_P(McpFilterIntegrationTest, InvalidJsonBodyPassedThrough) {
FakeAccessLogFactory factory;
Registry::InjectFactory<AccessLog::AccessLogInstanceFactory> factory_register(factory);

Expand All @@ -135,7 +135,8 @@ TEST_P(McpFilterIntegrationTest, InvalidJsonBodyRejected) {
if (it != dynamic_metadata.end()) {
Protobuf::Struct expected_metadata;
MessageUtil::loadFromJson(R"json({
"status": "mcp_parse_error",
"status": "mcp_ok",
"passthrough_reason": "mcp_parse_error",
"is_mcp_request": false
})json",
expected_metadata);
Expand Down Expand Up @@ -167,10 +168,12 @@ TEST_P(McpFilterIntegrationTest, InvalidJsonBodyRejected) {
{"content-type", "application/json"}},
R"({"jsonrpc": "2.0",)"); // Malformed JSON

waitForNextUpstreamRequest();
upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);

ASSERT_TRUE(response->waitForEndStream());
// The upstream should NOT receive a request because the filter sends a local reply.
EXPECT_FALSE(upstream_request_ != nullptr);
EXPECT_EQ("400", response->headers().getStatusValue());
EXPECT_TRUE(upstream_request_->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
EXPECT_TRUE(metadata_verified);
}

Expand Down Expand Up @@ -240,6 +243,31 @@ TEST_P(McpFilterIntegrationTest, WrongContentTypePostRequestIgnored) {
EXPECT_EQ("200", response->headers().getStatusValue());
}

// Test that invalid JSON is allowed in PASS_THROUGH mode and forwarded to upstream.
TEST_P(McpFilterIntegrationTest, PassThroughModeAllowsInvalidJson) {
initializeFilter();

codec_client_ = makeHttpConnection(lookupPort("http"));
// Invalid JSON (unclosed brace)
const std::string request_body = R"({"jsonrpc": "2.0", "method": "test")";
auto response = codec_client_->makeRequestWithBody(
Http::TestRequestHeaderMapImpl{{":method", "POST"},
{":path", "/"},
{":scheme", "http"},
{":authority", "host"},
{"accept", "application/json"},
{"accept", "text/event-stream"},
{"content-type", "application/json"}},
request_body);

waitForNextUpstreamRequest();
upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);

ASSERT_TRUE(response->waitForEndStream());
EXPECT_TRUE(upstream_request_->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
}

// Test no-MCP traffic is passed through without both accept headers
TEST_P(McpFilterIntegrationTest, NoAcceptHeaderReject) {
initializeFilter(R"EOF(
Expand Down Expand Up @@ -482,6 +510,8 @@ TEST_P(McpFilterIntegrationTest, PerRouteRejectDuplicateKeysOverride) {

envoy::extensions::filters::http::mcp::v3::McpOverride mcp_override;
mcp_override.set_reject_duplicate_keys(true);
mcp_override.set_traffic_mode(
envoy::extensions::filters::http::mcp::v3::Mcp::REJECT_NO_MCP);
std::ignore =
(*route->mutable_typed_per_filter_config())["envoy.filters.http.mcp"].PackFrom(
mcp_override);
Expand Down
Loading
Loading