diff --git a/changelog.d/unreleased/3759.security.md b/changelog.d/unreleased/3759.security.md new file mode 100644 index 0000000000..6487c4e1f3 --- /dev/null +++ b/changelog.d/unreleased/3759.security.md @@ -0,0 +1,19 @@ +--- +category: security +issues: + - 3759 +affected: + - src/CodeIndex/Indexer/IsolatedWorkerProcessLauncher.cs + - src/CodeIndex/WorkerProtocolJsonValidator.cs + - src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs + - src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs + - tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +--- + +## English + +- **Isolated workers now scrub inherited environments and validate protocol payload shape (#3759)** — symbol extraction and hook callback workers now start with a small environment allowlist, keep `CDIDX_TEST_*` only for test harnesses, and reject oversized JSON property/string payloads before deserialization. + +## 日本語 + +- **isolated worker が継承環境を scrub し、protocol payload 形状を検証するようになりました (#3759)** — symbol extraction worker と hook callback worker は小さな環境 allowlist で起動し、test harness 用の `CDIDX_TEST_*` だけを保持し、deserialize 前に過大な JSON property/string payload を拒否します。 diff --git a/changelog.d/unreleased/3790.fixed.md b/changelog.d/unreleased/3790.fixed.md new file mode 100644 index 0000000000..b9df332b89 --- /dev/null +++ b/changelog.d/unreleased/3790.fixed.md @@ -0,0 +1,19 @@ +--- +category: fixed +issues: + - 3790 +affected: + - src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.cs + - src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PluginLoading.cs + - src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs + - tests/CodeIndex.Tests/ExtractorPluginRegistryTests.cs + - tests/CodeIndex.Tests/PostExtractionHookTests.cs +--- + +## English + +- **Plugin and hook assembly inspection now has type-count and lifetime bounds (#3790)** — plugin duplicate path checks now follow the probed filesystem casing policy, plugin and hook assemblies with excessive type counts are skipped with bounded diagnostics, and hook assemblies that retain no hook types unload their collectible load context. + +## 日本語 + +- **plugin/hook assembly inspection に type-count と lifetime の上限を追加しました (#3790)** — plugin duplicate path 判定は probe 済み filesystem casing policy に従い、type count が過大な plugin/hook assembly は bounded diagnostics 付きで skipped になり、hook type を保持しない hook assembly は collectible load context を unload します。 diff --git a/changelog.d/unreleased/3821.fixed.md b/changelog.d/unreleased/3821.fixed.md new file mode 100644 index 0000000000..c1c2e6d690 --- /dev/null +++ b/changelog.d/unreleased/3821.fixed.md @@ -0,0 +1,19 @@ +--- +category: fixed +issues: + - 3821 +affected: + - src/CodeIndex/Indexer/Extensibility/ConfiguredSymbolExtractor.cs + - src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.Diagnostics.cs + - src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PatternConfig.cs + - src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PluginLoading.cs + - tests/CodeIndex.Tests/ExtractorPluginRegistryTests.cs +--- + +## English + +- **Pattern extractor timeouts now appear in structured registry diagnostics (#3821)** — pattern regex timeouts are reported through extractor registry status as bounded diagnostics, pattern kind rejection messages use the same sanitizer path as invalid regex diagnostics, and plugin loading no longer materializes the candidate path list before loading. + +## 日本語 + +- **pattern extractor timeout が構造化された registry diagnostics に表示されるようになりました (#3821)** — pattern regex timeout は extractor registry status の bounded diagnostics として報告され、pattern kind の rejection message は invalid regex diagnostics と同じ sanitizer 経路を使い、plugin loading は load 前に candidate path list を materialize しないようになりました。 diff --git a/changelog.d/unreleased/3836.fixed.md b/changelog.d/unreleased/3836.fixed.md new file mode 100644 index 0000000000..9f2d6335b6 --- /dev/null +++ b/changelog.d/unreleased/3836.fixed.md @@ -0,0 +1,21 @@ +--- +category: fixed +issues: + - 3836 +affected: + - src/CodeIndex/BoundedLineReader.cs + - src/CodeIndex/BoundedTextWriter.cs + - src/CodeIndex/WorkerOutputBuffer.cs + - src/CodeIndex/SafeDiagnosticFormatter.cs + - src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs + - src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs + - tests/CodeIndex.Tests/BoundedLineReaderTests.cs +--- + +## English + +- **Worker protocol reads and diagnostics are more tightly bounded (#3836)** — async protocol line reads now buffer safely without dropping following lines, isolated worker stderr tails are capped and sanitized in exit diagnostics, and hook callback workers bound captured console output while accepting caller cancellation for response reads. + +## 日本語 + +- **worker protocol の読み取りと診断の上限を強化しました (#3836)** — async protocol line read は後続行を落とさず安全にバッファし、isolated worker の stderr tail は exit 診断内で上限付き・sanitize 済みにし、hook callback worker は captured console output を bounded にして response read で caller cancellation を受け付けるようになりました。 diff --git a/src/CodeIndex/BoundedLineReader.cs b/src/CodeIndex/BoundedLineReader.cs index 0e0471f0d9..5d66a9a17e 100644 --- a/src/CodeIndex/BoundedLineReader.cs +++ b/src/CodeIndex/BoundedLineReader.cs @@ -1,4 +1,5 @@ using System.Text; +using System.Runtime.CompilerServices; namespace CodeIndex; @@ -48,6 +49,9 @@ internal static int ResolveForSourceFileBytes(long? maxFileSizeBytes) internal static class BoundedLineReader { + private const int AsyncReadBufferSize = 4096; + private static readonly ConditionalWeakTable AsyncBuffers = new(); + internal static string? ReadLine(TextReader reader, int maxCharacters, int maxUtf8Bytes) { ArgumentNullException.ThrowIfNull(reader); @@ -72,16 +76,58 @@ internal static class BoundedLineReader { ArgumentNullException.ThrowIfNull(reader); var state = new LineState(maxCharacters, maxUtf8Bytes); - var buffer = new char[1]; + var asyncBuffer = AsyncBuffers.GetValue(reader, _ => new AsyncReadBuffer(AsyncReadBufferSize)); while (true) { - var read = await reader.ReadAsync(buffer.AsMemory(0, 1), cancellationToken).ConfigureAwait(false); + while (asyncBuffer.TryReadBufferedChar(out var bufferedChar)) + { + if (state.Process(bufferedChar, out var bufferedLine)) + return bufferedLine; + } + + var buffer = asyncBuffer.Buffer; + var read = await reader.ReadAsync(buffer.AsMemory(0, buffer.Length), cancellationToken).ConfigureAwait(false); if (read == 0) return state.HasAnyInput ? state.CompleteLine() : null; - if (state.Process(buffer[0], out var line)) + for (var index = 0; index < read; index++) + { + if (!state.Process(buffer[index], out var line)) + continue; + + asyncBuffer.StoreRemainder(index + 1, read); return line; + } + } + } + + private sealed class AsyncReadBuffer(int size) + { + private int _start; + private int _length; + + internal char[] Buffer { get; } = new char[size]; + + internal bool TryReadBufferedChar(out char ch) + { + if (_length == 0) + { + ch = default; + return false; + } + + ch = Buffer[_start++]; + _length--; + if (_length == 0) + _start = 0; + return true; + } + + internal void StoreRemainder(int start, int read) + { + _start = start; + _length = Math.Max(0, read - start); } } diff --git a/src/CodeIndex/BoundedTextWriter.cs b/src/CodeIndex/BoundedTextWriter.cs new file mode 100644 index 0000000000..9c1720f5a7 --- /dev/null +++ b/src/CodeIndex/BoundedTextWriter.cs @@ -0,0 +1,63 @@ +using System.Text; + +namespace CodeIndex; + +internal sealed class BoundedTextWriter(int maxChars) : TextWriter +{ + private readonly StringBuilder builder = new(); + private bool truncated; + + public override Encoding Encoding => Encoding.UTF8; + + public override void Write(char value) + { + if (builder.Length < maxChars) + { + builder.Append(value); + return; + } + + truncated = true; + } + + public override void Write(string? value) + { + if (string.IsNullOrEmpty(value)) + return; + + Append(value.AsSpan()); + } + + public override void Write(char[] buffer, int index, int count) + => Append(buffer.AsSpan(index, count)); + + internal string GetCapturedText() + { + if (!truncated) + return builder.ToString(); + + return builder + .AppendLine() + .Append("[cdidx] captured worker console output truncated.") + .ToString(); + } + + private void Append(ReadOnlySpan value) + { + var remaining = maxChars - builder.Length; + if (remaining <= 0) + { + truncated = true; + return; + } + + if (value.Length <= remaining) + { + builder.Append(value); + return; + } + + builder.Append(value[..remaining]); + truncated = true; + } +} diff --git a/src/CodeIndex/Indexer/Extensibility/ConfiguredSymbolExtractor.cs b/src/CodeIndex/Indexer/Extensibility/ConfiguredSymbolExtractor.cs index a295568231..a5715b81a7 100644 --- a/src/CodeIndex/Indexer/Extensibility/ConfiguredSymbolExtractor.cs +++ b/src/CodeIndex/Indexer/Extensibility/ConfiguredSymbolExtractor.cs @@ -1,5 +1,6 @@ using System.Text.RegularExpressions; using CodeIndex.Cli; +using CodeIndex.Diagnostics; using CodeIndex.Models; namespace CodeIndex.Indexer.Extensibility; @@ -13,7 +14,7 @@ internal sealed class ConfiguredSymbolExtractor( private readonly HashSet disabledTimeoutPatterns = []; private readonly HashSet timeoutWarnings = new(StringComparer.Ordinal); - internal sealed record PatternRule(string Kind, Regex Regex); + internal sealed record PatternRule(string Kind, Regex Regex, string SourcePath = ""); public string Language { get; } = language; @@ -84,7 +85,8 @@ private void DisablePatternAfterTimeout(PatternRule pattern) if (!shouldReport) return; + ExtractorPluginRegistry.ReportPatternExtractorTimeout(pattern.SourcePath, Language, pattern.Kind); CommandErrorWriter.WriteStderr( - $"[cdidx] Pattern extractor for language '{Language}' kind '{pattern.Kind}' timed out after {(int)ExtractorPluginRegistry.PatternRegexTimeout.TotalMilliseconds}ms; skipped this pattern."); + $"[cdidx] Pattern extractor for language '{DiagnosticSanitizer.ForMessage(Language)}' kind '{DiagnosticSanitizer.ForMessage(pattern.Kind)}' timed out after {(int)ExtractorPluginRegistry.PatternRegexTimeout.TotalMilliseconds}ms; skipped this pattern."); } } diff --git a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.Diagnostics.cs b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.Diagnostics.cs index 03f02aa9a6..11f523fc26 100644 --- a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.Diagnostics.cs +++ b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.Diagnostics.cs @@ -73,6 +73,18 @@ private static void ReportPluginDirectorySkipped(string path, string reason, str category: category); } + internal static void ReportPatternExtractorTimeout(string path, string language, string kind) + { + RecordDiagnostic( + "pattern", + path, + typeName: null, + severity: "warning", + $"Pattern extractor timeout: language '{DiagnosticSanitizer.ForMessage(language)}' kind '{DiagnosticSanitizer.ForMessage(kind)}'.", + countsAsSkippedFile: false, + category: "pattern_regex_timeout"); + } + private static void RecordDiagnostic( string kind, string path, @@ -87,14 +99,22 @@ private static void RecordDiagnostic( diagnosticTotalCount++; if (countsAsSkippedFile) skippedFileCount++; + var diagnostic = new ExtractorRegistryDiagnostic( + DiagnosticSanitizer.ForMessage(kind), + DiagnosticSanitizer.ForPath(path), + DiagnosticSanitizer.ForOptionalLabel(typeName), + DiagnosticSanitizer.ForMessage(severity), + DiagnosticSanitizer.ForMessage(category), + DiagnosticSanitizer.ForMessage(message)); if (Diagnostics.Count < DiagnosticLimit) - Diagnostics.Add(new ExtractorRegistryDiagnostic( - DiagnosticSanitizer.ForMessage(kind), - DiagnosticSanitizer.ForPath(path), - DiagnosticSanitizer.ForOptionalLabel(typeName), - DiagnosticSanitizer.ForMessage(severity), - DiagnosticSanitizer.ForMessage(category), - DiagnosticSanitizer.ForMessage(message))); + { + Diagnostics.Add(diagnostic); + } + else if (category.EndsWith("_candidate_limit_exceeded", StringComparison.Ordinal) + && !Diagnostics.Any(item => item.Category == diagnostic.Category && item.Path == diagnostic.Path)) + { + Diagnostics[^1] = diagnostic; + } } } } diff --git a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PatternConfig.cs b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PatternConfig.cs index 63eee892e0..002a128be3 100644 --- a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PatternConfig.cs +++ b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PatternConfig.cs @@ -89,7 +89,7 @@ private static void TryLoadPatternConfig(string path) if (value.Length > MaxPatternRegexLength) { - ReportPatternConfigRejected(path, $"regex for kind '{pendingKind}' is too long ({value.Length} characters; maximum {MaxPatternRegexLength})"); + ReportPatternConfigRejected(path, $"regex for kind '{DiagnosticSanitizer.ForMessage(pendingKind)}' is too long ({value.Length} characters; maximum {MaxPatternRegexLength})"); return; } @@ -112,7 +112,8 @@ private static void TryLoadPatternConfig(string path) patterns.Add(new ConfiguredSymbolExtractor.PatternRule( pendingKind, - regex)); + regex, + path)); pendingKind = null; } } diff --git a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PluginLoading.cs b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PluginLoading.cs index 12a9d8894d..86fd6e6040 100644 --- a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PluginLoading.cs +++ b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.PluginLoading.cs @@ -6,8 +6,7 @@ public static partial class ExtractorPluginRegistry { private static void LoadPluginAssemblies(IEnumerable directories) { - var pluginPaths = EnumeratePluginAssemblyPaths(directories).ToArray(); - foreach (var pluginPath in pluginPaths) + foreach (var pluginPath in EnumeratePluginAssemblyPaths(directories)) TryLoadPlugin(pluginPath); } @@ -20,7 +19,7 @@ private static void TryLoadPlugin(string pluginPath) fullPath = Path.GetFullPath(pluginPath); lock (Gate) { - if (!LoadedPluginAssemblyPaths.Add(fullPath)) + if (!TryMarkPluginAssemblyPathLoaded(fullPath)) return; } @@ -78,6 +77,9 @@ private static void TryLoadPlugin(string pluginPath) return; } + if (!PluginAssemblyTypesAreWithinBudget(fullPath, types)) + return; + lock (Gate) { pluginAssemblyCount++; @@ -182,6 +184,23 @@ private static bool PluginAssemblyCandidateIsWithinBudget(string fullPath) return true; } + private static bool PluginAssemblyTypesAreWithinBudget(string fullPath, IReadOnlyCollection types) + { + var limit = ResolveTypeInspectionLimit(); + if (types.Count <= limit) + return true; + + RecordDiagnostic( + "plugin", + fullPath, + typeName: null, + severity: "skipped", + $"Plugin assembly skipped: too many loadable types ({types.Count}; maximum {limit}).", + countsAsSkippedFile: true, + category: "plugin_type_limit_exceeded"); + return false; + } + private static void TryRegisterPluginType(Type type, string pluginPath) { try diff --git a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.cs b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.cs index 853c55ea56..ccf1291dd2 100644 --- a/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.cs +++ b/src/CodeIndex/Indexer/Extensibility/ExtractorPluginRegistry.cs @@ -1,4 +1,5 @@ using System.Runtime.Loader; +using CodeIndex.Cli; namespace CodeIndex.Indexer.Extensibility; @@ -17,12 +18,13 @@ public static partial class ExtractorPluginRegistry internal const int MaxPluginAssemblyCandidatesPerDirectory = 128; internal const int MaxPluginAssemblyCandidatesTotal = 256; internal const long MaxPluginAssemblyBytes = 64 * 1024 * 1024; + internal const int MaxExtensionAssemblyTypes = 4096; internal static readonly TimeSpan PatternRegexTimeout = TimeSpan.FromMilliseconds(100); private static readonly object Gate = new(); private static readonly Dictionary SymbolExtractors = new(StringComparer.Ordinal); private static readonly Dictionary ReferenceExtractors = new(StringComparer.Ordinal); - private static readonly HashSet LoadedPluginAssemblyPaths = new(StringComparer.OrdinalIgnoreCase); + private static readonly List LoadedPluginAssemblyPaths = []; private static readonly HashSet LoadedPatternConfigPaths = new(StringComparer.OrdinalIgnoreCase); private static readonly List LoadedPluginAssemblyContexts = []; private static readonly IReadOnlyList PatternConfigSearchPatterns = ["*.yaml", "*.yml"]; @@ -34,6 +36,7 @@ public static partial class ExtractorPluginRegistry private static int diagnosticTotalCount; private static int loadedPatternRuleCount; private static bool pluginsLoaded; + internal static int? TypeInspectionLimitForTesting { get; set; } public static IReadOnlyCollection SymbolLanguages { @@ -136,6 +139,7 @@ internal static void ResetForTests() diagnosticTotalCount = 0; loadedPatternRuleCount = 0; pluginsLoaded = true; + TypeInspectionLimitForTesting = null; } } @@ -155,6 +159,7 @@ internal static void ReloadForTests() diagnosticTotalCount = 0; loadedPatternRuleCount = 0; pluginsLoaded = false; + TypeInspectionLimitForTesting = null; } } @@ -182,6 +187,12 @@ internal static void LoadPluginAssembliesForTests(IReadOnlyList director internal static void LoadPluginForTests(string pluginPath) => TryLoadPlugin(pluginPath); + internal static bool TryMarkPluginAssemblyPathLoadedForTests(string pluginPath) + { + lock (Gate) + return TryMarkPluginAssemblyPathLoaded(Path.GetFullPath(pluginPath)); + } + internal static void LoadPluginsForProjectRoot(string? projectRoot) { EnsurePluginsLoaded(); @@ -238,6 +249,18 @@ private static void EnsurePluginsLoaded() } } + private static bool TryMarkPluginAssemblyPathLoaded(string fullPath) + { + if (LoadedPluginAssemblyPaths.Any(path => string.Equals(path, fullPath, PathCasing.ComparisonFor(fullPath)))) + return false; + + LoadedPluginAssemblyPaths.Add(fullPath); + return true; + } + + private static int ResolveTypeInspectionLimit() + => TypeInspectionLimitForTesting is > 0 ? TypeInspectionLimitForTesting.Value : MaxExtensionAssemblyTypes; + private static void AddLanguageExtensions( Dictionary target, IEnumerable<(string Language, IReadOnlyCollection FileExtensions)> plugins) diff --git a/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs b/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs index 265da612d5..283e581915 100644 --- a/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs +++ b/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs @@ -2,7 +2,6 @@ using System.Globalization; using System.Reflection; using System.Runtime.Loader; -using System.Text; using System.Text.Json; using System.Text.Json.Serialization; using CodeIndex.Cli; @@ -33,7 +32,7 @@ internal sealed class PostExtractionHookCallbackWorkerClient : IDisposable private readonly int maxProtocolLineBytes; private readonly object gate = new(); private Process? process; - private StringBuilder stderr = new(); + private WorkerOutputBuffer stderr = new(); private bool disposed; internal PostExtractionHookCallbackWorkerClient(PostExtractionHookInfo hook, int maxProtocolLineBytes = WorkerProtocolLineLimits.MaxLineUtf8Bytes) @@ -48,7 +47,8 @@ internal PostExtractionHookCallbackResult Invoke( FileContext context, IReadOnlyList? symbols, IReadOnlyList? references, - TimeSpan callbackBudget) + TimeSpan callbackBudget, + CancellationToken cancellationToken = default) { lock (gate) { @@ -80,7 +80,7 @@ internal PostExtractionHookCallbackResult Invoke( process!.StandardOutput, maxProtocolLineBytes, maxProtocolLineBytes, - CancellationToken.None); + cancellationToken); sendTask = SendRequestAsync(process.StandardInput, requestJson); } catch (Exception ex) @@ -90,7 +90,7 @@ internal PostExtractionHookCallbackResult Invoke( stopwatch); } - if (!WaitForTask(sendTask, waitMilliseconds, out var sendException)) + if (!WaitForTask(sendTask, waitMilliseconds, cancellationToken, out var sendException)) { return TimedOutAfterKill(stopwatch); } @@ -103,7 +103,7 @@ internal PostExtractionHookCallbackResult Invoke( } waitMilliseconds = GetRemainingWaitMilliseconds(stopwatch, callbackBudget); - if (waitMilliseconds <= 0 || !WaitForTask(responseTask, waitMilliseconds, out var responseException)) + if (waitMilliseconds <= 0 || !WaitForTask(responseTask, waitMilliseconds, cancellationToken, out var responseException)) { return TimedOutAfterKill(stopwatch); } @@ -124,7 +124,7 @@ internal PostExtractionHookCallbackResult Invoke( var responseJson = responseTask.GetAwaiter().GetResult(); if (responseJson == null) { - var workerError = BuildWorkerExitError(process, stderr.ToString(), "worker exited before returning a response."); + var workerError = BuildWorkerExitError(process, stderr.GetCapturedText(), "worker exited before returning a response."); ClearExitedWorker(); return Failure(workerError, stopwatch.ElapsedMilliseconds); } @@ -205,7 +205,7 @@ private bool EnsureStarted(out string error) } ClearExitedWorker(); - stderr = new StringBuilder(); + stderr = new WorkerOutputBuffer(); if (!PostExtractionHookCallbackWorker.TryCreateStartInfo(hook, maxProtocolLineBytes, out var startInfo, out error)) return false; @@ -299,11 +299,11 @@ private static async Task SendRequestAsync(TextWriter input, string requestJson) await input.FlushAsync().ConfigureAwait(false); } - private static bool WaitForTask(Task task, int milliseconds, out Exception? exception) + private bool WaitForTask(Task task, int milliseconds, CancellationToken cancellationToken, out Exception? exception) { try { - if (!task.Wait(milliseconds)) + if (!task.Wait(milliseconds, cancellationToken)) { exception = null; return false; @@ -317,6 +317,11 @@ private static bool WaitForTask(Task task, int milliseconds, out Exception? exce exception = ex.GetBaseException(); return true; } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + _ = KillWorker(); + throw; + } catch (Exception ex) { exception = ex; @@ -339,7 +344,7 @@ private static bool CallbackBudgetExceeded(Stopwatch stopwatch, TimeSpan callbac private static string BuildWorkerExitError(Process? process, string stderr, string fallback) { var exitCode = process == null ? (int?)null : process.ExitCode; - return SafeDiagnosticFormatter.FormatWorkerExit("worker_protocol_error", exitCode, fallback); + return SafeDiagnosticFormatter.FormatWorkerExit("worker_protocol_error", exitCode, fallback, stderr); } internal static WorkerProcessExitWaitResult WaitForWorkerExit(Process process, int milliseconds) @@ -357,6 +362,7 @@ internal static class PostExtractionHookCallbackWorker internal const string CommandName = "__cdidx-post-extraction-hook-callback"; internal const int WorkerKillWaitMilliseconds = 5000; private const string ProtocolMaxLineBytesOption = "--protocol-max-line-bytes"; + private const int CapturedConsoleMaxChars = 32 * 1024; internal static readonly JsonSerializerOptions JsonOptions = PostExtractionHookCallbackWorkerJsonContext.Default.Options; internal static bool TryRunCommand( @@ -517,6 +523,14 @@ private static int RunCommand( if (requestJson is null) break; + if (!WorkerProtocolJsonValidator.TryValidate(requestJson, maxProtocolLineCharacters, out var validationError)) + { + response = new WorkerResponse(null, null, null, validationError); + output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); + output.Flush(); + continue; + } + try { request = JsonSerializer.Deserialize(requestJson, JsonOptions) @@ -570,8 +584,8 @@ private static WorkerResponse InvokeInsideWorker(IPostExtractionHook hook, Worke { var originalOut = Console.Out; var originalError = Console.Error; - using var capturedOut = new StringWriter(); - using var capturedError = new StringWriter(); + using var capturedOut = new BoundedTextWriter(CapturedConsoleMaxChars); + using var capturedError = new BoundedTextWriter(CapturedConsoleMaxChars); Exception? callbackFailure = null; try { diff --git a/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs b/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs index f27e8d1874..24b47669b5 100644 --- a/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs +++ b/src/CodeIndex/Indexer/Hooks/PostExtractionHooks.cs @@ -44,6 +44,7 @@ public sealed class PostExtractionHookRunner : IDisposable public static readonly TimeSpan DefaultCallbackBudget = TimeSpan.FromSeconds(5); internal const int DefaultDiscoveryLimit = 128; internal const long DefaultDiscoveryMaxBytes = 64 * 1024 * 1024; + internal const int DefaultTypeInspectionLimit = 4096; private readonly List hooks; private readonly ConcurrentQueue diagnostics = new(); @@ -53,6 +54,8 @@ public sealed class PostExtractionHookRunner : IDisposable internal static Func? CallbackBudgetForTesting { get; set; } internal static Func? DiscoveryLimitForTesting { get; set; } internal static Func? DiscoveryMaxBytesForTesting { get; set; } + internal static Func? TypeInspectionLimitForTesting { get; set; } + internal static WeakReference? LastUnretainedLoadContextForTesting { get; set; } private PostExtractionHookRunner(List hooks, TimeSpan callbackBudget) { @@ -119,13 +122,15 @@ private static PostExtractionHookRunner Discover( var maxAssemblyBytes = ResolveDiscoveryMaxBytes(); foreach (var dllPath in EnumerateHookAssemblyPaths(hooksDirectory, runner, ResolveDiscoveryLimit())) { + ExtensionAssemblyLoadContext? loadContext = null; + var retainedLoadContext = false; Assembly assembly; try { if (!HookAssemblyCandidateIsWithinBudget(dllPath, runner, maxAssemblyBytes)) continue; - var loadContext = new ExtensionAssemblyLoadContext( + loadContext = new ExtensionAssemblyLoadContext( $"cdidx-hook:{Path.GetFileNameWithoutExtension(dllPath)}", dllPath); assembly = loadContext.LoadFromAssemblyPath(Path.GetFullPath(dllPath)); @@ -134,6 +139,7 @@ private static PostExtractionHookRunner Discover( { var diagnostic = ExtensionLoadDiagnosticClassifier.ClassifyAssemblyLoad("Hook assembly load", ex); runner.EnqueueDiagnostic(dllPath, null, diagnostic.Message, category: diagnostic.Category); + loadContext?.Unload(); continue; } @@ -146,6 +152,13 @@ private static PostExtractionHookRunner Discover( { var diagnostic = ExtensionLoadDiagnosticClassifier.ClassifyTypeLoad("Hook assembly type inspection", ex); runner.EnqueueDiagnostic(dllPath, null, diagnostic.Message, category: diagnostic.Category); + loadContext?.Unload(); + continue; + } + + if (!HookAssemblyTypesAreWithinBudget(dllPath, types, runner)) + { + loadContext?.Unload(); continue; } @@ -169,14 +182,22 @@ private static PostExtractionHookRunner Discover( var info = new PostExtractionHookInfo(type.Name, Path.GetFullPath(dllPath), type.FullName ?? type.Name); loaded.Add(new LoadedPostExtractionHook( info, - AssemblyLoadContext.GetLoadContext(type.Assembly), + loadContext, new PostExtractionHookCallbackWorkerClient(info, maxProtocolLineBytes))); + retainedLoadContext = true; } catch (Exception) { runner.EnqueueDiagnostic(dllPath, type.FullName, "Failed to instantiate hook.", category: "activation_failed"); } } + + if (!retainedLoadContext) + { + if (loadContext != null) + LastUnretainedLoadContextForTesting = new WeakReference(loadContext, trackResurrection: false); + loadContext?.Unload(); + } } return runner; @@ -514,6 +535,34 @@ private static long ResolveDiscoveryMaxBytes() private static long NormalizeDiscoveryMaxBytes(long value) => value <= 0 ? DefaultDiscoveryMaxBytes : value; + private static int ResolveTypeInspectionLimit() + { + if (TypeInspectionLimitForTesting != null) + return NormalizeTypeInspectionLimit(TypeInspectionLimitForTesting()); + + return DefaultTypeInspectionLimit; + } + + private static int NormalizeTypeInspectionLimit(int value) + => value <= 0 ? DefaultTypeInspectionLimit : value; + + private static bool HookAssemblyTypesAreWithinBudget( + string dllPath, + IReadOnlyCollection types, + PostExtractionHookRunner runner) + { + var limit = ResolveTypeInspectionLimit(); + if (types.Count <= limit) + return true; + + runner.EnqueueDiagnostic( + dllPath, + null, + $"Hook assembly skipped: too many loadable types ({types.Count}; maximum {limit}).", + category: "hook_type_limit_exceeded"); + return false; + } + private static TimeSpan NormalizeCallbackBudgetMilliseconds(long milliseconds) => milliseconds <= 0 ? DefaultCallbackBudget diff --git a/src/CodeIndex/Indexer/IsolatedWorkerProcessLauncher.cs b/src/CodeIndex/Indexer/IsolatedWorkerProcessLauncher.cs index c8546e4142..641c46c4a2 100644 --- a/src/CodeIndex/Indexer/IsolatedWorkerProcessLauncher.cs +++ b/src/CodeIndex/Indexer/IsolatedWorkerProcessLauncher.cs @@ -8,7 +8,8 @@ namespace CodeIndex.Indexer; internal static class IsolatedWorkerProcessLauncher { internal static ProcessStartInfo CreateStartInfo() - => new() + { + var startInfo = new ProcessStartInfo { UseShellExecute = false, RedirectStandardInput = true, @@ -19,6 +20,47 @@ internal static ProcessStartInfo CreateStartInfo() StandardErrorEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false), CreateNoWindow = true, }; + ApplyEnvironmentAllowlist(startInfo); + return startInfo; + } + + private static readonly string[] EnvironmentAllowlist = + [ + "PATH", + "DOTNET_ROOT", + "DOTNET_ROOT_X64", + "DOTNET_ROOT_X86", + "DOTNET_ROOT_ARM64", + "DOTNET_BUNDLE_EXTRACT_BASE_DIR", + "TMPDIR", + "TMP", + "TEMP", + "SystemRoot", + "WINDIR", + ]; + + private static void ApplyEnvironmentAllowlist(ProcessStartInfo startInfo) + { + startInfo.Environment.Clear(); + foreach (var name in EnvironmentAllowlist) + { + var value = Environment.GetEnvironmentVariable(name); + if (!string.IsNullOrEmpty(value)) + startInfo.Environment[name] = value; + } + + foreach (System.Collections.DictionaryEntry item in Environment.GetEnvironmentVariables()) + { + if (item.Key is not string name + || item.Value is not string value + || !name.StartsWith("CDIDX_TEST_", StringComparison.Ordinal)) + { + continue; + } + + startInfo.Environment[name] = value; + } + } internal static bool ShouldStartCurrentExecutable( string? currentProcessPath, diff --git a/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs b/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs index d156a3f429..f62468e288 100644 --- a/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs +++ b/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs @@ -1,6 +1,5 @@ using System.Diagnostics; using System.Globalization; -using System.Text; using System.Text.Json; using System.Text.Json.Serialization; using CodeIndex.Cli; @@ -23,7 +22,7 @@ internal sealed class SymbolExtractionWorkerClient : IDisposable private readonly int maxProtocolLineBytes; private readonly object gate = new(); private Process? process; - private StringBuilder stderr = new(); + private WorkerOutputBuffer stderr = new(); private bool disposed; internal SymbolExtractionWorkerClient(long? maxFileSizeBytes = null) @@ -110,7 +109,7 @@ internal SymbolExtractionWorkerResult Invoke( var responseJson = responseTask.GetAwaiter().GetResult(); if (responseJson == null) { - var workerError = BuildWorkerExitError(process, stderr.ToString(), "worker exited before returning a response."); + var workerError = BuildWorkerExitError(process, stderr.GetCapturedText(), "worker exited before returning a response."); ClearExitedWorker(); return Failure(workerError, stopwatch.ElapsedMilliseconds); } @@ -191,7 +190,7 @@ private bool EnsureStarted(out string error) } ClearExitedWorker(); - stderr = new StringBuilder(); + stderr = new WorkerOutputBuffer(); if (!SymbolExtractionWorker.TryCreateStartInfo(maxProtocolLineBytes, out var startInfo, out error)) return false; @@ -326,7 +325,7 @@ private static bool CallbackBudgetExceeded(Stopwatch stopwatch, TimeSpan callbac private static string BuildWorkerExitError(Process? process, string stderr, string fallback) { var exitCode = process == null ? (int?)null : process.ExitCode; - return SafeDiagnosticFormatter.FormatWorkerExit("worker_protocol_error", exitCode, fallback); + return SafeDiagnosticFormatter.FormatWorkerExit("worker_protocol_error", exitCode, fallback, stderr); } internal static WorkerProcessExitWaitResult WaitForWorkerExit(Process process, int milliseconds) @@ -501,6 +500,14 @@ private static int RunCommand( if (requestJson is null) break; + if (!WorkerProtocolJsonValidator.TryValidate(requestJson, maxProtocolLineCharacters, out var validationError)) + { + response = new WorkerResponse(null, validationError, null); + output.WriteLine(JsonSerializer.Serialize(response, JsonOptions)); + output.Flush(); + continue; + } + try { request = JsonSerializer.Deserialize(requestJson, JsonOptions) @@ -714,65 +721,6 @@ private sealed record WorkerOptions( int? DelayMillisecondsForTesting, string? ConsoleStdoutForTesting); - private sealed class BoundedTextWriter(int maxChars) : TextWriter - { - private readonly StringBuilder builder = new(); - private bool truncated; - - public override Encoding Encoding => Encoding.UTF8; - - public override void Write(char value) - { - if (builder.Length < maxChars) - { - builder.Append(value); - return; - } - - truncated = true; - } - - public override void Write(string? value) - { - if (string.IsNullOrEmpty(value)) - return; - - Append(value.AsSpan()); - } - - public override void Write(char[] buffer, int index, int count) - => Append(buffer.AsSpan(index, count)); - - internal string GetCapturedText() - { - if (!truncated) - return builder.ToString(); - - return builder - .AppendLine() - .Append("[cdidx] captured worker console output truncated.") - .ToString(); - } - - private void Append(ReadOnlySpan value) - { - var remaining = maxChars - builder.Length; - if (remaining <= 0) - { - truncated = true; - return; - } - - if (value.Length <= remaining) - { - builder.Append(value); - return; - } - - builder.Append(value[..remaining]); - truncated = true; - } - } } [JsonSourceGenerationOptions(PropertyNameCaseInsensitive = true)] diff --git a/src/CodeIndex/Mcp/McpToolHandlers.cs b/src/CodeIndex/Mcp/McpToolHandlers.cs index 9a30efcb74..c36b367e8f 100644 --- a/src/CodeIndex/Mcp/McpToolHandlers.cs +++ b/src/CodeIndex/Mcp/McpToolHandlers.cs @@ -3177,7 +3177,6 @@ private JsonNode ExecuteStatus(JsonNode? id, JsonNode? args) .ToList(); } status.Version = _version; - var requestToken = _currentRequestToken.Value; requestToken.ThrowIfCancellationRequested(); status.UpdateCheck = runUpdateCheck ? (StatusUpdateCheckForTesting ?? UpdateChecker.Check)(_version, requestToken) diff --git a/src/CodeIndex/SafeDiagnosticFormatter.cs b/src/CodeIndex/SafeDiagnosticFormatter.cs index a05f4d4614..f8fcf21b9b 100644 --- a/src/CodeIndex/SafeDiagnosticFormatter.cs +++ b/src/CodeIndex/SafeDiagnosticFormatter.cs @@ -1,4 +1,5 @@ using System.Text; +using CodeIndex.Diagnostics; namespace CodeIndex; @@ -6,6 +7,7 @@ internal static class SafeDiagnosticFormatter { private const int MaxCategoryCharacters = 64; private const int MaxExceptionTypeCharacters = 128; + private const int MaxWorkerStderrTailCharacters = 512; private const string TruncationMarker = "..."; internal static string FormatExceptionCategory(string category, Exception ex) @@ -14,13 +16,28 @@ internal static string FormatExceptionCategory(string category, Exception ex) internal static string FormatCategoryType(string category, string typeName) => $"{BoundToken(category, MaxCategoryCharacters)}: {BoundToken(typeName, MaxExceptionTypeCharacters)}"; - internal static string FormatWorkerExit(string category, int? exitCode, string fallback) + internal static string FormatWorkerExit(string category, int? exitCode, string fallback, string? stderrTail = null) { var safeCategory = BoundToken(category, MaxCategoryCharacters); var safeFallback = BoundToken(fallback, MaxExceptionTypeCharacters); - return exitCode.HasValue + var message = exitCode.HasValue ? $"{safeCategory}: worker exited with code {exitCode.Value}. {safeFallback}." : $"{safeCategory}: worker exited before the exit code was available. {safeFallback}."; + var safeStderrTail = FormatWorkerStderrTail(stderrTail); + return safeStderrTail.Length == 0 + ? message + : $"{message} stderr_tail=\"{safeStderrTail}\"."; + } + + private static string FormatWorkerStderrTail(string? stderrTail) + { + if (string.IsNullOrWhiteSpace(stderrTail)) + return string.Empty; + + var tail = stderrTail.Length <= MaxWorkerStderrTailCharacters + ? stderrTail + : stderrTail[^MaxWorkerStderrTailCharacters..]; + return DiagnosticSanitizer.ForMessage(tail); } private static string BoundToken(string? value, int maxCharacters) diff --git a/src/CodeIndex/WorkerOutputBuffer.cs b/src/CodeIndex/WorkerOutputBuffer.cs new file mode 100644 index 0000000000..defed98d5a --- /dev/null +++ b/src/CodeIndex/WorkerOutputBuffer.cs @@ -0,0 +1,42 @@ +using System.Text; + +namespace CodeIndex; + +internal sealed class WorkerOutputBuffer(int maxCharacters = 4096, int maxLines = 64, int maxLineCharacters = 512) +{ + private readonly Queue lines = new(); + private int characterCount; + private bool truncated; + + internal void AppendLine(string line) + { + if (line.Length > maxLineCharacters) + { + line = line[^maxLineCharacters..]; + truncated = true; + } + + lines.Enqueue(line); + characterCount += line.Length + Environment.NewLine.Length; + + while (lines.Count > maxLines || characterCount > maxCharacters) + { + var removed = lines.Dequeue(); + characterCount -= removed.Length + Environment.NewLine.Length; + truncated = true; + } + } + + internal string GetCapturedText() + { + if (lines.Count == 0) + return string.Empty; + + var builder = new StringBuilder(characterCount + 64); + if (truncated) + builder.AppendLine("[cdidx] worker stderr truncated."); + foreach (var line in lines) + builder.AppendLine(line); + return builder.ToString(); + } +} diff --git a/src/CodeIndex/WorkerProtocolJsonValidator.cs b/src/CodeIndex/WorkerProtocolJsonValidator.cs new file mode 100644 index 0000000000..81c68e718d --- /dev/null +++ b/src/CodeIndex/WorkerProtocolJsonValidator.cs @@ -0,0 +1,70 @@ +using System.Text.Json; + +namespace CodeIndex; + +internal static class WorkerProtocolJsonValidator +{ + private const int DefaultMaxJsonProperties = 1_000_000; + internal static int? MaxJsonPropertiesForTesting { get; set; } + internal static int? MaxStringCharactersForTesting { get; set; } + + internal static bool TryValidate(string json, int maxStringCharacters, out string error) + { + var maxProperties = MaxJsonPropertiesForTesting ?? DefaultMaxJsonProperties; + var effectiveMaxStringCharacters = MaxStringCharactersForTesting ?? maxStringCharacters; + var propertyCount = 0; + try + { + using var document = JsonDocument.Parse(json); + ValidateElement(document.RootElement, maxProperties, effectiveMaxStringCharacters, ref propertyCount, out error); + return error.Length == 0; + } + catch (JsonException) + { + error = SafeDiagnosticFormatter.FormatCategoryType("worker_protocol_error", nameof(JsonException)); + return false; + } + } + + private static void ValidateElement( + JsonElement element, + int maxProperties, + int maxStringCharacters, + ref int propertyCount, + out string error) + { + error = string.Empty; + switch (element.ValueKind) + { + case JsonValueKind.Object: + foreach (var property in element.EnumerateObject()) + { + propertyCount++; + if (propertyCount > maxProperties) + { + error = SafeDiagnosticFormatter.FormatCategoryType("worker_protocol_error", "json_property_limit_exceeded"); + return; + } + + ValidateElement(property.Value, maxProperties, maxStringCharacters, ref propertyCount, out error); + if (error.Length != 0) + return; + } + + break; + case JsonValueKind.Array: + foreach (var item in element.EnumerateArray()) + { + ValidateElement(item, maxProperties, maxStringCharacters, ref propertyCount, out error); + if (error.Length != 0) + return; + } + + break; + case JsonValueKind.String: + if ((element.GetString()?.Length ?? 0) > maxStringCharacters) + error = SafeDiagnosticFormatter.FormatCategoryType("worker_protocol_error", "json_string_length_exceeded"); + break; + } + } +} diff --git a/tests/CodeIndex.Tests/BoundedLineReaderTests.cs b/tests/CodeIndex.Tests/BoundedLineReaderTests.cs new file mode 100644 index 0000000000..ba2e10b295 --- /dev/null +++ b/tests/CodeIndex.Tests/BoundedLineReaderTests.cs @@ -0,0 +1,49 @@ +namespace CodeIndex.Tests; + +public class BoundedLineReaderTests +{ + [Fact] + public async Task ReadLineAsync_BuffersRemainderAfterChunkRead_Issue3836() + { + using var reader = new StringReader("first line\nsecond line\n"); + + var first = await BoundedLineReader.ReadLineAsync(reader, 100, 100, CancellationToken.None); + var second = await BoundedLineReader.ReadLineAsync(reader, 100, 100, CancellationToken.None); + var end = await BoundedLineReader.ReadLineAsync(reader, 100, 100, CancellationToken.None); + + Assert.Equal("first line", first); + Assert.Equal("second line", second); + Assert.Null(end); + } + + [Fact] + public void WorkerOutputBuffer_KeepsBoundedTail_Issue3836() + { + var buffer = new WorkerOutputBuffer(maxCharacters: 24, maxLines: 2, maxLineCharacters: 12); + + buffer.AppendLine("first-sensitive-line"); + buffer.AppendLine("second-line"); + buffer.AppendLine("third-line"); + + var captured = buffer.GetCapturedText(); + + Assert.Contains("worker stderr truncated", captured, StringComparison.Ordinal); + Assert.DoesNotContain("first-sensitive", captured, StringComparison.Ordinal); + Assert.Contains("third-line", captured, StringComparison.Ordinal); + } + + [Fact] + public void FormatWorkerExit_IncludesSanitizedBoundedStderrTail_Issue3836() + { + var message = SafeDiagnosticFormatter.FormatWorkerExit( + "worker_protocol_error", + 7, + "worker exited before returning a response", + "/private/secret/project/file.cs: raw stderr detail"); + + Assert.Contains("worker exited with code 7", message, StringComparison.Ordinal); + Assert.Contains("stderr_tail=", message, StringComparison.Ordinal); + Assert.Contains("", message, StringComparison.Ordinal); + Assert.DoesNotContain("/private/secret", message, StringComparison.Ordinal); + } +} diff --git a/tests/CodeIndex.Tests/ExtractorPluginRegistryTests.cs b/tests/CodeIndex.Tests/ExtractorPluginRegistryTests.cs index 457819337d..295ad9c942 100644 --- a/tests/CodeIndex.Tests/ExtractorPluginRegistryTests.cs +++ b/tests/CodeIndex.Tests/ExtractorPluginRegistryTests.cs @@ -1,5 +1,6 @@ using System.Reflection; using System.Runtime.Loader; +using CodeIndex.Cli; using CodeIndex.Indexer.Extensibility; using CodeIndex.Models; @@ -291,6 +292,66 @@ public void LoadPlugin_LoadsExtractorAssemblyInCollectibleContext_3413() } } + [Fact] + public void PluginAssemblyPathIdentity_FollowsPathCasingPolicy_Issue3790() + { + var projectRoot = TestProjectHelper.CreateTempProject("extractor_registry_path_casing_3790"); + lock (TestConsoleLock.Gate) + { + var originalProbe = PathCasing.IgnoreCaseProbeForTesting; + try + { + ExtractorPluginRegistry.ResetForTests(); + PathCasing.ResetCacheForTests(); + PathCasing.IgnoreCaseProbeForTesting = _ => true; + var pluginPath = Path.Combine(projectRoot, "Plugin.dll"); + var caseVariant = Path.Combine(projectRoot, "plugin.dll"); + + Assert.True(ExtractorPluginRegistry.TryMarkPluginAssemblyPathLoadedForTests(pluginPath)); + Assert.False(ExtractorPluginRegistry.TryMarkPluginAssemblyPathLoadedForTests(caseVariant)); + + ExtractorPluginRegistry.ResetForTests(); + PathCasing.ResetCacheForTests(); + PathCasing.IgnoreCaseProbeForTesting = _ => false; + + Assert.True(ExtractorPluginRegistry.TryMarkPluginAssemblyPathLoadedForTests(pluginPath)); + Assert.True(ExtractorPluginRegistry.TryMarkPluginAssemblyPathLoadedForTests(caseVariant)); + } + finally + { + PathCasing.IgnoreCaseProbeForTesting = originalProbe; + PathCasing.ResetCacheForTests(); + ExtractorPluginRegistry.ResetForTests(); + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + } + + [Fact] + public void LoadPlugin_SkipsAssembliesAboveTypeInspectionLimit_Issue3790() + { + lock (TestConsoleLock.Gate) + { + try + { + ExtractorPluginRegistry.ResetForTests(); + ExtractorPluginRegistry.TypeInspectionLimitForTesting = 1; + + ExtractorPluginRegistry.LoadPluginForTests(Assembly.GetExecutingAssembly().Location); + var diagnostic = Assert.Single(ExtractorPluginRegistry.GetStatusSnapshot().Diagnostics!); + + Assert.Equal("plugin", diagnostic.Kind); + Assert.Equal("skipped", diagnostic.Severity); + Assert.Equal("plugin_type_limit_exceeded", diagnostic.Category); + Assert.Contains("too many loadable types", diagnostic.Message, StringComparison.Ordinal); + } + finally + { + ExtractorPluginRegistry.ResetForTests(); + } + } + } + [Fact] public void LoadPatternConfigs_BoundsDiagnosticsAndCountsSkippedFiles() { @@ -469,6 +530,81 @@ public void LoadPatternConfigs_RejectsExtensionNormalizedBeyondLimit_3245() } } + [Fact] + public void LoadPatternConfigs_RecordsPatternTimeoutDiagnostic_Issue3821() + { + var projectRoot = TestProjectHelper.CreateTempProject("extractor_registry_pattern_timeout_3821"); + lock (TestConsoleLock.Gate) + { + try + { + ExtractorPluginRegistry.ResetForTests(); + WritePatternConfig( + projectRoot, + "slow.yaml", + "language: \"timeoutdsl\"\nextensions:\n - extension: \".timeouttoy\"\npatterns:\n - kind: \"class\"\n regex: \"^(a+)+$\"\n"); + + ExtractorPluginRegistry.LoadPatternConfigsForProjectRoot(projectRoot); + Assert.True(ExtractorPluginRegistry.TryGetSymbolExtractor("timeoutdsl", out var extractor)); + + var stderr = ConsoleCapture.CaptureError(() => + { + var symbols = extractor.Extract( + 1, + new string('a', 10_000) + "!", + new ExtractionContext("timeoutdsl", Path.Combine(projectRoot, "sample.timeouttoy"))); + + Assert.Empty(symbols); + }); + var diagnostic = Assert.Single( + ExtractorPluginRegistry.GetStatusSnapshot().Diagnostics!, + item => item.Category == "pattern_regex_timeout"); + + Assert.Equal("pattern", diagnostic.Kind); + Assert.Equal("warning", diagnostic.Severity); + Assert.Equal(".cdidx/patterns/slow.yaml", diagnostic.Path); + Assert.Contains("timeoutdsl", diagnostic.Message, StringComparison.Ordinal); + Assert.Contains("class", diagnostic.Message, StringComparison.Ordinal); + Assert.Contains("Pattern extractor", stderr, StringComparison.Ordinal); + Assert.DoesNotContain(projectRoot, diagnostic.Path, StringComparison.Ordinal); + } + finally + { + ExtractorPluginRegistry.ResetForTests(); + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + } + + [Fact] + public void LoadPatternConfigs_SanitizesKindInRegexLengthRejection_Issue3821() + { + var projectRoot = TestProjectHelper.CreateTempProject("extractor_registry_pattern_kind_sanitized_3821"); + lock (TestConsoleLock.Gate) + { + try + { + ExtractorPluginRegistry.ResetForTests(); + WritePatternConfig( + projectRoot, + "long-regex.yaml", + $"language: \"toydsl\"\nextensions:\n - extension: \".toy\"\npatterns:\n - kind: \"/private/secret/kind\"\n regex: \"{new string('x', ExtractorPluginRegistry.MaxPatternRegexLength + 1)}\"\n"); + + ExtractorPluginRegistry.LoadPatternConfigsForProjectRoot(projectRoot); + var diagnostic = Assert.Single(ExtractorPluginRegistry.GetStatusSnapshot().Diagnostics!); + + Assert.Equal("invalid_pattern_config", diagnostic.Category); + Assert.Contains("regex for kind", diagnostic.Message, StringComparison.Ordinal); + Assert.DoesNotContain("/private/secret", diagnostic.Message, StringComparison.Ordinal); + } + finally + { + ExtractorPluginRegistry.ResetForTests(); + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + } + private static void WritePatternConfig(string projectRoot, string fileName, string content) { var path = Path.Combine(projectRoot, ".cdidx", "patterns", fileName); diff --git a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs index 3b66db6459..572479ef11 100644 --- a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs @@ -503,6 +503,72 @@ public void PostExtractionHookCallbackWorker_OversizedRequestLineReturnsProtocol Assert.Equal("worker_protocol_error: BoundedLineLengthException", workerError); } + [Fact] + public void WorkerProtocol_RejectsExcessiveJsonProperties_Issue3759() + { + lock (TestConsoleLock.Gate) + { + try + { + WorkerProtocolJsonValidator.MaxJsonPropertiesForTesting = 1; + using var input = new StringReader("{\"FileId\":0,\"Lang\":\"csharp\"}\n"); + using var output = new StringWriter(); + using var error = new StringWriter(); + + var handled = SymbolExtractionWorker.TryRunCommand( + [SymbolExtractionWorker.CommandName], + input, + output, + error, + out var exitCode); + + Assert.True(handled); + Assert.Equal(0, exitCode); + Assert.Equal(string.Empty, error.ToString()); + using var document = JsonDocument.Parse(output.ToString()); + var workerError = document.RootElement.GetProperty("WorkerError").GetString(); + Assert.Equal("worker_protocol_error: json_property_limit_exceeded", workerError); + } + finally + { + WorkerProtocolJsonValidator.MaxJsonPropertiesForTesting = null; + } + } + } + + [Fact] + public void WorkerProtocol_RejectsOversizedJsonStrings_Issue3759() + { + lock (TestConsoleLock.Gate) + { + try + { + WorkerProtocolJsonValidator.MaxStringCharactersForTesting = 4; + using var input = new StringReader("{\"Callback\":\"OnSymbolsExtracted\"}\n"); + using var output = new StringWriter(); + using var error = new StringWriter(); + + var handled = PostExtractionHookCallbackWorker.TryRunCommand( + [PostExtractionHookCallbackWorker.CommandName, "/tmp/demo-hook.dll", "Demo.Hook"], + input, + output, + error, + out var exitCode); + + Assert.True(handled); + Assert.Equal(0, exitCode); + Assert.Equal(string.Empty, error.ToString()); + using var document = JsonDocument.Parse(output.ToString()); + var workerError = document.RootElement.GetProperty("WorkerError").GetString(); + Assert.Equal("worker_protocol_error: json_string_length_exceeded", workerError); + } + finally + { + WorkerProtocolJsonValidator.MaxStringCharactersForTesting = null; + } + } + } + [Fact] public void SymbolExtractionWorker_StartInfo_UsesCurrentCdidxExecutableWhenAvailable() { @@ -528,6 +594,30 @@ public void SymbolExtractionWorker_StartInfo_UsesCurrentCdidxExecutableWhenAvail Assert.True(startInfo.RedirectStandardError); } + [Fact] + public void IsolatedWorkers_StartInfo_ScrubsEnvironmentByAllowlist_Issue3759() + { + lock (TestConsoleLock.Gate) + { + using var env = EnvironmentVariableScope.Capture( + "CDIDX_TEST_WORKER_ALLOWLIST_3759", + "CDIDX_SECRET_WORKER_3759"); + env.Set("CDIDX_TEST_WORKER_ALLOWLIST_3759", "allowed"); + env.Set("CDIDX_SECRET_WORKER_3759", "secret"); + var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "cdidx.exe" : "cdidx"); + + var created = SymbolExtractionWorker.TryCreateStartInfo( + currentProcessPath, + runnerAssemblyPath: string.Empty, + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.Equal("allowed", startInfo.Environment["CDIDX_TEST_WORKER_ALLOWLIST_3759"]); + Assert.False(startInfo.Environment.ContainsKey("CDIDX_SECRET_WORKER_3759")); + } + } + [Fact] public void SymbolExtractionWorker_StartInfo_BoundsInternalTestDelay_Issue3398() { diff --git a/tests/CodeIndex.Tests/PostExtractionHookTests.cs b/tests/CodeIndex.Tests/PostExtractionHookTests.cs index 13a8397216..c4d81aa675 100644 --- a/tests/CodeIndex.Tests/PostExtractionHookTests.cs +++ b/tests/CodeIndex.Tests/PostExtractionHookTests.cs @@ -484,6 +484,70 @@ public void Discover_SkipsOversizeHookAssemblyCandidate() } } + [Fact] + public void Discover_SkipsAssembliesAboveTypeInspectionLimit_Issue3790() + { + var projectRoot = TestProjectHelper.CreateTempProject("post-extraction-hook-type-cap-3790"); + lock (TestConsoleLock.Gate) + { + var originalLimit = PostExtractionHookRunner.TypeInspectionLimitForTesting; + try + { + PostExtractionHookRunner.TypeInspectionLimitForTesting = () => 1; + var hooksDir = Path.Combine(projectRoot, "hooks"); + Directory.CreateDirectory(hooksDir); + File.Copy(Assembly.GetExecutingAssembly().Location, Path.Combine(hooksDir, "CodeIndex.Tests.dll")); + + using var runner = PostExtractionHookRunner.Discover(hooksDir); + + Assert.Empty(runner.Hooks); + var diagnostic = Assert.Single(runner.Diagnostics); + Assert.Equal("hook_type_limit_exceeded", diagnostic.Category); + Assert.Contains("too many loadable types", diagnostic.Message, StringComparison.Ordinal); + Assert.DoesNotContain(projectRoot, diagnostic.AssemblyPath, StringComparison.Ordinal); + } + finally + { + PostExtractionHookRunner.TypeInspectionLimitForTesting = originalLimit; + CollectUnloadedHookAssemblies(); + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + } + + [Fact] + public void Discover_UnloadsAssemblyWithoutRetainedHooks_Issue3790() + { + var projectRoot = TestProjectHelper.CreateTempProject("post-extraction-hook-no-hook-unload-3790"); + WeakReference? weakLoadContext; + lock (TestConsoleLock.Gate) + { + try + { + PostExtractionHookRunner.LastUnretainedLoadContextForTesting = null; + var hooksDir = Path.Combine(projectRoot, "hooks"); + Directory.CreateDirectory(hooksDir); + File.Copy(typeof(PostExtractionHookRunner).Assembly.Location, Path.Combine(hooksDir, "CodeIndex.dll")); + + using (var runner = PostExtractionHookRunner.Discover(hooksDir)) + { + Assert.Empty(runner.Hooks); + Assert.Empty(runner.Diagnostics); + weakLoadContext = PostExtractionHookRunner.LastUnretainedLoadContextForTesting; + Assert.NotNull(weakLoadContext); + } + + CollectUnloadedHookAssemblies(); + Assert.False(weakLoadContext!.IsAlive); + } + finally + { + PostExtractionHookRunner.LastUnretainedLoadContextForTesting = null; + TestProjectHelper.DeleteDirectory(projectRoot); + } + } + } + private static void CollectUnloadedHookAssemblies() { GC.Collect();