From 7e156c500d0a8637b1abe34f768dbd2b47a2c757 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 6 Jun 2026 21:50:39 +0900 Subject: [PATCH 1/3] Fix MCP audit argument key bounds (#3105) --- changelog.d/unreleased/3105.fixed.md | 18 +++++++++ src/CodeIndex/Mcp/AuditLogSink.cs | 7 ++++ src/CodeIndex/Mcp/McpServer.cs | 49 ++++++++++++++++------- tests/CodeIndex.Tests/McpAuditLogTests.cs | 10 +++-- tests/CodeIndex.Tests/McpServerTests.cs | 26 ++++++------ 5 files changed, 80 insertions(+), 30 deletions(-) create mode 100644 changelog.d/unreleased/3105.fixed.md diff --git a/changelog.d/unreleased/3105.fixed.md b/changelog.d/unreleased/3105.fixed.md new file mode 100644 index 0000000000..b2aff36aee --- /dev/null +++ b/changelog.d/unreleased/3105.fixed.md @@ -0,0 +1,18 @@ +--- +category: fixed +issues: + - 3105 +affected: + - src/CodeIndex/Mcp/McpServer.cs + - src/CodeIndex/Mcp/AuditLogSink.cs + - tests/CodeIndex.Tests/McpServerTests.cs + - tests/CodeIndex.Tests/McpAuditLogTests.cs +--- + +## English + +- **MCP audit argument metadata now reports bounded key truncation counters (#3105)** — audit and telemetry events cap argument key count and key display length with explicit omitted/truncated counters so large argument maps cannot silently inflate record metadata. + +## 日本語 + +- **MCP audit の引数メタデータが bounded なキー切り詰めカウンタを報告するようになりました (#3105)** — audit / telemetry event は引数キー数とキー表示長を上限内に収め、省略数と切り詰めキー数を明示するため、大きな引数 map がレコードメタデータを静かに膨らませないようになります。 diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index bfe8e8183d..7c71e888b3 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -39,6 +39,7 @@ internal sealed class AuditLogSink : IDisposable internal const int MaxArgValueStringChars = 512; internal const int MaxArgValuesSerializedBytes = 16 * 1024; internal const int MaxAuditArgumentCount = 64; + internal const int MaxAuditArgumentKeyChars = McpBoundedText.MaxDiagnosticDisplayChars; internal const int MaxRequestIdChars = 256; internal const int MaxSerializedEventBytes = 64 * 1024; @@ -323,6 +324,10 @@ private static string SerializeEventCore(AuditEvent evt, bool includeValues) jw.WriteStringValue(reason); jw.WriteEndArray(); } + if (evt.ArgKeysOmittedCount > 0) + jw.WriteNumber("arg_keys_omitted_count", evt.ArgKeysOmittedCount); + if (evt.ArgKeyNamesTruncatedCount > 0) + jw.WriteNumber("arg_key_names_truncated_count", evt.ArgKeyNamesTruncatedCount); if (includeValues && evt.ArgValues is { } values) { @@ -657,6 +662,8 @@ internal sealed record AuditEvent( IReadOnlyList>? ArgKeyLengths = null, bool ArgKeysTruncated = false, IReadOnlyList? ArgKeyTruncationReasons = null, + int ArgKeysOmittedCount = 0, + int ArgKeyNamesTruncatedCount = 0, bool ArgValuesRedacted = false, bool ArgValuesTruncated = false, IReadOnlyList? ArgValueTruncationReasons = null, diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index de96cb13b3..b308ba8bcf 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2621,7 +2621,9 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo out _, out _, out var argKeysTruncated, - out var argKeyTruncationReasons); + out var argKeyTruncationReasons, + out var argKeysOmittedCount, + out var argKeyNamesTruncatedCount); var toolDisplay = BoundToolNameForDisplay(toolName); var argsObject = new JsonObject(); foreach (var pair in argLengths) @@ -2643,7 +2645,7 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo ["arg_lengths"] = argsObject, }; toolDisplay.AddMetadata(evt, "tool"); - AddArgKeyMetadata(evt, argKeyLengths); + AddArgKeyMetadata(evt, argKeyLengths, argKeysOmittedCount, argKeyNamesTruncatedCount); if (argKeysTruncated) evt["arg_keys_truncated"] = true; if (argKeyTruncationReasons.Count > 0) @@ -2786,7 +2788,9 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod out var argValueTruncationReasons, out var argValuesSerializedBytes, out var argKeysTruncated, - out var argKeyTruncationReasons); + out var argKeyTruncationReasons, + out var argKeysOmittedCount, + out var argKeyNamesTruncatedCount); var toolDisplay = BoundToolNameForDisplay(toolName); var requestId = SerializeRequestId(id); BoundedMcpText? requestIdDisplay = requestId is null @@ -2810,6 +2814,8 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ArgKeyLengths: argKeyLengths, ArgKeysTruncated: argKeysTruncated, ArgKeyTruncationReasons: argKeyTruncationReasons, + ArgKeysOmittedCount: argKeysOmittedCount, + ArgKeyNamesTruncatedCount: argKeyNamesTruncatedCount, ArgValuesRedacted: argValuesRedacted, ArgValuesTruncated: argValuesTruncated, ArgValueTruncationReasons: argValueTruncationReasons, @@ -2897,7 +2903,7 @@ internal static (int Code, string? Type) ExtractErrorCode(JsonNode response) /// internal static (IReadOnlyList Keys, IReadOnlyList> Lengths, IReadOnlyList> KeyLengths, JsonNode? ValuesEcho) SanitizeArgs(JsonNode? args, bool includeValues) - => SanitizeArgs(args, includeValues, out _, out _, out _, out _, out _, out _); + => SanitizeArgs(args, includeValues, out _, out _, out _, out _, out _, out _, out _, out _); private static (IReadOnlyList Keys, IReadOnlyList> Lengths, IReadOnlyList> KeyLengths, JsonNode? ValuesEcho) SanitizeArgs( @@ -2908,13 +2914,17 @@ private static (IReadOnlyList Keys, IReadOnlyList argValueTruncationReasons, out int? argValuesSerializedBytes, out bool argKeysTruncated, - out IReadOnlyList argKeyTruncationReasons) + out IReadOnlyList argKeyTruncationReasons, + out int argKeysOmittedCount, + out int argKeyNamesTruncatedCount) { argValuesRedacted = false; argValuesTruncated = false; argValueTruncationReasons = Array.Empty(); argValuesSerializedBytes = null; argKeysTruncated = false; + argKeysOmittedCount = 0; + argKeyNamesTruncatedCount = 0; var argKeyReasons = new List(); argKeyTruncationReasons = argKeyReasons; if (args is not JsonObject argsObj) @@ -2933,11 +2943,12 @@ private static (IReadOnlyList Keys, IReadOnlyList= AuditLogSink.MaxAuditArgumentCount) { argKeysTruncated = true; + argKeysOmittedCount = argsObj.Count - argumentCount; AddUniqueReason(argKeyReasons, "arg_key_count_limit"); break; } - var keyDisplay = McpBoundedText.ForDisplay(key); + var keyDisplay = McpBoundedText.ForDisplay(key, AuditLogSink.MaxAuditArgumentKeyChars); var displayKey = MakeUniqueArgumentDisplayKey(key, keyDisplay, usedKeys); keys.Add(displayKey); lengths.Add(new KeyValuePair(displayKey, AuditLogSink.MeasureArgLength(value))); @@ -2945,6 +2956,7 @@ private static (IReadOnlyList Keys, IReadOnlyList(displayKey, keyDisplay.OriginalLength)); argKeysTruncated = true; + argKeyNamesTruncatedCount++; AddUniqueReason(argKeyReasons, "arg_key_length_limit"); } if (echoObject is not null && !argValueBudgetExhausted) @@ -3021,15 +3033,24 @@ private static string ShortStableHash(string value) return Convert.ToHexString(bytes.AsSpan(0, 4)).ToLowerInvariant(); } - private static void AddArgKeyMetadata(JsonObject target, IReadOnlyList> argKeyLengths) + private static void AddArgKeyMetadata( + JsonObject target, + IReadOnlyList> argKeyLengths, + int argKeysOmittedCount, + int argKeyNamesTruncatedCount) { - if (argKeyLengths.Count == 0) - return; - var lengths = new JsonObject(); - foreach (var pair in argKeyLengths) - lengths[pair.Key] = pair.Value; - target["arg_key_lengths"] = lengths; - target["arg_keys_truncated"] = true; + if (argKeyLengths.Count > 0) + { + var lengths = new JsonObject(); + foreach (var pair in argKeyLengths) + lengths[pair.Key] = pair.Value; + target["arg_key_lengths"] = lengths; + target["arg_keys_truncated"] = true; + } + if (argKeysOmittedCount > 0) + target["arg_keys_omitted_count"] = argKeysOmittedCount; + if (argKeyNamesTruncatedCount > 0) + target["arg_key_names_truncated_count"] = argKeyNamesTruncatedCount; } private static string? SerializeRequestId(JsonNode? id) diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index 94e0ee9ba9..70d746efee 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -262,12 +262,12 @@ public void ToolsCall_UnknownTool_TruncatesAuditToolName_Issue3118() } [Fact] - public void ToolsCall_IncludeValues_TruncatesArgumentKeysInAuditValues_Issue3117() + public void ToolsCall_IncludeValues_TruncatesArgumentKeysInAuditValues_Issue3117_Issue3105() { using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); using var server = CreateServer(sink); - var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); - var display = McpBoundedText.ForDisplay(argumentName); + var argumentName = new string('k', AuditLogSink.MaxAuditArgumentKeyChars + 25); + var display = McpBoundedText.ForDisplay(argumentName, AuditLogSink.MaxAuditArgumentKeyChars); var request = new JsonObject { ["jsonrpc"] = "2.0", @@ -294,6 +294,7 @@ public void ToolsCall_IncludeValues_TruncatesArgumentKeysInAuditValues_Issue3117 Assert.Equal(display.Text, record.GetProperty("arg_keys")[1].GetString()); Assert.Equal(argumentName.Length, record.GetProperty("arg_key_lengths").GetProperty(display.Text).GetInt32()); Assert.True(record.GetProperty("arg_keys_truncated").GetBoolean()); + Assert.Equal(1, record.GetProperty("arg_key_names_truncated_count").GetInt32()); Assert.True(record.GetProperty("arg_values").TryGetProperty(display.Text, out _)); } @@ -432,7 +433,7 @@ public void ToolsCall_IncludeValues_ChargesTopLevelArgumentKeysToValueBudget_Iss } [Fact] - public void ToolsCall_CapsAuditArgumentKeyCount_Issue3237() + public void ToolsCall_CapsAuditArgumentKeyCount_Issue3237_Issue3105() { using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); using var server = CreateServer(sink); @@ -458,6 +459,7 @@ public void ToolsCall_CapsAuditArgumentKeyCount_Issue3237() Assert.True(record.GetProperty("arg_keys_truncated").GetBoolean()); Assert.Contains(record.GetProperty("arg_key_truncation_reasons").EnumerateArray(), reason => reason.GetString() == "arg_key_count_limit"); + Assert.Equal(3, record.GetProperty("arg_keys_omitted_count").GetInt32()); Assert.False(record.GetProperty("arg_values").TryGetProperty( $"arg{AuditLogSink.MaxAuditArgumentCount.ToString(CultureInfo.InvariantCulture)}", out _)); } diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 226491f0fe..3f5388d269 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -618,12 +618,12 @@ await Task.Run(() => } [Fact] - public async Task ProcessLineAsync_UnknownArgumentName_TruncatesTelemetryKeyMetadata_Issue3117() + public async Task ProcessLineAsync_UnknownArgumentName_TruncatesTelemetryKeyMetadata_Issue3117_Issue3105() { using var writer = new StringWriter(); using var error = new StringWriter(); - var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); - var display = McpBoundedText.ForDisplay(argumentName); + var argumentName = new string('k', AuditLogSink.MaxAuditArgumentKeyChars + 25); + var display = McpBoundedText.ForDisplay(argumentName, AuditLogSink.MaxAuditArgumentKeyChars); var request = new JsonObject { ["jsonrpc"] = "2.0", @@ -671,10 +671,11 @@ await Task.Run(() => Assert.Contains(root.GetProperty("arg_keys").EnumerateArray(), key => key.GetString() == display.Text); Assert.Equal(argumentName.Length, root.GetProperty("arg_key_lengths").GetProperty(display.Text).GetInt32()); Assert.True(root.GetProperty("arg_keys_truncated").GetBoolean()); + Assert.Equal(1, root.GetProperty("arg_key_names_truncated_count").GetInt32()); } [Fact] - public async Task ProcessLineAsync_CapsTelemetryArgumentKeyCount_Issue3237() + public async Task ProcessLineAsync_CapsTelemetryArgumentKeyCount_Issue3237_Issue3105() { using var writer = new StringWriter(); using var error = new StringWriter(); @@ -722,6 +723,7 @@ await Task.Run(() => Assert.True(root.GetProperty("arg_keys_truncated").GetBoolean()); Assert.Contains(root.GetProperty("arg_key_truncation_reasons").EnumerateArray(), reason => reason.GetString() == "arg_key_count_limit"); + Assert.Equal(3, root.GetProperty("arg_keys_omitted_count").GetInt32()); Assert.DoesNotContain(root.GetProperty("arg_keys").EnumerateArray(), key => key.GetString() == $"arg{AuditLogSink.MaxAuditArgumentCount}"); } @@ -8755,10 +8757,10 @@ public void ToolsCall_UnknownToolName_TruncatesMetricsLabels_Issue3118() } [Fact] - public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117() + public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117_Issue3105() { - var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 1); - var display = McpBoundedText.ForDisplay(argumentName); + var argumentName = new string('k', AuditLogSink.MaxAuditArgumentKeyChars + 1); + var display = McpBoundedText.ForDisplay(argumentName, AuditLogSink.MaxAuditArgumentKeyChars); var args = new JsonObject { [argumentName] = "value", @@ -8777,10 +8779,10 @@ public void SanitizeArgs_TruncatesArgumentKeysForAuditAndTelemetry_Issue3117() } [Fact] - public void SanitizeArgs_TruncatesArgumentKeysInValuesEcho_Issue3117() + public void SanitizeArgs_TruncatesArgumentKeysInValuesEcho_Issue3117_Issue3105() { - var argumentName = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); - var display = McpBoundedText.ForDisplay(argumentName); + var argumentName = new string('k', AuditLogSink.MaxAuditArgumentKeyChars + 25); + var display = McpBoundedText.ForDisplay(argumentName, AuditLogSink.MaxAuditArgumentKeyChars); var args = new JsonObject { [argumentName] = "value", @@ -8798,9 +8800,9 @@ public void SanitizeArgs_TruncatesArgumentKeysInValuesEcho_Issue3117() } [Fact] - public void SanitizeArgs_DisambiguatesCollidingTruncatedKeys_Issue3117() + public void SanitizeArgs_DisambiguatesCollidingTruncatedKeys_Issue3117_Issue3105() { - var sharedPrefix = new string('c', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var sharedPrefix = new string('c', AuditLogSink.MaxAuditArgumentKeyChars + 25); var firstArgumentName = sharedPrefix + "a"; var secondArgumentName = sharedPrefix + "b"; var args = new JsonObject From ad4aab5d84cb75f9e9f899ce7fee5fe156b1846d Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 6 Jun 2026 21:56:40 +0900 Subject: [PATCH 2/3] Cap MCP client response payloads (#3098) --- changelog.d/unreleased/3098.security.md | 16 +++++ src/CodeIndex/Mcp/McpServer.cs | 60 +++++++++++++++++- tests/CodeIndex.Tests/McpServerTests.cs | 81 +++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 changelog.d/unreleased/3098.security.md diff --git a/changelog.d/unreleased/3098.security.md b/changelog.d/unreleased/3098.security.md new file mode 100644 index 0000000000..87e660a8c1 --- /dev/null +++ b/changelog.d/unreleased/3098.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3098 +affected: + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **MCP out-of-band client responses are capped before cloning (#3098)** — client-supplied result and error payloads are measured with a bounded JSON writer before they are retained, and oversized responses are rejected with payload-free diagnostics. + +## 日本語 + +- **MCP の out-of-band client response を clone 前に上限チェックするようになりました (#3098)** — client supplied な result / error payload は保持前に bounded JSON writer で測定され、過大な応答は payload を含まない診断で拒否されます。 diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index b308ba8bcf..a4ef886116 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -165,6 +165,7 @@ public partial class McpServer : IDisposable internal const int MaxLineByteLength = 1_048_576; internal const int DefaultMaxResponseBytes = 10 * 1024 * 1024; internal const int MaxConfiguredResponseBytes = 64 * 1024 * 1024; + internal const int MaxClientResponseJsonBytes = 1 * 1024 * 1024; internal const int MaxMcpPaginationOffset = 10_000; internal const double MinKeepAliveIntervalSeconds = 1.0; internal const double MaxKeepAliveIntervalSeconds = 300.0; @@ -978,16 +979,40 @@ private bool TryCompletePendingClientRequest(JsonNode request) return false; if (obj.TryGetPropertyValue("error", out var error) && error is not null) - pending.TrySetException(new InvalidOperationException(error.ToJsonString(_jsonOptions))); + { + if (!TrySerializeClientResponseError(error, out var serializedError, out var errorBytes)) + { + DeferFrameLog(BuildClientResponseTooLargeLog("error", errorBytes)); + pending.TrySetException(new InvalidOperationException(BuildClientResponseTooLargeMessage(errorBytes))); + } + else + { + pending.TrySetException(new InvalidOperationException(serializedError)); + } + } + else if (!TryCloneClientResponsePayload(obj["result"], out var resultClone, out var resultBytes)) + { + DeferFrameLog(BuildClientResponseTooLargeLog("result", resultBytes)); + pending.TrySetException(new InvalidOperationException(BuildClientResponseTooLargeMessage(resultBytes))); + } else - pending.TrySetResult(obj["result"]?.DeepClone()); + { + pending.TrySetResult(resultClone); + } return true; } private async Task SendClientRequestAsync(string method, JsonObject? @params, CancellationToken cancellationToken) { if (ClientRequestHandlerForTests is { } handler) - return handler(method, @params)?.DeepClone(); + { + if (!TryCloneClientResponsePayload(handler(method, @params), out var handlerClone, out var handlerBytes)) + { + DeferFrameLog(BuildClientResponseTooLargeLog("result", handlerBytes)); + return null; + } + return handlerClone; + } var writer = _currentOutOfBandFrameWriter.Value; if (writer is null || !_canAwaitClientResponses.Value) @@ -1036,6 +1061,29 @@ private bool TryCompletePendingClientRequest(JsonNode request) } } + internal bool TryCloneClientResponsePayloadForTests(JsonNode? payload, out JsonNode? clone, out int bytesWritten) + => TryCloneClientResponsePayload(payload, out clone, out bytesWritten); + + internal bool TrySerializeClientResponseErrorForTests(JsonNode error, out string? serialized, out int bytesWritten) + => TrySerializeClientResponseError(error, out serialized, out bytesWritten); + + private bool TryCloneClientResponsePayload(JsonNode? payload, out JsonNode? clone, out int bytesWritten) + { + clone = null; + bytesWritten = 0; + if (payload is null) + return true; + + if (!TryMeasureJsonUtf8BytesWithinLimit(payload, _jsonOptions, MaxClientResponseJsonBytes, out bytesWritten)) + return false; + + clone = payload.DeepClone(); + return true; + } + + private bool TrySerializeClientResponseError(JsonNode error, out string? serialized, out int bytesWritten) + => TrySerializeJsonNodeWithinByteLimit(error, _jsonOptions, MaxClientResponseJsonBytes, captureSerialized: true, out serialized, out bytesWritten); + private static string? TryGetMcpTraceParent(JsonNode request) { if (request is not JsonObject obj || @@ -3103,6 +3151,12 @@ internal static string BuildResponseWriteErrorLog(string detail) => internal static string BuildToolErrorLog(string toolName, string detail) => $"[cdidx-mcp] Tool error ({BoundToolNameForDisplay(toolName).Text}): {detail}. Fix the tool arguments, refresh the index if needed, then retry."; + internal static string BuildClientResponseTooLargeLog(string member, int bytesWritten) => + $"[cdidx-mcp] Client response {member} exceeded the server byte limit ({bytesWritten} > {MaxClientResponseJsonBytes}); rejecting without retaining the payload."; + + private static string BuildClientResponseTooLargeMessage(int bytesWritten) => + $"MCP client response exceeded the server byte limit ({bytesWritten} > {MaxClientResponseJsonBytes})."; + // Stderr log emitted when the rate limiter denies a tool call. Mirrors the JSON-RPC // `-32000` payload (tool + caller + retry_after_ms) so operators tailing the MCP log // can correlate spikes with the structured error returned on the wire (#1560). diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 3f5388d269..9e370f80b1 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -6401,6 +6401,42 @@ public void ResponseLimitSerializer_StopsBeforeFullStringMaterialization_Issue28 Assert.True(bytesWritten < 10_000); } + [Fact] + public void ClientResponsePayload_RejectsOversizedResultBeforeClone_Issue3098() + { + var payload = new JsonObject + { + ["value"] = new string('x', McpServer.MaxClientResponseJsonBytes + 1), + }; + + var withinLimit = _server.TryCloneClientResponsePayloadForTests(payload, out var clone, out var bytesWritten); + + Assert.False(withinLimit); + Assert.Null(clone); + Assert.True(bytesWritten > McpServer.MaxClientResponseJsonBytes); + Assert.True(bytesWritten < McpServer.MaxClientResponseJsonBytes + 100); + } + + [Fact] + public void ClientResponsePayload_RejectsOversizedErrorBeforeMessageMaterialization_Issue3098() + { + var oversized = new string('e', McpServer.MaxClientResponseJsonBytes + 1); + var error = new JsonObject + { + ["code"] = -32000, + ["message"] = oversized, + }; + + var withinLimit = _server.TrySerializeClientResponseErrorForTests(error, out var serialized, out var bytesWritten); + var log = McpServer.BuildClientResponseTooLargeLog("error", bytesWritten); + + Assert.False(withinLimit); + Assert.Null(serialized); + Assert.True(bytesWritten > McpServer.MaxClientResponseJsonBytes); + Assert.True(bytesWritten < McpServer.MaxClientResponseJsonBytes + 100); + Assert.DoesNotContain(oversized, log, StringComparison.Ordinal); + } + [Fact] public void ResponseLimitSerializer_ReturnsCapturedJsonWhenWithinLimit_Issue2860() { @@ -11859,6 +11895,51 @@ public void SuggestImprovement_WhenSamplingResponseIsTooLarge_IgnoresSampledMeta Assert.Null(stored.SampledTags); } + [Fact] + public void SuggestImprovement_WhenSamplingClientResponseJsonIsTooLarge_IgnoresSampledMetadata_Issue3098() + { + _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"capabilities":{"sampling":{}}}}""")!); + _server.ClientRequestHandlerForTests = (method, _) => + { + Assert.Equal("sampling/createMessage", method); + return new JsonObject + { + ["content"] = new JsonObject + { + ["type"] = "text", + ["text"] = new string('A', McpServer.MaxClientResponseJsonBytes + 1), + }, + }; + }; + var uniqueDesc = $"Oversized sampling client response regression {Guid.NewGuid():N}"; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "suggest_improvement", + ["arguments"] = new JsonObject + { + ["category"] = "other", + ["description"] = uniqueDesc, + } + } + }; + + var response = _server.HandleMessage(request)!; + + var structured = response["result"]!["structuredContent"]!; + Assert.Equal("recorded", structured["status"]!.GetValue()); + Assert.Null(structured["sampled_title"]); + var stored = new SuggestionStore(Path.GetDirectoryName(_dbPath)!, Path.GetFileNameWithoutExtension(_dbPath)).LoadAll() + .Single(s => s.Description == uniqueDesc); + Assert.Null(stored.SampledTitle); + Assert.Null(stored.SampledTags); + } + [Fact] public void SuggestImprovement_WhenSamplingResponseJsonIsTooDeep_IgnoresSampledMetadata() { From 9e57ba47cbf578f92970006934fedbc29f6078a7 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 6 Jun 2026 22:01:29 +0900 Subject: [PATCH 3/3] Cap MCP audit log event serialization (#3107) --- changelog.d/unreleased/3107.fixed.md | 16 + src/CodeIndex/Mcp/AuditLogSink.cs | 344 +++++++++++++++------ tests/CodeIndex.Tests/AuditLogSinkTests.cs | 44 ++- 3 files changed, 306 insertions(+), 98 deletions(-) create mode 100644 changelog.d/unreleased/3107.fixed.md diff --git a/changelog.d/unreleased/3107.fixed.md b/changelog.d/unreleased/3107.fixed.md new file mode 100644 index 0000000000..9b7889ec93 --- /dev/null +++ b/changelog.d/unreleased/3107.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3107 +affected: + - src/CodeIndex/Mcp/AuditLogSink.cs + - tests/CodeIndex.Tests/AuditLogSinkTests.cs +--- + +## English + +- **MCP audit log events are now serialized through a per-record byte cap (#3107)** — oversized audit records are reduced with an explicit `event_truncated` marker before writing, preventing one event from bypassing rotation by forcing a large serialized line. + +## 日本語 + +- **MCP audit log event をレコード単位の byte 上限内で serialize するようになりました (#3107)** — 過大な audit record は書き込み前に明示的な `event_truncated` marker 付きで縮小され、単一eventが大きなserialized lineを作ってrotationを迂回することを防ぎます。 diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index 7c71e888b3..bf5747a62a 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -220,8 +220,8 @@ public void Dispose() internal static string SerializeEvent(AuditEvent evt, bool includeValues) { - var serialized = SerializeEventCore(evt, includeValues); - if (Encoding.UTF8.GetByteCount(serialized) <= MaxSerializedEventBytes) + evt = BoundEventScalarFields(evt); + if (TrySerializeEventCore(evt, includeValues, out var serialized)) return serialized; if (includeValues && evt.ArgValues is not null) @@ -232,9 +232,10 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) ArgValuesTruncated = true, ArgValueTruncationReasons = AppendTruncationReason(evt.ArgValueTruncationReasons, "event_size_limit"), ArgValuesSerializedBytes = null, + EventTruncated = true, + EventTruncationReasons = AppendTruncationReason(evt.EventTruncationReasons, "event_size_limit"), }; - serialized = SerializeEventCore(fallback, includeValues: false); - if (Encoding.UTF8.GetByteCount(serialized) <= MaxSerializedEventBytes) + if (TrySerializeEventCore(fallback, includeValues: false, out serialized)) return serialized; evt = fallback; @@ -247,121 +248,171 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) ArgKeyLengths = null, ArgKeysTruncated = true, ArgKeyTruncationReasons = AppendTruncationReason(evt.ArgKeyTruncationReasons, "event_size_limit"), + ArgKeysOmittedCount = Math.Max(evt.ArgKeysOmittedCount, evt.ArgKeys.Count), + EventTruncated = true, + EventTruncationReasons = AppendTruncationReason(evt.EventTruncationReasons, "event_size_limit"), }; - serialized = SerializeEventCore(compact, includeValues && compact.ArgValues is not null); - if (Encoding.UTF8.GetByteCount(serialized) <= MaxSerializedEventBytes) + if (TrySerializeEventCore(compact, includeValues && compact.ArgValues is not null, out serialized)) return serialized; - return serialized; + var minimal = compact with + { + CallerName = null, + CallerVersion = null, + RequestId = null, + ResultCount = null, + ErrorType = null, + ArgKeys = Array.Empty(), + ArgLengths = Array.Empty>(), + ArgValues = null, + ArgKeyLengths = null, + ArgValuesSerializedBytes = null, + ArgValuesTruncated = compact.ArgValues is not null || compact.ArgValuesTruncated, + ArgValueTruncationReasons = AppendTruncationReason(compact.ArgValueTruncationReasons, "event_size_limit"), + EventTruncated = true, + EventTruncationReasons = AppendTruncationReason(compact.EventTruncationReasons, "event_size_limit"), + }; + if (TrySerializeEventCore(minimal, includeValues: false, out serialized)) + return serialized; + + return "{\"event_truncated\":true,\"event_truncation_reasons\":[\"event_size_limit\"]}"; } - private static string SerializeEventCore(AuditEvent evt, bool includeValues) + private static bool TrySerializeEventCore(AuditEvent evt, bool includeValues, out string serialized) { - using var buffer = new MemoryStream(); - using (var jw = new Utf8JsonWriter(buffer, new JsonWriterOptions + serialized = string.Empty; + using var buffer = new BoundedAuditEventUtf8Stream(MaxSerializedEventBytes); + try { - Indented = false, - // Mirror MetricsSink: local-only JSONL stays human readable in tail/grep. - // 出力は local 限定なので tail/grep で読める relaxed encoder を使う。 - Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, - })) + using (var jw = new Utf8JsonWriter(buffer, new JsonWriterOptions + { + Indented = false, + // Mirror MetricsSink: local-only JSONL stays human readable in tail/grep. + // 出力は local 限定なので tail/grep で読める relaxed encoder を使う。 + Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, + })) + { + WriteEventCore(jw, evt, includeValues); + } + serialized = buffer.GetCapturedString(); + return true; + } + catch (AuditEventByteLimitExceededException) { - jw.WriteStartObject(); - jw.WriteString("timestamp", evt.Timestamp.ToString("O", CultureInfo.InvariantCulture)); - jw.WriteString("tool", evt.Tool); - if (evt.ToolLength is { } toolLength) - jw.WriteNumber("tool_length", toolLength); - if (evt.ToolTruncated) - jw.WriteBoolean("tool_truncated", true); - if (evt.CallerName is { } caller) - jw.WriteString("caller", caller); - if (evt.CallerNameLength is { } callerLength) - jw.WriteNumber("caller_length", callerLength); - if (evt.CallerNameTruncated) - jw.WriteBoolean("caller_truncated", true); - if (evt.CallerVersion is { } callerVersion) - jw.WriteString("caller_version", callerVersion); - if (evt.CallerVersionLength is { } callerVersionLength) - jw.WriteNumber("caller_version_length", callerVersionLength); - if (evt.CallerVersionTruncated) - jw.WriteBoolean("caller_version_truncated", true); - if (evt.RequestId is { } reqId) - jw.WriteString("request_id", reqId); - if (evt.RequestIdLength is { } requestIdLength) - jw.WriteNumber("request_id_length", requestIdLength); - if (evt.RequestIdTruncated) - jw.WriteBoolean("request_id_truncated", true); - - jw.WritePropertyName("arg_keys"); - jw.WriteStartArray(); - foreach (var key in evt.ArgKeys) - jw.WriteStringValue(key); - jw.WriteEndArray(); + return false; + } + } - jw.WritePropertyName("arg_lengths"); + private static void WriteEventCore(Utf8JsonWriter jw, AuditEvent evt, bool includeValues) + { + jw.WriteStartObject(); + jw.WriteString("timestamp", evt.Timestamp.ToString("O", CultureInfo.InvariantCulture)); + jw.WriteString("tool", evt.Tool); + if (evt.ToolLength is { } toolLength) + jw.WriteNumber("tool_length", toolLength); + if (evt.ToolTruncated) + jw.WriteBoolean("tool_truncated", true); + if (evt.CallerName is { } caller) + jw.WriteString("caller", caller); + if (evt.CallerNameLength is { } callerLength) + jw.WriteNumber("caller_length", callerLength); + if (evt.CallerNameTruncated) + jw.WriteBoolean("caller_truncated", true); + if (evt.CallerVersion is { } callerVersion) + jw.WriteString("caller_version", callerVersion); + if (evt.CallerVersionLength is { } callerVersionLength) + jw.WriteNumber("caller_version_length", callerVersionLength); + if (evt.CallerVersionTruncated) + jw.WriteBoolean("caller_version_truncated", true); + if (evt.RequestId is { } reqId) + jw.WriteString("request_id", reqId); + if (evt.RequestIdLength is { } requestIdLength) + jw.WriteNumber("request_id_length", requestIdLength); + if (evt.RequestIdTruncated) + jw.WriteBoolean("request_id_truncated", true); + + jw.WritePropertyName("arg_keys"); + jw.WriteStartArray(); + foreach (var key in evt.ArgKeys) + jw.WriteStringValue(key); + jw.WriteEndArray(); + + jw.WritePropertyName("arg_lengths"); + jw.WriteStartObject(); + foreach (var kv in evt.ArgLengths) + jw.WriteNumber(kv.Key, kv.Value); + jw.WriteEndObject(); + + var argKeysTruncated = evt.ArgKeysTruncated; + if (evt.ArgKeyLengths is { Count: > 0 } argKeyLengths) + { + jw.WritePropertyName("arg_key_lengths"); jw.WriteStartObject(); - foreach (var kv in evt.ArgLengths) + foreach (var kv in argKeyLengths) jw.WriteNumber(kv.Key, kv.Value); jw.WriteEndObject(); + argKeysTruncated = true; + } + if (argKeysTruncated) + jw.WriteBoolean("arg_keys_truncated", true); + if (evt.ArgKeyTruncationReasons is { Count: > 0 } argKeyReasons) + { + jw.WritePropertyName("arg_key_truncation_reasons"); + jw.WriteStartArray(); + foreach (var reason in argKeyReasons) + jw.WriteStringValue(reason); + jw.WriteEndArray(); + } + if (evt.ArgKeysOmittedCount > 0) + jw.WriteNumber("arg_keys_omitted_count", evt.ArgKeysOmittedCount); + if (evt.ArgKeyNamesTruncatedCount > 0) + jw.WriteNumber("arg_key_names_truncated_count", evt.ArgKeyNamesTruncatedCount); - var argKeysTruncated = evt.ArgKeysTruncated; - if (evt.ArgKeyLengths is { Count: > 0 } argKeyLengths) - { - jw.WritePropertyName("arg_key_lengths"); - jw.WriteStartObject(); - foreach (var kv in argKeyLengths) - jw.WriteNumber(kv.Key, kv.Value); - jw.WriteEndObject(); - argKeysTruncated = true; - } - if (argKeysTruncated) - jw.WriteBoolean("arg_keys_truncated", true); - if (evt.ArgKeyTruncationReasons is { Count: > 0 } argKeyReasons) + if (includeValues && evt.ArgValues is { } values) + { + jw.WritePropertyName("arg_values"); + values.WriteTo(jw); + } + if (evt.ArgValuesRedacted) + jw.WriteBoolean("arg_values_redacted", true); + if (evt.ArgValuesTruncated) + { + jw.WriteBoolean("arg_values_truncated", true); + jw.WriteNumber("arg_values_max_bytes", MaxArgValuesSerializedBytes); + if (evt.ArgValuesSerializedBytes is { } argValuesSerializedBytes) + jw.WriteNumber("arg_values_serialized_bytes", argValuesSerializedBytes); + if (evt.ArgValueTruncationReasons is { Count: > 0 } reasons) { - jw.WritePropertyName("arg_key_truncation_reasons"); + jw.WritePropertyName("arg_values_truncation_reasons"); jw.WriteStartArray(); - foreach (var reason in argKeyReasons) + foreach (var reason in reasons) jw.WriteStringValue(reason); jw.WriteEndArray(); } - if (evt.ArgKeysOmittedCount > 0) - jw.WriteNumber("arg_keys_omitted_count", evt.ArgKeysOmittedCount); - if (evt.ArgKeyNamesTruncatedCount > 0) - jw.WriteNumber("arg_key_names_truncated_count", evt.ArgKeyNamesTruncatedCount); + } - if (includeValues && evt.ArgValues is { } values) - { - jw.WritePropertyName("arg_values"); - values.WriteTo(jw); - } - if (evt.ArgValuesRedacted) - jw.WriteBoolean("arg_values_redacted", true); - if (evt.ArgValuesTruncated) + if (evt.EventTruncated) + { + jw.WriteBoolean("event_truncated", true); + jw.WriteNumber("event_max_bytes", MaxSerializedEventBytes); + if (evt.EventTruncationReasons is { Count: > 0 } eventReasons) { - jw.WriteBoolean("arg_values_truncated", true); - jw.WriteNumber("arg_values_max_bytes", MaxArgValuesSerializedBytes); - if (evt.ArgValuesSerializedBytes is { } argValuesSerializedBytes) - jw.WriteNumber("arg_values_serialized_bytes", argValuesSerializedBytes); - if (evt.ArgValueTruncationReasons is { Count: > 0 } reasons) - { - jw.WritePropertyName("arg_values_truncation_reasons"); - jw.WriteStartArray(); - foreach (var reason in reasons) - jw.WriteStringValue(reason); - jw.WriteEndArray(); - } + jw.WritePropertyName("event_truncation_reasons"); + jw.WriteStartArray(); + foreach (var reason in eventReasons) + jw.WriteStringValue(reason); + jw.WriteEndArray(); } + } - if (evt.ResultCount is { } rc) - jw.WriteNumber("result_count", rc); + if (evt.ResultCount is { } rc) + jw.WriteNumber("result_count", rc); - jw.WriteNumber("elapsed_ms", Math.Round(evt.ElapsedMs, 3)); - jw.WriteNumber("error_code", evt.ErrorCode); - if (evt.ErrorType is { } et) - jw.WriteString("error", et); - jw.WriteEndObject(); - } - return Encoding.UTF8.GetString(buffer.ToArray()); + jw.WriteNumber("elapsed_ms", Math.Round(evt.ElapsedMs, 3)); + jw.WriteNumber("error_code", evt.ErrorCode); + if (evt.ErrorType is { } et) + jw.WriteString("error", et); + jw.WriteEndObject(); } private static IReadOnlyList AppendTruncationReason(IReadOnlyList? reasons, string reason) @@ -380,6 +431,105 @@ private static IReadOnlyList AppendTruncationReason(IReadOnlyList false; + public override bool CanSeek => false; + public override bool CanWrite => true; + public override long Length => throw new NotSupportedException(); + + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + public string GetCapturedString() + => Encoding.UTF8.GetString(_buffer.GetBuffer(), 0, (int)_buffer.Length); + + public override void Flush() + { + } + + public override int Read(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + + public override long Seek(long offset, SeekOrigin origin) + => throw new NotSupportedException(); + + public override void SetLength(long value) + => throw new NotSupportedException(); + + public override void Write(byte[] buffer, int offset, int count) + => Write(buffer.AsSpan(offset, count)); + + public override void Write(ReadOnlySpan buffer) + { + if (buffer.Length == 0) + return; + + var remaining = maxBytes - BytesWritten; + if (remaining < buffer.Length) + { + if (remaining > 0) + _buffer.Write(buffer[..remaining]); + BytesWritten = maxBytes == int.MaxValue ? int.MaxValue : maxBytes + 1; + throw new AuditEventByteLimitExceededException(BytesWritten); + } + + _buffer.Write(buffer); + BytesWritten += buffer.Length; + } + } + internal static JsonNode? SanitizeArgValue(string key, JsonNode? value, out bool redacted) { var state = new ArgValueSanitizationState(); @@ -668,6 +818,8 @@ internal sealed record AuditEvent( bool ArgValuesTruncated = false, IReadOnlyList? ArgValueTruncationReasons = null, int? ArgValuesSerializedBytes = null, + bool EventTruncated = false, + IReadOnlyList? EventTruncationReasons = null, int? RequestIdLength = null, bool RequestIdTruncated = false, int? CallerNameLength = null, diff --git a/tests/CodeIndex.Tests/AuditLogSinkTests.cs b/tests/CodeIndex.Tests/AuditLogSinkTests.cs index b80495aa16..adb34558e1 100644 --- a/tests/CodeIndex.Tests/AuditLogSinkTests.cs +++ b/tests/CodeIndex.Tests/AuditLogSinkTests.cs @@ -226,7 +226,7 @@ public void SanitizeArgValue_TruncatesNestedObjectKeys_Issue3237() } [Fact] - public void SerializeEvent_DropsArgValues_WhenRecordExceedsEventBudget_Issue3237() + public void SerializeEvent_DropsArgValues_WhenRecordExceedsEventBudget_Issue3237_Issue3107() { var evt = new AuditLogSink.AuditEvent( Timestamp: DateTimeOffset.UtcNow, @@ -250,13 +250,17 @@ public void SerializeEvent_DropsArgValues_WhenRecordExceedsEventBudget_Issue3237 Assert.True(Encoding.UTF8.GetByteCount(json) <= AuditLogSink.MaxSerializedEventBytes); using var doc = JsonDocument.Parse(json); Assert.False(doc.RootElement.TryGetProperty("arg_values", out _)); + Assert.True(doc.RootElement.GetProperty("event_truncated").GetBoolean()); + Assert.Equal(AuditLogSink.MaxSerializedEventBytes, doc.RootElement.GetProperty("event_max_bytes").GetInt32()); + Assert.Contains(doc.RootElement.GetProperty("event_truncation_reasons").EnumerateArray(), + reason => reason.GetString() == "event_size_limit"); Assert.True(doc.RootElement.GetProperty("arg_values_truncated").GetBoolean()); Assert.Contains(doc.RootElement.GetProperty("arg_values_truncation_reasons").EnumerateArray(), reason => reason.GetString() == "event_size_limit"); } [Fact] - public void SerializeEvent_DropsArgKeyMetadata_WhenRecordExceedsEventBudget_Issue3237() + public void SerializeEvent_DropsArgKeyMetadata_WhenRecordExceedsEventBudget_Issue3237_Issue3107() { var keys = new List(); var lengths = new List>(); @@ -292,11 +296,47 @@ public void SerializeEvent_DropsArgKeyMetadata_WhenRecordExceedsEventBudget_Issu Assert.Empty(root.GetProperty("arg_keys").EnumerateArray()); Assert.Empty(root.GetProperty("arg_lengths").EnumerateObject()); Assert.False(root.TryGetProperty("arg_key_lengths", out _)); + Assert.True(root.GetProperty("event_truncated").GetBoolean()); + Assert.Equal(AuditLogSink.MaxSerializedEventBytes, root.GetProperty("event_max_bytes").GetInt32()); Assert.True(root.GetProperty("arg_keys_truncated").GetBoolean()); Assert.Contains(root.GetProperty("arg_key_truncation_reasons").EnumerateArray(), reason => reason.GetString() == "event_size_limit"); } + [Fact] + public void SerializeEvent_BoundsScalarFieldsBeforeBudgetedSerialization_Issue3107() + { + var huge = new string('z', AuditLogSink.MaxSerializedEventBytes + 1000); + var evt = new AuditLogSink.AuditEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: huge, + CallerName: huge, + CallerVersion: huge, + RequestId: huge, + ArgKeys: new[] { huge }, + ArgLengths: new[] { new KeyValuePair(huge, huge.Length) }, + ArgValues: null, + ResultCount: 0, + ElapsedMs: 1.0, + ErrorCode: 0, + ErrorType: huge, + ArgKeyLengths: new[] { new KeyValuePair(huge, huge.Length) }); + + var json = AuditLogSink.SerializeEvent(evt, includeValues: false); + + Assert.True(Encoding.UTF8.GetByteCount(json) <= AuditLogSink.MaxSerializedEventBytes); + Assert.DoesNotContain(huge, json, StringComparison.Ordinal); + using var doc = JsonDocument.Parse(json); + var root = doc.RootElement; + Assert.True(root.GetProperty("tool_truncated").GetBoolean()); + Assert.True(root.GetProperty("caller_truncated").GetBoolean()); + Assert.True(root.GetProperty("caller_version_truncated").GetBoolean()); + Assert.True(root.GetProperty("request_id_truncated").GetBoolean()); + Assert.True(root.GetProperty("event_truncated").GetBoolean()); + Assert.Contains(root.GetProperty("event_truncation_reasons").EnumerateArray(), + reason => reason.GetString() == "event_size_limit"); + } + [Fact] public void MeasureArgLength_ReportsTypeSpecificCounts() {