From 9f071086e0cb81063d6104bd4e84445058d69554 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 13 Jun 2026 23:37:27 +0900 Subject: [PATCH 1/4] Make private log hardening failures observable (#3475) --- changelog.d/unreleased/3475.fixed.md | 17 ++++++ src/CodeIndex/Cli/GlobalToolLog.cs | 47 ++++++++++++--- src/CodeIndex/Cli/PrivateLogFile.cs | 67 ++++++++++++++++++--- tests/CodeIndex.Tests/GlobalToolLogTests.cs | 59 ++++++++++++++++++ 4 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 changelog.d/unreleased/3475.fixed.md diff --git a/changelog.d/unreleased/3475.fixed.md b/changelog.d/unreleased/3475.fixed.md new file mode 100644 index 0000000000..c47750705f --- /dev/null +++ b/changelog.d/unreleased/3475.fixed.md @@ -0,0 +1,17 @@ +--- +category: fixed +issues: + - 3475 +affected: + - src/CodeIndex/Cli/PrivateLogFile.cs + - src/CodeIndex/Cli/GlobalToolLog.cs + - tests/CodeIndex.Tests/GlobalToolLogTests.cs +--- + +## English + +- **Private log hardening failures are now observable (#3475)** — best-effort permission and pruning failures now emit bounded `private_log_diagnostic` WARN entries with stable reason codes instead of disappearing silently. + +## 日本語 + +- **private log の hardening 失敗を観測できるようになりました (#3475)** — 権限設定や prune のベストエフォート失敗は、黙って消える代わりに、安定した reason code を含む bounded な `private_log_diagnostic` WARN として記録されます。 diff --git a/src/CodeIndex/Cli/GlobalToolLog.cs b/src/CodeIndex/Cli/GlobalToolLog.cs index caf67b7ed6..d218be2664 100644 --- a/src/CodeIndex/Cli/GlobalToolLog.cs +++ b/src/CodeIndex/Cli/GlobalToolLog.cs @@ -26,6 +26,7 @@ internal static class GlobalToolLog internal const int RedactionArgumentLengthLimit = 8192; internal const string RedactionTruncationMarker = ""; private const string RedactedValue = ""; + private const int PrivateLogDiagnosticEmitLimit = 16; private static readonly TimeSpan RedactionRegexTimeout = TimeSpan.FromSeconds(1); internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; private static readonly AsyncLocal CurrentSession = new(); @@ -68,15 +69,16 @@ internal static class GlobalToolLog if (!ShouldEnable()) return null; + var privateLogDiagnostics = new List(); var logDirectory = ResolveLogDirectory(); Directory.CreateDirectory(logDirectory); - HardenLogFiles(logDirectory); + HardenLogFiles(logDirectory, privateLogDiagnostics.Add); var options = LogOptions.FromEnvironment(); var logPath = ResolveLogPath(logDirectory, options); writer = createWriter?.Invoke(logPath) ?? CreateLogWriter(logPath); afterWriterCreated?.Invoke(); - SetLogFilePermissions(logPath); - PruneOldLogs(logDirectory, options.RetainCount); + SetLogFilePermissions(logPath, privateLogDiagnostics.Add); + PruneOldLogs(logDirectory, options.RetainCount, privateLogDiagnostics.Add); var session = new Session(writer, logPath, options.Format); writer = null; @@ -87,6 +89,7 @@ internal static class GlobalToolLog session.Write("INFO", $"base_dir={AppContext.BaseDirectory}"); session.Write("INFO", $"cwd={Environment.CurrentDirectory}"); session.Write("INFO", $"args={FormatArgs(args)}"); + WritePrivateLogDiagnostics(session, privateLogDiagnostics); return session; } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) @@ -410,11 +413,14 @@ private static string CreateProcessLogSuffix() return FormattableString.Invariant($"p{Environment.ProcessId}-{startTime}"); } - private static void PruneOldLogs(string logDirectory, int retainedLogFileCount) + private static void PruneOldLogs( + string logDirectory, + int retainedLogFileCount, + Action? diagnosticSink) { try { - PrivateLogFile.PruneOldFiles(logDirectory, "stderr-*.log", retainedLogFileCount); + PrivateLogFile.PruneOldFiles(logDirectory, "stderr-*.log", retainedLogFileCount, diagnosticSink); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -422,14 +428,14 @@ private static void PruneOldLogs(string logDirectory, int retainedLogFileCount) } } - private static void HardenLogFiles(string logDirectory) + private static void HardenLogFiles(string logDirectory, Action? diagnosticSink) { if (OperatingSystem.IsWindows()) return; try { - PrivateLogFile.HardenExisting(logDirectory, "stderr-*.log"); + PrivateLogFile.HardenExisting(logDirectory, "stderr-*.log", diagnosticSink); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -437,14 +443,14 @@ private static void HardenLogFiles(string logDirectory) } } - private static void SetLogFilePermissions(string logPath) + private static void SetLogFilePermissions(string logPath, Action? diagnosticSink) { if (OperatingSystem.IsWindows()) return; try { - PrivateLogFile.TrySetPrivatePermissions(logPath); + PrivateLogFile.TrySetPrivatePermissions(logPath, diagnosticSink); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -452,6 +458,29 @@ private static void SetLogFilePermissions(string logPath) } } + private static void WritePrivateLogDiagnostics(Session session, IReadOnlyList diagnostics) + { + var emitted = Math.Min(diagnostics.Count, PrivateLogDiagnosticEmitLimit); + for (var i = 0; i < emitted; i++) + { + var diagnostic = diagnostics[i]; + session.Write( + "WARN", + "private_log_diagnostic" + + $" operation={QuoteLogValue(diagnostic.Operation)}" + + $" reason={QuoteLogValue(diagnostic.Reason)}" + + $" target={QuoteLogValue(diagnostic.Target)}"); + } + + if (diagnostics.Count > emitted) + { + session.Write( + "WARN", + "private_log_diagnostics_truncated" + + $" omitted={(diagnostics.Count - emitted).ToString(CultureInfo.InvariantCulture)}"); + } + } + internal static string FormatArgs(string[] args) { if (args.Length == 0) diff --git a/src/CodeIndex/Cli/PrivateLogFile.cs b/src/CodeIndex/Cli/PrivateLogFile.cs index 57f589fec5..afbc2faec2 100644 --- a/src/CodeIndex/Cli/PrivateLogFile.cs +++ b/src/CodeIndex/Cli/PrivateLogFile.cs @@ -7,6 +7,7 @@ internal static class PrivateLogFile { internal const UnixFileMode PrivateFileMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; internal const int MaxExistingFilesToHarden = 128; + private const int MaxDiagnosticTargetChars = 160; internal static FileStream OpenAppend(string path, FileShare share = FileShare.ReadWrite) { @@ -28,7 +29,7 @@ internal static StreamWriter OpenAppendText(string path) AutoFlush = true, }; - internal static void TrySetPrivatePermissions(string path) + internal static void TrySetPrivatePermissions(string path, Action? diagnosticSink = null) { if (OperatingSystem.IsWindows()) return; @@ -39,11 +40,11 @@ internal static void TrySetPrivatePermissions(string path) } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) { - // Best-effort only / ベストエフォートのみ + ReportDiagnostic(diagnosticSink, "set_private_permissions", path, ex); } } - internal static void HardenExisting(string directory, string pattern) + internal static void HardenExisting(string directory, string pattern, Action? diagnosticSink = null) { if (OperatingSystem.IsWindows()) return; @@ -53,7 +54,7 @@ internal static void HardenExisting(string directory, string pattern) var hardened = 0; foreach (var file in new DirectoryInfo(directory).EnumerateFiles(pattern, SearchOption.TopDirectoryOnly)) { - TrySetPrivatePermissions(file.FullName); + TrySetPrivatePermissions(file.FullName, diagnosticSink); hardened++; if (hardened >= MaxExistingFilesToHarden) break; @@ -61,11 +62,11 @@ internal static void HardenExisting(string directory, string pattern) } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - // Best-effort only / ベストエフォートのみ + ReportDiagnostic(diagnosticSink, "harden_existing", directory, ex); } } - internal static void PruneOldFiles(string directory, string pattern, int retainedFileCount) + internal static void PruneOldFiles(string directory, string pattern, int retainedFileCount, Action? diagnosticSink = null) { try { @@ -85,7 +86,7 @@ internal static void PruneOldFiles(string directory, string pattern, int retaine } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - // Best-effort only / ベストエフォートのみ + ReportDiagnostic(diagnosticSink, "prune_old_files", directory, ex); } } @@ -190,4 +191,56 @@ private static void SafeDelete(string path) // Ignore: rotation is best-effort. } } + + private static void ReportDiagnostic( + Action? diagnosticSink, + string operation, + string path, + Exception exception) + { + if (diagnosticSink is null) + return; + + try + { + diagnosticSink(new PrivateLogFileDiagnostic( + operation, + ClassifyFailure(exception), + FormatDiagnosticTarget(path))); + } + catch + { + // Diagnostics must not make best-effort log hardening fail. + } + } + + private static string ClassifyFailure(Exception exception) => + exception switch + { + UnauthorizedAccessException => "permission_denied", + FileNotFoundException or DirectoryNotFoundException => "not_found", + NotSupportedException => "not_supported", + IOException => "io_error", + _ => "operation_failed", + }; + + private static string FormatDiagnosticTarget(string path) + { + try + { + var trimmed = path.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + var target = Path.GetFileName(trimmed); + if (string.IsNullOrWhiteSpace(target)) + target = ""; + return target.Length <= MaxDiagnosticTargetChars + ? target + : target[..MaxDiagnosticTargetChars] + "..."; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException) + { + return ""; + } + } } + +internal sealed record PrivateLogFileDiagnostic(string Operation, string Reason, string Target); diff --git a/tests/CodeIndex.Tests/GlobalToolLogTests.cs b/tests/CodeIndex.Tests/GlobalToolLogTests.cs index a183f2b0db..d4550bcd6a 100644 --- a/tests/CodeIndex.Tests/GlobalToolLogTests.cs +++ b/tests/CodeIndex.Tests/GlobalToolLogTests.cs @@ -79,6 +79,24 @@ public void PrivateLogFile_HardenExisting_CapsBestEffortWork_Issue3027() } } + [Fact] + public void PrivateLogFile_TrySetPrivatePermissions_WhenPathMissing_ReportsDiagnostic_Issue3475() + { + if (OperatingSystem.IsWindows()) + return; + + var path = Path.Combine(Path.GetTempPath(), $"cdidx_missing_private_log_{Guid.NewGuid():N}.log"); + var diagnostics = new List(); + + PrivateLogFile.TrySetPrivatePermissions(path, diagnostics.Add); + + var diagnostic = Assert.Single(diagnostics); + Assert.Equal("set_private_permissions", diagnostic.Operation); + Assert.Equal("not_found", diagnostic.Reason); + Assert.Equal(Path.GetFileName(path), diagnostic.Target); + Assert.DoesNotContain(Path.GetTempPath(), diagnostic.Target, StringComparison.Ordinal); + } + [Fact] public void PrivateLogFile_PruneOldFiles_KeepsNewestFilesWithoutMaterializingAll_Issue3028() { @@ -115,6 +133,47 @@ public void PrivateLogFile_PruneOldFiles_KeepsNewestFilesWithoutMaterializingAll } } + [Fact] + public void TryStart_WritesPrivateLogDiagnostics_Issue3475() + { + if (OperatingSystem.IsWindows()) + return; + + var logRoot = Path.Combine(Path.GetTempPath(), $"cdidx_global_log_private_diag_{Guid.NewGuid():N}"); + var capturedLogPath = Path.Combine(Path.GetTempPath(), $"cdidx_captured_private_diag_{Guid.NewGuid():N}.log"); + try + { + using var env = EnvironmentVariableScope.Capture( + "CDIDX_FORCE_GLOBAL_TOOL_LOG", + "CDIDX_DISABLE_PERSISTENT_LOG", + "CDIDX_GLOBAL_TOOL_LOG_DIR"); + env.Set("CDIDX_FORCE_GLOBAL_TOOL_LOG", "1"); + env.Set("CDIDX_DISABLE_PERSISTENT_LOG", null); + env.Set("CDIDX_GLOBAL_TOOL_LOG_DIR", logRoot); + + using (var session = GlobalToolLog.TryStartForTesting( + ["status"], + "test", + createWriter: _ => PrivateLogFile.OpenAppendText(capturedLogPath))) + { + Assert.NotNull(session); + } + + var log = File.ReadAllText(capturedLogPath); + Assert.Contains("private_log_diagnostic", log); + Assert.Contains("operation=\"set_private_permissions\"", log); + Assert.Contains("reason=\"not_found\"", log); + Assert.DoesNotContain(logRoot, log, StringComparison.Ordinal); + } + finally + { + if (Directory.Exists(logRoot)) + Directory.Delete(logRoot, recursive: true); + if (File.Exists(capturedLogPath)) + File.Delete(capturedLogPath); + } + } + [Fact] public void FormatArgs_RedactsSensitiveArgumentsByDefault() { From 1a423d4df87907ba3709ef599193e09582e11e19 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 13 Jun 2026 23:38:08 +0900 Subject: [PATCH 2/4] Sanitize index lock metadata and cleanup diagnostics (#3462) --- changelog.d/unreleased/3462.fixed.md | 19 ++++++ src/CodeIndex/Cli/IndexLock.cs | 62 ++++++++----------- src/CodeIndex/Cli/LockCleanupDiagnostic.cs | 21 +++++++ src/CodeIndex/Mcp/McpIndexRunLock.cs | 10 ++- .../IndexCommandRunnerTests.cs | 49 +++++++++++++++ tests/CodeIndex.Tests/McpServerTests.cs | 42 +++++++++++++ 6 files changed, 166 insertions(+), 37 deletions(-) create mode 100644 changelog.d/unreleased/3462.fixed.md create mode 100644 src/CodeIndex/Cli/LockCleanupDiagnostic.cs diff --git a/changelog.d/unreleased/3462.fixed.md b/changelog.d/unreleased/3462.fixed.md new file mode 100644 index 0000000000..95ce6a06b3 --- /dev/null +++ b/changelog.d/unreleased/3462.fixed.md @@ -0,0 +1,19 @@ +--- +category: fixed +issues: + - 3462 +affected: + - src/CodeIndex/Cli/IndexLock.cs + - src/CodeIndex/Cli/LockCleanupDiagnostic.cs + - src/CodeIndex/Mcp/McpIndexRunLock.cs + - tests/CodeIndex.Tests/IndexCommandRunnerTests.cs + - tests/CodeIndex.Tests/McpServerTests.cs +--- + +## English + +- **Index lock metadata and cleanup failures are now safer (#3462)** — new CLI lock sidecars no longer include host or project-path metadata, and CLI/MCP lock cleanup failures now report bounded reason-code diagnostics without raw exception text. + +## 日本語 + +- **index lock metadata と cleanup 失敗をより安全に扱うようになりました (#3462)** — 新しい CLI lock sidecar は host や project path を含めず、CLI / MCP lock の cleanup 失敗は生の例外文ではなく bounded な reason-code diagnostic として記録します。 diff --git a/src/CodeIndex/Cli/IndexLock.cs b/src/CodeIndex/Cli/IndexLock.cs index 8ae9083b64..86588c43c0 100644 --- a/src/CodeIndex/Cli/IndexLock.cs +++ b/src/CodeIndex/Cli/IndexLock.cs @@ -30,6 +30,9 @@ internal sealed class IndexLock : IDisposable private readonly string _infoPath; private bool _disposed; + internal static Action DeleteFileForTesting { get; set; } = File.Delete; + internal static Action? CleanupDiagnosticSinkForTesting { get; set; } + private IndexLock(FileStream stream, string lockPath, string infoPath) { _stream = stream; @@ -99,9 +102,7 @@ public static IndexLock Acquire(string lockPath, string projectPath) { var info = new IndexLockInfo( Pid: Environment.ProcessId, - StartedAt: DateTime.UtcNow, - Host: Environment.MachineName, - ProjectPath: Path.GetFullPath(projectPath)); + StartedAt: DateTime.UtcNow); DataDirectorySecurity.WritePrivateText(infoPath, SerializeInfo(info), Encoding.UTF8); } catch (Exception) @@ -148,14 +149,7 @@ public void Dispose() return; _disposed = true; - try - { - File.Delete(_infoPath); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - // Best-effort. / ベストエフォート。 - } + TryDeleteCleanupTarget(_infoPath, "metadata"); try { @@ -166,16 +160,7 @@ public void Dispose() // Best-effort. / ベストエフォート。 } - try - { - File.Delete(_lockPath); - } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) - { - // Best-effort cleanup; a leftover empty lockfile does not block future - // acquires because the next Acquire opens it with OpenOrCreate. - // 残った空 lockfile も次回 Acquire の OpenOrCreate で再利用できる。 - } + TryDeleteCleanupTarget(_lockPath, "lockfile"); } // --- Tiny key=value serializer (avoids touching JsonSerializerContext) --- @@ -186,8 +171,6 @@ private static string SerializeInfo(IndexLockInfo info) var sb = new StringBuilder(); sb.Append("pid=").Append(info.Pid.ToString(CultureInfo.InvariantCulture)).Append('\n'); sb.Append("started_at=").Append(info.StartedAt.ToString("o", CultureInfo.InvariantCulture)).Append('\n'); - sb.Append("host=").Append(EscapeValue(info.Host)).Append('\n'); - sb.Append("project=").Append(EscapeValue(info.ProjectPath)).Append('\n'); return sb.ToString(); } @@ -195,8 +178,6 @@ private static string SerializeInfo(IndexLockInfo info) { int? pid = null; DateTime? started = null; - string? host = null; - string? project = null; foreach (var rawLine in text.Split('\n')) { var line = rawLine.TrimEnd('\r'); @@ -217,18 +198,12 @@ private static string SerializeInfo(IndexLockInfo info) if (DateTime.TryParse(value, CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var s)) started = s; break; - case "host": - host = value; - break; - case "project": - project = value; - break; } } if (pid is null || started is null) return null; - return new IndexLockInfo(pid.Value, started.Value, host ?? string.Empty, project ?? string.Empty); + return new IndexLockInfo(pid.Value, started.Value); } private static string EscapeValue(string? value) @@ -264,13 +239,30 @@ private static string UnescapeValue(string value) } return sb.ToString(); } + + private static void TryDeleteCleanupTarget(string path, string target) + { + try + { + DeleteFileForTesting(path); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) + { + ReportCleanupFailure(target, ex); + } + } + + private static void ReportCleanupFailure(string target, Exception exception) + { + var diagnostic = LockCleanupDiagnostic.Create("index_lock", target, exception); + GlobalToolLog.Error(diagnostic.ToLogMessage()); + CleanupDiagnosticSinkForTesting?.Invoke(diagnostic); + } } internal sealed record IndexLockInfo( int Pid, - DateTime StartedAt, - string Host, - string ProjectPath); + DateTime StartedAt); internal sealed class IndexLockConflictException : Exception { diff --git a/src/CodeIndex/Cli/LockCleanupDiagnostic.cs b/src/CodeIndex/Cli/LockCleanupDiagnostic.cs new file mode 100644 index 0000000000..7568a5c76a --- /dev/null +++ b/src/CodeIndex/Cli/LockCleanupDiagnostic.cs @@ -0,0 +1,21 @@ +namespace CodeIndex.Cli; + +internal sealed record LockCleanupDiagnostic(string Component, string Target, string Reason) +{ + internal string ToLogMessage() => + $"{Component}_cleanup_failed target={Target} reason={Reason}"; + + internal static LockCleanupDiagnostic Create(string component, string target, Exception exception) => + new(component, target, ClassifyFailure(exception)); + + private static string ClassifyFailure(Exception exception) => + exception switch + { + UnauthorizedAccessException => "permission_denied", + FileNotFoundException or DirectoryNotFoundException => "not_found", + NotSupportedException => "not_supported", + IOException => "io_error", + ObjectDisposedException => "object_disposed", + _ => "operation_failed", + }; +} diff --git a/src/CodeIndex/Mcp/McpIndexRunLock.cs b/src/CodeIndex/Mcp/McpIndexRunLock.cs index c49f771057..755db53b3e 100644 --- a/src/CodeIndex/Mcp/McpIndexRunLock.cs +++ b/src/CodeIndex/Mcp/McpIndexRunLock.cs @@ -19,6 +19,9 @@ internal sealed class McpIndexRunLock : IDisposable private readonly string _infoPath; private bool _disposed; + internal static Action DeleteFileForTesting { get; set; } = File.Delete; + internal static Action? CleanupDiagnosticSinkForTesting { get; set; } + private McpIndexRunLock(FileStream stream, string infoPath) { _stream = stream; @@ -148,10 +151,13 @@ public void Dispose() _disposed = true; try { - File.Delete(_infoPath); + DeleteFileForTesting(_infoPath); } - catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) { + var diagnostic = LockCleanupDiagnostic.Create("mcp_index_lock", "metadata", ex); + GlobalToolLog.Error(diagnostic.ToLogMessage()); + CleanupDiagnosticSinkForTesting?.Invoke(diagnostic); } _stream.Dispose(); } diff --git a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs index eaac4a9950..06d73078d1 100644 --- a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs @@ -11397,6 +11397,12 @@ public void IndexLock_Acquire_OnPosix_WritesPrivateInfoFile() using var indexLock = IndexLock.Acquire(lockPath, projectRoot); Assert.True(File.Exists(infoPath)); + var info = File.ReadAllText(infoPath); + Assert.Contains("pid=", info); + Assert.Contains("started_at=", info); + Assert.DoesNotContain("host=", info); + Assert.DoesNotContain("project=", info); + Assert.DoesNotContain(projectRoot, info); Assert.Equal( DataDirectorySecurity.PrivateFileMode, File.GetUnixFileMode(infoPath) & DataDirectorySecurity.PermissionBits); @@ -11413,6 +11419,49 @@ public void IndexLock_Acquire_OnPosix_WritesPrivateInfoFile() } } + [Fact] + public void IndexLock_Dispose_WhenMetadataCleanupFails_ReportsSanitizedDiagnostic_Issue3462() + { + var projectRoot = CreateTempProject(); + var dbPath = Path.Combine(Path.GetTempPath(), $"cdidx_cleanup_diag_{Guid.NewGuid():N}.db"); + var lockPath = dbPath + ".lock"; + var infoPath = lockPath + ".info"; + var diagnostics = new List(); + try + { + IndexLock.CleanupDiagnosticSinkForTesting = diagnostics.Add; + IndexLock.DeleteFileForTesting = path => + { + if (string.Equals(path, infoPath, StringComparison.Ordinal)) + throw new IOException($"sensitive cleanup path {path}"); + File.Delete(path); + }; + + using (IndexLock.Acquire(lockPath, projectRoot)) + { + } + + var diagnostic = Assert.Single(diagnostics); + Assert.Equal("index_lock", diagnostic.Component); + Assert.Equal("metadata", diagnostic.Target); + Assert.Equal("io_error", diagnostic.Reason); + Assert.DoesNotContain("sensitive", diagnostic.ToLogMessage(), StringComparison.Ordinal); + Assert.DoesNotContain(infoPath, diagnostic.ToLogMessage(), StringComparison.Ordinal); + } + finally + { + IndexLock.CleanupDiagnosticSinkForTesting = null; + IndexLock.DeleteFileForTesting = File.Delete; + DeleteDirectory(projectRoot); + if (File.Exists(infoPath)) + File.Delete(infoPath); + if (File.Exists(lockPath)) + File.Delete(lockPath); + if (File.Exists(dbPath)) + File.Delete(dbPath); + } + } + [Fact] public void IndexLock_TryReadHolderInfo_WhenInfoFileTooLarge_ReturnsNull() { diff --git a/tests/CodeIndex.Tests/McpServerTests.cs b/tests/CodeIndex.Tests/McpServerTests.cs index 45129cab13..9c9defb565 100644 --- a/tests/CodeIndex.Tests/McpServerTests.cs +++ b/tests/CodeIndex.Tests/McpServerTests.cs @@ -10465,6 +10465,48 @@ public void McpIndexRunLock_TryAcquire_OnPosix_WritesPrivateInfoFile() } } + [Fact] + public void McpIndexRunLock_Dispose_WhenInfoCleanupFails_ReportsSanitizedDiagnostic_Issue3462() + { + var dbPath = Path.Combine(Path.GetTempPath(), $"cdidx_mcp_cleanup_diag_{Guid.NewGuid():N}.db"); + var lockPath = McpIndexRunLock.ResolveLockPath(dbPath); + var infoPath = lockPath + ".info"; + var diagnostics = new List(); + try + { + McpIndexRunLock.CleanupDiagnosticSinkForTesting = diagnostics.Add; + McpIndexRunLock.DeleteFileForTesting = path => + { + if (string.Equals(path, infoPath, StringComparison.Ordinal)) + throw new IOException($"sensitive cleanup path {path}"); + File.Delete(path); + }; + + Assert.True(McpIndexRunLock.TryAcquire(dbPath, out var runLock, out var error), error); + using (runLock!) + { + } + + var diagnostic = Assert.Single(diagnostics); + Assert.Equal("mcp_index_lock", diagnostic.Component); + Assert.Equal("metadata", diagnostic.Target); + Assert.Equal("io_error", diagnostic.Reason); + Assert.DoesNotContain("sensitive", diagnostic.ToLogMessage(), StringComparison.Ordinal); + Assert.DoesNotContain(infoPath, diagnostic.ToLogMessage(), StringComparison.Ordinal); + } + finally + { + McpIndexRunLock.CleanupDiagnosticSinkForTesting = null; + McpIndexRunLock.DeleteFileForTesting = File.Delete; + if (File.Exists(infoPath)) + File.Delete(infoPath); + if (File.Exists(lockPath)) + File.Delete(lockPath); + if (File.Exists(dbPath)) + File.Delete(dbPath); + } + } + [Fact] public void ToolsCall_Index_WhenDbLockInfoTooLarge_ReturnsBusyWithoutHolderDetails() { From 7cb35b0fb6f8105f6aa0cb77791d78ce4a181ba8 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sat, 13 Jun 2026 23:38:41 +0900 Subject: [PATCH 3/4] Sanitize changelog cleanup diagnostics (#3457) --- changelog.d/unreleased/3457.fixed.md | 16 ++++ tests/CodeIndex.Tests/ChangelogToolTests.cs | 83 +++++++++++++++++++++ tools/CodeIndex.Changelog/Program.cs | 83 +++++++++++++++++++-- 3 files changed, 175 insertions(+), 7 deletions(-) create mode 100644 changelog.d/unreleased/3457.fixed.md diff --git a/changelog.d/unreleased/3457.fixed.md b/changelog.d/unreleased/3457.fixed.md new file mode 100644 index 0000000000..12a0402705 --- /dev/null +++ b/changelog.d/unreleased/3457.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 3457 +affected: + - tools/CodeIndex.Changelog/Program.cs + - tests/CodeIndex.Tests/ChangelogToolTests.cs +--- + +## English + +- **Changelog cleanup failures now use sanitized diagnostics (#3457)** — release preparation cleanup and rollback failures now report stable categories, affected relative paths, reason codes, and recovery hints without raw filesystem exception messages. + +## 日本語 + +- **changelog cleanup 失敗が sanitized diagnostic を返すようになりました (#3457)** — release preparation の cleanup / rollback 失敗は、生の filesystem 例外文ではなく、安定した category、影響する相対パス、reason code、復旧ヒントを返します。 diff --git a/tests/CodeIndex.Tests/ChangelogToolTests.cs b/tests/CodeIndex.Tests/ChangelogToolTests.cs index d2908d9695..38fd1ce98e 100644 --- a/tests/CodeIndex.Tests/ChangelogToolTests.cs +++ b/tests/CodeIndex.Tests/ChangelogToolTests.cs @@ -284,6 +284,89 @@ public void PrepareFailureBeforeFragmentDeletionRollsBackReleaseFiles() Assert.DoesNotContain(scope.ListFiles("."), path => Path.GetFileName(path).EndsWith(".tmp", StringComparison.Ordinal)); } + [Fact] + public void PrepareConsumedFragmentDeleteFailureReportsSanitizedDiagnostic_Issue3457() + { + using var scope = new TestRepositoryScope(); + scope.WriteFile("CHANGELOG.md", SampleChangelog); + scope.WriteFile("version.json", """ + { + "version": "1.16.0" + } + """); + scope.WriteFile("changelog.d/unreleased/195.fixed.md", SampleFragment); + var fragmentPath = Path.Combine(scope.Root, "changelog.d/unreleased/195.fixed.md"); + + var tool = new ChangelogTool(scope.Root); + ChangelogException? ex = null; + ChangelogTool.DeleteFileForTesting = path => + { + if (string.Equals(path, fragmentPath, StringComparison.Ordinal)) + throw new IOException($"raw filesystem detail {path}"); + File.Delete(path); + }; + try + { + ex = Assert.Throws(() => tool.Prepare(new Version(1, 17, 0), new DateOnly(2026, 5, 1), writeChanges: true)); + } + finally + { + ChangelogTool.DeleteFileForTesting = File.Delete; + } + + Assert.NotNull(ex); + Assert.Contains("fragment_delete_failed", ex.Message); + Assert.Contains("affected_paths=changelog.d/unreleased/195.fixed.md", ex.Message); + Assert.Contains("reason=io_error", ex.Message); + Assert.Contains("Hint: Delete the listed fragment manually before retrying prepare.", ex.Message); + Assert.DoesNotContain("raw filesystem detail", ex.Message); + Assert.DoesNotContain(scope.Root, ex.Message); + } + + [Fact] + public void PrepareRollbackFailureReportsSanitizedDiagnostic_Issue3457() + { + using var scope = new TestRepositoryScope(); + scope.WriteFile("CHANGELOG.md", SampleChangelog); + scope.WriteFile("version.json", """ + { + "version": "1.16.0" + } + """); + scope.WriteFile("changelog.d/unreleased/195.fixed.md", SampleFragment); + var versionPath = Path.Combine(scope.Root, "version.json"); + + var tool = new ChangelogTool(scope.Root); + ChangelogException? ex = null; + ChangelogTool.PrepareWritePhaseForTesting = phase => + { + if (phase == PrepareWritePhase.BeforeFragmentsDeleted) + throw new ChangelogException("injected fragment deletion failure"); + }; + ChangelogTool.BeforeRestoreTextForTesting = path => + { + if (string.Equals(path, versionPath, StringComparison.Ordinal)) + throw new IOException($"raw rollback detail {path}"); + }; + try + { + ex = Assert.Throws(() => tool.Prepare(new Version(1, 17, 0), new DateOnly(2026, 5, 1), writeChanges: true)); + } + finally + { + ChangelogTool.PrepareWritePhaseForTesting = null; + ChangelogTool.BeforeRestoreTextForTesting = null; + } + + Assert.NotNull(ex); + Assert.Contains("rollback_failed", ex.Message); + Assert.Contains("affected_paths=version.json,CHANGELOG.md", ex.Message); + Assert.Contains("reason=io_error", ex.Message); + Assert.Contains("Hint: Restore the listed release files from version control or backup before retrying prepare.", ex.Message); + Assert.DoesNotContain("raw rollback detail", ex.Message); + Assert.DoesNotContain(scope.Root, ex.Message); + } + [Fact] public void RenderReleaseNotesUsesProvidedPreviousVersion() { diff --git a/tools/CodeIndex.Changelog/Program.cs b/tools/CodeIndex.Changelog/Program.cs index a5036c2371..7278d7d12e 100644 --- a/tools/CodeIndex.Changelog/Program.cs +++ b/tools/CodeIndex.Changelog/Program.cs @@ -170,6 +170,8 @@ public sealed class ChangelogTool public const long MaxChangelogBytes = 8 * 1024 * 1024; public const long MaxVersionJsonBytes = 16 * 1024; internal static Action? PrepareWritePhaseForTesting { get; set; } + internal static Action DeleteFileForTesting { get; set; } = File.Delete; + internal static Action? BeforeRestoreTextForTesting { get; set; } private static readonly string[] AllowedCategories = [ @@ -621,7 +623,7 @@ private static string ReadAllTextBounded(string absolutePath, string repositoryR return reader.ReadToEnd(); } - private static void WritePreparedFiles( + private void WritePreparedFiles( string changelogPath, string originalChangelog, string updatedChangelog, @@ -735,19 +737,23 @@ private static void ReplaceWithStagedFile(string stagedPath, string targetPath) File.Move(stagedPath, targetPath, overwrite: true); } - private static void DeleteConsumedFragment(Fragment fragment) + private void DeleteConsumedFragment(Fragment fragment) { try { - File.Delete(fragment.AbsolutePath); + DeleteFileForTesting(fragment.AbsolutePath); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - throw new ChangelogException($"{fragment.RelativePath}: failed to delete consumed changelog fragment after CHANGELOG.md and version.json were updated; delete this fragment manually before retrying prepare. {ex.Message}"); + throw new ChangelogException(BuildCleanupFailureMessage( + "fragment_delete_failed", + [fragment.RelativePath], + ex, + "Delete the listed fragment manually before retrying prepare.")); } } - private static void RollBackPreparedFiles( + private void RollBackPreparedFiles( string changelogPath, string originalChangelog, bool changelogReplaced, @@ -765,12 +771,17 @@ private static void RollBackPreparedFiles( } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - throw new ChangelogException($"prepare failed before fragment deletion and rollback also failed: {ex.Message}"); + throw new ChangelogException(BuildCleanupFailureMessage( + "rollback_failed", + GetRollbackAffectedPaths(changelogPath, changelogReplaced, versionPath, versionReplaced), + ex, + "Restore the listed release files from version control or backup before retrying prepare.")); } } private static void RestoreText(string targetPath, string contents) { + BeforeRestoreTextForTesting?.Invoke(targetPath); var tempPath = string.Empty; try { @@ -806,13 +817,71 @@ private static void TryDelete(string path) try { - File.Delete(path); + DeleteFileForTesting(path); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { } } + private string BuildCleanupFailureMessage( + string category, + IReadOnlyList affectedPaths, + Exception exception, + string recoveryHint) + { + var paths = affectedPaths + .Select(FormatRelativePath) + .Where(path => !string.IsNullOrWhiteSpace(path)) + .Distinct(StringComparer.Ordinal) + .DefaultIfEmpty("") + .ToArray(); + + return $"{category}: affected_paths={string.Join(",", paths)}; reason={ClassifyFileFailure(exception)}; Hint: {recoveryHint}"; + } + + private IReadOnlyList GetRollbackAffectedPaths( + string changelogPath, + bool changelogReplaced, + string versionPath, + bool versionReplaced) + { + var paths = new List(capacity: 2); + if (versionReplaced) + paths.Add(versionPath); + if (changelogReplaced) + paths.Add(changelogPath); + return paths; + } + + private string FormatRelativePath(string path) + { + string relative; + try + { + relative = Path.IsPathFullyQualified(path) + ? Path.GetRelativePath(_repositoryRoot, path) + : path; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException) + { + return ""; + } + + if (relative.StartsWith("..", StringComparison.Ordinal) || Path.IsPathRooted(relative)) + relative = Path.GetFileName(path); + return relative.Replace('\\', '/'); + } + + private static string ClassifyFileFailure(Exception exception) => + exception switch + { + UnauthorizedAccessException => "permission_denied", + FileNotFoundException or DirectoryNotFoundException => "not_found", + IOException => "io_error", + _ => "operation_failed", + }; + private static List PrepareLanguageSection( IReadOnlyList existingBlocks, Version targetVersion, From 58471ecdf4fad89b5114ec71008b04669fa26910 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Sun, 14 Jun 2026 01:46:35 +0900 Subject: [PATCH 4/4] Fix Windows changelog delete hook test (#3457) --- tests/CodeIndex.Tests/ChangelogToolTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/CodeIndex.Tests/ChangelogToolTests.cs b/tests/CodeIndex.Tests/ChangelogToolTests.cs index 38fd1ce98e..43f8adf2bf 100644 --- a/tests/CodeIndex.Tests/ChangelogToolTests.cs +++ b/tests/CodeIndex.Tests/ChangelogToolTests.cs @@ -296,12 +296,16 @@ public void PrepareConsumedFragmentDeleteFailureReportsSanitizedDiagnostic_Issue """); scope.WriteFile("changelog.d/unreleased/195.fixed.md", SampleFragment); var fragmentPath = Path.Combine(scope.Root, "changelog.d/unreleased/195.fixed.md"); + var fullFragmentPath = Path.GetFullPath(fragmentPath); + var pathComparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; var tool = new ChangelogTool(scope.Root); ChangelogException? ex = null; ChangelogTool.DeleteFileForTesting = path => { - if (string.Equals(path, fragmentPath, StringComparison.Ordinal)) + if (string.Equals(Path.GetFullPath(path), fullFragmentPath, pathComparison)) throw new IOException($"raw filesystem detail {path}"); File.Delete(path); };