From f5805ff1e8a157e18e0a850bb73a165d90fe0c45 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 02:00:44 +0900 Subject: [PATCH 1/4] Enforce MCP audit log max-byte cap (#3180) --- USER_GUIDE.md | 4 ++-- changelog.d/unreleased/3180.security.md | 18 ++++++++++++++++++ src/CodeIndex/Cli/ProgramRunner.cs | 5 +++-- src/CodeIndex/Mcp/AuditLogSink.cs | 3 +++ tests/CodeIndex.Tests/ProgramRunnerTests.cs | 10 ++++++++++ 5 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 changelog.d/unreleased/3180.security.md diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 7aec5917fd..d2243e6dca 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1377,7 +1377,7 @@ Example output: |---|---|---| | `--audit-log ` | (off) | Enable audit emission and write JSONL records to ``. The parent directory is created if missing. | | `--audit-log-include-values` | off | Echo the full argument payload into each record. Requires `--audit-log`. Off by default because `query` / `name` arguments may contain literal source snippets or secret-shaped strings. | -| `--audit-log-max-bytes ` | `52428800` (50 MiB) | Size threshold (bytes) at which the active log rotates. Must be ≥ 4096. | +| `--audit-log-max-bytes ` | `52428800` (50 MiB) | Size threshold (bytes) at which the active log rotates. Must be between 4096 and 1073741824. | Each record is a single JSON object on its own line with these fields: @@ -3583,7 +3583,7 @@ MCP ツールで catch-all まで突き抜けた例外（想定外の SQLite 例 |---|---|---| | `--audit-log ` | (無効) | 監査出力を有効化し `` に JSONL を書き出す。親ディレクトリは無ければ自動作成 | | `--audit-log-include-values` | off | 引数の値をレコードに含める。`--audit-log` 必須。既定で off なのは `query` / `name` 引数にソース片や secret 風の文字列が入りうるため | -| `--audit-log-max-bytes ` | `52428800` (50 MiB) | ローテーションの閾値（バイト）。最小値は 4096 | +| `--audit-log-max-bytes ` | `52428800` (50 MiB) | ローテーションの閾値（バイト）。4096 以上 1073741824 以下 | 各レコードは独立した行に 1 つの JSON オブジェクトとして書き出され、フィールドは次の通りです。 diff --git a/changelog.d/unreleased/3180.security.md b/changelog.d/unreleased/3180.security.md new file mode 100644 index 0000000000..d57ee76a61 --- /dev/null +++ b/changelog.d/unreleased/3180.security.md @@ -0,0 +1,18 @@ +--- +category: security +issues: + - 3180 +affected: + - src/CodeIndex/Mcp/AuditLogSink.cs + - src/CodeIndex/Cli/ProgramRunner.cs + - tests/CodeIndex.Tests/ProgramRunnerTests.cs + - USER_GUIDE.md +--- + +## English + +- **MCP audit-log rotation now rejects oversized caps (#3180)** — `--audit-log-max-bytes` now enforces a documented 1 GiB upper bound while preserving the existing 4 KiB lower bound. + +## 日本語 + +- **MCP 監査ログのローテーション上限が過大値を拒否するようになりました (#3180)** — `--audit-log-max-bytes` は既存の 4 KiB 下限を維持しつつ、ドキュメント化された 1 GiB 上限を超える値を拒否します。 diff --git a/src/CodeIndex/Cli/ProgramRunner.cs b/src/CodeIndex/Cli/ProgramRunner.cs index 125db8b2e7..2b6d1260fa 100644 --- a/src/CodeIndex/Cli/ProgramRunner.cs +++ b/src/CodeIndex/Cli/ProgramRunner.cs @@ -2833,9 +2833,10 @@ private static bool TryConsumeAuditLogMaxBytes( } if (!long.TryParse(raw, System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out var parsed) - || parsed < AuditLogSink.MinMaxBytes) + || parsed < AuditLogSink.MinMaxBytes + || parsed > AuditLogSink.MaxMaxBytes) { - error = $"Error: --audit-log-max-bytes must be an integer >= {AuditLogSink.MinMaxBytes}."; + error = $"Error: --audit-log-max-bytes must be an integer between {AuditLogSink.MinMaxBytes} and {AuditLogSink.MaxMaxBytes}."; return false; } diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index 79135f5f40..0e7bc9264c 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -27,6 +27,7 @@ internal sealed class AuditLogSink : IDisposable { internal const long DefaultMaxBytes = 50L * 1024 * 1024; // 50 MiB internal const long MinMaxBytes = 4 * 1024; // 4 KiB + internal const long MaxMaxBytes = 1024L * 1024 * 1024; // 1 GiB internal const int RotationKeep = 3; // path, path.1, path.2 private readonly object _gate = new(); @@ -43,6 +44,8 @@ internal AuditLogSink(string path, long maxBytes, bool includeValues) throw new ArgumentException("Audit log path must be non-empty.", nameof(path)); if (maxBytes < MinMaxBytes) throw new ArgumentOutOfRangeException(nameof(maxBytes), $"maxBytes must be >= {MinMaxBytes} bytes."); + if (maxBytes > MaxMaxBytes) + throw new ArgumentOutOfRangeException(nameof(maxBytes), $"maxBytes must be <= {MaxMaxBytes} bytes."); _path = System.IO.Path.GetFullPath(path); _maxBytes = maxBytes; diff --git a/tests/CodeIndex.Tests/ProgramRunnerTests.cs b/tests/CodeIndex.Tests/ProgramRunnerTests.cs index c22536e82f..db481f3d97 100644 --- a/tests/CodeIndex.Tests/ProgramRunnerTests.cs +++ b/tests/CodeIndex.Tests/ProgramRunnerTests.cs @@ -2186,6 +2186,16 @@ public void TryConsumeAuditLogFlags_MaxBytesBelowMin_ReturnsError() Assert.Contains("--audit-log-max-bytes must be an integer", error); } + [Fact] + public void TryConsumeAuditLogFlags_MaxBytesAboveMax_ReturnsError() + { + var args = new[] { "--audit-log", "/tmp/a.jsonl", "--audit-log-max-bytes", (AuditLogSink.MaxMaxBytes + 1).ToString(CultureInfo.InvariantCulture) }; + var ok = ProgramRunner.TryConsumeAuditLogFlags(ref args, out _, out var error); + + Assert.False(ok); + Assert.Contains(AuditLogSink.MaxMaxBytes.ToString(CultureInfo.InvariantCulture), error); + } + [Fact] public void TryConsumeAuditLogFlags_NonNumericMaxBytes_ReturnsError() { From 5bb0d5f3377eda4e7cadb7b7d0cac2fd62b71a47 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 11:01:00 +0900 Subject: [PATCH 2/4] Redact MCP audit argument values (#3067) --- DEVELOPER_GUIDE.md | 8 +- USER_GUIDE.md | 10 ++- changelog.d/unreleased/3067.security.md | 20 +++++ src/CodeIndex/Mcp/AuditLogSink.cs | 100 +++++++++++++++++++++ src/CodeIndex/Mcp/McpServer.cs | 12 ++- tests/CodeIndex.Tests/AuditLogSinkTests.cs | 49 ++++++++++ tests/CodeIndex.Tests/McpAuditLogTests.cs | 69 ++++++++++++++ 7 files changed, 258 insertions(+), 10 deletions(-) create mode 100644 changelog.d/unreleased/3067.security.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 01629f2612..0394840bfc 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1999,10 +1999,10 @@ who opens a cloud session, not by a real user after release. Contract guarantees that downstream consumers can rely on: -- **Field stability.** `timestamp`, `tool`, `arg_keys`, `arg_lengths`, `elapsed_ms`, `error_code` are emitted on every record. `caller`, `caller_version`, `request_id`, `arg_values`, `result_count`, `error` are emitted only when non-null; renaming or repurposing any published field is a breaking change, the same policy as the CLI `--metrics` schema. +- **Field stability.** `timestamp`, `tool`, `arg_keys`, `arg_lengths`, `elapsed_ms`, `error_code` are emitted on every record. `caller`, `caller_version`, `request_id`, `arg_values`, `arg_values_redacted`, `result_count`, `error` are emitted only when non-null or true; renaming or repurposing any published field is a breaking change, the same policy as the CLI `--metrics` schema. - **Error code semantics.** `0` = success, `1` = MCP tool error (`isError: true`), negative = the verbatim JSON-RPC error code (e.g. `-32602` for invalid params, `-32603` for internal error). The companion `error` string is one of `jsonrpc_error`, `tool_error`, `missing_tool_name`, or the sanitized exception type name (`McpServer.BuildSanitizedToolErrorMessage` keeps `ex.Message` out of the wire and out of the audit, #1530). - **Result count.** `ExtractResultCount` prefers `structuredContent.count` over `structuredContent.results.length`; tool errors and JSON-RPC errors omit the field. Tools that return no count-shaped payload (e.g. `ping`) leave `result_count` absent rather than emitting `0`. -- **Argument privacy.** `arg_keys` and `arg_lengths` are always recorded so query *shape* is recoverable. `arg_values` is gated behind `--audit-log-include-values` because cdidx queries can carry literal source snippets or secret-shaped strings. The echo is a `DeepClone` so later mutation of the request payload cannot retroactively change the audit trail. +- **Argument privacy.** `arg_keys` and `arg_lengths` are always recorded so query *shape* is recoverable. `arg_values` is gated behind `--audit-log-include-values` because cdidx queries can carry literal source snippets or secret-shaped strings. The echo is a sanitized clone: secret-like keys and known token patterns are replaced with `[REDACTED]`, and `arg_values_redacted` records when redaction happened. - **Caller identity.** `_clientName` / `_clientVersion` are captured from every `initialize.clientInfo` and overwrite on reconnection within the same session, so a long-running MCP loop with multiple `initialize` handshakes attributes records to the *currently connected* client rather than the first one. - **Rotation.** Writes go through an open-append-close cycle so external `tail -F` consumers follow rotations and so the file is closed during the rename. When `_bytesWritten >= MaxBytes`, `RotateLocked` drops `.(RotationKeep-1)` (currently `.2`), cascades surviving slots up by one, and moves `` to `.1`. `RotationKeep = 3`, so `.3` is never created — exercised by `AuditLogSinkTests.Record_KeepsAtMostThreeFiles_DropsOldestOnRotationOverflow`. - **Best effort.** Serialization failures, IO failures, and rotation failures are swallowed (the audit must not crash the underlying tool call). The constructor still fails fast on impossible paths so the operator sees the misconfiguration before any tool dispatch happens. @@ -3576,10 +3576,10 @@ Cloud セッションは開発ループの中で `dotnet build` にフォール 下流コンシューマが依存できる契約: -- **フィールドの安定性。** `timestamp`、`tool`、`arg_keys`、`arg_lengths`、`elapsed_ms`、`error_code` は全レコードで出力する。`caller`、`caller_version`、`request_id`、`arg_values`、`result_count`、`error` は値が non-null のときだけ含める。既存フィールドの改名や流用は破壊的変更扱い（CLI `--metrics` と同じ運用）。 +- **フィールドの安定性。** `timestamp`、`tool`、`arg_keys`、`arg_lengths`、`elapsed_ms`、`error_code` は全レコードで出力する。`caller`、`caller_version`、`request_id`、`arg_values`、`arg_values_redacted`、`result_count`、`error` は値が non-null または true のときだけ含める。既存フィールドの改名や流用は破壊的変更扱い（CLI `--metrics` と同じ運用）。 - **エラーコード意味論。** `0`=成功、`1`=MCP ツールエラー (`isError: true`)、負値=JSON-RPC エラーコードそのまま（例: invalid params なら `-32602`、internal error なら `-32603`）。同伴する `error` 文字列は `jsonrpc_error` / `tool_error` / `missing_tool_name` / サニタイズ済み例外型名のいずれか。`McpServer.BuildSanitizedToolErrorMessage` が `ex.Message` をワイヤーと audit から除外している（#1530）。 - **result count。** `ExtractResultCount` は `structuredContent.count` を優先し、無ければ `structuredContent.results.length`、いずれも無ければ省略する。ツールエラー / JSON-RPC エラー時も省略する（`0` ではなく欠落）。 -- **引数のプライバシー。** `arg_keys` / `arg_lengths` は常に記録するので呼び出しの *形状* は復元できる。`arg_values` は `--audit-log-include-values` に gated（cdidx クエリにはソース片や secret 風文字列が混入しうる）。echo は `DeepClone` で取るので、後段のリクエスト改変が監査記録を遡及的に書き換えることはない。 +- **引数のプライバシー。** `arg_keys` / `arg_lengths` は常に記録するので呼び出しの *形状* は復元できる。`arg_values` は `--audit-log-include-values` に gated（cdidx クエリにはソース片や secret 風文字列が混入しうる）。echo は sanitize 済み clone として作り、secret 風のキーや既知 token pattern は `[REDACTED]` に置換し、redaction が発生した場合は `arg_values_redacted` を記録する。 - **呼び出し元の特定。** `_clientName` / `_clientVersion` は `initialize.clientInfo` から毎回キャプチャし、同一セッション内で再 `initialize` があれば上書きされる。複数 handshake が走る長寿命 MCP ループでも、*現在接続中の*クライアントに対して記録が紐付く。 - **ローテーション。** 1 レコードごとに open-append-close する。外部 `tail -F` の追従と rename 時の close-state 維持のため。`_bytesWritten >= MaxBytes` を超えた時点で `RotateLocked` が `.(RotationKeep-1)`（現在は `.2`）を破棄し、生存スロットを 1 つ古い側へ寄せ、`` を `.1` へ移す。`RotationKeep = 3` なので `.3` は決して生成されない（`AuditLogSinkTests.Record_KeepsAtMostThreeFiles_DropsOldestOnRotationOverflow` で常時検証）。 - **ベストエフォート。** シリアライズ失敗・IO 失敗・rotation 失敗はすべて握り潰す（監査の失敗で本体ツール呼び出しを壊さない）。一方、構築時の不正パスはコンストラクタが早期失敗させ、ディスパッチ前にオペレーターに気付かせる。 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index d2243e6dca..c46548625d 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1376,7 +1376,7 @@ Example output: | Flag | Default | Effect | |---|---|---| | `--audit-log ` | (off) | Enable audit emission and write JSONL records to ``. The parent directory is created if missing. | -| `--audit-log-include-values` | off | Echo the full argument payload into each record. Requires `--audit-log`. Off by default because `query` / `name` arguments may contain literal source snippets or secret-shaped strings. | +| `--audit-log-include-values` | off | Echo a redacted copy of the argument payload into each record. Requires `--audit-log`. Off by default because `query` / `name` arguments may contain literal source snippets or secret-shaped strings. | | `--audit-log-max-bytes ` | `52428800` (50 MiB) | Size threshold (bytes) at which the active log rotates. Must be between 4096 and 1073741824. | Each record is a single JSON object on its own line with these fields: @@ -1390,7 +1390,8 @@ Each record is a single JSON object on its own line with these fields: | `request_id` | string (optional) | JSON-encoded JSON-RPC request id, when present | | `arg_keys` | string[] | Ordered list of argument names supplied to the tool | | `arg_lengths` | object | Per-argument length sketch — string→char count, array→element count, object→key count, scalar→0 | -| `arg_values` | object (optional) | Full argument payload. Present only when `--audit-log-include-values` is enabled | +| `arg_values` | object (optional) | Redacted argument payload. Present only when `--audit-log-include-values` is enabled | +| `arg_values_redacted` | boolean (optional) | `true` when secret-like keys or token patterns were replaced with `[REDACTED]` | | `result_count` | number (optional) | `structuredContent.count` or `structuredContent.results.length` for successful calls; omitted otherwise | | `elapsed_ms` | number | Wall-clock duration in milliseconds (3 decimal places) | | `error_code` | number | `0` on success, `1` for MCP tool errors (`isError: true`), or the verbatim JSON-RPC error code (e.g. `-32602`) | @@ -3582,7 +3583,7 @@ MCP ツールで catch-all まで突き抜けた例外（想定外の SQLite 例 | フラグ | 既定 | 効果 | |---|---|---| | `--audit-log ` | (無効) | 監査出力を有効化し `` に JSONL を書き出す。親ディレクトリは無ければ自動作成 | -| `--audit-log-include-values` | off | 引数の値をレコードに含める。`--audit-log` 必須。既定で off なのは `query` / `name` 引数にソース片や secret 風の文字列が入りうるため | +| `--audit-log-include-values` | off | redaction 済みの引数値をレコードに含める。`--audit-log` 必須。既定で off なのは `query` / `name` 引数にソース片や secret 風の文字列が入りうるため | | `--audit-log-max-bytes ` | `52428800` (50 MiB) | ローテーションの閾値（バイト）。4096 以上 1073741824 以下 | 各レコードは独立した行に 1 つの JSON オブジェクトとして書き出され、フィールドは次の通りです。 @@ -3596,7 +3597,8 @@ MCP ツールで catch-all まで突き抜けた例外（想定外の SQLite 例 | `request_id` | string（任意） | JSON-RPC リクエスト id を JSON エンコードしたもの | | `arg_keys` | string[] | ツールへ渡された引数名の順序付きリスト | | `arg_lengths` | object | 引数ごとの長さ概算（文字列→文字数、配列→要素数、オブジェクト→キー数、スカラ→0） | -| `arg_values` | object（任意） | 引数本体。`--audit-log-include-values` 指定時のみ付与 | +| `arg_values` | object（任意） | redaction 済みの引数本体。`--audit-log-include-values` 指定時のみ付与 | +| `arg_values_redacted` | boolean（任意） | secret 風のキーまたは token pattern が `[REDACTED]` に置き換えられた場合に `true` | | `result_count` | number（任意） | 成功時の `structuredContent.count` または `structuredContent.results.length`。それ以外は省略 | | `elapsed_ms` | number | ウォールクロック経過ミリ秒（小数 3 桁） | | `error_code` | number | 成功=`0`、MCP ツールエラー（`isError: true`）=`1`、JSON-RPC エラー=そのコード（例: `-32000` のレート制限、`-32602` の引数エラー） | diff --git a/changelog.d/unreleased/3067.security.md b/changelog.d/unreleased/3067.security.md new file mode 100644 index 0000000000..6774bc7603 --- /dev/null +++ b/changelog.d/unreleased/3067.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3067 +affected: + - src/CodeIndex/Mcp/AuditLogSink.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/AuditLogSinkTests.cs + - tests/CodeIndex.Tests/McpAuditLogTests.cs + - USER_GUIDE.md + - DEVELOPER_GUIDE.md +--- + +## English + +- **MCP audit-log include-values now redacts secret-like values (#3067)** — include-values audit records replace secret-like argument keys and known token patterns with `[REDACTED]` and mark the record with `arg_values_redacted`. + +## 日本語 + +- **MCP 監査ログの include-values が secret 風の値を redaction するようになりました (#3067)** — include-values の監査レコードは secret 風の引数キーや既知 token pattern を `[REDACTED]` に置き換え、`arg_values_redacted` で記録します。 diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index 0e7bc9264c..c0d7803275 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -4,6 +4,7 @@ using System.Text.Encodings.Web; using System.Text.Json; using System.Text.Json.Nodes; +using System.Text.RegularExpressions; using CodeIndex.Cli; using CodeIndex.Indexer; @@ -29,6 +30,11 @@ internal sealed class AuditLogSink : IDisposable internal const long MinMaxBytes = 4 * 1024; // 4 KiB internal const long MaxMaxBytes = 1024L * 1024 * 1024; // 1 GiB internal const int RotationKeep = 3; // path, path.1, path.2 + internal const string RedactedValue = "[REDACTED]"; + + private static readonly Regex SecretValuePattern = new( + "(?i)(github_pat_[A-Za-z0-9_]{20,}|gh[pousr]_[A-Za-z0-9_]{20,}|sk-(?:proj-)?[A-Za-z0-9_-]{20,}|xox[baprs]-[A-Za-z0-9-]{20,}|AKIA[0-9A-Z]{16}|://[^/\\s:@]+:[^/\\s:@]+@|(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key|authorization)=[^&\\s]+)", + RegexOptions.Compiled | RegexOptions.CultureInvariant); private readonly object _gate = new(); private readonly string _path; @@ -261,6 +267,8 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) jw.WritePropertyName("arg_values"); values.WriteTo(jw); } + if (evt.ArgValuesRedacted) + jw.WriteBoolean("arg_values_redacted", true); if (evt.ResultCount is { } rc) jw.WriteNumber("result_count", rc); @@ -274,6 +282,97 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) return Encoding.UTF8.GetString(buffer.ToArray()); } + internal static JsonNode? SanitizeArgValue(string key, JsonNode? value, out bool redacted) + { + redacted = false; + return SanitizeArgValueCore(key, value, ref redacted); + } + + private static JsonNode? SanitizeArgValueCore(string key, JsonNode? value, ref bool redacted) + { + if (IsSecretLikeKey(key)) + { + redacted = true; + return JsonValue.Create(RedactedValue); + } + + return value switch + { + null => null, + JsonObject obj => SanitizeObject(obj, ref redacted), + JsonArray arr => SanitizeArray(arr, ref redacted), + JsonValue jsonValue => SanitizeScalar(jsonValue, ref redacted), + _ => null, + }; + } + + private static JsonObject SanitizeObject(JsonObject obj, ref bool redacted) + { + var clone = new JsonObject(); + foreach (var (key, value) in obj) + clone[key] = SanitizeArgValueCore(key, value, ref redacted); + return clone; + } + + private static JsonArray SanitizeArray(JsonArray arr, ref bool redacted) + { + var clone = new JsonArray(); + foreach (var value in arr) + clone.Add(SanitizeArgValueCore(string.Empty, value, ref redacted)); + return clone; + } + + private static JsonNode? SanitizeScalar(JsonValue value, ref bool redacted) + { + if (value.TryGetValue(out var text)) + { + if (SecretValuePattern.IsMatch(text)) + { + redacted = true; + return JsonValue.Create(RedactedValue); + } + + return JsonValue.Create(text); + } + + try + { + return JsonNode.Parse(value.ToJsonString()); + } + catch + { + return null; + } + } + + private static bool IsSecretLikeKey(string key) + { + var normalized = NormalizeKey(key); + return normalized.Contains("pwd", StringComparison.Ordinal) + || normalized.Contains("auth", StringComparison.Ordinal) + || normalized.Contains("password", StringComparison.Ordinal) + || normalized.Contains("passwd", StringComparison.Ordinal) + || normalized.Contains("secret", StringComparison.Ordinal) + || normalized.Contains("token", StringComparison.Ordinal) + || normalized.Contains("apikey", StringComparison.Ordinal) + || normalized.Contains("accesskey", StringComparison.Ordinal) + || normalized.Contains("privatekey", StringComparison.Ordinal) + || normalized.Contains("authorization", StringComparison.Ordinal) + || normalized.Contains("credential", StringComparison.Ordinal) + || normalized.Contains("sessioncookie", StringComparison.Ordinal); + } + + private static string NormalizeKey(string key) + { + var sb = new StringBuilder(key.Length); + foreach (var ch in key) + { + if (char.IsLetterOrDigit(ch)) + sb.Append(char.ToLowerInvariant(ch)); + } + return sb.ToString(); + } + /// /// Compute the per-key length sketch used by audit records. Strings → char count; /// arrays → element count; objects → key count; scalars (number / bool / null) → 0. @@ -307,6 +406,7 @@ internal sealed record AuditEvent( int? ToolLength = null, bool ToolTruncated = false, IReadOnlyList>? ArgKeyLengths = null, + bool ArgValuesRedacted = false, int? CallerNameLength = null, bool CallerNameTruncated = false, int? CallerVersionLength = null, diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index 32a85f6f89..bcc2790bf3 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2659,7 +2659,8 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod { var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); - var (argKeys, argLengths, argKeyLengths, argValuesEcho) = SanitizeArgs(args, _auditLog.IncludeValues); + var (argKeys, argLengths, argKeyLengths, argValuesEcho) = + SanitizeArgs(args, _auditLog.IncludeValues, out var argValuesRedacted); var toolDisplay = BoundToolNameForDisplay(toolName); var evt = new AuditLogSink.AuditEvent( Timestamp: startedAt, @@ -2677,6 +2678,7 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ToolLength: toolDisplay.Truncated ? toolDisplay.OriginalLength : null, ToolTruncated: toolDisplay.Truncated, ArgKeyLengths: argKeyLengths, + ArgValuesRedacted: argValuesRedacted, CallerNameLength: _clientNameDisplay?.Truncated == true ? _clientNameDisplay.Value.OriginalLength : null, CallerNameTruncated: _clientNameDisplay?.Truncated == true, CallerVersionLength: _clientVersionDisplay?.Truncated == true ? _clientVersionDisplay.Value.OriginalLength : null, @@ -2758,7 +2760,12 @@ 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 _); + + private static (IReadOnlyList Keys, IReadOnlyList> Lengths, IReadOnlyList> KeyLengths, JsonNode? ValuesEcho) + SanitizeArgs(JsonNode? args, bool includeValues, out bool argValuesRedacted) { + argValuesRedacted = false; if (args is not JsonObject argsObj) return (Array.Empty(), Array.Empty>(), Array.Empty>(), null); @@ -2779,7 +2786,8 @@ internal static (IReadOnlyList Keys, IReadOnlyList("token", 12) }, + ArgValues: args, + ResultCount: 0, + ElapsedMs: 1.0, + ErrorCode: 0, + ErrorType: null, + ArgValuesRedacted: true); + + var json = AuditLogSink.SerializeEvent(evt, includeValues: true); + using var doc = JsonDocument.Parse(json); + Assert.True(doc.RootElement.GetProperty("arg_values_redacted").GetBoolean()); + } + + [Fact] + public void SanitizeArgValue_RedactsSecretKeysAndTokenPatterns_Issue3067() + { + var args = JsonNode.Parse(""" + { + "access_token": "plain-secret", + "query": "token=ghp_abcdefghijklmnopqrstuvwxyz123456", + "nested": { + "password": "hunter2", + "db_pwd": "hunter2", + "auth_header": "Bearer abcdefghijklmnop" + } + } + """); + + var sanitized = AuditLogSink.SanitizeArgValue("arguments", args, out var redacted)!.AsObject(); + + Assert.True(redacted); + Assert.Equal(AuditLogSink.RedactedValue, sanitized["access_token"]!.GetValue()); + Assert.Equal(AuditLogSink.RedactedValue, sanitized["query"]!.GetValue()); + Assert.Equal(AuditLogSink.RedactedValue, sanitized["nested"]!["password"]!.GetValue()); + Assert.Equal(AuditLogSink.RedactedValue, sanitized["nested"]!["db_pwd"]!.GetValue()); + Assert.Equal(AuditLogSink.RedactedValue, sanitized["nested"]!["auth_header"]!.GetValue()); + } + [Fact] public void MeasureArgLength_ReportsTypeSpecificCounts() { diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index b410989cda..6486dbf436 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -281,6 +281,41 @@ public void ToolsCall_IncludeValues_EchoesArgValuesIntoRecord() Assert.Equal(0, lengths.GetProperty("limit").GetInt32()); } + [Fact] + public void ToolsCall_IncludeValues_RedactsSecretLikeArgumentValues_Issue3067() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); + using var server = CreateServer(sink); + + var token = "ghp_abcdefghijklmnopqrstuvwxyz123456"; + var unknown = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "does_not_exist", + ["arguments"] = new JsonObject + { + ["apiToken"] = token, + ["query"] = "https://user:pass@example.test/repo", + }, + }, + }; + _ = server.HandleMessage(unknown); + + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(token, rawLog, StringComparison.Ordinal); + Assert.DoesNotContain("user:pass", rawLog, StringComparison.Ordinal); + + var record = ReadOnlyRecord(); + var values = record.GetProperty("arg_values"); + Assert.Equal(AuditLogSink.RedactedValue, values.GetProperty("apiToken").GetString()); + Assert.Equal(AuditLogSink.RedactedValue, values.GetProperty("query").GetString()); + Assert.True(record.GetProperty("arg_values_redacted").GetBoolean()); + } + [Fact] public void ToolsCall_ValuesOmitted_ByDefault() { @@ -296,6 +331,40 @@ public void ToolsCall_ValuesOmitted_ByDefault() Assert.Equal(6, record.GetProperty("arg_lengths").GetProperty("query").GetInt32()); } + [Fact] + public void ToolsCall_IncludeValues_RedactsLongSecretLikeTopLevelArgumentKey_Issue3067() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); + using var server = CreateServer(sink); + + var secret = "top-secret-long-key-value"; + var secretKey = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 10) + "Password"; + var display = McpBoundedText.ForDisplay(secretKey); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "does_not_exist", + ["arguments"] = new JsonObject + { + [secretKey] = secret, + }, + }, + }; + + _ = server.HandleMessage(request); + + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(secret, rawLog, StringComparison.Ordinal); + var record = ReadOnlyRecord(); + Assert.Equal(display.Text, record.GetProperty("arg_keys")[0].GetString()); + Assert.Equal(AuditLogSink.RedactedValue, record.GetProperty("arg_values").GetProperty(display.Text).GetString()); + Assert.True(record.GetProperty("arg_values_redacted").GetBoolean()); + } + [Fact] public void ExtractErrorCode_NoError_ReturnsZero() { From 608c223168247ab7dc98ef23ca92cd0f01ceacc2 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 11:21:08 +0900 Subject: [PATCH 3/4] Budget MCP audit argument values (#3106) --- DEVELOPER_GUIDE.md | 8 +- USER_GUIDE.md | 12 +- changelog.d/unreleased/3106.security.md | 20 +++ src/CodeIndex/Mcp/AuditLogSink.cs | 186 +++++++++++++++++++-- src/CodeIndex/Mcp/McpServer.cs | 44 ++++- tests/CodeIndex.Tests/AuditLogSinkTests.cs | 22 +++ tests/CodeIndex.Tests/McpAuditLogTests.cs | 80 +++++++++ 7 files changed, 343 insertions(+), 29 deletions(-) create mode 100644 changelog.d/unreleased/3106.security.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 0394840bfc..116a177db3 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1999,10 +1999,10 @@ who opens a cloud session, not by a real user after release. Contract guarantees that downstream consumers can rely on: -- **Field stability.** `timestamp`, `tool`, `arg_keys`, `arg_lengths`, `elapsed_ms`, `error_code` are emitted on every record. `caller`, `caller_version`, `request_id`, `arg_values`, `arg_values_redacted`, `result_count`, `error` are emitted only when non-null or true; renaming or repurposing any published field is a breaking change, the same policy as the CLI `--metrics` schema. +- **Field stability.** `timestamp`, `tool`, `arg_keys`, `arg_lengths`, `elapsed_ms`, `error_code` are emitted on every record. `caller`, `caller_version`, `request_id`, `arg_values`, `arg_values_redacted`, `arg_values_truncated`, `arg_values_truncation_reasons`, `arg_values_serialized_bytes`, `arg_values_max_bytes`, `result_count`, `error` are emitted only when non-null or true; renaming or repurposing any published field is a breaking change, the same policy as the CLI `--metrics` schema. - **Error code semantics.** `0` = success, `1` = MCP tool error (`isError: true`), negative = the verbatim JSON-RPC error code (e.g. `-32602` for invalid params, `-32603` for internal error). The companion `error` string is one of `jsonrpc_error`, `tool_error`, `missing_tool_name`, or the sanitized exception type name (`McpServer.BuildSanitizedToolErrorMessage` keeps `ex.Message` out of the wire and out of the audit, #1530). - **Result count.** `ExtractResultCount` prefers `structuredContent.count` over `structuredContent.results.length`; tool errors and JSON-RPC errors omit the field. Tools that return no count-shaped payload (e.g. `ping`) leave `result_count` absent rather than emitting `0`. -- **Argument privacy.** `arg_keys` and `arg_lengths` are always recorded so query *shape* is recoverable. `arg_values` is gated behind `--audit-log-include-values` because cdidx queries can carry literal source snippets or secret-shaped strings. The echo is a sanitized clone: secret-like keys and known token patterns are replaced with `[REDACTED]`, and `arg_values_redacted` records when redaction happened. +- **Argument privacy.** `arg_keys` and `arg_lengths` are always recorded so query *shape* is recoverable. `arg_values` is gated behind `--audit-log-include-values` because cdidx queries can carry literal source snippets or secret-shaped strings. The echo is a sanitized, budgeted clone: secret-like keys and known token patterns are replaced with `[REDACTED]`, and depth, object-property, array-item, total-node, string-length, and serialized-byte limits can mark `arg_values_truncated` before values are written. - **Caller identity.** `_clientName` / `_clientVersion` are captured from every `initialize.clientInfo` and overwrite on reconnection within the same session, so a long-running MCP loop with multiple `initialize` handshakes attributes records to the *currently connected* client rather than the first one. - **Rotation.** Writes go through an open-append-close cycle so external `tail -F` consumers follow rotations and so the file is closed during the rename. When `_bytesWritten >= MaxBytes`, `RotateLocked` drops `.(RotationKeep-1)` (currently `.2`), cascades surviving slots up by one, and moves `` to `.1`. `RotationKeep = 3`, so `.3` is never created — exercised by `AuditLogSinkTests.Record_KeepsAtMostThreeFiles_DropsOldestOnRotationOverflow`. - **Best effort.** Serialization failures, IO failures, and rotation failures are swallowed (the audit must not crash the underlying tool call). The constructor still fails fast on impossible paths so the operator sees the misconfiguration before any tool dispatch happens. @@ -3576,10 +3576,10 @@ Cloud セッションは開発ループの中で `dotnet build` にフォール 下流コンシューマが依存できる契約: -- **フィールドの安定性。** `timestamp`、`tool`、`arg_keys`、`arg_lengths`、`elapsed_ms`、`error_code` は全レコードで出力する。`caller`、`caller_version`、`request_id`、`arg_values`、`arg_values_redacted`、`result_count`、`error` は値が non-null または true のときだけ含める。既存フィールドの改名や流用は破壊的変更扱い（CLI `--metrics` と同じ運用）。 +- **フィールドの安定性。** `timestamp`、`tool`、`arg_keys`、`arg_lengths`、`elapsed_ms`、`error_code` は全レコードで出力する。`caller`、`caller_version`、`request_id`、`arg_values`、`arg_values_redacted`、`arg_values_truncated`、`arg_values_truncation_reasons`、`arg_values_serialized_bytes`、`arg_values_max_bytes`、`result_count`、`error` は値が non-null または true のときだけ含める。既存フィールドの改名や流用は破壊的変更扱い（CLI `--metrics` と同じ運用）。 - **エラーコード意味論。** `0`=成功、`1`=MCP ツールエラー (`isError: true`)、負値=JSON-RPC エラーコードそのまま（例: invalid params なら `-32602`、internal error なら `-32603`）。同伴する `error` 文字列は `jsonrpc_error` / `tool_error` / `missing_tool_name` / サニタイズ済み例外型名のいずれか。`McpServer.BuildSanitizedToolErrorMessage` が `ex.Message` をワイヤーと audit から除外している（#1530）。 - **result count。** `ExtractResultCount` は `structuredContent.count` を優先し、無ければ `structuredContent.results.length`、いずれも無ければ省略する。ツールエラー / JSON-RPC エラー時も省略する（`0` ではなく欠落）。 -- **引数のプライバシー。** `arg_keys` / `arg_lengths` は常に記録するので呼び出しの *形状* は復元できる。`arg_values` は `--audit-log-include-values` に gated（cdidx クエリにはソース片や secret 風文字列が混入しうる）。echo は sanitize 済み clone として作り、secret 風のキーや既知 token pattern は `[REDACTED]` に置換し、redaction が発生した場合は `arg_values_redacted` を記録する。 +- **引数のプライバシー。** `arg_keys` / `arg_lengths` は常に記録するので呼び出しの *形状* は復元できる。`arg_values` は `--audit-log-include-values` に gated（cdidx クエリにはソース片や secret 風文字列が混入しうる）。echo は sanitize と budget を適用した clone として作り、secret 風のキーや既知 token pattern は `[REDACTED]` に置換し、depth / object property / array item / total node / string length / serialized byte の上限に達した場合は値を書き出す前に `arg_values_truncated` を記録する。 - **呼び出し元の特定。** `_clientName` / `_clientVersion` は `initialize.clientInfo` から毎回キャプチャし、同一セッション内で再 `initialize` があれば上書きされる。複数 handshake が走る長寿命 MCP ループでも、*現在接続中の*クライアントに対して記録が紐付く。 - **ローテーション。** 1 レコードごとに open-append-close する。外部 `tail -F` の追従と rename 時の close-state 維持のため。`_bytesWritten >= MaxBytes` を超えた時点で `RotateLocked` が `.(RotationKeep-1)`（現在は `.2`）を破棄し、生存スロットを 1 つ古い側へ寄せ、`` を `.1` へ移す。`RotationKeep = 3` なので `.3` は決して生成されない（`AuditLogSinkTests.Record_KeepsAtMostThreeFiles_DropsOldestOnRotationOverflow` で常時検証）。 - **ベストエフォート。** シリアライズ失敗・IO 失敗・rotation 失敗はすべて握り潰す（監査の失敗で本体ツール呼び出しを壊さない）。一方、構築時の不正パスはコンストラクタが早期失敗させ、ディスパッチ前にオペレーターに気付かせる。 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index c46548625d..9214515643 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1390,8 +1390,12 @@ Each record is a single JSON object on its own line with these fields: | `request_id` | string (optional) | JSON-encoded JSON-RPC request id, when present | | `arg_keys` | string[] | Ordered list of argument names supplied to the tool | | `arg_lengths` | object | Per-argument length sketch — string→char count, array→element count, object→key count, scalar→0 | -| `arg_values` | object (optional) | Redacted argument payload. Present only when `--audit-log-include-values` is enabled | +| `arg_values` | object (optional) | Redacted and budgeted argument payload. Present only when `--audit-log-include-values` is enabled | | `arg_values_redacted` | boolean (optional) | `true` when secret-like keys or token patterns were replaced with `[REDACTED]` | +| `arg_values_truncated` | boolean (optional) | `true` when include-values output hit a depth, count, string, or byte budget | +| `arg_values_truncation_reasons` | string[] (optional) | Stable truncation reason codes when `arg_values_truncated` is true | +| `arg_values_serialized_bytes` | number (optional) | Approximate serialized-byte budget consumed by retained `arg_values` | +| `arg_values_max_bytes` | number (optional) | Maximum serialized-byte budget for retained `arg_values` | | `result_count` | number (optional) | `structuredContent.count` or `structuredContent.results.length` for successful calls; omitted otherwise | | `elapsed_ms` | number | Wall-clock duration in milliseconds (3 decimal places) | | `error_code` | number | `0` on success, `1` for MCP tool errors (`isError: true`), or the verbatim JSON-RPC error code (e.g. `-32602`) | @@ -3597,8 +3601,12 @@ MCP ツールで catch-all まで突き抜けた例外（想定外の SQLite 例 | `request_id` | string（任意） | JSON-RPC リクエスト id を JSON エンコードしたもの | | `arg_keys` | string[] | ツールへ渡された引数名の順序付きリスト | | `arg_lengths` | object | 引数ごとの長さ概算（文字列→文字数、配列→要素数、オブジェクト→キー数、スカラ→0） | -| `arg_values` | object（任意） | redaction 済みの引数本体。`--audit-log-include-values` 指定時のみ付与 | +| `arg_values` | object（任意） | redaction および budget 適用済みの引数本体。`--audit-log-include-values` 指定時のみ付与 | | `arg_values_redacted` | boolean（任意） | secret 風のキーまたは token pattern が `[REDACTED]` に置き換えられた場合に `true` | +| `arg_values_truncated` | boolean（任意） | include-values 出力が depth / count / string / byte budget に到達した場合に `true` | +| `arg_values_truncation_reasons` | string[]（任意） | `arg_values_truncated` が true の場合の安定した truncation reason code | +| `arg_values_serialized_bytes` | number（任意） | 保持された `arg_values` が消費した概算 serialized byte budget | +| `arg_values_max_bytes` | number（任意） | 保持される `arg_values` の最大 serialized byte budget | | `result_count` | number（任意） | 成功時の `structuredContent.count` または `structuredContent.results.length`。それ以外は省略 | | `elapsed_ms` | number | ウォールクロック経過ミリ秒（小数 3 桁） | | `error_code` | number | 成功=`0`、MCP ツールエラー（`isError: true`）=`1`、JSON-RPC エラー=そのコード（例: `-32000` のレート制限、`-32602` の引数エラー） | diff --git a/changelog.d/unreleased/3106.security.md b/changelog.d/unreleased/3106.security.md new file mode 100644 index 0000000000..77951c18c9 --- /dev/null +++ b/changelog.d/unreleased/3106.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3106 +affected: + - src/CodeIndex/Mcp/AuditLogSink.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/AuditLogSinkTests.cs + - tests/CodeIndex.Tests/McpAuditLogTests.cs + - USER_GUIDE.md + - DEVELOPER_GUIDE.md +--- + +## English + +- **MCP audit-log include-values now budgets argument payloads (#3106)** — audit argument values are cloned through depth, count, string, and serialized-byte budgets before they are written, with truncation metadata recorded on oversized payloads. + +## 日本語 + +- **MCP 監査ログの include-values が引数 payload に budget を適用するようになりました (#3106)** — 監査ログの引数値は書き出し前に depth / count / string / serialized-byte budget を通して clone され、過大 payload では truncation metadata を記録します。 diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index c0d7803275..2374bb441a 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -31,6 +31,13 @@ internal sealed class AuditLogSink : IDisposable internal const long MaxMaxBytes = 1024L * 1024 * 1024; // 1 GiB internal const int RotationKeep = 3; // path, path.1, path.2 internal const string RedactedValue = "[REDACTED]"; + internal const string TruncatedValue = "[TRUNCATED]"; + internal const int MaxArgValueDepth = 8; + internal const int MaxArgValueProperties = 64; + internal const int MaxArgValueArrayItems = 64; + internal const int MaxArgValueTotalNodes = 512; + internal const int MaxArgValueStringChars = 512; + internal const int MaxArgValuesSerializedBytes = 16 * 1024; private static readonly Regex SecretValuePattern = new( "(?i)(github_pat_[A-Za-z0-9_]{20,}|gh[pousr]_[A-Za-z0-9_]{20,}|sk-(?:proj-)?[A-Za-z0-9_-]{20,}|xox[baprs]-[A-Za-z0-9-]{20,}|AKIA[0-9A-Z]{16}|://[^/\\s:@]+:[^/\\s:@]+@|(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key|authorization)=[^&\\s]+)", @@ -269,6 +276,21 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) } 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_values_truncation_reasons"); + jw.WriteStartArray(); + foreach (var reason in reasons) + jw.WriteStringValue(reason); + jw.WriteEndArray(); + } + } if (evt.ResultCount is { } rc) jw.WriteNumber("result_count", rc); @@ -284,67 +306,141 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) internal static JsonNode? SanitizeArgValue(string key, JsonNode? value, out bool redacted) { - redacted = false; - return SanitizeArgValueCore(key, value, ref redacted); + var state = new ArgValueSanitizationState(); + var sanitized = SanitizeArgValue(key, value, state); + redacted = state.Redacted; + return sanitized; } - private static JsonNode? SanitizeArgValueCore(string key, JsonNode? value, ref bool redacted) + internal static JsonNode? SanitizeArgValue(string key, JsonNode? value, ArgValueSanitizationState state) + => SanitizeArgValueCore(key, value, state, depth: 0); + + private static JsonNode? SanitizeArgValueCore(string key, JsonNode? value, ArgValueSanitizationState state, int depth) { + if (!state.TryReserveNode()) + return CreateTruncatedValue(); + if (IsSecretLikeKey(key)) { - redacted = true; + state.MarkRedacted(); + state.TryReserveSerializedBytes(EstimateStringJsonBytes(RedactedValue)); return JsonValue.Create(RedactedValue); } return value switch { - null => null, - JsonObject obj => SanitizeObject(obj, ref redacted), - JsonArray arr => SanitizeArray(arr, ref redacted), - JsonValue jsonValue => SanitizeScalar(jsonValue, ref redacted), + null => ReserveNull(state), + JsonObject obj => SanitizeObject(obj, state, depth), + JsonArray arr => SanitizeArray(arr, state, depth), + JsonValue jsonValue => SanitizeScalar(jsonValue, state), _ => null, }; } - private static JsonObject SanitizeObject(JsonObject obj, ref bool redacted) + private static JsonNode? ReserveNull(ArgValueSanitizationState state) + { + state.TryReserveSerializedBytes("null".Length); + return null; + } + + private static JsonNode SanitizeObject(JsonObject obj, ArgValueSanitizationState state, int depth) { + if (depth >= MaxArgValueDepth) + { + state.AddTruncationReason("depth_limit"); + return CreateTruncatedValue(); + } + + if (!state.TryReserveSerializedBytes(2)) + return CreateTruncatedValue(); + var clone = new JsonObject(); + var propertyCount = 0; foreach (var (key, value) in obj) - clone[key] = SanitizeArgValueCore(key, value, ref redacted); + { + if (propertyCount >= MaxArgValueProperties) + { + state.AddTruncationReason("object_property_count_limit"); + break; + } + + if (!state.TryReservePropertyName(key)) + break; + + clone[key] = SanitizeArgValueCore(key, value, state, depth + 1); + propertyCount++; + } return clone; } - private static JsonArray SanitizeArray(JsonArray arr, ref bool redacted) + private static JsonNode SanitizeArray(JsonArray arr, ArgValueSanitizationState state, int depth) { + if (depth >= MaxArgValueDepth) + { + state.AddTruncationReason("depth_limit"); + return CreateTruncatedValue(); + } + + if (!state.TryReserveSerializedBytes(2)) + return CreateTruncatedValue(); + var clone = new JsonArray(); + var itemCount = 0; foreach (var value in arr) - clone.Add(SanitizeArgValueCore(string.Empty, value, ref redacted)); + { + if (itemCount >= MaxArgValueArrayItems) + { + state.AddTruncationReason("array_item_count_limit"); + break; + } + + clone.Add(SanitizeArgValueCore(string.Empty, value, state, depth + 1)); + itemCount++; + } return clone; } - private static JsonNode? SanitizeScalar(JsonValue value, ref bool redacted) + private static JsonNode SanitizeScalar(JsonValue value, ArgValueSanitizationState state) { if (value.TryGetValue(out var text)) { if (SecretValuePattern.IsMatch(text)) { - redacted = true; + state.MarkRedacted(); + state.TryReserveSerializedBytes(EstimateStringJsonBytes(RedactedValue)); return JsonValue.Create(RedactedValue); } - return JsonValue.Create(text); + var display = McpBoundedText.ForDisplay(text, MaxArgValueStringChars); + if (display.Truncated) + state.AddTruncationReason("string_length_limit"); + if (!state.TryReserveSerializedBytes(EstimateStringJsonBytes(display.Text))) + return CreateTruncatedValue(); + return JsonValue.Create(display.Text); } try { - return JsonNode.Parse(value.ToJsonString()); + var json = value.ToJsonString(); + if (!state.TryReserveSerializedBytes(Encoding.UTF8.GetByteCount(json))) + return CreateTruncatedValue(); + return JsonNode.Parse(json) ?? CreateTruncatedValue(); } catch { - return null; + state.AddTruncationReason("scalar_serialization_failed"); + return CreateTruncatedValue(); } } + private static JsonValue CreateTruncatedValue() => JsonValue.Create(TruncatedValue); + + private static int EstimateStringJsonBytes(string value) + => Encoding.UTF8.GetByteCount(value) + 2; + + private static int EstimatePropertyNameJsonBytes(string key) + => EstimateStringJsonBytes(key) + 1; + private static bool IsSecretLikeKey(string key) { var normalized = NormalizeKey(key); @@ -373,6 +469,59 @@ private static string NormalizeKey(string key) return sb.ToString(); } + internal sealed class ArgValueSanitizationState + { + private readonly List _truncationReasons = new(); + private int _nodeCount; + private int _serializedBytes; + + internal bool Redacted { get; private set; } + internal bool Truncated => _truncationReasons.Count > 0; + internal IReadOnlyList TruncationReasons => _truncationReasons; + internal int SerializedBytes => _serializedBytes; + + internal void MarkRedacted() => Redacted = true; + + internal bool TryReserveNode() + { + if (_nodeCount >= MaxArgValueTotalNodes) + { + AddTruncationReason("node_count_limit"); + return false; + } + + _nodeCount++; + return true; + } + + internal bool TryReserveSerializedBytes(int byteCount) + { + var next = _serializedBytes + Math.Max(0, byteCount); + if (next > MaxArgValuesSerializedBytes) + { + AddTruncationReason("serialized_bytes_limit"); + return false; + } + + _serializedBytes = next; + return true; + } + + internal bool TryReservePropertyName(string key) + => TryReserveSerializedBytes(EstimatePropertyNameJsonBytes(key)); + + internal void AddTruncationReason(string reason) + { + foreach (var existing in _truncationReasons) + { + if (StringComparer.Ordinal.Equals(existing, reason)) + return; + } + + _truncationReasons.Add(reason); + } + } + /// /// Compute the per-key length sketch used by audit records. Strings → char count; /// arrays → element count; objects → key count; scalars (number / bool / null) → 0. @@ -407,6 +556,9 @@ internal sealed record AuditEvent( bool ToolTruncated = false, IReadOnlyList>? ArgKeyLengths = null, bool ArgValuesRedacted = false, + bool ArgValuesTruncated = false, + IReadOnlyList? ArgValueTruncationReasons = null, + int? ArgValuesSerializedBytes = null, int? CallerNameLength = null, bool CallerNameTruncated = false, int? CallerVersionLength = null, diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index bcc2790bf3..b88457874a 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2660,7 +2660,11 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); var (argKeys, argLengths, argKeyLengths, argValuesEcho) = - SanitizeArgs(args, _auditLog.IncludeValues, out var argValuesRedacted); + SanitizeArgs(args, _auditLog.IncludeValues, + out var argValuesRedacted, + out var argValuesTruncated, + out var argValueTruncationReasons, + out var argValuesSerializedBytes); var toolDisplay = BoundToolNameForDisplay(toolName); var evt = new AuditLogSink.AuditEvent( Timestamp: startedAt, @@ -2679,6 +2683,9 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ToolTruncated: toolDisplay.Truncated, ArgKeyLengths: argKeyLengths, ArgValuesRedacted: argValuesRedacted, + ArgValuesTruncated: argValuesTruncated, + ArgValueTruncationReasons: argValueTruncationReasons, + ArgValuesSerializedBytes: argValuesSerializedBytes, CallerNameLength: _clientNameDisplay?.Truncated == true ? _clientNameDisplay.Value.OriginalLength : null, CallerNameTruncated: _clientNameDisplay?.Truncated == true, CallerVersionLength: _clientVersionDisplay?.Truncated == true ? _clientVersionDisplay.Value.OriginalLength : null, @@ -2760,12 +2767,21 @@ 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 _); + => SanitizeArgs(args, includeValues, out _, out _, out _, out _); private static (IReadOnlyList Keys, IReadOnlyList> Lengths, IReadOnlyList> KeyLengths, JsonNode? ValuesEcho) - SanitizeArgs(JsonNode? args, bool includeValues, out bool argValuesRedacted) + SanitizeArgs( + JsonNode? args, + bool includeValues, + out bool argValuesRedacted, + out bool argValuesTruncated, + out IReadOnlyList argValueTruncationReasons, + out int? argValuesSerializedBytes) { argValuesRedacted = false; + argValuesTruncated = false; + argValueTruncationReasons = Array.Empty(); + argValuesSerializedBytes = null; if (args is not JsonObject argsObj) return (Array.Empty(), Array.Empty>(), Array.Empty>(), null); @@ -2774,6 +2790,8 @@ private static (IReadOnlyList Keys, IReadOnlyList>(); var usedKeys = new HashSet(StringComparer.Ordinal); JsonObject? echoObject = includeValues ? new JsonObject() : null; + AuditLogSink.ArgValueSanitizationState? valueState = includeValues ? new AuditLogSink.ArgValueSanitizationState() : null; + var argValueBudgetExhausted = false; foreach (var (key, value) in argsObj) { var keyDisplay = McpBoundedText.ForDisplay(key); @@ -2782,12 +2800,18 @@ private static (IReadOnlyList Keys, IReadOnlyList(displayKey, AuditLogSink.MeasureArgLength(value))); if (keyDisplay.Truncated) keyLengths.Add(new KeyValuePair(displayKey, keyDisplay.OriginalLength)); - if (echoObject is not null) + if (echoObject is not null && !argValueBudgetExhausted) { try { - echoObject[displayKey] = AuditLogSink.SanitizeArgValue(key, value, out var valueRedacted); - argValuesRedacted |= valueRedacted; + if (!valueState!.TryReservePropertyName(displayKey)) + { + argValueBudgetExhausted = true; + continue; + } + + echoObject[displayKey] = AuditLogSink.SanitizeArgValue(key, value, valueState!); + argValuesRedacted = valueState!.Redacted; } catch { @@ -2795,6 +2819,14 @@ private static (IReadOnlyList Keys, IReadOnlyList()); } + [Fact] + public void SanitizeArgValue_BudgetsPayloadBeforeClone_Issue3106() + { + var items = new JsonArray(); + for (var i = 0; i < AuditLogSink.MaxArgValueArrayItems + 2; i++) + items.Add(i); + var args = new JsonObject + { + ["query"] = new string('x', AuditLogSink.MaxArgValueStringChars + 10), + ["items"] = items, + }; + var state = new AuditLogSink.ArgValueSanitizationState(); + + var sanitized = AuditLogSink.SanitizeArgValue("arguments", args, state)!.AsObject(); + + Assert.True(state.Truncated); + Assert.Contains("string_length_limit", state.TruncationReasons); + Assert.Contains("array_item_count_limit", state.TruncationReasons); + Assert.EndsWith("...", sanitized["query"]!.GetValue(), StringComparison.Ordinal); + Assert.Equal(AuditLogSink.MaxArgValueArrayItems, sanitized["items"]!.AsArray().Count); + } + [Fact] public void MeasureArgLength_ReportsTypeSpecificCounts() { diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index 6486dbf436..0681c92463 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -316,6 +316,86 @@ public void ToolsCall_IncludeValues_RedactsSecretLikeArgumentValues_Issue3067() Assert.True(record.GetProperty("arg_values_redacted").GetBoolean()); } + [Fact] + public void ToolsCall_IncludeValues_BudgetsArgumentPayload_Issue3106() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); + using var server = CreateServer(sink); + var longQuery = new string('q', AuditLogSink.MaxArgValueStringChars + 25); + var items = new JsonArray(); + for (var i = 0; i < AuditLogSink.MaxArgValueArrayItems + 3; i++) + items.Add(i); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "does_not_exist", + ["arguments"] = new JsonObject + { + ["query"] = longQuery, + ["items"] = items, + }, + }, + }; + + _ = server.HandleMessage(request); + + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(longQuery, rawLog, StringComparison.Ordinal); + var record = ReadOnlyRecord(); + Assert.True(record.GetProperty("arg_values_truncated").GetBoolean()); + var reasons = record.GetProperty("arg_values_truncation_reasons").EnumerateArray() + .Select(reason => reason.GetString()) + .ToArray(); + Assert.Contains("string_length_limit", reasons); + Assert.Contains("array_item_count_limit", reasons); + var values = record.GetProperty("arg_values"); + Assert.EndsWith("...", values.GetProperty("query").GetString(), StringComparison.Ordinal); + Assert.Equal(AuditLogSink.MaxArgValueArrayItems, values.GetProperty("items").GetArrayLength()); + } + + [Fact] + public void ToolsCall_IncludeValues_ChargesTopLevelArgumentKeysToValueBudget_Issue3106() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); + using var server = CreateServer(sink); + + var arguments = new JsonObject(); + string? lastDisplayKey = null; + for (var i = 0; i < 64; i++) + { + var key = "arg" + i.ToString("D2", System.Globalization.CultureInfo.InvariantCulture) + + "_" + new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); + lastDisplayKey = McpBoundedText.ForDisplay(key).Text; + arguments[key] = new string('v', 200); + } + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "does_not_exist", + ["arguments"] = arguments, + }, + }; + + _ = server.HandleMessage(request); + + var record = ReadOnlyRecord(); + Assert.True(record.GetProperty("arg_values_truncated").GetBoolean()); + Assert.True(record.GetProperty("arg_values_serialized_bytes").GetInt32() <= AuditLogSink.MaxArgValuesSerializedBytes); + var reasons = record.GetProperty("arg_values_truncation_reasons").EnumerateArray() + .Select(reason => reason.GetString()) + .ToArray(); + Assert.Contains("serialized_bytes_limit", reasons); + Assert.False(record.GetProperty("arg_values").TryGetProperty(lastDisplayKey!, out _)); + } + [Fact] public void ToolsCall_ValuesOmitted_ByDefault() { From 0e0b7edc754683d14c324a8040b2fcbae6fc0f62 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Fri, 5 Jun 2026 10:44:59 +0900 Subject: [PATCH 4/4] Cap MCP audit log event fields (#3237) --- DEVELOPER_GUIDE.md | 8 +- USER_GUIDE.md | 10 ++ changelog.d/unreleased/3237.security.md | 20 ++++ src/CodeIndex/Mcp/AuditLogSink.cs | 108 ++++++++++++++++++++- src/CodeIndex/Mcp/McpServer.cs | 69 +++++++++++-- tests/CodeIndex.Tests/AuditLogSinkTests.cs | 92 ++++++++++++++++++ tests/CodeIndex.Tests/McpAuditLogTests.cs | 64 +++++++++++- tests/CodeIndex.Tests/McpServerTests.cs | 53 ++++++++++ 8 files changed, 408 insertions(+), 16 deletions(-) create mode 100644 changelog.d/unreleased/3237.security.md diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 116a177db3..b83f72eade 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1999,10 +1999,10 @@ who opens a cloud session, not by a real user after release. Contract guarantees that downstream consumers can rely on: -- **Field stability.** `timestamp`, `tool`, `arg_keys`, `arg_lengths`, `elapsed_ms`, `error_code` are emitted on every record. `caller`, `caller_version`, `request_id`, `arg_values`, `arg_values_redacted`, `arg_values_truncated`, `arg_values_truncation_reasons`, `arg_values_serialized_bytes`, `arg_values_max_bytes`, `result_count`, `error` are emitted only when non-null or true; renaming or repurposing any published field is a breaking change, the same policy as the CLI `--metrics` schema. +- **Field stability.** `timestamp`, `tool`, `arg_keys`, `arg_lengths`, `elapsed_ms`, `error_code` are emitted on every record. `caller`, `caller_version`, `request_id`, `request_id_length`, `request_id_truncated`, `arg_key_lengths`, `arg_keys_truncated`, `arg_key_truncation_reasons`, `arg_values`, `arg_values_redacted`, `arg_values_truncated`, `arg_values_truncation_reasons`, `arg_values_serialized_bytes`, `arg_values_max_bytes`, `result_count`, `error` are emitted only when non-null or true; renaming or repurposing any published field is a breaking change, the same policy as the CLI `--metrics` schema. - **Error code semantics.** `0` = success, `1` = MCP tool error (`isError: true`), negative = the verbatim JSON-RPC error code (e.g. `-32602` for invalid params, `-32603` for internal error). The companion `error` string is one of `jsonrpc_error`, `tool_error`, `missing_tool_name`, or the sanitized exception type name (`McpServer.BuildSanitizedToolErrorMessage` keeps `ex.Message` out of the wire and out of the audit, #1530). - **Result count.** `ExtractResultCount` prefers `structuredContent.count` over `structuredContent.results.length`; tool errors and JSON-RPC errors omit the field. Tools that return no count-shaped payload (e.g. `ping`) leave `result_count` absent rather than emitting `0`. -- **Argument privacy.** `arg_keys` and `arg_lengths` are always recorded so query *shape* is recoverable. `arg_values` is gated behind `--audit-log-include-values` because cdidx queries can carry literal source snippets or secret-shaped strings. The echo is a sanitized, budgeted clone: secret-like keys and known token patterns are replaced with `[REDACTED]`, and depth, object-property, array-item, total-node, string-length, and serialized-byte limits can mark `arg_values_truncated` before values are written. +- **Argument privacy.** `arg_keys` and `arg_lengths` are always recorded so query *shape* is recoverable, but argument-key count and displayed key length are capped and marked with `arg_keys_truncated`. `arg_values` is gated behind `--audit-log-include-values` because cdidx queries can carry literal source snippets or secret-shaped strings. The echo is a sanitized, budgeted clone: secret-like keys and known token patterns are replaced with `[REDACTED]`, and depth, object-property, array-item, total-node, string-length, serialized-byte, and event-byte limits can mark `arg_values_truncated` before values are written. - **Caller identity.** `_clientName` / `_clientVersion` are captured from every `initialize.clientInfo` and overwrite on reconnection within the same session, so a long-running MCP loop with multiple `initialize` handshakes attributes records to the *currently connected* client rather than the first one. - **Rotation.** Writes go through an open-append-close cycle so external `tail -F` consumers follow rotations and so the file is closed during the rename. When `_bytesWritten >= MaxBytes`, `RotateLocked` drops `.(RotationKeep-1)` (currently `.2`), cascades surviving slots up by one, and moves `` to `.1`. `RotationKeep = 3`, so `.3` is never created — exercised by `AuditLogSinkTests.Record_KeepsAtMostThreeFiles_DropsOldestOnRotationOverflow`. - **Best effort.** Serialization failures, IO failures, and rotation failures are swallowed (the audit must not crash the underlying tool call). The constructor still fails fast on impossible paths so the operator sees the misconfiguration before any tool dispatch happens. @@ -3576,10 +3576,10 @@ Cloud セッションは開発ループの中で `dotnet build` にフォール 下流コンシューマが依存できる契約: -- **フィールドの安定性。** `timestamp`、`tool`、`arg_keys`、`arg_lengths`、`elapsed_ms`、`error_code` は全レコードで出力する。`caller`、`caller_version`、`request_id`、`arg_values`、`arg_values_redacted`、`arg_values_truncated`、`arg_values_truncation_reasons`、`arg_values_serialized_bytes`、`arg_values_max_bytes`、`result_count`、`error` は値が non-null または true のときだけ含める。既存フィールドの改名や流用は破壊的変更扱い（CLI `--metrics` と同じ運用）。 +- **フィールドの安定性。** `timestamp`、`tool`、`arg_keys`、`arg_lengths`、`elapsed_ms`、`error_code` は全レコードで出力する。`caller`、`caller_version`、`request_id`、`request_id_length`、`request_id_truncated`、`arg_key_lengths`、`arg_keys_truncated`、`arg_key_truncation_reasons`、`arg_values`、`arg_values_redacted`、`arg_values_truncated`、`arg_values_truncation_reasons`、`arg_values_serialized_bytes`、`arg_values_max_bytes`、`result_count`、`error` は値が non-null または true のときだけ含める。既存フィールドの改名や流用は破壊的変更扱い（CLI `--metrics` と同じ運用）。 - **エラーコード意味論。** `0`=成功、`1`=MCP ツールエラー (`isError: true`)、負値=JSON-RPC エラーコードそのまま（例: invalid params なら `-32602`、internal error なら `-32603`）。同伴する `error` 文字列は `jsonrpc_error` / `tool_error` / `missing_tool_name` / サニタイズ済み例外型名のいずれか。`McpServer.BuildSanitizedToolErrorMessage` が `ex.Message` をワイヤーと audit から除外している（#1530）。 - **result count。** `ExtractResultCount` は `structuredContent.count` を優先し、無ければ `structuredContent.results.length`、いずれも無ければ省略する。ツールエラー / JSON-RPC エラー時も省略する（`0` ではなく欠落）。 -- **引数のプライバシー。** `arg_keys` / `arg_lengths` は常に記録するので呼び出しの *形状* は復元できる。`arg_values` は `--audit-log-include-values` に gated（cdidx クエリにはソース片や secret 風文字列が混入しうる）。echo は sanitize と budget を適用した clone として作り、secret 風のキーや既知 token pattern は `[REDACTED]` に置換し、depth / object property / array item / total node / string length / serialized byte の上限に達した場合は値を書き出す前に `arg_values_truncated` を記録する。 +- **引数のプライバシー。** `arg_keys` / `arg_lengths` は常に記録するので呼び出しの *形状* は復元できるが、引数キー数と表示キー長は capped され `arg_keys_truncated` で明示される。`arg_values` は `--audit-log-include-values` に gated（cdidx クエリにはソース片や secret 風文字列が混入しうる）。echo は sanitize と budget を適用した clone として作り、secret 風のキーや既知 token pattern は `[REDACTED]` に置換し、depth / object property / array item / total node / string length / serialized byte / event byte の上限に達した場合は値を書き出す前に `arg_values_truncated` を記録する。 - **呼び出し元の特定。** `_clientName` / `_clientVersion` は `initialize.clientInfo` から毎回キャプチャし、同一セッション内で再 `initialize` があれば上書きされる。複数 handshake が走る長寿命 MCP ループでも、*現在接続中の*クライアントに対して記録が紐付く。 - **ローテーション。** 1 レコードごとに open-append-close する。外部 `tail -F` の追従と rename 時の close-state 維持のため。`_bytesWritten >= MaxBytes` を超えた時点で `RotateLocked` が `.(RotationKeep-1)`（現在は `.2`）を破棄し、生存スロットを 1 つ古い側へ寄せ、`` を `.1` へ移す。`RotationKeep = 3` なので `.3` は決して生成されない（`AuditLogSinkTests.Record_KeepsAtMostThreeFiles_DropsOldestOnRotationOverflow` で常時検証）。 - **ベストエフォート。** シリアライズ失敗・IO 失敗・rotation 失敗はすべて握り潰す（監査の失敗で本体ツール呼び出しを壊さない）。一方、構築時の不正パスはコンストラクタが早期失敗させ、ディスパッチ前にオペレーターに気付かせる。 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 9214515643..1d1e1d2026 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -1388,7 +1388,12 @@ Each record is a single JSON object on its own line with these fields: | `caller` | string (optional) | `initialize.clientInfo.name` from the connected MCP client | | `caller_version` | string (optional) | `initialize.clientInfo.version` from the connected MCP client | | `request_id` | string (optional) | JSON-encoded JSON-RPC request id, when present | +| `request_id_length` | number (optional) | Original request id length when `request_id` is truncated | +| `request_id_truncated` | boolean (optional) | `true` when `request_id` was shortened for the audit record | | `arg_keys` | string[] | Ordered list of argument names supplied to the tool | +| `arg_key_lengths` | object (optional) | Original key lengths for truncated argument names | +| `arg_keys_truncated` | boolean (optional) | `true` when argument names or the argument-key list were truncated | +| `arg_key_truncation_reasons` | string[] (optional) | Stable truncation reason codes for argument-key truncation | | `arg_lengths` | object | Per-argument length sketch — string→char count, array→element count, object→key count, scalar→0 | | `arg_values` | object (optional) | Redacted and budgeted argument payload. Present only when `--audit-log-include-values` is enabled | | `arg_values_redacted` | boolean (optional) | `true` when secret-like keys or token patterns were replaced with `[REDACTED]` | @@ -3599,7 +3604,12 @@ MCP ツールで catch-all まで突き抜けた例外（想定外の SQLite 例 | `caller` | string（任意） | 接続中クライアントの `initialize.clientInfo.name` | | `caller_version` | string（任意） | 接続中クライアントの `initialize.clientInfo.version` | | `request_id` | string（任意） | JSON-RPC リクエスト id を JSON エンコードしたもの | +| `request_id_length` | number（任意） | `request_id` が短縮された場合の元の長さ | +| `request_id_truncated` | boolean（任意） | audit record 用に `request_id` が短縮された場合に `true` | | `arg_keys` | string[] | ツールへ渡された引数名の順序付きリスト | +| `arg_key_lengths` | object（任意） | 短縮された引数名の元の長さ | +| `arg_keys_truncated` | boolean（任意） | 引数名または引数キー一覧が短縮された場合に `true` | +| `arg_key_truncation_reasons` | string[]（任意） | 引数キー truncation の安定した reason code | | `arg_lengths` | object | 引数ごとの長さ概算（文字列→文字数、配列→要素数、オブジェクト→キー数、スカラ→0） | | `arg_values` | object（任意） | redaction および budget 適用済みの引数本体。`--audit-log-include-values` 指定時のみ付与 | | `arg_values_redacted` | boolean（任意） | secret 風のキーまたは token pattern が `[REDACTED]` に置き換えられた場合に `true` | diff --git a/changelog.d/unreleased/3237.security.md b/changelog.d/unreleased/3237.security.md new file mode 100644 index 0000000000..77ada9ffe5 --- /dev/null +++ b/changelog.d/unreleased/3237.security.md @@ -0,0 +1,20 @@ +--- +category: security +issues: + - 3237 +affected: + - src/CodeIndex/Mcp/AuditLogSink.cs + - src/CodeIndex/Mcp/McpServer.cs + - tests/CodeIndex.Tests/AuditLogSinkTests.cs + - tests/CodeIndex.Tests/McpAuditLogTests.cs + - USER_GUIDE.md + - DEVELOPER_GUIDE.md +--- + +## English + +- **MCP audit log events now cap field and record sizes (#3237)** — audit records now bound request ids, argument key lists, nested include-values keys, and oversized event payloads while marking truncation explicitly. + +## 日本語 + +- **MCP 監査ログイベントが field と record size を制限するようになりました (#3237)** — audit record は request id、引数キー一覧、include-values 内の nested key、過大 event payload を制限し、truncation を明示的に記録します。 diff --git a/src/CodeIndex/Mcp/AuditLogSink.cs b/src/CodeIndex/Mcp/AuditLogSink.cs index 2374bb441a..bfe8e8183d 100644 --- a/src/CodeIndex/Mcp/AuditLogSink.cs +++ b/src/CodeIndex/Mcp/AuditLogSink.cs @@ -38,6 +38,9 @@ internal sealed class AuditLogSink : IDisposable internal const int MaxArgValueTotalNodes = 512; internal const int MaxArgValueStringChars = 512; internal const int MaxArgValuesSerializedBytes = 16 * 1024; + internal const int MaxAuditArgumentCount = 64; + internal const int MaxRequestIdChars = 256; + internal const int MaxSerializedEventBytes = 64 * 1024; private static readonly Regex SecretValuePattern = new( "(?i)(github_pat_[A-Za-z0-9_]{20,}|gh[pousr]_[A-Za-z0-9_]{20,}|sk-(?:proj-)?[A-Za-z0-9_-]{20,}|xox[baprs]-[A-Za-z0-9-]{20,}|AKIA[0-9A-Z]{16}|://[^/\\s:@]+:[^/\\s:@]+@|(?:password|passwd|pwd|secret|token|api[_-]?key|access[_-]?key|authorization)=[^&\\s]+)", @@ -215,6 +218,43 @@ public void Dispose() } internal static string SerializeEvent(AuditEvent evt, bool includeValues) + { + var serialized = SerializeEventCore(evt, includeValues); + if (Encoding.UTF8.GetByteCount(serialized) <= MaxSerializedEventBytes) + return serialized; + + if (includeValues && evt.ArgValues is not null) + { + var fallback = evt with + { + ArgValues = null, + ArgValuesTruncated = true, + ArgValueTruncationReasons = AppendTruncationReason(evt.ArgValueTruncationReasons, "event_size_limit"), + ArgValuesSerializedBytes = null, + }; + serialized = SerializeEventCore(fallback, includeValues: false); + if (Encoding.UTF8.GetByteCount(serialized) <= MaxSerializedEventBytes) + return serialized; + + evt = fallback; + } + + var compact = evt with + { + ArgKeys = Array.Empty(), + ArgLengths = Array.Empty>(), + ArgKeyLengths = null, + ArgKeysTruncated = true, + ArgKeyTruncationReasons = AppendTruncationReason(evt.ArgKeyTruncationReasons, "event_size_limit"), + }; + serialized = SerializeEventCore(compact, includeValues && compact.ArgValues is not null); + if (Encoding.UTF8.GetByteCount(serialized) <= MaxSerializedEventBytes) + return serialized; + + return serialized; + } + + private static string SerializeEventCore(AuditEvent evt, bool includeValues) { using var buffer = new MemoryStream(); using (var jw = new Utf8JsonWriter(buffer, new JsonWriterOptions @@ -246,6 +286,10 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) 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(); @@ -259,6 +303,7 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) jw.WriteNumber(kv.Key, kv.Value); jw.WriteEndObject(); + var argKeysTruncated = evt.ArgKeysTruncated; if (evt.ArgKeyLengths is { Count: > 0 } argKeyLengths) { jw.WritePropertyName("arg_key_lengths"); @@ -266,7 +311,17 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) 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 (includeValues && evt.ArgValues is { } values) @@ -304,6 +359,22 @@ internal static string SerializeEvent(AuditEvent evt, bool includeValues) return Encoding.UTF8.GetString(buffer.ToArray()); } + private static IReadOnlyList AppendTruncationReason(IReadOnlyList? reasons, string reason) + { + var result = new List(); + if (reasons is not null) + { + foreach (var existing in reasons) + { + if (StringComparer.Ordinal.Equals(existing, reason)) + return reasons; + result.Add(existing); + } + } + result.Add(reason); + return result; + } + internal static JsonNode? SanitizeArgValue(string key, JsonNode? value, out bool redacted) { var state = new ArgValueSanitizationState(); @@ -355,6 +426,7 @@ private static JsonNode SanitizeObject(JsonObject obj, ArgValueSanitizationState return CreateTruncatedValue(); var clone = new JsonObject(); + var usedKeys = new HashSet(StringComparer.Ordinal); var propertyCount = 0; foreach (var (key, value) in obj) { @@ -364,10 +436,15 @@ private static JsonNode SanitizeObject(JsonObject obj, ArgValueSanitizationState break; } - if (!state.TryReservePropertyName(key)) + var keyDisplay = McpBoundedText.ForDisplay(key); + if (keyDisplay.Truncated) + state.AddTruncationReason("object_property_key_length_limit"); + var displayKey = MakeUniqueObjectDisplayKey(key, keyDisplay, usedKeys); + + if (!state.TryReservePropertyName(displayKey)) break; - clone[key] = SanitizeArgValueCore(key, value, state, depth + 1); + clone[displayKey] = SanitizeArgValueCore(key, value, state, depth + 1); propertyCount++; } return clone; @@ -441,6 +518,29 @@ private static int EstimateStringJsonBytes(string value) private static int EstimatePropertyNameJsonBytes(string key) => EstimateStringJsonBytes(key) + 1; + private static string MakeUniqueObjectDisplayKey(string rawKey, BoundedMcpText display, ISet usedKeys) + { + if (usedKeys.Add(display.Text)) + return display.Text; + + var disambiguator = 2; + while (true) + { + var suffix = "#" + disambiguator.ToString(CultureInfo.InvariantCulture); + var candidate = ComposeObjectDisplayKeyWithSuffix(rawKey, suffix); + if (usedKeys.Add(candidate)) + return candidate; + disambiguator++; + } + } + + private static string ComposeObjectDisplayKeyWithSuffix(string rawKey, string suffix) + { + const int maxDisplayTextChars = McpBoundedText.MaxDiagnosticDisplayChars + 3; + var maxPrefixChars = Math.Max(0, maxDisplayTextChars - suffix.Length - 3); + return McpBoundedText.ForDisplay(rawKey, maxPrefixChars).Text + suffix; + } + private static bool IsSecretLikeKey(string key) { var normalized = NormalizeKey(key); @@ -555,10 +655,14 @@ internal sealed record AuditEvent( int? ToolLength = null, bool ToolTruncated = false, IReadOnlyList>? ArgKeyLengths = null, + bool ArgKeysTruncated = false, + IReadOnlyList? ArgKeyTruncationReasons = null, bool ArgValuesRedacted = false, bool ArgValuesTruncated = false, IReadOnlyList? ArgValueTruncationReasons = null, int? ArgValuesSerializedBytes = null, + int? RequestIdLength = null, + bool RequestIdTruncated = false, int? CallerNameLength = null, bool CallerNameTruncated = false, int? CallerVersionLength = null, diff --git a/src/CodeIndex/Mcp/McpServer.cs b/src/CodeIndex/Mcp/McpServer.cs index b88457874a..b15bf79fb2 100644 --- a/src/CodeIndex/Mcp/McpServer.cs +++ b/src/CodeIndex/Mcp/McpServer.cs @@ -2566,7 +2566,15 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo var context = CurrentCorrelationContext.Value; var (errorCode, observedErrorType) = ExtractErrorCode(response); var resultCount = ExtractResultCount(response); - var (argKeys, argLengths, argKeyLengths, _) = SanitizeArgs(args, includeValues: false); + var (argKeys, argLengths, argKeyLengths, _) = SanitizeArgs( + args, + includeValues: false, + out _, + out _, + out _, + out _, + out var argKeysTruncated, + out var argKeyTruncationReasons); var toolDisplay = BoundToolNameForDisplay(toolName); var argsObject = new JsonObject(); foreach (var pair in argLengths) @@ -2589,6 +2597,10 @@ private void EmitToolInvocationTelemetry(string toolName, JsonNode? args, JsonNo }; toolDisplay.AddMetadata(evt, "tool"); AddArgKeyMetadata(evt, argKeyLengths); + if (argKeysTruncated) + evt["arg_keys_truncated"] = true; + if (argKeyTruncationReasons.Count > 0) + evt["arg_key_truncation_reasons"] = JsonSerializer.SerializeToNode(argKeyTruncationReasons, _jsonOptions); DeferFrameLog(() => WriteMcpLogLine(evt.ToJsonString(_jsonOptions))); } @@ -2664,14 +2676,20 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod out var argValuesRedacted, out var argValuesTruncated, out var argValueTruncationReasons, - out var argValuesSerializedBytes); + out var argValuesSerializedBytes, + out var argKeysTruncated, + out var argKeyTruncationReasons); var toolDisplay = BoundToolNameForDisplay(toolName); + var requestId = SerializeRequestId(id); + BoundedMcpText? requestIdDisplay = requestId is null + ? null + : McpBoundedText.ForDisplay(requestId, AuditLogSink.MaxRequestIdChars); var evt = new AuditLogSink.AuditEvent( Timestamp: startedAt, Tool: toolDisplay.Text, CallerName: _clientName, CallerVersion: _clientVersion, - RequestId: SerializeRequestId(id), + RequestId: requestIdDisplay?.Text, ArgKeys: argKeys, ArgLengths: argLengths, ArgValues: argValuesEcho, @@ -2682,10 +2700,14 @@ private void TryEmitAudit(string toolName, JsonNode? id, JsonNode? args, JsonNod ToolLength: toolDisplay.Truncated ? toolDisplay.OriginalLength : null, ToolTruncated: toolDisplay.Truncated, ArgKeyLengths: argKeyLengths, + ArgKeysTruncated: argKeysTruncated, + ArgKeyTruncationReasons: argKeyTruncationReasons, ArgValuesRedacted: argValuesRedacted, ArgValuesTruncated: argValuesTruncated, ArgValueTruncationReasons: argValueTruncationReasons, ArgValuesSerializedBytes: argValuesSerializedBytes, + RequestIdLength: requestIdDisplay?.Truncated == true ? requestIdDisplay.Value.OriginalLength : null, + RequestIdTruncated: requestIdDisplay?.Truncated == true, CallerNameLength: _clientNameDisplay?.Truncated == true ? _clientNameDisplay.Value.OriginalLength : null, CallerNameTruncated: _clientNameDisplay?.Truncated == true, CallerVersionLength: _clientVersionDisplay?.Truncated == true ? _clientVersionDisplay.Value.OriginalLength : null, @@ -2767,7 +2789,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 _); + => SanitizeArgs(args, includeValues, out _, out _, out _, out _, out _, out _); private static (IReadOnlyList Keys, IReadOnlyList> Lengths, IReadOnlyList> KeyLengths, JsonNode? ValuesEcho) SanitizeArgs( @@ -2776,12 +2798,17 @@ private static (IReadOnlyList Keys, IReadOnlyList argValueTruncationReasons, - out int? argValuesSerializedBytes) + out int? argValuesSerializedBytes, + out bool argKeysTruncated, + out IReadOnlyList argKeyTruncationReasons) { argValuesRedacted = false; argValuesTruncated = false; argValueTruncationReasons = Array.Empty(); argValuesSerializedBytes = null; + argKeysTruncated = false; + var argKeyReasons = new List(); + argKeyTruncationReasons = argKeyReasons; if (args is not JsonObject argsObj) return (Array.Empty(), Array.Empty>(), Array.Empty>(), null); @@ -2792,14 +2819,26 @@ private static (IReadOnlyList Keys, IReadOnlyList= AuditLogSink.MaxAuditArgumentCount) + { + argKeysTruncated = true; + AddUniqueReason(argKeyReasons, "arg_key_count_limit"); + break; + } + var keyDisplay = McpBoundedText.ForDisplay(key); var displayKey = MakeUniqueArgumentDisplayKey(key, keyDisplay, usedKeys); keys.Add(displayKey); lengths.Add(new KeyValuePair(displayKey, AuditLogSink.MeasureArgLength(value))); if (keyDisplay.Truncated) + { keyLengths.Add(new KeyValuePair(displayKey, keyDisplay.OriginalLength)); + argKeysTruncated = true; + AddUniqueReason(argKeyReasons, "arg_key_length_limit"); + } if (echoObject is not null && !argValueBudgetExhausted) { try @@ -2807,17 +2846,19 @@ private static (IReadOnlyList Keys, IReadOnlyList Keys, IReadOnlyList reasons, string reason) + { + foreach (var existing in reasons) + { + if (StringComparer.Ordinal.Equals(existing, reason)) + return; + } + reasons.Add(reason); + } + private static string MakeUniqueArgumentDisplayKey(string rawKey, BoundedMcpText display, ISet usedKeys) { if (usedKeys.Add(display.Text)) diff --git a/tests/CodeIndex.Tests/AuditLogSinkTests.cs b/tests/CodeIndex.Tests/AuditLogSinkTests.cs index e793020816..b80495aa16 100644 --- a/tests/CodeIndex.Tests/AuditLogSinkTests.cs +++ b/tests/CodeIndex.Tests/AuditLogSinkTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Text; using System.Text.Json; using System.Text.Json.Nodes; using CodeIndex.Cli; @@ -205,6 +206,97 @@ public void SanitizeArgValue_BudgetsPayloadBeforeClone_Issue3106() Assert.Equal(AuditLogSink.MaxArgValueArrayItems, sanitized["items"]!.AsArray().Count); } + [Fact] + public void SanitizeArgValue_TruncatesNestedObjectKeys_Issue3237() + { + var rawKey = new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); + var display = McpBoundedText.ForDisplay(rawKey); + var args = new JsonObject + { + [rawKey] = "value", + }; + var state = new AuditLogSink.ArgValueSanitizationState(); + + var sanitized = AuditLogSink.SanitizeArgValue("arguments", args, state)!.AsObject(); + + Assert.True(state.Truncated); + Assert.Contains("object_property_key_length_limit", state.TruncationReasons); + Assert.False(sanitized.ContainsKey(rawKey)); + Assert.True(sanitized.ContainsKey(display.Text)); + } + + [Fact] + public void SerializeEvent_DropsArgValues_WhenRecordExceedsEventBudget_Issue3237() + { + var evt = new AuditLogSink.AuditEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: "search", + CallerName: null, + CallerVersion: null, + RequestId: null, + ArgKeys: new[] { "query" }, + ArgLengths: new[] { new KeyValuePair("query", AuditLogSink.MaxSerializedEventBytes) }, + ArgValues: new JsonObject + { + ["query"] = new string('x', AuditLogSink.MaxSerializedEventBytes + 100), + }, + ResultCount: 0, + ElapsedMs: 1.0, + ErrorCode: 0, + ErrorType: null); + + var json = AuditLogSink.SerializeEvent(evt, includeValues: true); + + 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("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() + { + var keys = new List(); + var lengths = new List>(); + var keyLengths = new List>(); + for (var i = 0; i < AuditLogSink.MaxAuditArgumentCount; i++) + { + var key = "arg" + i.ToString(System.Globalization.CultureInfo.InvariantCulture) + + "_" + new string('\u6f22', 512); + keys.Add(key); + lengths.Add(new KeyValuePair(key, i)); + keyLengths.Add(new KeyValuePair(key, key.Length)); + } + var evt = new AuditLogSink.AuditEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: "search", + CallerName: null, + CallerVersion: null, + RequestId: null, + ArgKeys: keys, + ArgLengths: lengths, + ArgValues: null, + ResultCount: 0, + ElapsedMs: 1.0, + ErrorCode: 0, + ErrorType: null, + ArgKeyLengths: keyLengths); + + var json = AuditLogSink.SerializeEvent(evt, includeValues: false); + + Assert.True(Encoding.UTF8.GetByteCount(json) <= AuditLogSink.MaxSerializedEventBytes); + using var doc = JsonDocument.Parse(json); + var root = doc.RootElement; + 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("arg_keys_truncated").GetBoolean()); + Assert.Contains(root.GetProperty("arg_key_truncation_reasons").EnumerateArray(), + reason => reason.GetString() == "event_size_limit"); + } + [Fact] public void MeasureArgLength_ReportsTypeSpecificCounts() { diff --git a/tests/CodeIndex.Tests/McpAuditLogTests.cs b/tests/CodeIndex.Tests/McpAuditLogTests.cs index 0681c92463..89c80fb763 100644 --- a/tests/CodeIndex.Tests/McpAuditLogTests.cs +++ b/tests/CodeIndex.Tests/McpAuditLogTests.cs @@ -1,3 +1,4 @@ +using System.Globalization; using System.Text.Json; using System.Text.Json.Nodes; using CodeIndex.Cli; @@ -367,7 +368,7 @@ public void ToolsCall_IncludeValues_ChargesTopLevelArgumentKeysToValueBudget_Iss string? lastDisplayKey = null; for (var i = 0; i < 64; i++) { - var key = "arg" + i.ToString("D2", System.Globalization.CultureInfo.InvariantCulture) + var key = "arg" + i.ToString("D2", CultureInfo.InvariantCulture) + "_" + new string('k', McpBoundedText.MaxDiagnosticDisplayChars + 25); lastDisplayKey = McpBoundedText.ForDisplay(key).Text; arguments[key] = new string('v', 200); @@ -396,6 +397,67 @@ public void ToolsCall_IncludeValues_ChargesTopLevelArgumentKeysToValueBudget_Iss Assert.False(record.GetProperty("arg_values").TryGetProperty(lastDisplayKey!, out _)); } + [Fact] + public void ToolsCall_CapsAuditArgumentKeyCount_Issue3237() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: true); + using var server = CreateServer(sink); + var arguments = new JsonObject(); + for (var i = 0; i < AuditLogSink.MaxAuditArgumentCount + 3; i++) + arguments[$"arg{i.ToString(CultureInfo.InvariantCulture)}"] = i; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "does_not_exist", + ["arguments"] = arguments, + }, + }; + + _ = server.HandleMessage(request); + + var record = ReadOnlyRecord(); + Assert.Equal(AuditLogSink.MaxAuditArgumentCount, record.GetProperty("arg_keys").GetArrayLength()); + 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.False(record.GetProperty("arg_values").TryGetProperty( + $"arg{AuditLogSink.MaxAuditArgumentCount.ToString(CultureInfo.InvariantCulture)}", out _)); + } + + [Fact] + public void ToolsCall_TruncatesAuditRequestId_Issue3237() + { + using var sink = new AuditLogSink(_auditPath, AuditLogSink.DefaultMaxBytes, includeValues: false); + using var server = CreateServer(sink); + var id = new string('r', AuditLogSink.MaxRequestIdChars + 25); + var serializedId = JsonSerializer.Serialize(id); + var display = McpBoundedText.ForDisplay(serializedId, AuditLogSink.MaxRequestIdChars); + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "ping", + ["arguments"] = new JsonObject(), + }, + }; + + _ = server.HandleMessage(request); + + var rawLog = File.ReadAllText(_auditPath); + Assert.DoesNotContain(id, rawLog, StringComparison.Ordinal); + var record = ReadOnlyRecord(); + Assert.Equal(display.Text, record.GetProperty("request_id").GetString()); + Assert.Equal(serializedId.Length, record.GetProperty("request_id_length").GetInt32()); + Assert.True(record.GetProperty("request_id_truncated").GetBoolean()); + } + [Fact] public void ToolsCall_ValuesOmitted_ByDefault() { diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 85d3cfe26e..e34d938fc1 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -642,6 +642,59 @@ await Task.Run(() => Assert.True(root.GetProperty("arg_keys_truncated").GetBoolean()); } + [Fact] + public async Task ProcessLineAsync_CapsTelemetryArgumentKeyCount_Issue3237() + { + using var writer = new StringWriter(); + using var error = new StringWriter(); + var arguments = new JsonObject(); + for (var i = 0; i < AuditLogSink.MaxAuditArgumentCount + 3; i++) + arguments[$"arg{i}"] = i; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 123, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "does_not_exist", + ["arguments"] = arguments, + }, + }; + + await Task.Run(() => + { + lock (TestConsoleLock.Gate) + { + var previousError = Console.Error; + try + { + Console.SetError(error); +#pragma warning disable xUnit1031 + _server.ProcessLineAsync(request.ToJsonString(), writer).GetAwaiter().GetResult(); +#pragma warning restore xUnit1031 + } + finally + { + Console.SetError(previousError); + } + } + }); + + var line = error.ToString() + .Split('\n', StringSplitOptions.RemoveEmptyEntries) + .Single(l => l.Contains("\"event\":\"mcp.tool.invocation\"", StringComparison.Ordinal)); + var jsonStart = line.IndexOf('{'); + using var document = JsonDocument.Parse(line[jsonStart..]); + var root = document.RootElement; + Assert.Equal(AuditLogSink.MaxAuditArgumentCount, root.GetProperty("arg_keys").GetArrayLength()); + 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.DoesNotContain(root.GetProperty("arg_keys").EnumerateArray(), + key => key.GetString() == $"arg{AuditLogSink.MaxAuditArgumentCount}"); + } + [Fact] public async Task ProcessLineAsync_FallbackErrorIncludesCorrelationData() {