diff --git a/changelog.d/unreleased/3005.security.md b/changelog.d/unreleased/3005.security.md new file mode 100644 index 0000000000..bf101b73e4 --- /dev/null +++ b/changelog.d/unreleased/3005.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3005 +affected: + - src/CodeIndex/Cli/GlobalToolLog.cs + - tests/CodeIndex.Tests/GlobalToolLogTests.cs +--- + +## English + +- **Global tool logs now redact underscore-separated secret options (#3005)** — lifecycle argument logging now treats `--api_key` and `--access_key` forms like existing API/access key spellings, so both assignment and split-value forms are redacted. + +## 日本語 + +- **グローバルツールログが underscore 区切りの secret option を redact するようになりました (#3005)** — lifecycle 引数ログは `--api_key` と `--access_key` 形式も既存の API/access key 表記と同様に扱い、assignment 形式と値分離形式の両方を redact します。 diff --git a/changelog.d/unreleased/3006.security.md b/changelog.d/unreleased/3006.security.md new file mode 100644 index 0000000000..d3af322ed1 --- /dev/null +++ b/changelog.d/unreleased/3006.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3006 +affected: + - src/CodeIndex/Cli/SuggestionStore.cs + - tests/CodeIndex.Tests/SuggestionStoreTests.cs +--- + +## English + +- **Suggestion persistence now redacts common key assignments (#3006)** — suggestion storage and GitHub submission scrubbing now covers low-entropy `token=`, `api_key=`, and access-key assignment forms, including hyphen and underscore variants. + +## 日本語 + +- **Suggestion の永続化が一般的な key assignment を redact するようになりました (#3006)** — suggestion storage と GitHub submission の scrub は、低エントロピーの `token=`、`api_key=`、access-key assignment 形式を hyphen / underscore variants も含めて処理します。 diff --git a/changelog.d/unreleased/3070.security.md b/changelog.d/unreleased/3070.security.md new file mode 100644 index 0000000000..a92f6c1310 --- /dev/null +++ b/changelog.d/unreleased/3070.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3070 +affected: + - src/CodeIndex/Cli/GlobalToolLog.cs + - tests/CodeIndex.Tests/GlobalToolLogTests.cs +--- + +## English + +- **Global tool log redaction is now bounded (#3070)** — lifecycle argument scrubbing now uses timeout-bounded regexes and caps each argument before expensive redaction passes, replacing overlong suffixes with a truncation marker instead of logging them. + +## 日本語 + +- **グローバルツールログの redaction が bounded になりました (#3070)** — lifecycle 引数の scrub は timeout 付き regex と引数ごとの入力長 cap を使い、長大な suffix はログへ残さず truncation marker に置き換えます。 diff --git a/changelog.d/unreleased/3071.security.md b/changelog.d/unreleased/3071.security.md new file mode 100644 index 0000000000..f60827219c --- /dev/null +++ b/changelog.d/unreleased/3071.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3071 +affected: + - src/CodeIndex/Cli/SuggestionStore.cs + - tests/CodeIndex.Tests/SuggestionStoreTests.cs +--- + +## English + +- **Suggestion text redaction is now bounded (#3071)** — suggestion persistence and GitHub submission scrubbing now uses timeout-bounded regexes and caps each text field before redaction, replacing overlong suffixes with an explicit truncation marker. + +## 日本語 + +- **Suggestion text redaction が bounded になりました (#3071)** — suggestion persistence と GitHub submission の scrub は timeout 付き regex とフィールドごとの入力長 cap を使い、長大な suffix を明示的な truncation marker に置き換えます。 diff --git a/src/CodeIndex/Cli/GlobalToolLog.cs b/src/CodeIndex/Cli/GlobalToolLog.cs index e025e5d27b..4bdbaf300f 100644 --- a/src/CodeIndex/Cli/GlobalToolLog.cs +++ b/src/CodeIndex/Cli/GlobalToolLog.cs @@ -21,21 +21,28 @@ internal static class GlobalToolLog private const long DefaultLogMaxSizeBytes = 50L * 1024L * 1024L; internal const int MaxLogSizeMb = 1024; internal const long MaxLogSizeBytes = MaxLogSizeMb * 1024L * 1024L; + internal const int RedactionArgumentLengthLimit = 8192; + internal const string RedactionTruncationMarker = ""; private const string RedactedValue = ""; + private static readonly TimeSpan RedactionRegexTimeout = TimeSpan.FromSeconds(1); internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; private static readonly AsyncLocal CurrentSession = new(); private static readonly Regex SensitiveAssignmentPattern = new( - @"^(?--?[^=\s]*(?:token|password|passwd|pwd|secret|auth|apikey|api-key|access-key|credential)[^=\s]*)=(?.+)$", - RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled); + @"^(?--?[^=\s]*(?:token|password|passwd|pwd|secret|auth|apikey|api-key|api_key|access-key|access_key|credential)[^=\s]*)=(?.+)$", + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled, + RedactionRegexTimeout); private static readonly Regex UriUserInfoPattern = new( @"(?[a-z][a-z0-9+\-.]*://)(?[^:@/\s]+):(?[^@/\s]+)@", - RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled); + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled, + RedactionRegexTimeout); private static readonly Regex LongHexPattern = new( @"\b[0-9a-f]{32,}\b", - RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled); + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled, + RedactionRegexTimeout); private static readonly Regex LongBase64Pattern = new( @"\b[A-Za-z0-9+/]{40,}={0,2}\b", - RegexOptions.CultureInvariant | RegexOptions.Compiled); + RegexOptions.CultureInvariant | RegexOptions.Compiled, + RedactionRegexTimeout); internal static IDisposable? TryStart(string[] args, string appVersion) => TryStart(args, appVersion, createWriter: null, afterWriterCreated: null); @@ -488,20 +495,74 @@ private static bool IsSensitiveFlag(string arg) || arg.Contains("auth", StringComparison.OrdinalIgnoreCase) || arg.Contains("apikey", StringComparison.OrdinalIgnoreCase) || arg.Contains("api-key", StringComparison.OrdinalIgnoreCase) + || arg.Contains("api_key", StringComparison.OrdinalIgnoreCase) || arg.Contains("access-key", StringComparison.OrdinalIgnoreCase) + || arg.Contains("access_key", StringComparison.OrdinalIgnoreCase) || arg.Contains("credential", StringComparison.OrdinalIgnoreCase); } private static string RedactSensitiveText(string value) { - var assignment = SensitiveAssignmentPattern.Match(value); - if (assignment.Success) - value = $"{assignment.Groups["name"].Value}={RedactedValue}"; - - value = UriUserInfoPattern.Replace(value, match => $"{match.Groups["scheme"].Value}{match.Groups["user"].Value}:{RedactedValue}@"); - value = LongHexPattern.Replace(value, RedactedValue); - value = LongBase64Pattern.Replace(value, RedactedValue); - return value; + var truncated = false; + if (value.Length > RedactionArgumentLengthLimit) + { + value = value[..RedactionArgumentLengthLimit]; + truncated = true; + } + + try + { + if (value.Contains('=', StringComparison.Ordinal)) + { + var assignment = SensitiveAssignmentPattern.Match(value); + if (assignment.Success) + return $"{assignment.Groups["name"].Value}={RedactedValue}"; + } + + value = UriUserInfoPattern.Replace(value, match => $"{match.Groups["scheme"].Value}{match.Groups["user"].Value}:{RedactedValue}@"); + value = LongHexPattern.Replace(value, RedactedValue); + value = LongBase64Pattern.Replace(value, RedactedValue); + } + catch (RegexMatchTimeoutException) + { + return RedactedValue; + } + + if (truncated && LooksLikeTruncatedUriUserInfo(value)) + return RedactedValue; + + return truncated ? value + RedactionTruncationMarker : value; + } + + private static bool LooksLikeTruncatedUriUserInfo(string value) + { + var schemeEnd = value.IndexOf("://", StringComparison.Ordinal); + if (schemeEnd <= 0) + return false; + + var authorityStart = schemeEnd + 3; + if (authorityStart >= value.Length) + return false; + + var authorityEnd = value.Length; + for (var i = authorityStart; i < value.Length; i++) + { + var ch = value[i]; + if (char.IsWhiteSpace(ch) || ch == '/' || ch == '?' || ch == '#') + { + authorityEnd = i; + break; + } + } + + if (authorityEnd <= authorityStart) + return false; + + var authorityLength = authorityEnd - authorityStart; + if (value.IndexOf('@', authorityStart, authorityLength) >= 0) + return false; + + return value.IndexOf(':', authorityStart, authorityLength) >= 0; } private static string RedactPathLikeValue(string value) diff --git a/src/CodeIndex/Cli/SuggestionStore.cs b/src/CodeIndex/Cli/SuggestionStore.cs index 5633ebdef0..09157c1be3 100644 --- a/src/CodeIndex/Cli/SuggestionStore.cs +++ b/src/CodeIndex/Cli/SuggestionStore.cs @@ -40,10 +40,14 @@ public class SuggestionStore private const string RedactedBearerToken = "[REDACTED:bearer_token]"; private const string RedactedCredential = "[REDACTED:credential]"; private const string RedactedHighEntropyToken = "[REDACTED:high_entropy_token]"; - private static readonly Regex s_awsAccessKeyRegex = new(@"\bAKIA[0-9A-Z]{16}\b", RegexOptions.Compiled | RegexOptions.CultureInvariant); - private static readonly Regex s_bearerTokenRegex = new(@"\bBearer\s+[A-Za-z0-9._~+/=-]{16,}\b", RegexOptions.Compiled | RegexOptions.CultureInvariant); - private static readonly Regex s_namedSecretRegex = new(@"(?i)\b(password|secret)=([^&\s]{1,200})", RegexOptions.Compiled | RegexOptions.CultureInvariant); - private static readonly Regex s_highEntropyTokenRegex = new(@"\b(?=[A-Za-z0-9._~+/=-]{32,}\b)(?=.*[A-Z])(?=.*[a-z])(?=.*\d)[A-Za-z0-9._~+/=-]+\b", RegexOptions.Compiled | RegexOptions.CultureInvariant); + private const string RedactedRegexTimeout = "[REDACTED:redaction_timeout]"; + internal const int RedactionFieldLengthLimit = 32768; + internal const string RedactionTruncationMarker = "[REDACTED:truncated]"; + private static readonly TimeSpan RedactionRegexTimeout = TimeSpan.FromSeconds(1); + private static readonly Regex s_awsAccessKeyRegex = new(@"\bAKIA[0-9A-Z]{16}\b", RegexOptions.Compiled | RegexOptions.CultureInvariant, RedactionRegexTimeout); + private static readonly Regex s_bearerTokenRegex = new(@"\bBearer\s+[A-Za-z0-9._~+/=-]{16,}\b", RegexOptions.Compiled | RegexOptions.CultureInvariant, RedactionRegexTimeout); + private static readonly Regex s_namedSecretRegex = new(@"(?i)(^|[^\p{L}\p{N}_-])(?[\p{L}\p{N}_-]*(?:password|passwd|pwd|secret|token|api[-_]?key|access[-_]?key|credential)[\p{L}\p{N}_-]*)=(?[^&\s]+)", RegexOptions.Compiled | RegexOptions.CultureInvariant, RedactionRegexTimeout); + private static readonly Regex s_highEntropyTokenRegex = new(@"\b(?=[A-Za-z0-9._~+/=-]{32,}\b)(?=.*[A-Z])(?=.*[a-z])(?=.*\d)[A-Za-z0-9._~+/=-]+\b", RegexOptions.Compiled | RegexOptions.CultureInvariant, RedactionRegexTimeout); private static readonly HashSet s_dedupStopWords = new(StringComparer.Ordinal) { @@ -174,7 +178,8 @@ public record AddAndSubmitResult( string? UpstreamUrl, string? SubmissionError = null, string? DuplicateOfHash = null, - double? DuplicateScore = null); + double? DuplicateScore = null, + string? StoredHash = null); /// /// Result of a GitHub submission attempt. @@ -281,7 +286,8 @@ record = RedactRecordForPersistence(record); reservation.UpstreamUrl, null, reservation.DuplicateOfHash, - reservation.DuplicateScore); + reservation.DuplicateScore, + reservation.Hash); } SubmitAttemptResult submitResult; @@ -309,7 +315,8 @@ record = RedactRecordForPersistence(record); reservation.UpstreamUrl, null, reservation.DuplicateOfHash, - reservation.DuplicateScore); + reservation.DuplicateScore, + reservation.Hash); } var issueUrl = submitResult.IssueUrl; @@ -325,7 +332,8 @@ record = RedactRecordForPersistence(record); issueUrl ?? found.UpstreamUrl, submitResult.Error, reservation.DuplicateOfHash, - reservation.DuplicateScore); + reservation.DuplicateScore, + found.Hash); }); } @@ -873,6 +881,9 @@ private static void StampSubmitResult(SuggestionRecord record, SubmitAttemptResu McpClientName = record.McpClientName, McpClientVersion = record.McpClientVersion, ToolInvocationContext = record.ToolInvocationContext, + SampledTitle = record.SampledTitle, + SampledTags = record.SampledTags?.ToArray(), + EvidencePaths = record.EvidencePaths?.ToArray(), UpstreamIssueNumber = record.UpstreamIssueNumber, UpstreamUrl = record.UpstreamUrl, LastSyncedAt = record.LastSyncedAt, @@ -890,31 +901,48 @@ private static void StampSubmitResult(SuggestionRecord record, SubmitAttemptResu internal static string RedactSensitiveText(string text, out IReadOnlyCollection redactedTypes) { var types = new SortedSet(StringComparer.Ordinal); - var redacted = s_awsAccessKeyRegex.Replace(text, match => + var truncated = false; + if (text.Length > RedactionFieldLengthLimit) { - types.Add("aws_access_key"); - return RedactedAwsAccessKey; - }); - redacted = s_bearerTokenRegex.Replace(redacted, match => - { - types.Add("bearer_token"); - return RedactedBearerToken; - }); - redacted = s_namedSecretRegex.Replace(redacted, match => + text = text[..RedactionFieldLengthLimit]; + truncated = true; + types.Add("truncated"); + } + + try { - types.Add("credential"); - return $"{match.Groups[1].Value}={RedactedCredential}"; - }); - redacted = s_highEntropyTokenRegex.Replace(redacted, match => + var redacted = s_awsAccessKeyRegex.Replace(text, match => + { + types.Add("aws_access_key"); + return RedactedAwsAccessKey; + }); + redacted = s_bearerTokenRegex.Replace(redacted, match => + { + types.Add("bearer_token"); + return RedactedBearerToken; + }); + redacted = s_namedSecretRegex.Replace(redacted, match => + { + types.Add("credential"); + return $"{match.Groups[1].Value}{match.Groups["name"].Value}={RedactedCredential}"; + }); + redacted = s_highEntropyTokenRegex.Replace(redacted, match => + { + if (match.Value.StartsWith("[REDACTED:", StringComparison.Ordinal)) + return match.Value; + types.Add("high_entropy_token"); + return RedactedHighEntropyToken; + }); + + redactedTypes = types; + return truncated ? redacted + RedactionTruncationMarker : redacted; + } + catch (RegexMatchTimeoutException) { - if (match.Value.StartsWith("[REDACTED:", StringComparison.Ordinal)) - return match.Value; - types.Add("high_entropy_token"); - return RedactedHighEntropyToken; - }); - - redactedTypes = types; - return redacted; + types.Add("redaction_timeout"); + redactedTypes = types; + return RedactedRegexTimeout; + } } private static SuggestionRecord RedactRecordForPersistence(SuggestionRecord record) @@ -922,7 +950,16 @@ private static SuggestionRecord RedactRecordForPersistence(SuggestionRecord reco var redactedDescription = RedactNullable(record.Description, out var descriptionTypes) ?? string.Empty; var redactedContext = RedactNullable(record.Context, out var contextTypes); var redactedToolInvocationContext = RedactNullable(record.ToolInvocationContext, out var toolInvocationTypes); - var allTypes = descriptionTypes.Concat(contextTypes).Concat(toolInvocationTypes).Distinct(StringComparer.Ordinal).Order(StringComparer.Ordinal).ToArray(); + var redactedSampledTitle = RedactNullable(record.SampledTitle, out var sampledTitleTypes); + var redactedSampledTags = RedactArray(record.SampledTags, out var sampledTagTypes); + var allTypes = descriptionTypes + .Concat(contextTypes) + .Concat(toolInvocationTypes) + .Concat(sampledTitleTypes) + .Concat(sampledTagTypes) + .Distinct(StringComparer.Ordinal) + .Order(StringComparer.Ordinal) + .ToArray(); if (allTypes.Length == 0) return record; @@ -932,10 +969,33 @@ private static SuggestionRecord RedactRecordForPersistence(SuggestionRecord reco copy.Description = redactedDescription; copy.Context = redactedContext; copy.ToolInvocationContext = redactedToolInvocationContext; + copy.SampledTitle = redactedSampledTitle; + copy.SampledTags = redactedSampledTags; copy.Hash = ComputeHash(copy.Category, copy.Language, copy.Description); return copy; } + private static string[]? RedactArray(string[]? values, out IReadOnlyCollection redactedTypes) + { + if (values == null) + { + redactedTypes = Array.Empty(); + return null; + } + + var types = new SortedSet(StringComparer.Ordinal); + var redacted = new string[values.Length]; + for (var i = 0; i < values.Length; i++) + { + redacted[i] = RedactSensitiveText(values[i] ?? string.Empty, out var valueTypes); + foreach (var type in valueTypes) + types.Add(type); + } + + redactedTypes = types; + return redacted; + } + private static string? RedactNullable(string? value, out IReadOnlyCollection redactedTypes) { if (value == null) diff --git a/src/CodeIndex/Cli/SuggestionsCommandRunner.cs b/src/CodeIndex/Cli/SuggestionsCommandRunner.cs index 7028939719..f16efdcc86 100644 --- a/src/CodeIndex/Cli/SuggestionsCommandRunner.cs +++ b/src/CodeIndex/Cli/SuggestionsCommandRunner.cs @@ -305,8 +305,8 @@ private static string FormatTitle(string description, int maxLength) record.McpClientName, record.McpClientVersion, record.ToolInvocationContext, - record.SampledTitle, - NormalizeNullableArray(record.SampledTags), + RedactSuggestionOutputValue(record.SampledTitle), + RedactSuggestionOutputArray(record.SampledTags), NormalizeEvidencePaths(record), record.Description, record.Context, @@ -351,9 +351,19 @@ private static string BuildIssueDraftTitle(SuggestionRecord record) var titleSource = !string.IsNullOrWhiteSpace(record.SampledTitle) ? record.SampledTitle : record.Description; - return GitHubIssueReporter.BuildIssueTitle(record.Category, titleSource); + return GitHubIssueReporter.BuildIssueTitle(record.Category, RedactSuggestionOutputValue(titleSource) ?? string.Empty); } + private static string? RedactSuggestionOutputValue(string? value) + => value == null ? null : SuggestionStore.RedactSensitiveText(value, out _); + + private static List RedactSuggestionOutputArray(string[]? values) + => NormalizeNullableArray(values) + .Select(RedactSuggestionOutputValue) + .Where(value => value != null) + .Cast() + .ToList(); + private static string BuildIssueDraftBody(SuggestionRecord record, IReadOnlyList evidencePaths) { var sb = new StringBuilder(); diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 828d8c1a87..61240f2519 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -4356,7 +4356,13 @@ private async Task ExecuteSuggestImprovementAsync(JsonNode? id, JsonNo if (toolInvocationContext != null && SourceCodeDetector.ContainsSourceCode(toolInvocationContext)) return CreateToolErrorResponse(id, "Tool invocation context appears to contain source code. Please describe the invocation without including code."); - var sampling = await TrySampleSuggestionMetadataAsync(category, language, description, context, toolInvocationContext).ConfigureAwait(false); + var sampling = await TrySampleSuggestionMetadataAsync( + category, + language, + RedactSuggestionSamplingInput(description), + context == null ? null : RedactSuggestionSamplingInput(context), + toolInvocationContext == null ? null : RedactSuggestionSamplingInput(toolInvocationContext)).ConfigureAwait(false); + sampling = RedactSuggestionSamplingResult(sampling); // 4. Compute dedup hash / 重複排除ハッシュを計算 var hash = SuggestionStore.ComputeHash(category, language, description); @@ -4413,13 +4419,14 @@ private async Task ExecuteSuggestImprovementAsync(JsonNode? id, JsonNo } var result = await store.TryAddAndSubmitAsync(record, githubCallback).ConfigureAwait(false); + var storedHash = result.StoredHash ?? hash; if (!result.IsNew) { var dupPayload = new JsonObject { ["status"] = "duplicate", - ["hash"] = hash, + ["hash"] = storedHash, ["message"] = result.AlreadySubmitted ? "This suggestion has already been recorded and submitted." : result.UpstreamUrl != null @@ -4448,7 +4455,7 @@ private async Task ExecuteSuggestImprovementAsync(JsonNode? id, JsonNo var payload = new JsonObject { ["status"] = "recorded", - ["hash"] = hash, + ["hash"] = storedHash, ["category"] = category, ["language"] = language, ["stored_locally"] = true, @@ -4539,6 +4546,33 @@ private static bool StartsWithHttpStatusCode(string value) private sealed record SuggestionSamplingResult(string? Title, string[]? Tags); + private static string RedactSuggestionSamplingInput(string value) + => SuggestionStore.RedactSensitiveText(value, out _); + + private static SuggestionSamplingResult? RedactSuggestionSamplingResult(SuggestionSamplingResult? sampling) + { + if (sampling == null) + return null; + + var title = SanitizeSampledTitle(RedactNullableSamplingValue(sampling.Title)); + var tags = sampling.Tags? + .Select(RedactNullableSamplingValue) + .Where(t => !string.IsNullOrWhiteSpace(t)) + .Select(SanitizeSampledTag) + .Where(t => t != null) + .Cast() + .Distinct(StringComparer.Ordinal) + .Take(6) + .ToArray(); + + return title == null && (tags == null || tags.Length == 0) + ? null + : new SuggestionSamplingResult(title, tags is { Length: > 0 } ? tags : null); + } + + private static string? RedactNullableSamplingValue(string? value) + => value == null ? null : SuggestionStore.RedactSensitiveText(value, out _); + private async Task TrySampleSuggestionMetadataAsync( string category, string? language, @@ -4576,9 +4610,11 @@ private sealed record SuggestionSamplingResult(string? Title, string[]? Tags); try { var parsed = JsonNode.Parse(text, documentOptions: new JsonDocumentOptions { MaxDepth = MaxSamplingResponseJsonDepth }); - var title = SanitizeSampledTitle(TryReadStringValue(parsed?["title"])); + var title = SanitizeSampledTitle(RedactNullableSamplingValue(TryReadStringValue(parsed?["title"]))); var tags = parsed?["tags"] is JsonArray tagArray ? tagArray.Select(TryReadStringValue) + .Where(t => !string.IsNullOrWhiteSpace(t)) + .Select(RedactNullableSamplingValue) .Where(t => !string.IsNullOrWhiteSpace(t)) .Select(SanitizeSampledTag) .Where(t => t != null) diff --git a/tests/CodeIndex.Tests/GlobalToolLogTests.cs b/tests/CodeIndex.Tests/GlobalToolLogTests.cs index 969b91e4d7..8a88f7dd3a 100644 --- a/tests/CodeIndex.Tests/GlobalToolLogTests.cs +++ b/tests/CodeIndex.Tests/GlobalToolLogTests.cs @@ -52,6 +52,64 @@ public void FormatArgs_RedactsSensitiveArgumentsByDefault() Assert.DoesNotContain("0123456789abcdef0123456789abcdef", formatted); } + [Fact] + public void FormatArgs_RedactsUnderscoreSeparatedSecretArguments() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_LOG_REDACT"); + env.Set("CDIDX_LOG_REDACT", null); + + var formatted = GlobalToolLog.FormatArgs([ + "--api_key=api-secret", + "--access_key", + "access-secret", + ]); + + Assert.Contains("--api_key=", formatted); + Assert.Contains("--access_key ", formatted); + Assert.DoesNotContain("api-secret", formatted); + Assert.DoesNotContain("access-secret", formatted); + } + + [Fact] + public void FormatArgs_TruncatesOverlongArgumentBeforeRedaction() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_LOG_REDACT"); + env.Set("CDIDX_LOG_REDACT", null); + var tailSecret = "tail-secret-value-should-not-survive"; + var argument = "safe:" + new string('!', GlobalToolLog.RedactionArgumentLengthLimit) + tailSecret; + + var formatted = GlobalToolLog.FormatArgs([argument]); + + Assert.Contains(GlobalToolLog.RedactionTruncationMarker, formatted); + Assert.DoesNotContain(tailSecret, formatted); + Assert.True(formatted.Length < argument.Length); + } + + [Fact] + public void FormatArgs_RedactsOverlongUriUserInfoBeforeTruncation() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_LOG_REDACT"); + env.Set("CDIDX_LOG_REDACT", null); + var argument = "https://user:" + new string('!', GlobalToolLog.RedactionArgumentLengthLimit) + "@example.test/repo.git"; + + var formatted = GlobalToolLog.FormatArgs([argument]); + + Assert.Equal("", formatted); + Assert.DoesNotContain("user:", formatted); + } + + [Fact] + public void FormatArgs_RedactsOverlongSensitiveAssignment() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_LOG_REDACT"); + env.Set("CDIDX_LOG_REDACT", null); + var argument = "--api_key=" + new string('x', GlobalToolLog.RedactionArgumentLengthLimit * 2); + + var formatted = GlobalToolLog.FormatArgs([argument]); + + Assert.Equal("--api_key=", formatted); + } + [Fact] public void FormatArgs_AllowsExplicitNoRedaction() { diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 95eb0e1fef..fab7f08c6b 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -10706,6 +10706,40 @@ public void SuggestImprovement_RecordsClientAttributionFromInitialize() Assert.Equal(["src/CodeIndex/Mcp/McpToolHandlers.cs"], stored.EvidencePaths); } + [Fact] + public void SuggestImprovement_RedactedDescriptionReturnsStoredHash() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_GITHUB_TOKEN"); + env.Set("CDIDX_GITHUB_TOKEN", null); + var secret = $"secret-{Guid.NewGuid():N}"; + var description = $"MCP redaction hash regression api_key={secret}"; + var json = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "suggest_improvement", + ["arguments"] = new JsonObject + { + ["category"] = "other", + ["description"] = description, + } + } + }; + + var response = _server.HandleMessage((JsonNode)json)!; + + var structured = response["result"]!["structuredContent"]!; + var responseHash = structured["hash"]!.GetValue(); + var stored = new SuggestionStore(Path.GetDirectoryName(_dbPath)!, Path.GetFileNameWithoutExtension(_dbPath)).LoadAll() + .Single(s => s.Hash == responseHash); + Assert.Equal(stored.Hash, responseHash); + Assert.Contains("api_key=[REDACTED:credential]", stored.Description); + Assert.DoesNotContain(secret, stored.Description); + } + [Fact] public void SuggestImprovement_RejectsNonRelativeEvidencePath() { @@ -10780,6 +10814,64 @@ public void SuggestImprovement_WhenSamplingAvailable_StoresSampledMetadata() Assert.Contains("symbol_extraction", stored.SampledTags!); } + [Fact] + public void SuggestImprovement_WhenSamplingReturnsSensitiveMetadata_RedactsBeforeResponseAndPersistence() + { + using var env = EnvironmentVariableScope.Capture("CDIDX_GITHUB_TOKEN"); + env.Set("CDIDX_GITHUB_TOKEN", null); + _server.HandleMessage(JsonNode.Parse( + """{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"capabilities":{"sampling":{}}}}""")!); + var secret = $"sample-secret-{Guid.NewGuid():N}"; + string? capturedPrompt = null; + _server.ClientRequestHandlerForTests = (method, parameters) => + { + Assert.Equal("sampling/createMessage", method); + capturedPrompt = parameters?["messages"]?[0]?["content"]?["text"]?.GetValue(); + return new JsonObject + { + ["content"] = new JsonObject + { + ["type"] = "text", + ["text"] = $$"""{"title":"Echoed api_key={{secret}}","tags":["github_token={{secret}}"]}""" + } + }; + }; + var description = $"Sampling metadata redaction regression api_key={secret}"; + var request = new JsonObject + { + ["jsonrpc"] = "2.0", + ["id"] = 1, + ["method"] = "tools/call", + ["params"] = new JsonObject + { + ["name"] = "suggest_improvement", + ["arguments"] = new JsonObject + { + ["category"] = "other", + ["description"] = description, + } + } + }; + + var response = _server.HandleMessage(request)!; + + var structured = response["result"]!["structuredContent"]!; + var sampledTitle = structured["sampled_title"]!.GetValue(); + var sampledTags = string.Join(" ", structured["sampled_tags"]!.AsArray().Select(tag => tag!.GetValue())); + Assert.Contains("api_key=[REDACTED:credential]", sampledTitle); + Assert.Contains("redacted", sampledTags); + Assert.NotNull(capturedPrompt); + Assert.DoesNotContain(secret, capturedPrompt); + Assert.DoesNotContain(secret, sampledTitle); + Assert.DoesNotContain(secret, sampledTags); + var responseHash = structured["hash"]!.GetValue(); + var stored = new SuggestionStore(Path.GetDirectoryName(_dbPath)!, Path.GetFileNameWithoutExtension(_dbPath)).LoadAll() + .Single(s => s.Hash == responseHash); + Assert.DoesNotContain(secret, stored.Description); + Assert.DoesNotContain(secret, stored.SampledTitle!); + Assert.DoesNotContain(secret, string.Join(" ", stored.SampledTags!)); + } + [Fact] public void SuggestImprovement_WhenSamplingResponseIsTooLarge_IgnoresSampledMetadata() { diff --git a/tests/CodeIndex.Tests/ProgramCliTests.cs b/tests/CodeIndex.Tests/ProgramCliTests.cs index 1288683970..8573cd49ea 100644 --- a/tests/CodeIndex.Tests/ProgramCliTests.cs +++ b/tests/CodeIndex.Tests/ProgramCliTests.cs @@ -748,6 +748,31 @@ public void Suggestions_ExportIssueDraftsIncludesEvidenceAndDuplicatePreflight() Assert.Equal("title_exact", preflight.GetProperty("matches")[0].GetProperty("reason").GetString()); } + [Fact] + public void Suggestions_ExportIssueDraftsRedactsSensitiveSampledTitle() + { + using var fixture = SuggestionFixture.Create(); + var secret = $"issue-draft-secret-{Guid.NewGuid():N}"; + fixture.Add( + "output_format", + "csharp", + "Issue draft export should redact sampled metadata", + submitted: false, + sampledTitle: $"Leaked api_key={secret}"); + var openIssuesPath = fixture.WriteOpenIssuesJson("[]"); + + var (exitCode, stdout, stderr) = RunCliInSubprocess([ + "suggestions", "export", "--db", fixture.DbPath, "--format", "issue-drafts", "--open-issues", openIssuesPath + ]); + + Assert.Equal(0, exitCode); + Assert.Equal(string.Empty, stderr); + Assert.DoesNotContain(secret, stdout); + using var doc = JsonDocument.Parse(stdout); + var title = doc.RootElement.GetProperty("drafts")[0].GetProperty("title").GetString(); + Assert.Contains("REDACTED:credential", title!); + } + [Fact] public void Suggestions_ExportIssueDraftsRejectsOversizedOpenIssuesPreflight() { diff --git a/tests/CodeIndex.Tests/SuggestionStoreTests.cs b/tests/CodeIndex.Tests/SuggestionStoreTests.cs index c0994adae0..b137c5c0e6 100644 --- a/tests/CodeIndex.Tests/SuggestionStoreTests.cs +++ b/tests/CodeIndex.Tests/SuggestionStoreTests.cs @@ -330,21 +330,75 @@ public void TryAdd_RedactsSensitiveTextBeforePersistence() var record = MakeRecord( "other", null, - "AWS AKIA1234567890ABCDEF and password=swordfish and Bearer AbCdEfGhIjKlMnOpQrStUvWxYz123456 should not persist"); + "AWS AKIA1234567890ABCDEF and password=swordfish and token=tok123 and github_token=git123 and api_key=abc123 and openai_api_key=oa123 and access-key=def456 and CDIDX_GITHUB_TOKEN=cdidx123 and Bearer AbCdEfGhIjKlMnOpQrStUvWxYz123456 should not persist"); record.Context = "token aaBB11ccDD22eeFF33ggHH44iiJJ55kk"; - record.ToolInvocationContext = "secret=hunter2"; + record.ToolInvocationContext = "secret=hunter2 access_key=ghi789"; + record.SampledTitle = "Sensitive text redaction"; + record.SampledTags = ["security", "suggestions"]; + record.EvidencePaths = ["src/CodeIndex/Cli/SuggestionStore.cs"]; Assert.True(_store.TryAdd(record)); var stored = Assert.Single(_store.LoadAll()); Assert.Contains("[REDACTED:aws_access_key]", stored.Description); Assert.Contains("password=[REDACTED:credential]", stored.Description); + Assert.Contains("token=[REDACTED:credential]", stored.Description); + Assert.Contains("github_token=[REDACTED:credential]", stored.Description); + Assert.Contains("api_key=[REDACTED:credential]", stored.Description); + Assert.Contains("openai_api_key=[REDACTED:credential]", stored.Description); + Assert.Contains("access-key=[REDACTED:credential]", stored.Description); + Assert.Contains("CDIDX_GITHUB_TOKEN=[REDACTED:credential]", stored.Description); Assert.Contains("[REDACTED:bearer_token]", stored.Description); Assert.Contains("[REDACTED:high_entropy_token]", stored.Context); Assert.Contains("secret=[REDACTED:credential]", stored.ToolInvocationContext); + Assert.Contains("access_key=[REDACTED:credential]", stored.ToolInvocationContext); + Assert.Equal("Sensitive text redaction", stored.SampledTitle); + Assert.Equal(["security", "suggestions"], stored.SampledTags); + Assert.Equal(["src/CodeIndex/Cli/SuggestionStore.cs"], stored.EvidencePaths); Assert.DoesNotContain("AKIA1234567890ABCDEF", stored.Description); Assert.DoesNotContain("swordfish", stored.Description); + Assert.DoesNotContain("tok123", stored.Description); + Assert.DoesNotContain("git123", stored.Description); + Assert.DoesNotContain("abc123", stored.Description); + Assert.DoesNotContain("oa123", stored.Description); + Assert.DoesNotContain("def456", stored.Description); + Assert.DoesNotContain("cdidx123", stored.Description); Assert.DoesNotContain("hunter2", stored.ToolInvocationContext); + Assert.DoesNotContain("ghi789", stored.ToolInvocationContext); + } + + [Fact] + public void TryAdd_RedactsSensitiveSampledMetadataBeforePersistence() + { + var record = MakeRecord("other", null, "Sampled metadata redaction"); + record.SampledTitle = "Sampled title api_key=sample-title-secret"; + record.SampledTags = ["security", "github_token=sample-tag-secret"]; + + Assert.True(_store.TryAdd(record)); + + var stored = Assert.Single(_store.LoadAll()); + Assert.Contains("api_key=[REDACTED:credential]", stored.SampledTitle!); + Assert.Contains("github_token=[REDACTED:credential]", stored.SampledTags!); + Assert.DoesNotContain("sample-title-secret", stored.SampledTitle!); + Assert.DoesNotContain("sample-tag-secret", string.Join(" ", stored.SampledTags!)); + } + + [Fact] + public void TryAdd_TruncatesLargeSensitiveFieldsBeforePersistence() + { + var tailSecret = "tail-secret-value-should-not-survive"; + var record = MakeRecord( + "other", + null, + "api_key=" + new string('a', SuggestionStore.RedactionFieldLengthLimit) + tailSecret); + + Assert.True(_store.TryAdd(record)); + + var stored = Assert.Single(_store.LoadAll()); + Assert.Contains("api_key=[REDACTED:credential]", stored.Description); + Assert.Contains(SuggestionStore.RedactionTruncationMarker, stored.Description); + Assert.DoesNotContain(tailSecret, stored.Description); + Assert.True(stored.Description.Length < record.Description.Length); } [Fact]