From a765386148ab4204699fdaefa4aac9f9a955985b Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 26 Aug 2026 06:03:18 +0900 Subject: [PATCH 1/6] Harden repository-configured output paths (#5181) --- DEVELOPER_GUIDE.md | 25 +- changelog.d/unreleased/5181.security.md | 23 + src/CodeIndex/Cli/CdidxConfigFile.cs | 51 +- src/CodeIndex/Cli/GlobalToolLog.cs | 108 ++- src/CodeIndex/Cli/MetricsSink.cs | 46 +- src/CodeIndex/Cli/PrivateLogFile.cs | 115 ++- .../Cli/RepositoryOutputPathBoundary.cs | 864 ++++++++++++++++++ tests/CodeIndex.Tests/CdidxConfigFileTests.cs | 234 +++++ tests/CodeIndex.Tests/GlobalToolLogTests.cs | 114 +++ tests/CodeIndex.Tests/MetricsSinkTests.cs | 124 +++ 10 files changed, 1612 insertions(+), 92 deletions(-) create mode 100644 changelog.d/unreleased/5181.security.md create mode 100644 src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 092c693fd1..ac982e008e 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -2948,7 +2948,18 @@ If every platform candidate is unavailable, the final temp fallback is a hashed per-user `cdidx-u.../logs` directory under the OS temp root. Each candidate is probed with a create/write/delete round trip before the logger commits to it, so read-only state/cache/runtime mounts fall through to the -next candidate instead of losing the first log write. The file name is +next candidate instead of losing the first log write. Repository-configured +`metrics_path` and `global_tool_log_dir` values from `.cdidxrc.json` or +`.cdidx/config.json` use a stricter boundary: every existing component below +the config workspace is rejected when it is a symbolic link, junction, +cross-device mount point, reparse point, device, or dangling link. The boundary +is revalidated before each mutation; on POSIX, directory creation, append, +permission changes, rotation, and deletion are additionally anchored to the +workspace directory handle with no-follow relative operations. An unsafe value fails config +validation with the bounded `unsafe_output_path` diagnostic and does not create, +append, rotate, delete, or chmod the external target. Explicit CLI and process +environment destinations retain their existing operator-controlled behavior. +The file name is `stderr-YYYYMMDD.log`, timestamps inside the file are ISO-8601 UTC (`yyyy-MM-ddTHH:mm:ss.fffZ`) using invariant culture, and the logger keeps only the newest 30 daily files. `CDIDX_LOG_FORMAT` / `--log-format` switch @@ -6950,7 +6961,17 @@ platform candidate がすべて使えない場合、最後の temp fallback は 配下のユーザー別 hashed `cdidx-u.../logs` ディレクトリです。各 candidate は logger が採用する前に create/write/delete の往復で probe されるため、 read-only な state/cache/runtime mount は最初の log write を失うのではなく -次の candidate へ fall through します。ファイル名は +次の candidate へ fall through します。`.cdidxrc.json` または +`.cdidx/config.json` の repository config に由来する `metrics_path` と +`global_tool_log_dir` には、より厳格な境界を適用します。config workspace +配下の既存 component が symbolic link、junction、cross-device mount point、 +reparse point、device、dangling link のいずれかであれば拒否し、各 mutation の +直前にも境界を再検証します。POSIX ではさらに directory 作成、append、permission +変更、rotation、delete を workspace directory handle 起点の no-follow relative +operation へ固定します。安全でない値は上限付きの `unsafe_output_path` 診断で config validation +に失敗し、外部 target の作成、追記、rotation、削除、chmod は行いません。明示的な +CLI と process environment の保存先は、operator が制御する従来の挙動を維持します。 +ファイル名は `stderr-YYYYMMDD.log`、ファイル内 timestamp は invariant culture の ISO-8601 UTC（`yyyy-MM-ddTHH:mm:ss.fffZ`）で、logger は新しい 30 日次 ファイルだけを保持します。`CDIDX_LOG_FORMAT` / `--log-format` は text と diff --git a/changelog.d/unreleased/5181.security.md b/changelog.d/unreleased/5181.security.md new file mode 100644 index 0000000000..e7420acfe4 --- /dev/null +++ b/changelog.d/unreleased/5181.security.md @@ -0,0 +1,23 @@ +--- +category: security +issues: + - 5181 +affected: + - src/CodeIndex/Cli/CdidxConfigFile.cs + - src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs + - src/CodeIndex/Cli/MetricsSink.cs + - src/CodeIndex/Cli/GlobalToolLog.cs + - src/CodeIndex/Cli/PrivateLogFile.cs + - tests/CodeIndex.Tests/CdidxConfigFileTests.cs + - tests/CodeIndex.Tests/MetricsSinkTests.cs + - tests/CodeIndex.Tests/GlobalToolLogTests.cs + - DEVELOPER_GUIDE.md +--- + +## English + +- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, cross-device mount, reparse-point, device, and dangling-link components below the config workspace. Mutations are revalidated, and POSIX writes, directory creation, permission changes, rotation, and deletion are anchored to the workspace directory handle so an untrusted checkout cannot redirect them to an external target. + +## 日本語 + +- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、cross-device mount、reparse point、device、dangling link を拒否します。mutation 前の再検証に加え、POSIX の書き込み、directory 作成、permission 変更、rotation、削除を workspace directory handle に固定し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 diff --git a/src/CodeIndex/Cli/CdidxConfigFile.cs b/src/CodeIndex/Cli/CdidxConfigFile.cs index c9c20b14da..b9e873a9f5 100644 --- a/src/CodeIndex/Cli/CdidxConfigFile.cs +++ b/src/CodeIndex/Cli/CdidxConfigFile.cs @@ -573,7 +573,7 @@ private static bool IsConfigDiscoveryBoundary(DirectoryInfo directory) return false; } - private static string ResolveConfigWorkspaceRoot(string configPath) + internal static string ResolveConfigWorkspaceRoot(string configPath) { var fullConfigPath = Path.GetFullPath(configPath); var configDirectory = Path.GetDirectoryName(fullConfigPath) ?? Path.GetFullPath("."); @@ -1005,45 +1005,24 @@ private static bool TryReadWorkspaceOutputPath(JsonElement element, string key, return false; var workspaceRoot = ResolveConfigWorkspaceRoot(path); - if (!TryResolveWorkspaceOutputPath(raw!, workspaceRoot, out value, out var pathError)) - { - error = pathError; + var destinationIsDirectory = string.Equals(key, "global_tool_log_dir", StringComparison.Ordinal); + if (!RepositoryOutputPathBoundary.TryResolveConfiguredPath( + raw!, + workspaceRoot, + destinationIsDirectory, + out var resolvedPath, + out var pathFailure)) + { + error = pathFailure == "outside_workspace" + ? $"{FormatConfigDiagnosticPrefix(path)} `{key}` must resolve inside the config workspace root `{FormatConfigDiagnosticPath(workspaceRoot)}`." + : pathFailure == RepositoryOutputPathBoundary.UnsafeReason + ? $"{FormatConfigDiagnosticPrefix(path)} `{key}` is unsafe ({RepositoryOutputPathBoundary.UnsafeReason}); symbolic links, junctions, cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root." + : $"{FormatConfigDiagnosticPrefix(path)} `{key}` path is invalid (invalid_path)."; return false; } + value = resolvedPath; return true; - - bool TryResolveWorkspaceOutputPath(string rawPath, string root, out string? resolved, out string? pathError) - { - resolved = null; - pathError = null; - try - { - var normalizedRoot = PathCasing.NormalizeBoundaryPath(root); - var fullPath = Path.IsPathRooted(rawPath) - ? Path.GetFullPath(rawPath) - : Path.GetFullPath(Path.Combine(normalizedRoot, rawPath)); - var normalizedPath = PathCasing.NormalizeBoundaryPath(fullPath); - - if (!PathCasing.IsPathEqualOrParent(normalizedRoot, normalizedPath)) - { - pathError = $"{FormatConfigDiagnosticPrefix(path)} `{key}` must resolve inside the config workspace root `{FormatConfigDiagnosticPath(normalizedRoot)}`."; - return false; - } - - resolved = fullPath; - return true; - } - catch (Exception ex) when (ex is ArgumentException - or IOException - or NotSupportedException - or PathTooLongException - or UnauthorizedAccessException) - { - pathError = $"{FormatConfigDiagnosticPrefix(path)} `{key}` path is invalid (invalid_path)."; - return false; - } - } } private static bool TryReadStringArray(JsonElement element, string key, string path, out string[]? value, out string? error) diff --git a/src/CodeIndex/Cli/GlobalToolLog.cs b/src/CodeIndex/Cli/GlobalToolLog.cs index 45f2b0e351..ece0a74b59 100644 --- a/src/CodeIndex/Cli/GlobalToolLog.cs +++ b/src/CodeIndex/Cli/GlobalToolLog.cs @@ -32,6 +32,7 @@ internal static class GlobalToolLog private const int PrivateLogDiagnosticEmitLimit = 16; internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; private static readonly AsyncLocal CurrentSession = new(); + private sealed record LogDirectorySelection(string Path, RepositoryOutputPathGuard? Boundary); internal static IDisposable? TryStart(string[] args, string appVersion) => TryStart(args, appVersion, createWriter: null, afterWriterCreated: null); @@ -56,15 +57,20 @@ internal static class GlobalToolLog return null; var privateLogDiagnostics = new List(); - var logDirectory = ResolveLogDirectory(); - Directory.CreateDirectory(logDirectory); - HardenLogFiles(logDirectory, privateLogDiagnostics.Add); + var selection = ResolveLogDirectorySelection(requireWritableCandidate: true); + var logDirectory = selection.Path; + var boundary = selection.Boundary; + if (boundary is null) + Directory.CreateDirectory(logDirectory); + else + boundary.CreateSensitiveDestinationDirectory(); + HardenLogFiles(logDirectory, privateLogDiagnostics.Add, boundary); var options = LogOptions.FromEnvironment(); var logPath = ResolveLogPath(logDirectory, options); - writer = createWriter?.Invoke(logPath) ?? CreateLogWriter(logPath); + writer = createWriter?.Invoke(logPath) ?? CreateLogWriter(logPath, boundary); afterWriterCreated?.Invoke(); - SetLogFilePermissions(logPath, privateLogDiagnostics.Add); - PruneOldLogs(logDirectory, options.RetainCount, privateLogDiagnostics.Add); + SetLogFilePermissions(logPath, privateLogDiagnostics.Add, boundary); + PruneOldLogs(logDirectory, options.RetainCount, privateLogDiagnostics.Add, boundary); var session = new Session(writer, logPath, options.Format); writer = null; @@ -78,6 +84,15 @@ internal static class GlobalToolLog WritePrivateLogDiagnostics(session, privateLogDiagnostics); return session; } + catch (RepositoryOutputPathBoundaryException) + { + writer?.Dispose(); + CurrentSession.Value = null; + CommandErrorWriter.WriteWarning( + "persistent log disabled; repository-configured `global_tool_log_dir` is unsafe " + + $"({RepositoryOutputPathBoundary.UnsafeReason})."); + return null; + } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { writer?.Dispose(); @@ -86,8 +101,8 @@ internal static class GlobalToolLog } } - private static StreamWriter CreateLogWriter(string logPath) => - PrivateLogFile.OpenAppendText(logPath); + private static StreamWriter CreateLogWriter(string logPath, RepositoryOutputPathGuard? boundary) => + PrivateLogFile.OpenAppendText(logPath, boundary); internal static void Info(string message) => CurrentSession.Value?.Write("INFO", message); @@ -275,23 +290,40 @@ private static bool ContainsPathSegments(string path, string[] expectedSegments, internal static string ResolveLogDirectoryForStatus() => ResolveLogDirectory(); - private static string ResolveLogDirectory() => ResolveLogDirectory(requireWritableCandidate: true); + private static string ResolveLogDirectory() => ResolveLogDirectorySelection(requireWritableCandidate: true).Path; internal static string ResolveLogDirectoryWithoutWriteProbeForTesting() - => ResolveLogDirectory(requireWritableCandidate: false); + => ResolveLogDirectorySelection(requireWritableCandidate: false).Path; - private static string ResolveLogDirectory(bool requireWritableCandidate) + private static LogDirectorySelection ResolveLogDirectorySelection(bool requireWritableCandidate) { + var configuredOverride = CdidxEnvironment.GetEnvironmentVariable("CDIDX_GLOBAL_TOOL_LOG_DIR"); + var configuredSource = CdidxConfigSourceResolver.GetSource("CDIDX_GLOBAL_TOOL_LOG_DIR"); foreach (var candidate in EnumerateLogDirectoryCandidates()) { if (!TryNormalizeLogDirectoryCandidate(candidate, out var fullPath)) continue; - if (!requireWritableCandidate || CanWriteProbe(fullPath)) - return fullPath; + RepositoryOutputPathGuard? boundary = null; + if (!string.IsNullOrWhiteSpace(configuredOverride) + && !string.IsNullOrWhiteSpace(configuredSource) + && string.Equals( + PathCasing.NormalizeBoundaryPath(ExpandUserLogDirectory(configuredOverride)), + PathCasing.NormalizeBoundaryPath(candidate), + PathCasing.ComparisonFor(candidate))) + { + boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + "CDIDX_GLOBAL_TOOL_LOG_DIR", + "global_tool_log_dir", + fullPath, + destinationIsDirectory: true); + } + + if (!requireWritableCandidate || CanWriteProbe(fullPath, boundary)) + return new LogDirectorySelection(fullPath, boundary); } - return ResolveTempFallbackLogDirectory(); + return new LogDirectorySelection(ResolveTempFallbackLogDirectory(), null); } internal static bool TryNormalizeLogDirectoryCandidate(string candidate, out string fullPath) @@ -352,14 +384,34 @@ private static IEnumerable EnumerateLogDirectoryCandidates() private static string ResolveTempFallbackLogDirectory() => DataDirectorySecurity.ResolveSensitiveTempFallbackDirectory("logs"); - private static bool CanWriteProbe(string directory) + private static bool CanWriteProbe(string directory, RepositoryOutputPathGuard? boundary) { try { - DataDirectorySecurity.CreateSensitiveDirectory(directory); + if (boundary is null) + DataDirectorySecurity.CreateSensitiveDirectory(directory); + else + boundary.CreateSensitiveDestinationDirectory(); var probePath = Path.Combine(directory, $".cdidx-write-probe-{Guid.NewGuid():N}.tmp"); + if (boundary is not null) + { + using (PrivateLogFile.OpenAppend(probePath, boundary: boundary)) + { + } + boundary.PrepareMutation("write_probe_delete", probePath); + if (OperatingSystem.IsWindows()) + File.Delete(LongPath.EnsureWindowsPrefix(probePath)); + else + boundary.DeleteFileUnix(probePath); + boundary.CompleteMutation(probePath); + return true; + } return FileWriteProbe.TryWriteAndDeleteEmptyFile(probePath, Encoding.UTF8); } + catch (RepositoryOutputPathBoundaryException) + { + throw; + } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) { return false; @@ -429,11 +481,17 @@ private static string CreateProcessLogSuffix() private static void PruneOldLogs( string logDirectory, int retainedLogFileCount, - Action? diagnosticSink) + Action? diagnosticSink, + RepositoryOutputPathGuard? boundary) { try { - PrivateLogFile.PruneOldFiles(logDirectory, "stderr-*.log", retainedLogFileCount, diagnosticSink); + PrivateLogFile.PruneOldFiles( + logDirectory, + "stderr-*.log", + retainedLogFileCount, + diagnosticSink, + boundary: boundary); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -441,14 +499,17 @@ private static void PruneOldLogs( } } - private static void HardenLogFiles(string logDirectory, Action? diagnosticSink) + private static void HardenLogFiles( + string logDirectory, + Action? diagnosticSink, + RepositoryOutputPathGuard? boundary) { if (OperatingSystem.IsWindows()) return; try { - PrivateLogFile.HardenExisting(logDirectory, "stderr-*.log", diagnosticSink); + PrivateLogFile.HardenExisting(logDirectory, "stderr-*.log", diagnosticSink, boundary); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -456,14 +517,17 @@ private static void HardenLogFiles(string logDirectory, Action? diagnosticSink) + private static void SetLogFilePermissions( + string logPath, + Action? diagnosticSink, + RepositoryOutputPathGuard? boundary) { if (OperatingSystem.IsWindows()) return; try { - PrivateLogFile.TrySetPrivatePermissions(logPath, diagnosticSink); + PrivateLogFile.TrySetPrivatePermissions(logPath, diagnosticSink, boundary); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { diff --git a/src/CodeIndex/Cli/MetricsSink.cs b/src/CodeIndex/Cli/MetricsSink.cs index 96eaef7bd9..37541d0861 100644 --- a/src/CodeIndex/Cli/MetricsSink.cs +++ b/src/CodeIndex/Cli/MetricsSink.cs @@ -63,19 +63,45 @@ internal static class MetricsSink try { var fullPath = Path.GetFullPath(path); - DataDirectorySecurity.CreateSensitiveParentDirectoryForFile(fullPath); + var boundary = string.IsNullOrWhiteSpace(explicitPath) + ? RepositoryOutputPathBoundary.CreateGuardForConfigSource( + EnvVarName, + "metrics_path", + fullPath, + destinationIsDirectory: false) + : null; + if (boundary is null) + DataDirectorySecurity.CreateSensitiveParentDirectoryForFile(fullPath); + else + boundary.CreateSensitiveDestinationDirectory(); long bytesWritten; - using (var probe = PrivateLogFile.OpenAppend(fullPath, FileShare.ReadWrite)) + using (var probe = PrivateLogFile.OpenAppend(fullPath, FileShare.ReadWrite, boundary)) { bytesWritten = probe.Length; } - PrivateLogFile.TrySetPrivatePermissions(fullPath); - - var session = new Session(fullPath, maxBytes, bytesWritten, warningSink, queueCapacity, retryDelay, disposeWriterTimeout); + PrivateLogFile.TrySetPrivatePermissions(fullPath, boundary: boundary); + + var session = new Session( + fullPath, + maxBytes, + bytesWritten, + warningSink, + queueCapacity, + retryDelay, + disposeWriterTimeout, + boundary); CurrentSession.Value = session; return session; } + catch (RepositoryOutputPathBoundaryException) + { + warningSink?.Invoke( + "metrics output disabled; repository-configured `metrics_path` is unsafe " + + $"({RepositoryOutputPathBoundary.UnsafeReason})."); + CurrentSession.Value = null; + return null; + } catch (Exception ex) { // Best-effort: a metrics sink that cannot open its file must not block the command. @@ -133,6 +159,7 @@ internal sealed class Session : IDisposable private readonly Action? _warningSink; private readonly Func _retryDelay; private readonly TimeSpan _disposeWriterTimeout; + private readonly RepositoryOutputPathGuard? _boundary; private long _bytesWritten; private long _pendingEventCount; private long _queueDepth; @@ -162,7 +189,8 @@ public Session( Action? warningSink, int? queueCapacity = null, Func? retryDelay = null, - TimeSpan? disposeWriterTimeout = null) + TimeSpan? disposeWriterTimeout = null, + RepositoryOutputPathGuard? boundary = null) { Path = path; _maxBytes = maxBytes; @@ -170,6 +198,7 @@ public Session( _warningSink = warningSink; _retryDelay = retryDelay ?? ((delay, cancellationToken) => Task.Delay(delay, cancellationToken)); _disposeWriterTimeout = disposeWriterTimeout ?? DisposeWriterTimeout; + _boundary = boundary; if (_disposeWriterTimeout < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(disposeWriterTimeout)); _queueCapacity = ResolveQueueCapacity(queueCapacity); @@ -413,7 +442,7 @@ private void WriteBatch(IReadOnlyList sourceBatch) try { - using (var stream = PrivateLogFile.OpenAppend(Path, FileShare.ReadWrite)) + using (var stream = PrivateLogFile.OpenAppend(Path, FileShare.ReadWrite, _boundary)) { for (var i = start; i < index; i++) stream.Write(batch[i].Encoded, 0, batch[i].Encoded.Length); @@ -450,7 +479,8 @@ private bool TryRotate(IReadOnlyList batch, int dropStart) if (PrivateLogFile.TryRotateSlots( Path, RotationKeep, - onFailure: ex => capturedFailure = ex)) + onFailure: ex => capturedFailure = ex, + boundary: _boundary)) { Volatile.Write(ref _bytesWritten, 0); return true; diff --git a/src/CodeIndex/Cli/PrivateLogFile.cs b/src/CodeIndex/Cli/PrivateLogFile.cs index 2b3770a0d3..b9c7d5a195 100644 --- a/src/CodeIndex/Cli/PrivateLogFile.cs +++ b/src/CodeIndex/Cli/PrivateLogFile.cs @@ -9,37 +9,69 @@ internal static class PrivateLogFile internal const int MaxExistingFilesToHarden = 128; private const int MaxDiagnosticTargetChars = 160; - internal static FileStream OpenAppend(string path, FileShare share = FileShare.ReadWrite) + internal static Stream OpenAppend( + string path, + FileShare share = FileShare.ReadWrite, + RepositoryOutputPathGuard? boundary = null) { + boundary?.PrepareMutation("open_append", path); RejectUnsafeTarget(path); - if (OperatingSystem.IsWindows()) - return new FileStream(path, FileMode.Append, FileAccess.Write, share); + Stream stream; + if (boundary is not null && !OperatingSystem.IsWindows()) + { + stream = boundary.OpenAppendUnix(path); + } + else if (OperatingSystem.IsWindows()) + { + stream = new FileStream(path, FileMode.Append, FileAccess.Write, share); + } + else + { + stream = new FileStream(path, new FileStreamOptions + { + Mode = FileMode.Append, + Access = FileAccess.Write, + Share = share, + UnixCreateMode = PrivateFileMode, + }); + } - return new FileStream(path, new FileStreamOptions + try { - Mode = FileMode.Append, - Access = FileAccess.Write, - Share = share, - UnixCreateMode = PrivateFileMode, - }); + boundary?.CompleteMutation(path); + return stream; + } + catch + { + stream.Dispose(); + throw; + } } - internal static StreamWriter OpenAppendText(string path) - => new(OpenAppend(path), new UTF8Encoding(false)) + internal static StreamWriter OpenAppendText(string path, RepositoryOutputPathGuard? boundary = null) + => new(OpenAppend(path, boundary: boundary), new UTF8Encoding(false)) { AutoFlush = true, }; - internal static void TrySetPrivatePermissions(string path, Action? diagnosticSink = null) + internal static void TrySetPrivatePermissions( + string path, + Action? diagnosticSink = null, + RepositoryOutputPathGuard? boundary = null) { if (OperatingSystem.IsWindows()) return; try { + boundary?.PrepareMutation("set_private_permissions", path); RejectUnsafeTarget(path); - File.SetUnixFileMode(path, PrivateFileMode); + if (boundary is null) + File.SetUnixFileMode(path, PrivateFileMode); + else + boundary.SetPrivateFileModeUnix(path); + boundary?.CompleteMutation(path); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) { @@ -47,7 +79,11 @@ internal static void TrySetPrivatePermissions(string path, Action? diagnosticSink = null) + internal static void HardenExisting( + string directory, + string pattern, + Action? diagnosticSink = null, + RepositoryOutputPathGuard? boundary = null) { if (OperatingSystem.IsWindows()) return; @@ -65,7 +101,7 @@ internal static void HardenExisting(string directory, string pattern, Action? diagnosticSink = null, - Action? deleteOverride = null) + Action? deleteOverride = null, + RepositoryOutputPathGuard? boundary = null) { try { @@ -131,10 +168,19 @@ internal static void PruneOldFiles( { if (ShouldPruneFile(file, retainedPaths, retainedFiles, retainedFileCount)) { - AtomicFileWriter.TryDeleteFile( - file.FullName, - ex => ReportDiagnostic(diagnosticSink, "prune_old_file_delete", file.FullName, ex), - deleteOverride); + boundary?.PrepareMutation("prune_old_file", file.FullName); + if (boundary is not null && !OperatingSystem.IsWindows()) + { + boundary.DeleteFileUnix(file.FullName); + } + else + { + AtomicFileWriter.TryDeleteFile( + file.FullName, + ex => ReportDiagnostic(diagnosticSink, "prune_old_file_delete", file.FullName, ex), + deleteOverride); + } + boundary?.CompleteMutation(file.FullName); } } } @@ -201,11 +247,18 @@ internal static bool TryRotateSlots( int retainedFileCount, Action? afterMove = null, Action? onFailure = null, - Action? onCleanupFailure = null) + Action? onCleanupFailure = null, + RepositoryOutputPathGuard? boundary = null) { try { - AtomicFileWriter.TryDeleteFile(SlotPath(path, retainedFileCount - 1), onCleanupFailure); + var lastSlot = SlotPath(path, retainedFileCount - 1); + boundary?.PrepareMutation("rotate_delete", lastSlot); + if (boundary is not null && !OperatingSystem.IsWindows()) + boundary.DeleteFileUnix(lastSlot); + else + AtomicFileWriter.TryDeleteFile(lastSlot, onCleanupFailure); + boundary?.CompleteMutation(lastSlot); for (var slot = retainedFileCount - 2; slot >= 1; slot--) { @@ -213,14 +266,28 @@ internal static bool TryRotateSlots( var next = SlotPath(path, slot + 1); if (!File.Exists(LongPath.EnsureWindowsPrefix(current))) continue; - AtomicFileWriter.MoveReplacing(current, next); + boundary?.PrepareMutation("rotate_source", current); + boundary?.PrepareMutation("rotate_destination", next); + if (boundary is not null && !OperatingSystem.IsWindows()) + boundary.MoveReplacingUnix(current, next); + else + AtomicFileWriter.MoveReplacing(current, next); + boundary?.CompleteMutation(current); + boundary?.CompleteMutation(next); afterMove?.Invoke(next); } if (File.Exists(LongPath.EnsureWindowsPrefix(path))) { var first = SlotPath(path, 1); - AtomicFileWriter.MoveReplacing(path, first); + boundary?.PrepareMutation("rotate_source", path); + boundary?.PrepareMutation("rotate_destination", first); + if (boundary is not null && !OperatingSystem.IsWindows()) + boundary.MoveReplacingUnix(path, first); + else + AtomicFileWriter.MoveReplacing(path, first); + boundary?.CompleteMutation(path); + boundary?.CompleteMutation(first); afterMove?.Invoke(first); } diff --git a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs new file mode 100644 index 0000000000..df71cac319 --- /dev/null +++ b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs @@ -0,0 +1,864 @@ +using System.Runtime.InteropServices; +using CodeIndex.Indexer; +using Microsoft.Win32.SafeHandles; + +namespace CodeIndex.Cli; + +/// +/// Keeps writable destinations supplied by repository configuration below the config +/// workspace without following repository-controlled filesystem aliases. +/// repository config が指定する writable destination を、repository-controlled な +/// filesystem alias を辿らず config workspace 配下に保持する。 +/// +internal static class RepositoryOutputPathBoundary +{ + internal const string UnsafeReason = "unsafe_output_path"; + private const string UnsafeMessage = + "symbolic links, junctions, cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root"; + private static readonly AsyncLocal?> BeforeMutation = new(); + + internal static Action? BeforeMutationForTesting + { + get => BeforeMutation.Value; + set => BeforeMutation.Value = value; + } + + internal static bool TryResolveConfiguredPath( + string rawPath, + string workspaceRoot, + bool destinationIsDirectory, + out string resolvedPath, + out string failureReason) + { + resolvedPath = string.Empty; + failureReason = string.Empty; + try + { + var normalizedRoot = PathCasing.NormalizeBoundaryPath(workspaceRoot); + var fullPath = Path.IsPathRooted(rawPath) + ? Path.GetFullPath(rawPath) + : Path.GetFullPath(Path.Combine(normalizedRoot, rawPath)); + var normalizedPath = PathCasing.NormalizeBoundaryPath(fullPath); + if (!PathCasing.IsPathEqualOrParent(normalizedRoot, normalizedPath)) + { + failureReason = "outside_workspace"; + return false; + } + + ValidatePathComponents(normalizedRoot, normalizedPath, destinationIsDirectory); + resolvedPath = fullPath; + return true; + } + catch (RepositoryOutputPathBoundaryException) + { + failureReason = UnsafeReason; + return false; + } + catch (Exception ex) when (IsPathException(ex)) + { + failureReason = "invalid_path"; + return false; + } + } + + internal static RepositoryOutputPathGuard? CreateGuardForConfigSource( + string environmentVariable, + string fieldName, + string destinationPath, + bool destinationIsDirectory) + { + var sourcePath = CdidxConfigSourceResolver.GetSource(environmentVariable); + if (string.IsNullOrWhiteSpace(sourcePath)) + return null; + + var workspaceRoot = CdidxConfigFile.ResolveConfigWorkspaceRoot(sourcePath); + if (!TryResolveConfiguredPath( + destinationPath, + workspaceRoot, + destinationIsDirectory, + out var resolvedPath, + out var failureReason)) + { + throw CreateException(fieldName, failureReason); + } + + return new RepositoryOutputPathGuard( + fieldName, + PathCasing.NormalizeBoundaryPath(workspaceRoot), + PathCasing.NormalizeBoundaryPath(resolvedPath), + destinationIsDirectory); + } + + internal static RepositoryOutputPathBoundaryException CreateException(string fieldName, string reason) + => new($"repository-configured `{fieldName}` rejected ({reason}): {UnsafeMessage}."); + + internal static string ResolveCanonicalWorkspaceRoot(string workspaceRoot) + { + if (OperatingSystem.IsWindows()) + return PathCasing.NormalizeBoundaryPath(workspaceRoot); + + IntPtr pointer = IntPtr.Zero; + try + { + pointer = UnixRealPath(workspaceRoot, IntPtr.Zero); + var resolved = pointer == IntPtr.Zero ? null : Marshal.PtrToStringUTF8(pointer); + return string.IsNullOrWhiteSpace(resolved) + ? PathCasing.NormalizeBoundaryPath(workspaceRoot) + : PathCasing.NormalizeBoundaryPath(resolved); + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + return PathCasing.NormalizeBoundaryPath(workspaceRoot); + } + finally + { + if (pointer != IntPtr.Zero) + UnixFree(pointer); + } + } + + internal static void ValidatePathComponents( + string workspaceRoot, + string path, + bool destinationIsDirectory) + { + var normalizedRoot = PathCasing.NormalizeBoundaryPath(workspaceRoot); + var normalizedPath = PathCasing.NormalizeBoundaryPath(path); + if (!PathCasing.IsPathEqualOrParent(normalizedRoot, normalizedPath)) + throw CreateException("output path", "outside_workspace"); + + var rootAttributesStatus = FileSystemBoundary.TryGetAttributes(normalizedRoot, out var rootAttributes); + if (rootAttributesStatus != FileSystemBoundaryProbeStatus.Found + || (rootAttributes & FileAttributes.Directory) == 0 + || FileSystemBoundary.IsDevice(rootAttributes)) + { + throw CreateException("output path", UnsafeReason); + } + + var rootDevice = TryGetUnixDevice(normalizedRoot, out var device) ? device : (long?)null; + var relative = Path.GetRelativePath(normalizedRoot, normalizedPath); + if (relative == ".") + return; + + var components = relative.Split( + [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], + StringSplitOptions.RemoveEmptyEntries); + var current = normalizedRoot; + for (var index = 0; index < components.Length; index++) + { + var component = components[index]; + if (component is "." or "..") + throw CreateException("output path", UnsafeReason); + + current = Path.Combine(current, component); + if (IsLinkOrReparseEntry(current)) + throw CreateException("output path", UnsafeReason); + + var status = FileSystemBoundary.TryGetAttributes(current, out var attributes); + if (status == FileSystemBoundaryProbeStatus.Missing) + continue; + if (status != FileSystemBoundaryProbeStatus.Found + || FileSystemBoundary.IsSymlinkOrReparsePoint(attributes) + || FileSystemBoundary.IsDevice(attributes)) + { + throw CreateException("output path", UnsafeReason); + } + + var isFinal = index == components.Length - 1; + var isDirectory = (attributes & FileAttributes.Directory) != 0; + if ((!isFinal && !isDirectory) || (isFinal && destinationIsDirectory && !isDirectory)) + throw CreateException("output path", UnsafeReason); + if (isFinal && !destinationIsDirectory && isDirectory) + throw CreateException("output path", UnsafeReason); + + if (rootDevice.HasValue + && TryGetUnixDevice(current, out var currentDevice) + && currentDevice != rootDevice.Value) + { + throw CreateException("output path", UnsafeReason); + } + } + } + + private static bool IsLinkOrReparseEntry(string path) + { + try + { + var directory = new DirectoryInfo(path); + directory.Refresh(); + if (!string.IsNullOrEmpty(directory.LinkTarget)) + return true; + } + catch (Exception ex) when (IsPathException(ex)) + { + if (ex is UnauthorizedAccessException) + throw; + } + + try + { + var file = new FileInfo(path); + file.Refresh(); + return !string.IsNullOrEmpty(file.LinkTarget); + } + catch (Exception ex) when (IsPathException(ex)) + { + if (ex is UnauthorizedAccessException) + throw; + return false; + } + } + + private static bool TryGetUnixDevice(string path, out long device) + { + device = 0; + if (OperatingSystem.IsWindows()) + return false; + + try + { + if (UnixStat(LongPath.EnsureWindowsPrefix(path), out var status) != 0) + return false; + device = status.Device; + return true; + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + return false; + } + } + + private static bool IsPathException(Exception ex) + => ex is ArgumentException + or IOException + or NotSupportedException + or PathTooLongException + or UnauthorizedAccessException; + + [DllImport("libSystem.Native", EntryPoint = "SystemNative_Stat", CharSet = CharSet.Ansi)] + private static extern int UnixStat(string path, out UnixFileStatus status); + + [DllImport("libc", EntryPoint = "realpath", SetLastError = true)] + private static extern IntPtr UnixRealPath(string path, IntPtr resolvedPath); + + [DllImport("libc", EntryPoint = "free")] + private static extern void UnixFree(IntPtr pointer); + + [StructLayout(LayoutKind.Sequential)] + private struct UnixFileStatus + { + internal uint Flags; + internal int Mode; + internal uint Uid; + internal uint Gid; + internal long Size; + internal long ATime; + internal long ATimeNsec; + internal long MTime; + internal long MTimeNsec; + internal long CTime; + internal long CTimeNsec; + internal long BirthTime; + internal long BirthTimeNsec; + internal long Device; + internal long RDevice; + internal long Inode; + internal uint UserFlags; + } +} + +internal sealed class RepositoryOutputPathGuard +{ + internal RepositoryOutputPathGuard( + string fieldName, + string workspaceRoot, + string destinationPath, + bool destinationIsDirectory) + { + FieldName = fieldName; + WorkspaceRoot = workspaceRoot; + CanonicalWorkspaceRoot = RepositoryOutputPathBoundary.ResolveCanonicalWorkspaceRoot(workspaceRoot); + DestinationPath = destinationPath; + DestinationIsDirectory = destinationIsDirectory; + } + + internal string FieldName { get; } + internal string WorkspaceRoot { get; } + internal string CanonicalWorkspaceRoot { get; } + internal string DestinationPath { get; } + internal bool DestinationIsDirectory { get; } + + internal void CreateSensitiveDestinationDirectory() + { + var directory = DestinationIsDirectory + ? DestinationPath + : Path.GetDirectoryName(DestinationPath); + if (string.IsNullOrWhiteSpace(directory)) + return; + + CreateSensitiveDirectories(directory); + } + + internal void PrepareMutation(string operation, string path, bool expectsDirectory = false) + { + ValidateRelatedPath(path, expectsDirectory); + RepositoryOutputPathBoundary.BeforeMutationForTesting?.Invoke(operation, path); + ValidateRelatedPath(path, expectsDirectory); + } + + internal void CompleteMutation(string path, bool expectsDirectory = false) + => ValidateRelatedPath(path, expectsDirectory); + + internal Stream OpenAppendUnix(string path) + { + if (OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + var handle = OpenUnixFile(path, create: true, append: true); + try + { + return new UnixAppendStream(handle); + } + catch + { + handle.Dispose(); + throw; + } + } + + internal void SetPrivateFileModeUnix(string path) + { + if (OperatingSystem.IsWindows()) + return; + + using var handle = OpenUnixFile(path, create: false, append: false); + if (UnixFChmod(handle.DangerousGetHandle().ToInt32(), (uint)PrivateLogFile.PrivateFileMode) != 0) + throw CreateNativeIOException(); + } + + internal bool DeleteFileUnix(string path) + { + if (OperatingSystem.IsWindows()) + return false; + + using var parent = OpenUnixParentDirectory(path); + var result = UnixUnlinkAt(parent.DangerousGetHandle().ToInt32(), Path.GetFileName(path), flags: 0); + if (result == 0) + return true; + if (Marshal.GetLastWin32Error() == 2) + return false; + throw CreateNativeIOException(); + } + + internal void MoveReplacingUnix(string source, string destination) + { + if (OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + using var sourceParent = OpenUnixParentDirectory(source); + using var destinationParent = OpenUnixParentDirectory(destination); + if (UnixRenameAt( + sourceParent.DangerousGetHandle().ToInt32(), + Path.GetFileName(source), + destinationParent.DangerousGetHandle().ToInt32(), + Path.GetFileName(destination)) != 0) + { + throw CreateNativeIOException(); + } + } + + internal void ValidateRelatedPath(string path, bool expectsDirectory = false) + { + var fullPath = Path.GetFullPath(path); + if (!Allows(fullPath, expectsDirectory)) + throw RepositoryOutputPathBoundary.CreateException(FieldName, "outside_workspace"); + + expectsDirectory |= string.Equals( + PathCasing.NormalizeBoundaryPath(fullPath), + PathCasing.NormalizeBoundaryPath(DestinationPath), + PathCasing.ComparisonFor(DestinationPath)) + && DestinationIsDirectory; + RepositoryOutputPathBoundary.ValidatePathComponents(WorkspaceRoot, fullPath, expectsDirectory); + } + + private void CreateSensitiveDirectories(string directory) + { + if (!OperatingSystem.IsWindows()) + { + CreateSensitiveDirectoriesUnix(directory); + return; + } + + var normalizedDirectory = PathCasing.NormalizeBoundaryPath(directory); + var relative = Path.GetRelativePath(WorkspaceRoot, normalizedDirectory); + if (relative == ".") + return; + + var components = relative.Split( + [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], + StringSplitOptions.RemoveEmptyEntries); + var current = WorkspaceRoot; + foreach (var component in components) + { + current = Path.Combine(current, component); + RepositoryOutputPathBoundary.ValidatePathComponents(WorkspaceRoot, current, destinationIsDirectory: true); + var created = false; + if (!Directory.Exists(LongPath.EnsureWindowsPrefix(current))) + { + PrepareMutation("create_directory", current, expectsDirectory: true); + if (OperatingSystem.IsWindows()) + Directory.CreateDirectory(LongPath.EnsureWindowsPrefix(current)); + else + Directory.CreateDirectory(LongPath.EnsureWindowsPrefix(current), DataDirectorySecurity.PrivateDirectoryMode); + CompleteMutation(current, expectsDirectory: true); + created = true; + } + + if (!OperatingSystem.IsWindows() + && (created || string.Equals( + PathCasing.NormalizeBoundaryPath(current), + normalizedDirectory, + PathCasing.ComparisonFor(normalizedDirectory)))) + { + PrepareMutation("set_directory_permissions", current, expectsDirectory: true); + File.SetUnixFileMode(LongPath.EnsureWindowsPrefix(current), DataDirectorySecurity.PrivateDirectoryMode); + CompleteMutation(current, expectsDirectory: true); + } + } + } + + private void CreateSensitiveDirectoriesUnix(string directory) + { + var normalizedDirectory = PathCasing.NormalizeBoundaryPath(directory); + var components = GetRelativeComponents(normalizedDirectory); + using var root = OpenUnixRootDirectory(out var rootDevice); + SafeFileHandle current = root; + try + { + for (var index = 0; index < components.Length; index++) + { + var component = components[index]; + var currentPath = Path.Combine(WorkspaceRoot, Path.Combine(components[..(index + 1)])); + SafeFileHandle? next = null; + try + { + next = TryOpenUnixDirectoryAt(current, component); + var created = false; + if (next is null) + { + PrepareMutation("create_directory", currentPath, expectsDirectory: true); + if (UnixMkdirAt( + current.DangerousGetHandle().ToInt32(), + component, + (uint)DataDirectorySecurity.PrivateDirectoryMode) != 0 + && Marshal.GetLastWin32Error() != 17) + { + throw CreateNativeIOException(); + } + next = TryOpenUnixDirectoryAt(current, component) ?? throw CreateNativeIOException(); + CompleteMutation(currentPath, expectsDirectory: true); + created = true; + } + + EnsureUnixObject(next, rootDevice, expectedType: UnixDirectoryType); + if (created || index == components.Length - 1) + { + PrepareMutation("set_directory_permissions", currentPath, expectsDirectory: true); + if (UnixFChmod( + next.DangerousGetHandle().ToInt32(), + (uint)DataDirectorySecurity.PrivateDirectoryMode) != 0) + { + throw CreateNativeIOException(); + } + CompleteMutation(currentPath, expectsDirectory: true); + } + } + catch + { + next?.Dispose(); + throw; + } + + if (!ReferenceEquals(current, root)) + current.Dispose(); + current = next; + } + } + finally + { + if (!ReferenceEquals(current, root)) + current.Dispose(); + } + } + + private SafeFileHandle OpenUnixFile(string path, bool create, bool append) + { + using var parent = OpenUnixParentDirectory(path, out var rootDevice); + var flags = UnixWriteOnly | UnixCloseOnExec | UnixNoFollow; + if (append) + flags |= UnixAppend; + if (create) + flags |= UnixCreate; + var descriptor = UnixOpenAt( + parent.DangerousGetHandle().ToInt32(), + Path.GetFileName(path), + flags, + (uint)PrivateLogFile.PrivateFileMode); + if (descriptor < 0) + throw CreateNativeIOException(); + + var handle = new SafeFileHandle(new IntPtr(descriptor), ownsHandle: true); + try + { + EnsureUnixObject(handle, rootDevice, expectedType: UnixRegularFileType); + if (create + && UnixFChmod(handle.DangerousGetHandle().ToInt32(), (uint)PrivateLogFile.PrivateFileMode) != 0) + { + throw CreateNativeIOException(); + } + return handle; + } + catch + { + handle.Dispose(); + throw; + } + } + + private SafeFileHandle OpenUnixParentDirectory(string path) + => OpenUnixParentDirectory(path, out _); + + private SafeFileHandle OpenUnixParentDirectory(string path, out long rootDevice) + { + var parentPath = Path.GetDirectoryName(Path.GetFullPath(path)) + ?? throw RepositoryOutputPathBoundary.CreateException(FieldName, "invalid_path"); + return OpenUnixDirectory(parentPath, out rootDevice); + } + + private SafeFileHandle OpenUnixDirectory(string directory, out long rootDevice) + { + var components = GetRelativeComponents(PathCasing.NormalizeBoundaryPath(directory)); + var current = OpenUnixRootDirectory(out rootDevice); + try + { + foreach (var component in components) + { + var next = TryOpenUnixDirectoryAt(current, component) ?? throw CreateNativeIOException(); + try + { + EnsureUnixObject(next, rootDevice, expectedType: UnixDirectoryType); + } + catch + { + next.Dispose(); + throw; + } + current.Dispose(); + current = next; + } + return current; + } + catch + { + current.Dispose(); + throw; + } + } + + private SafeFileHandle OpenUnixRootDirectory(out long rootDevice) + { + var descriptor = UnixOpen( + CanonicalWorkspaceRoot, + UnixReadOnly | UnixCloseOnExec | UnixNoFollow | UnixDirectory); + if (descriptor < 0) + throw CreateNativeIOException(); + + var handle = new SafeFileHandle(new IntPtr(descriptor), ownsHandle: true); + try + { + if (UnixFStat(handle.DangerousGetHandle(), out var status) != 0 + || (status.Mode & UnixFileTypeMask) != UnixDirectoryType) + { + throw CreateNativeIOException(); + } + rootDevice = status.Device; + return handle; + } + catch + { + handle.Dispose(); + throw; + } + } + + private static SafeFileHandle? TryOpenUnixDirectoryAt(SafeFileHandle parent, string component) + { + var descriptor = UnixOpenAt( + parent.DangerousGetHandle().ToInt32(), + component, + UnixReadOnly | UnixCloseOnExec | UnixNoFollow | UnixDirectory, + mode: 0); + if (descriptor >= 0) + return new SafeFileHandle(new IntPtr(descriptor), ownsHandle: true); + if (Marshal.GetLastWin32Error() == 2) + return null; + throw CreateNativeIOException(); + } + + private static void EnsureUnixObject(SafeFileHandle handle, long rootDevice, int expectedType) + { + if (UnixFStat(handle.DangerousGetHandle(), out var status) != 0 + || (status.Mode & UnixFileTypeMask) != expectedType + || status.Device != rootDevice) + { + throw CreateNativeIOException(); + } + } + + private string[] GetRelativeComponents(string path) + { + if (!PathCasing.IsPathEqualOrParent(WorkspaceRoot, path)) + throw RepositoryOutputPathBoundary.CreateException(FieldName, "outside_workspace"); + var relative = Path.GetRelativePath(WorkspaceRoot, path); + if (relative == ".") + return []; + var components = relative.Split( + [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], + StringSplitOptions.RemoveEmptyEntries); + if (components.Any(static component => component is "." or "..")) + throw RepositoryOutputPathBoundary.CreateException("output path", "outside_workspace"); + return components; + } + + private static RepositoryOutputPathBoundaryException CreateNativeIOException() + => RepositoryOutputPathBoundary.CreateException("output path", RepositoryOutputPathBoundary.UnsafeReason); + + private bool Allows(string path, bool expectsDirectory) + { + if (DestinationIsDirectory) + { + return PathCasing.IsPathEqualOrParent(DestinationPath, path) + || (expectsDirectory + && PathCasing.IsPathEqualOrParent(path, DestinationPath) + && PathCasing.IsPathEqualOrParent(WorkspaceRoot, path)); + } + + if (expectsDirectory && PathCasing.IsPathEqualOrParent(path, DestinationPath)) + return PathCasing.IsPathEqualOrParent(WorkspaceRoot, path); + + var comparison = PathCasing.ComparisonFor(DestinationPath); + if (string.Equals(DestinationPath, path, comparison)) + return true; + + var destinationDirectory = Path.GetDirectoryName(DestinationPath); + var pathDirectory = Path.GetDirectoryName(path); + if (string.IsNullOrEmpty(destinationDirectory) + || string.IsNullOrEmpty(pathDirectory) + || !string.Equals( + PathCasing.NormalizeBoundaryPath(destinationDirectory), + PathCasing.NormalizeBoundaryPath(pathDirectory), + comparison)) + { + return false; + } + + var destinationName = Path.GetFileName(DestinationPath); + var candidateName = Path.GetFileName(path); + if (!candidateName.StartsWith(destinationName + ".", comparison)) + return false; + + return int.TryParse( + candidateName.AsSpan(destinationName.Length + 1), + System.Globalization.NumberStyles.None, + System.Globalization.CultureInfo.InvariantCulture, + out _); + } + + private const int UnixReadOnly = 0; + private const int UnixWriteOnly = 1; + private const int UnixFileTypeMask = 0xF000; + private const int UnixDirectoryType = 0x4000; + private const int UnixRegularFileType = 0x8000; + + private static int UnixCloseOnExec => OperatingSystem.IsMacOS() ? 0x01000000 : 0x00080000; + private static int UnixNoFollow => OperatingSystem.IsMacOS() ? 0x00000100 : 0x00020000; + private static int UnixDirectory => OperatingSystem.IsMacOS() ? 0x00100000 : 0x00010000; + private static int UnixCreate => OperatingSystem.IsMacOS() ? 0x00000200 : 0x00000040; + private static int UnixAppend => OperatingSystem.IsMacOS() ? 0x00000008 : 0x00000400; + + [DllImport("libc", EntryPoint = "open", SetLastError = true)] + private static extern int UnixOpen(string path, int flags); + + [DllImport("libc", EntryPoint = "openat", SetLastError = true)] + private static extern int UnixOpenAt(int directory, string path, int flags, uint mode); + + [DllImport("libc", EntryPoint = "mkdirat", SetLastError = true)] + private static extern int UnixMkdirAt(int directory, string path, uint mode); + + [DllImport("libc", EntryPoint = "unlinkat", SetLastError = true)] + private static extern int UnixUnlinkAt(int directory, string path, int flags); + + [DllImport("libc", EntryPoint = "renameat", SetLastError = true)] + private static extern int UnixRenameAt( + int sourceDirectory, + string sourcePath, + int destinationDirectory, + string destinationPath); + + [DllImport("libc", EntryPoint = "fchmod", SetLastError = true)] + private static extern int UnixFChmod(int descriptor, uint mode); + + [DllImport("libSystem.Native", EntryPoint = "SystemNative_FStat", SetLastError = true)] + private static extern int UnixFStat(IntPtr descriptor, out UnixFileStatus status); + + [StructLayout(LayoutKind.Sequential)] + private struct UnixFileStatus + { + internal uint Flags; + internal int Mode; + internal uint Uid; + internal uint Gid; + internal long Size; + internal long ATime; + internal long ATimeNsec; + internal long MTime; + internal long MTimeNsec; + internal long CTime; + internal long CTimeNsec; + internal long BirthTime; + internal long BirthTimeNsec; + internal long Device; + internal long RDevice; + internal long Inode; + internal uint UserFlags; + } +} + +internal sealed class RepositoryOutputPathBoundaryException(string message) : IOException(message); + +internal sealed class UnixAppendStream : Stream +{ + private readonly SafeFileHandle _handle; + private bool _disposed; + + internal UnixAppendStream(SafeFileHandle handle) + { + _handle = handle; + } + + public override bool CanRead => false; + public override bool CanSeek => false; + public override bool CanWrite => !_disposed; + public override long Length + { + get + { + ThrowIfDisposed(); + return RandomAccess.GetLength(_handle); + } + } + + public override long Position + { + get => Length; + set => throw new NotSupportedException(); + } + + public override void Flush() + { + ThrowIfDisposed(); + } + + public override Task FlushAsync(CancellationToken cancellationToken) + { + ThrowIfDisposed(); + return cancellationToken.IsCancellationRequested + ? Task.FromCanceled(cancellationToken) + : Task.CompletedTask; + } + + public override void Write(byte[] buffer, int offset, int count) + { + ArgumentNullException.ThrowIfNull(buffer); + ArgumentOutOfRangeException.ThrowIfNegative(offset); + ArgumentOutOfRangeException.ThrowIfNegative(count); + if (buffer.Length - offset < count) + throw new ArgumentException("The offset and count exceed the buffer length."); + ThrowIfDisposed(); + if (count == 0) + return; + + if (offset == 0 && count == buffer.Length) + { + WriteAll(buffer, count); + return; + } + + var slice = new byte[count]; + Buffer.BlockCopy(buffer, offset, slice, 0, count); + WriteAll(slice, count); + } + + public override void Write(ReadOnlySpan buffer) + { + if (buffer.IsEmpty) + return; + Write(buffer.ToArray(), 0, buffer.Length); + } + + public override int Read(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + + public override long Seek(long offset, SeekOrigin origin) + => throw new NotSupportedException(); + + public override void SetLength(long value) + => throw new NotSupportedException(); + + protected override void Dispose(bool disposing) + { + if (disposing && !_disposed) + { + _disposed = true; + _handle.Dispose(); + } + base.Dispose(disposing); + } + + private void WriteAll(byte[] buffer, int count) + { + var written = 0; + while (written < count) + { + byte[] nextBuffer; + if (written == 0) + { + nextBuffer = buffer; + } + else + { + nextBuffer = new byte[count - written]; + Buffer.BlockCopy(buffer, written, nextBuffer, 0, nextBuffer.Length); + } + + var result = UnixWrite( + _handle.DangerousGetHandle().ToInt32(), + nextBuffer, + (nuint)(count - written)); + if (result > 0) + { + written += checked((int)result); + continue; + } + if (result < 0 && Marshal.GetLastWin32Error() == 4) + continue; + throw new IOException("repository-configured append failed (unsafe_output_path)."); + } + } + + private void ThrowIfDisposed() + => ObjectDisposedException.ThrowIf(_disposed, this); + + [DllImport("libc", EntryPoint = "write", SetLastError = true)] + private static extern nint UnixWrite(int descriptor, byte[] buffer, nuint count); +} diff --git a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs index eab89d6f9b..efec29c3c5 100644 --- a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs +++ b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs @@ -282,6 +282,204 @@ public void LoadAndApply_OutputPathOutsideWorkspace_ReturnsError(string key) finally { TestProjectHelper.DeleteDirectory(dir); } } + [Theory] + [InlineData("metrics_path", "bridge/new/metrics.jsonl")] + [InlineData("global_tool_log_dir", "bridge/new/logs")] + public void LoadAndApply_OutputPathUnderSymlinkAncestorIsRejectedWithoutExternalMutation_Issue5181( + string key, + string configuredPath) + { + var workspace = CreateTempDir(); + var outside = CreateTempDir(); + var bridge = Path.Combine(workspace, "bridge"); + try + { + if (!TryCreateDirectoryLink(bridge, outside)) + return; + + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + JsonSerializer.Serialize(new Dictionary { [key] = configuredPath })); + + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + + Assert.True(result.Failed); + Assert.Contains(key, result.Error, StringComparison.Ordinal); + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, result.Error, StringComparison.Ordinal); + Assert.DoesNotContain(outside, result.Error, StringComparison.Ordinal); + Assert.False(Directory.Exists(Path.Combine(outside, "new"))); + Assert.Empty(result.Settings); + } + finally + { + DeleteLinkEntry(bridge); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + + [Theory] + [InlineData("metrics_path", false, false)] + [InlineData("metrics_path", false, true)] + [InlineData("global_tool_log_dir", true, false)] + [InlineData("global_tool_log_dir", true, true)] + public void LoadAndApply_FinalLinkIsRejectedWithoutChangingTarget_Issue5181( + string key, + bool directoryLink, + bool targetExists) + { + var workspace = CreateTempDir(); + var outside = CreateTempDir(); + var destinationName = directoryLink ? "logs" : "metrics.jsonl"; + var destination = Path.Combine(workspace, destinationName); + var target = Path.Combine(outside, "target"); + UnixFileMode? originalOutsideMode = null; + UnixFileMode? originalTargetMode = null; + try + { + if (targetExists) + { + if (directoryLink) + Directory.CreateDirectory(target); + else + File.WriteAllText(target, "outside-target-content"); + } + if (!OperatingSystem.IsWindows()) + { + originalOutsideMode = File.GetUnixFileMode(outside); + if (targetExists) + originalTargetMode = File.GetUnixFileMode(target); + } + if (!TryCreateLink(destination, target, directoryLink)) + return; + + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + JsonSerializer.Serialize(new Dictionary { [key] = destinationName })); + + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + + Assert.True(result.Failed); + Assert.Contains(key, result.Error, StringComparison.Ordinal); + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, result.Error, StringComparison.Ordinal); + Assert.DoesNotContain(outside, result.Error, StringComparison.Ordinal); + if (targetExists && !directoryLink) + Assert.Equal("outside-target-content", File.ReadAllText(target)); + if (!targetExists) + { + Assert.False(File.Exists(target)); + Assert.False(Directory.Exists(target)); + } + if (!OperatingSystem.IsWindows()) + { + Assert.Equal(originalOutsideMode, File.GetUnixFileMode(outside)); + if (targetExists) + Assert.Equal(originalTargetMode, File.GetUnixFileMode(target)); + } + } + finally + { + DeleteLinkEntry(destination); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + + [Fact] + public void LoadAndApply_WorkspaceRootAliasStillAllowsOrdinaryContainedOutput_Issue5181() + { + var container = CreateTempDir(); + var physicalWorkspace = Path.Combine(container, "physical"); + var workspaceAlias = Path.Combine(container, "workspace-alias"); + try + { + Directory.CreateDirectory(physicalWorkspace); + if (!TryCreateDirectoryLink(workspaceAlias, physicalWorkspace)) + return; + + File.WriteAllText( + Path.Combine(physicalWorkspace, CdidxConfigFile.FileName), + """{ "metrics_path": "safe/metrics.jsonl" }"""); + + var result = CdidxConfigFile.Load(workspaceAlias, new TestEnvironment().Read); + + Assert.True(result.Loaded); + Assert.Null(result.Error); + Assert.Equal( + Path.Combine(workspaceAlias, "safe", "metrics.jsonl"), + result.Settings[MetricsSink.EnvVarName]); + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + using var session = MetricsSink.TryStartForTesting(explicitPath: null, maxBytes: 1024 * 1024); + Assert.NotNull(session); + MetricsSink.Record(new MetricsEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: "status", + Source: "cli", + ElapsedMs: 1, + ExitCode: 0)); + Assert.True(session.WaitForIdle(TimeSpan.FromSeconds(5))); + Assert.Contains( + "\"tool\":\"status\"", + File.ReadAllText(Path.Combine(physicalWorkspace, "safe", "metrics.jsonl")), + StringComparison.Ordinal); + } + finally + { + DeleteLinkEntry(workspaceAlias); + TestProjectHelper.DeleteDirectory(container); + } + } + + [Fact] + public void RepositoryOutputBoundary_RevalidatesAfterInjectedAncestorSwap_Issue5181() + { + var workspace = CreateTempDir(); + var outside = CreateTempDir(); + var safeDirectory = Path.Combine(workspace, "safe"); + var configuredPath = Path.Combine(safeDirectory, "new", "metrics.jsonl"); + try + { + Directory.CreateDirectory(safeDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "safe/new/metrics.jsonl" }"""); + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + Assert.True(result.Loaded); + + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + configuredPath, + destinationIsDirectory: false); + Assert.NotNull(boundary); + + var swapped = false; + RepositoryOutputPathBoundary.BeforeMutationForTesting = (operation, _) => + { + if (swapped || operation != "create_directory") + return; + swapped = true; + Directory.Delete(safeDirectory); + Directory.CreateSymbolicLink(safeDirectory, outside); + }; + + var exception = Assert.Throws( + () => boundary!.CreateSensitiveDestinationDirectory()); + + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, exception.Message, StringComparison.Ordinal); + Assert.False(Directory.Exists(Path.Combine(outside, "new"))); + Assert.False(File.Exists(Path.Combine(outside, "new", "metrics.jsonl"))); + } + finally + { + RepositoryOutputPathBoundary.BeforeMutationForTesting = null; + DeleteLinkEntry(safeDirectory); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + [Theory] [InlineData("""{ "search": { "limit": 0 } }""", "positive integer")] [InlineData("""{ "search": { "snippet_lines": -1 } }""", "positive integer")] @@ -1098,6 +1296,42 @@ private static string CreateTempDir() return TestProjectHelper.CreateTempProject("cdidx_config"); } + private static bool TryCreateDirectoryLink(string linkPath, string targetPath) + => TryCreateLink(linkPath, targetPath, directoryLink: true); + + private static bool TryCreateLink(string linkPath, string targetPath, bool directoryLink) + { + try + { + if (directoryLink) + Directory.CreateSymbolicLink(linkPath, targetPath); + else + File.CreateSymbolicLink(linkPath, targetPath); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) + { + return false; + } + } + + private static void DeleteLinkEntry(string path) + { + try + { + var attributes = File.GetAttributes(path); + if ((attributes & FileAttributes.ReparsePoint) == 0) + return; + if ((attributes & FileAttributes.Directory) != 0) + Directory.Delete(path); + else + File.Delete(path); + } + catch (Exception ex) when (ex is FileNotFoundException or DirectoryNotFoundException) + { + } + } + private static (int ExitCode, string Stdout, string Stderr) CaptureConsole(Func action) => ConsoleCapture.Capture(action); diff --git a/tests/CodeIndex.Tests/GlobalToolLogTests.cs b/tests/CodeIndex.Tests/GlobalToolLogTests.cs index d7eda2475c..2397b48eee 100644 --- a/tests/CodeIndex.Tests/GlobalToolLogTests.cs +++ b/tests/CodeIndex.Tests/GlobalToolLogTests.cs @@ -69,6 +69,108 @@ public void PrivateLogFile_OpenAppend_OnUnixRejectsSymlinkTargets_Issue3824() } } + [Fact] + public void TryStart_RepositoryConfiguredOrdinaryDirectoryWritesSuccessfully_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_global_log_config_5181"); + var logDirectory = Path.Combine(workspace, "state", "logs"); + var sourceVariable = CdidxConfigFile.ConfigSourceEnvironmentVariablePrefix + "CDIDX_GLOBAL_TOOL_LOG_DIR"; + using var environment = EnvironmentVariableScope.Capture( + "CDIDX_FORCE_GLOBAL_TOOL_LOG", + "CDIDX_DISABLE_PERSISTENT_LOG", + "CDIDX_GLOBAL_TOOL_LOG_DIR", + sourceVariable); + environment.Set("CDIDX_FORCE_GLOBAL_TOOL_LOG", "1"); + environment.Set("CDIDX_DISABLE_PERSISTENT_LOG", null); + environment.Set("CDIDX_GLOBAL_TOOL_LOG_DIR", null); + environment.Set(sourceVariable, null); + try + { + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "global_tool_log_dir": "state/logs" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + + using (CdidxEnvironment.Push(config.Settings, config.Sources)) + using (var session = GlobalToolLog.TryStartForTesting(["status"], "test")) + { + Assert.NotNull(session); + GlobalToolLog.Info("repository_configured_log_boundary_ok"); + } + + var logPath = Assert.Single(Directory.GetFiles(logDirectory, "stderr-*.log")); + Assert.Contains("repository_configured_log_boundary_ok", File.ReadAllText(logPath), StringComparison.Ordinal); + if (!OperatingSystem.IsWindows()) + { + Assert.Equal( + DataDirectorySecurity.PrivateDirectoryMode, + File.GetUnixFileMode(logDirectory) & PermissionBits); + Assert.Equal(PrivateLogFile.PrivateFileMode, File.GetUnixFileMode(logPath) & PermissionBits); + } + } + finally + { + TestProjectHelper.DeleteDirectory(workspace); + } + } + + [Fact] + public void TryStart_RepositoryConfiguredAncestorSwapIsRejectedWithoutFallbackWrite_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_global_log_race_5181"); + var outside = TestProjectHelper.CreateTempProject("cdidx_global_log_outside_5181"); + var safeDirectory = Path.Combine(workspace, "safe"); + var sourceVariable = CdidxConfigFile.ConfigSourceEnvironmentVariablePrefix + "CDIDX_GLOBAL_TOOL_LOG_DIR"; + using var environment = EnvironmentVariableScope.Capture( + "CDIDX_FORCE_GLOBAL_TOOL_LOG", + "CDIDX_DISABLE_PERSISTENT_LOG", + "CDIDX_GLOBAL_TOOL_LOG_DIR", + sourceVariable); + environment.Set("CDIDX_FORCE_GLOBAL_TOOL_LOG", "1"); + environment.Set("CDIDX_DISABLE_PERSISTENT_LOG", null); + environment.Set("CDIDX_GLOBAL_TOOL_LOG_DIR", null); + environment.Set(sourceVariable, null); + try + { + Directory.CreateDirectory(safeDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "global_tool_log_dir": "safe/new/logs" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + var swapped = false; + RepositoryOutputPathBoundary.BeforeMutationForTesting = (operation, _) => + { + if (swapped || operation != "create_directory") + return; + swapped = true; + Directory.Delete(safeDirectory); + Directory.CreateSymbolicLink(safeDirectory, outside); + }; + + using var scopedConfig = CdidxEnvironment.Push(config.Settings, config.Sources); + var capture = ConsoleCapture.Capture(() => + { + using var session = GlobalToolLog.TryStartForTesting(["status"], "test"); + return session is null ? 0 : 1; + }); + + Assert.Equal(0, capture.ExitCode); + Assert.Contains("global_tool_log_dir", capture.Stderr, StringComparison.Ordinal); + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, capture.Stderr, StringComparison.Ordinal); + Assert.False(Directory.Exists(Path.Combine(outside, "new"))); + Assert.Empty(Directory.GetFiles(outside, "stderr-*.log", SearchOption.AllDirectories)); + } + finally + { + RepositoryOutputPathBoundary.BeforeMutationForTesting = null; + DeleteDirectoryLink(safeDirectory); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + [Fact] public void PrivateLogFile_HardenExisting_CapsBestEffortWork_Issue3027() { @@ -746,6 +848,18 @@ public void TryStart_ErrorMirrorTruncatesLargeWrites_Issue3166() private static void ThrowForGlobalToolLogTest() => throw new InvalidOperationException("global log stack trace test"); + private static void DeleteDirectoryLink(string path) + { + try + { + if ((File.GetAttributes(path) & FileAttributes.ReparsePoint) != 0) + Directory.Delete(path); + } + catch (Exception ex) when (ex is FileNotFoundException or DirectoryNotFoundException) + { + } + } + private sealed class ThrowingTextWriter : TextWriter { public override Encoding Encoding => Encoding.UTF8; diff --git a/tests/CodeIndex.Tests/MetricsSinkTests.cs b/tests/CodeIndex.Tests/MetricsSinkTests.cs index 5fada4d5b1..1323d61173 100644 --- a/tests/CodeIndex.Tests/MetricsSinkTests.cs +++ b/tests/CodeIndex.Tests/MetricsSinkTests.cs @@ -170,6 +170,130 @@ public void Run_WithMetricsFlag_OnUnixCreatesPrivateParentDirectory_Issue3686() } } + [Fact] + public void Record_RepositoryConfiguredOrdinaryPathWritesSuccessfully_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_metrics_config_5181"); + var metricsPath = Path.Combine(workspace, "logs", "metrics.jsonl"); + var configPath = Path.Combine(workspace, CdidxConfigFile.FileName); + var warnings = new List(); + try + { + File.WriteAllText(configPath, """{ "metrics_path": "logs/metrics.jsonl" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + using var environment = CdidxEnvironment.Push(config.Settings, config.Sources); + using var session = MetricsSink.TryStartForTesting( + explicitPath: null, + maxBytes: 1024 * 1024, + warningSink: warnings.Add); + Assert.NotNull(session); + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + metricsPath, + destinationIsDirectory: false); + if (!OperatingSystem.IsWindows()) + { + Assert.Equal(PrivateLogFile.PrivateFileMode, File.GetUnixFileMode(metricsPath)); + Assert.Equal( + DataDirectorySecurity.PrivateDirectoryMode, + File.GetUnixFileMode(Path.GetDirectoryName(metricsPath)!)); + } + using (var stream = PrivateLogFile.OpenAppend(metricsPath, boundary: boundary)) + { + stream.WriteByte((byte)'\n'); + } + + MetricsSink.Record(new MetricsEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: "status", + Source: "cli", + ElapsedMs: 1, + ExitCode: 0)); + + Assert.True(session.WaitForIdle(TimeSpan.FromSeconds(5))); + var diagnostics = session.SnapshotDiagnostics(); + Assert.False(diagnostics.Degraded, diagnostics.LastFailure); + Assert.Empty(warnings); + Assert.Contains("\"tool\":\"status\"", File.ReadAllText(metricsPath), StringComparison.Ordinal); + } + finally + { + TestProjectHelper.DeleteDirectory(workspace); + } + } + + [Fact] + public void Record_RepositoryConfiguredPathRotatesWithinWorkspace_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_metrics_config_rotate_5181"); + var metricsPath = Path.Combine(workspace, "logs", "metrics.jsonl"); + try + { + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "logs/metrics.jsonl" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + using var environment = CdidxEnvironment.Push(config.Settings, config.Sources); + using var session = MetricsSink.TryStartForTesting(explicitPath: null, maxBytes: 1); + Assert.NotNull(session); + + MetricsSink.Record(new MetricsEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: "status", + Source: "cli", + ElapsedMs: 1, + ExitCode: 0)); + + Assert.True(session.WaitForIdle(TimeSpan.FromSeconds(5))); + var diagnostics = session.SnapshotDiagnostics(); + Assert.False(diagnostics.Degraded, diagnostics.LastFailure); + Assert.True(File.Exists(metricsPath + ".1")); + Assert.Contains("\"tool\":\"status\"", File.ReadAllText(metricsPath + ".1"), StringComparison.Ordinal); + } + finally + { + TestProjectHelper.DeleteDirectory(workspace); + } + } + + [Fact] + public void Record_ExplicitPathRetainsOperatorControlledBehavior_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_metrics_config_explicit_5181"); + var explicitDirectory = TestProjectHelper.CreateTempProject("cdidx_metrics_explicit_5181"); + var explicitPath = Path.Combine(explicitDirectory, "metrics.jsonl"); + try + { + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "logs/config-metrics.jsonl" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + using var environment = CdidxEnvironment.Push(config.Settings, config.Sources); + using var session = MetricsSink.TryStartForTesting(explicitPath, maxBytes: 1024 * 1024); + Assert.NotNull(session); + + MetricsSink.Record(new MetricsEvent( + Timestamp: DateTimeOffset.UtcNow, + Tool: "status", + Source: "cli", + ElapsedMs: 1, + ExitCode: 0)); + + Assert.True(session.WaitForIdle(TimeSpan.FromSeconds(5))); + Assert.Contains("\"tool\":\"status\"", File.ReadAllText(explicitPath), StringComparison.Ordinal); + Assert.False(File.Exists(Path.Combine(workspace, "logs", "config-metrics.jsonl"))); + } + finally + { + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(explicitDirectory); + } + } + [Fact] public void Record_RotatesMetricsLogAtMaxBytes() { From 074d1bd206ec784f640b191e81d8b11688e33d8c Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 26 Aug 2026 06:36:12 +0900 Subject: [PATCH 2/6] Guard auxiliary repository log writes (#5181) --- DEVELOPER_GUIDE.md | 16 ++- changelog.d/unreleased/5181.security.md | 6 +- src/CodeIndex/Cli/GlobalToolLog.cs | 5 +- src/CodeIndex/Cli/LastFailureEventStore.cs | 12 +- src/CodeIndex/Cli/PrivateLogFile.cs | 59 ++++++++++ src/CodeIndex/Cli/ProgramRunner.Metrics.cs | 23 ++-- .../Cli/RepositoryOutputPathBoundary.cs | 58 ++++++--- tests/CodeIndex.Tests/CdidxConfigFileTests.cs | 53 +++++++++ tests/CodeIndex.Tests/GlobalToolLogTests.cs | 111 ++++++++++++++++++ 9 files changed, 310 insertions(+), 33 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index ac982e008e..32da278f7b 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -2955,9 +2955,11 @@ the config workspace is rejected when it is a symbolic link, junction, cross-device mount point, reparse point, device, or dangling link. The boundary is revalidated before each mutation; on POSIX, directory creation, append, permission changes, rotation, and deletion are additionally anchored to the -workspace directory handle with no-follow relative operations. An unsafe value fails config -validation with the bounded `unsafe_output_path` diagnostic and does not create, -append, rotate, delete, or chmod the external target. Explicit CLI and process +workspace directory handle with no-follow relative operations. The same +`global_tool_log_dir` guard covers lifecycle logs, file query traces, and the +bounded `last-failure.json` diagnostic. An unsafe value fails config validation +with the bounded `unsafe_output_path` diagnostic and does not create, append, +rotate, replace, delete, or chmod the external target. Explicit CLI and process environment destinations retain their existing operator-controlled behavior. The file name is `stderr-YYYYMMDD.log`, timestamps inside the file are ISO-8601 UTC @@ -6968,9 +6970,11 @@ read-only な state/cache/runtime mount は最初の log write を失うので reparse point、device、dangling link のいずれかであれば拒否し、各 mutation の 直前にも境界を再検証します。POSIX ではさらに directory 作成、append、permission 変更、rotation、delete を workspace directory handle 起点の no-follow relative -operation へ固定します。安全でない値は上限付きの `unsafe_output_path` 診断で config validation -に失敗し、外部 target の作成、追記、rotation、削除、chmod は行いません。明示的な -CLI と process environment の保存先は、operator が制御する従来の挙動を維持します。 +operation へ固定します。同じ `global_tool_log_dir` guard を lifecycle log、file +query trace、上限付きの `last-failure.json` 診断にも適用します。安全でない値は +上限付きの `unsafe_output_path` 診断で config validation に失敗し、外部 target の +作成、追記、rotation、置換、削除、chmod は行いません。明示的な CLI と process +environment の保存先は、operator が制御する従来の挙動を維持します。 ファイル名は `stderr-YYYYMMDD.log`、ファイル内 timestamp は invariant culture の ISO-8601 UTC（`yyyy-MM-ddTHH:mm:ss.fffZ`）で、logger は新しい 30 日次 diff --git a/changelog.d/unreleased/5181.security.md b/changelog.d/unreleased/5181.security.md index e7420acfe4..a688d25ef6 100644 --- a/changelog.d/unreleased/5181.security.md +++ b/changelog.d/unreleased/5181.security.md @@ -7,6 +7,8 @@ affected: - src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs - src/CodeIndex/Cli/MetricsSink.cs - src/CodeIndex/Cli/GlobalToolLog.cs + - src/CodeIndex/Cli/ProgramRunner.Metrics.cs + - src/CodeIndex/Cli/LastFailureEventStore.cs - src/CodeIndex/Cli/PrivateLogFile.cs - tests/CodeIndex.Tests/CdidxConfigFileTests.cs - tests/CodeIndex.Tests/MetricsSinkTests.cs @@ -16,8 +18,8 @@ affected: ## English -- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, cross-device mount, reparse-point, device, and dangling-link components below the config workspace. Mutations are revalidated, and POSIX writes, directory creation, permission changes, rotation, and deletion are anchored to the workspace directory handle so an untrusted checkout cannot redirect them to an external target. +- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, cross-device mount, reparse-point, device, and dangling-link components below the config workspace. Mutations are revalidated, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle; the global-log guard also covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. ## 日本語 -- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、cross-device mount、reparse point、device、dangling link を拒否します。mutation 前の再検証に加え、POSIX の書き込み、directory 作成、permission 変更、rotation、削除を workspace directory handle に固定し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 +- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、cross-device mount、reparse point、device、dangling link を拒否します。mutation 前の再検証に加え、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を workspace directory handle に固定します。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 diff --git a/src/CodeIndex/Cli/GlobalToolLog.cs b/src/CodeIndex/Cli/GlobalToolLog.cs index ece0a74b59..70835ed1c0 100644 --- a/src/CodeIndex/Cli/GlobalToolLog.cs +++ b/src/CodeIndex/Cli/GlobalToolLog.cs @@ -32,7 +32,7 @@ internal static class GlobalToolLog private const int PrivateLogDiagnosticEmitLimit = 16; internal static TimeProvider TimeProvider { get; set; } = TimeProvider.System; private static readonly AsyncLocal CurrentSession = new(); - private sealed record LogDirectorySelection(string Path, RepositoryOutputPathGuard? Boundary); + internal sealed record LogDirectorySelection(string Path, RepositoryOutputPathGuard? Boundary); internal static IDisposable? TryStart(string[] args, string appVersion) => TryStart(args, appVersion, createWriter: null, afterWriterCreated: null); @@ -290,6 +290,9 @@ private static bool ContainsPathSegments(string path, string[] expectedSegments, internal static string ResolveLogDirectoryForStatus() => ResolveLogDirectory(); + internal static LogDirectorySelection ResolveLogDirectorySelectionForRepositoryWrite() + => ResolveLogDirectorySelection(requireWritableCandidate: true); + private static string ResolveLogDirectory() => ResolveLogDirectorySelection(requireWritableCandidate: true).Path; internal static string ResolveLogDirectoryWithoutWriteProbeForTesting() diff --git a/src/CodeIndex/Cli/LastFailureEventStore.cs b/src/CodeIndex/Cli/LastFailureEventStore.cs index 0e4c3a36cf..9d765f3fcf 100644 --- a/src/CodeIndex/Cli/LastFailureEventStore.cs +++ b/src/CodeIndex/Cli/LastFailureEventStore.cs @@ -73,9 +73,15 @@ internal static bool TryPersist( if (Encoding.UTF8.GetByteCount(json) > MaxEventBytes) return false; - var logDirectory = GlobalToolLog.ResolveLogDirectoryForReport(); - DataDirectorySecurity.CreateSensitiveDirectory(logDirectory); - DataDirectorySecurity.WritePrivateText(Path.Combine(logDirectory, FileName), json + "\n"); + var selection = GlobalToolLog.ResolveLogDirectorySelectionForRepositoryWrite(); + if (selection.Boundary is null) + DataDirectorySecurity.CreateSensitiveDirectory(selection.Path); + else + selection.Boundary.CreateSensitiveDestinationDirectory(); + PrivateLogFile.WritePrivateTextReplacing( + Path.Combine(selection.Path, FileName), + json + "\n", + selection.Boundary); return true; } catch (Exception ex) when (ex is not OutOfMemoryException) diff --git a/src/CodeIndex/Cli/PrivateLogFile.cs b/src/CodeIndex/Cli/PrivateLogFile.cs index b9c7d5a195..d01fc97271 100644 --- a/src/CodeIndex/Cli/PrivateLogFile.cs +++ b/src/CodeIndex/Cli/PrivateLogFile.cs @@ -55,6 +55,65 @@ internal static StreamWriter OpenAppendText(string path, RepositoryOutputPathGua AutoFlush = true, }; + internal static void WritePrivateTextReplacing( + string path, + string contents, + RepositoryOutputPathGuard? boundary = null) + { + if (boundary is null) + { + DataDirectorySecurity.WritePrivateText(path, contents); + return; + } + + boundary.PrepareMutation("write_private_text", path); + var stagedPath = path + $".{Guid.NewGuid():N}.tmp"; + try + { + boundary.PrepareMutation("write_private_text_stage", stagedPath); + RejectUnsafeTarget(stagedPath); + using (var stream = OperatingSystem.IsWindows() + ? new FileStream(stagedPath, FileMode.CreateNew, FileAccess.Write, FileShare.None) + : boundary.OpenReplacingUnix(stagedPath)) + { + var encoded = Encoding.UTF8.GetBytes(contents); + stream.Write(encoded, 0, encoded.Length); + stream.Flush(flushToDisk: true); + } + boundary.CompleteMutation(stagedPath); + TrySetPrivatePermissions(stagedPath, boundary: boundary); + + boundary.PrepareMutation("write_private_text_publish_source", stagedPath); + boundary.PrepareMutation("write_private_text_publish_destination", path); + if (OperatingSystem.IsWindows()) + AtomicFileWriter.MoveReplacing(stagedPath, path); + else + boundary.MoveReplacingUnix(stagedPath, path); + boundary.CompleteMutation(path); + } + finally + { + TryDeleteStagedFile(stagedPath, boundary); + } + } + + private static void TryDeleteStagedFile(string path, RepositoryOutputPathGuard boundary) + { + try + { + boundary.PrepareMutation("write_private_text_cleanup", path); + if (OperatingSystem.IsWindows()) + File.Delete(path); + else + boundary.DeleteFileUnix(path); + boundary.CompleteMutation(path); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or NotSupportedException) + { + // Best-effort cleanup only / cleanup はベストエフォートのみ + } + } + internal static void TrySetPrivatePermissions( string path, Action? diagnosticSink = null, diff --git a/src/CodeIndex/Cli/ProgramRunner.Metrics.cs b/src/CodeIndex/Cli/ProgramRunner.Metrics.cs index a0a3e33fbc..6febd994aa 100644 --- a/src/CodeIndex/Cli/ProgramRunner.Metrics.cs +++ b/src/CodeIndex/Cli/ProgramRunner.Metrics.cs @@ -154,7 +154,7 @@ internal static bool TryConsumeQueryTraceFlag(string? commandName, ref string[] return true; } - private static void EmitQueryTrace(string mode, string commandName, string[] subArgs, DateTimeOffset startTimestamp, Stopwatch stopwatch, int exitCode, int? resultCount) + internal static void EmitQueryTrace(string mode, string commandName, string[] subArgs, DateTimeOffset startTimestamp, Stopwatch stopwatch, int exitCode, int? resultCount) { if (mode == "none") return; @@ -169,18 +169,27 @@ private static void EmitQueryTrace(string mode, string commandName, string[] sub return; } - var directory = GlobalToolLog.ResolveLogDirectoryForStatus(); - Directory.CreateDirectory(directory); - PrivateLogFile.HardenExisting(directory, "query-trace-*.jsonl"); + var selection = GlobalToolLog.ResolveLogDirectorySelectionForRepositoryWrite(); + var directory = selection.Path; + var boundary = selection.Boundary; + if (boundary is null) + Directory.CreateDirectory(directory); + else + boundary.CreateSensitiveDestinationDirectory(); + PrivateLogFile.HardenExisting(directory, "query-trace-*.jsonl", boundary: boundary); var path = ResolveQueryTracePath(directory); var encoded = Encoding.UTF8.GetBytes(payload + Environment.NewLine); - using (var stream = PrivateLogFile.OpenAppend(path, FileShare.ReadWrite)) + using (var stream = PrivateLogFile.OpenAppend(path, FileShare.ReadWrite, boundary)) { stream.Write(encoded, 0, encoded.Length); stream.Flush(); } - PrivateLogFile.TrySetPrivatePermissions(path); - PrivateLogFile.PruneOldFiles(directory, "query-trace-*.jsonl", RetainedQueryTraceFileCount); + PrivateLogFile.TrySetPrivatePermissions(path, boundary: boundary); + PrivateLogFile.PruneOldFiles( + directory, + "query-trace-*.jsonl", + RetainedQueryTraceFileCount, + boundary: boundary); } catch { diff --git a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs index df71cac319..a815f6ee19 100644 --- a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs +++ b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs @@ -16,6 +16,7 @@ internal static class RepositoryOutputPathBoundary private const string UnsafeMessage = "symbolic links, junctions, cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root"; private static readonly AsyncLocal?> BeforeMutation = new(); + private static readonly AsyncLocal?> ContainsPath = new(); internal static Action? BeforeMutationForTesting { @@ -23,6 +24,12 @@ internal static Action? BeforeMutationForTesting set => BeforeMutation.Value = value; } + internal static Func? ContainsPathForTesting + { + get => ContainsPath.Value; + set => ContainsPath.Value = value; + } + internal static bool TryResolveConfiguredPath( string rawPath, string workspaceRoot, @@ -39,7 +46,7 @@ internal static bool TryResolveConfiguredPath( ? Path.GetFullPath(rawPath) : Path.GetFullPath(Path.Combine(normalizedRoot, rawPath)); var normalizedPath = PathCasing.NormalizeBoundaryPath(fullPath); - if (!PathCasing.IsPathEqualOrParent(normalizedRoot, normalizedPath)) + if (!IsPathEqualOrParent(normalizedRoot, normalizedPath)) { failureReason = "outside_workspace"; return false; @@ -124,7 +131,7 @@ internal static void ValidatePathComponents( { var normalizedRoot = PathCasing.NormalizeBoundaryPath(workspaceRoot); var normalizedPath = PathCasing.NormalizeBoundaryPath(path); - if (!PathCasing.IsPathEqualOrParent(normalizedRoot, normalizedPath)) + if (!IsPathEqualOrParent(normalizedRoot, normalizedPath)) throw CreateException("output path", "outside_workspace"); var rootAttributesStatus = FileSystemBoundary.TryGetAttributes(normalizedRoot, out var rootAttributes); @@ -235,6 +242,10 @@ or NotSupportedException or PathTooLongException or UnauthorizedAccessException; + internal static bool IsPathEqualOrParent(string parent, string child) + => ContainsPathForTesting?.Invoke(parent, child) + ?? PathCasing.IsPathEqualOrParentByDirectoryNamespace(parent, child); + [DllImport("libSystem.Native", EntryPoint = "SystemNative_Stat", CharSet = CharSet.Ansi)] private static extern int UnixStat(string path, out UnixFileStatus status); @@ -326,6 +337,23 @@ internal Stream OpenAppendUnix(string path) } } + internal FileStream OpenReplacingUnix(string path) + { + if (OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + var handle = OpenUnixFile(path, create: true, append: false, truncate: true); + try + { + return new FileStream(handle, FileAccess.Write); + } + catch + { + handle.Dispose(); + throw; + } + } + internal void SetPrivateFileModeUnix(string path) { if (OperatingSystem.IsWindows()) @@ -491,7 +519,7 @@ private void CreateSensitiveDirectoriesUnix(string directory) } } - private SafeFileHandle OpenUnixFile(string path, bool create, bool append) + private SafeFileHandle OpenUnixFile(string path, bool create, bool append, bool truncate = false) { using var parent = OpenUnixParentDirectory(path, out var rootDevice); var flags = UnixWriteOnly | UnixCloseOnExec | UnixNoFollow; @@ -499,6 +527,8 @@ private SafeFileHandle OpenUnixFile(string path, bool create, bool append) flags |= UnixAppend; if (create) flags |= UnixCreate; + if (truncate) + flags |= UnixTruncate; var descriptor = UnixOpenAt( parent.DangerousGetHandle().ToInt32(), Path.GetFileName(path), @@ -617,7 +647,7 @@ private static void EnsureUnixObject(SafeFileHandle handle, long rootDevice, int private string[] GetRelativeComponents(string path) { - if (!PathCasing.IsPathEqualOrParent(WorkspaceRoot, path)) + if (!RepositoryOutputPathBoundary.IsPathEqualOrParent(WorkspaceRoot, path)) throw RepositoryOutputPathBoundary.CreateException(FieldName, "outside_workspace"); var relative = Path.GetRelativePath(WorkspaceRoot, path); if (relative == ".") @@ -637,31 +667,30 @@ private bool Allows(string path, bool expectsDirectory) { if (DestinationIsDirectory) { - return PathCasing.IsPathEqualOrParent(DestinationPath, path) + return RepositoryOutputPathBoundary.IsPathEqualOrParent(DestinationPath, path) || (expectsDirectory - && PathCasing.IsPathEqualOrParent(path, DestinationPath) - && PathCasing.IsPathEqualOrParent(WorkspaceRoot, path)); + && RepositoryOutputPathBoundary.IsPathEqualOrParent(path, DestinationPath) + && RepositoryOutputPathBoundary.IsPathEqualOrParent(WorkspaceRoot, path)); } - if (expectsDirectory && PathCasing.IsPathEqualOrParent(path, DestinationPath)) - return PathCasing.IsPathEqualOrParent(WorkspaceRoot, path); + if (expectsDirectory && RepositoryOutputPathBoundary.IsPathEqualOrParent(path, DestinationPath)) + return RepositoryOutputPathBoundary.IsPathEqualOrParent(WorkspaceRoot, path); - var comparison = PathCasing.ComparisonFor(DestinationPath); - if (string.Equals(DestinationPath, path, comparison)) + if (PathCasing.PathsEqualByDirectoryNamespace(DestinationPath, path)) return true; var destinationDirectory = Path.GetDirectoryName(DestinationPath); var pathDirectory = Path.GetDirectoryName(path); if (string.IsNullOrEmpty(destinationDirectory) || string.IsNullOrEmpty(pathDirectory) - || !string.Equals( + || !PathCasing.PathsEqualByDirectoryNamespace( PathCasing.NormalizeBoundaryPath(destinationDirectory), - PathCasing.NormalizeBoundaryPath(pathDirectory), - comparison)) + PathCasing.NormalizeBoundaryPath(pathDirectory))) { return false; } + var comparison = PathCasing.ComparisonFor(destinationDirectory); var destinationName = Path.GetFileName(DestinationPath); var candidateName = Path.GetFileName(path); if (!candidateName.StartsWith(destinationName + ".", comparison)) @@ -685,6 +714,7 @@ private bool Allows(string path, bool expectsDirectory) private static int UnixDirectory => OperatingSystem.IsMacOS() ? 0x00100000 : 0x00010000; private static int UnixCreate => OperatingSystem.IsMacOS() ? 0x00000200 : 0x00000040; private static int UnixAppend => OperatingSystem.IsMacOS() ? 0x00000008 : 0x00000400; + private static int UnixTruncate => OperatingSystem.IsMacOS() ? 0x00000400 : 0x00000200; [DllImport("libc", EntryPoint = "open", SetLastError = true)] private static extern int UnixOpen(string path, int flags); diff --git a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs index efec29c3c5..ad728e9789 100644 --- a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs +++ b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs @@ -1,4 +1,5 @@ using CodeIndex.Cli; +using CodeIndex.Indexer; using CodeIndex.Mcp; using System.Text; using System.Text.Json; @@ -430,6 +431,58 @@ public void LoadAndApply_WorkspaceRootAliasStillAllowsOrdinaryContainedOutput_Is } } + [Fact] + public void OutputBoundary_RejectsCaseOnlySiblingInDistinctParentNamespace_Issue5181Review() + { + var root = CreateTempDir(); + var upperWorkspace = Path.Combine(root, "Workspace"); + var lowerWorkspace = Path.Combine(root, "workspace"); + var configuredPath = Path.Combine(lowerWorkspace, "logs", "metrics.jsonl"); + var rootIdentity = new FileIndexer.FileIdentity(1, 1); + var upperIdentity = new FileIndexer.FileIdentity(2, 2); + var lowerIdentity = new FileIndexer.FileIdentity(3, 3); + try + { + Directory.CreateDirectory(upperWorkspace); + FileIndexer.FileIdentity? Identity(string path) + { + var fullPath = Path.GetFullPath(path); + if (string.Equals(fullPath, root, StringComparison.Ordinal)) + return rootIdentity; + if (string.Equals(fullPath, upperWorkspace, StringComparison.Ordinal)) + return upperIdentity; + if (string.Equals(fullPath, lowerWorkspace, StringComparison.Ordinal)) + return lowerIdentity; + return null; + } + + bool IgnoreCase(string path) + => !string.Equals(Path.GetFullPath(path), root, StringComparison.Ordinal); + + RepositoryOutputPathBoundary.ContainsPathForTesting = (parent, child) => + PathCasing.IsPathEqualOrParentByDirectoryNamespaceForTesting( + parent, + child, + IgnoreCase, + Identity); + + var accepted = RepositoryOutputPathBoundary.TryResolveConfiguredPath( + configuredPath, + upperWorkspace, + destinationIsDirectory: false, + out _, + out var failureReason); + + Assert.False(accepted); + Assert.Equal("outside_workspace", failureReason); + } + finally + { + RepositoryOutputPathBoundary.ContainsPathForTesting = null; + TestProjectHelper.DeleteDirectory(root); + } + } + [Fact] public void RepositoryOutputBoundary_RevalidatesAfterInjectedAncestorSwap_Issue5181() { diff --git a/tests/CodeIndex.Tests/GlobalToolLogTests.cs b/tests/CodeIndex.Tests/GlobalToolLogTests.cs index 2397b48eee..51f26bdfe3 100644 --- a/tests/CodeIndex.Tests/GlobalToolLogTests.cs +++ b/tests/CodeIndex.Tests/GlobalToolLogTests.cs @@ -171,6 +171,117 @@ public void TryStart_RepositoryConfiguredAncestorSwapIsRejectedWithoutFallbackWr } } + [Fact] + public void QueryTrace_RepositoryConfiguredAncestorSwapDoesNotWriteOutsideWorkspace_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_query_trace_race_5181"); + var outside = TestProjectHelper.CreateTempProject("cdidx_query_trace_outside_5181"); + var safeDirectory = Path.Combine(workspace, "safe"); + var originalDirectory = Path.Combine(workspace, "safe-original"); + var logDirectory = Path.Combine(safeDirectory, "logs"); + var sourceVariable = CdidxConfigFile.ConfigSourceEnvironmentVariablePrefix + "CDIDX_GLOBAL_TOOL_LOG_DIR"; + using var environment = EnvironmentVariableScope.Capture("CDIDX_GLOBAL_TOOL_LOG_DIR", sourceVariable); + environment.Set("CDIDX_GLOBAL_TOOL_LOG_DIR", null); + environment.Set(sourceVariable, null); + try + { + Directory.CreateDirectory(logDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "global_tool_log_dir": "safe/logs" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + var swapped = false; + RepositoryOutputPathBoundary.BeforeMutationForTesting = (operation, path) => + { + if (swapped + || operation != "open_append" + || !Path.GetFileName(path).StartsWith("query-trace-", StringComparison.Ordinal)) + { + return; + } + + swapped = true; + Directory.Move(safeDirectory, originalDirectory); + Directory.CreateSymbolicLink(safeDirectory, outside); + }; + + using var scopedConfig = CdidxEnvironment.Push(config.Settings, config.Sources); + ProgramRunner.EmitQueryTrace( + "file", + "search", + ["needle"], + DateTimeOffset.UtcNow, + System.Diagnostics.Stopwatch.StartNew(), + CommandExitCodes.Success, + resultCount: 0); + + Assert.True(swapped); + Assert.Empty(Directory.GetFiles(outside, "query-trace-*.jsonl", SearchOption.AllDirectories)); + } + finally + { + RepositoryOutputPathBoundary.BeforeMutationForTesting = null; + DeleteDirectoryLink(safeDirectory); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + + [Fact] + public void LastFailure_RepositoryConfiguredAncestorSwapDoesNotWriteOutsideWorkspace_Issue5181() + { + var workspace = TestProjectHelper.CreateTempProject("cdidx_last_failure_race_5181"); + var outside = TestProjectHelper.CreateTempProject("cdidx_last_failure_outside_5181"); + var safeDirectory = Path.Combine(workspace, "safe"); + var originalDirectory = Path.Combine(workspace, "safe-original"); + var logDirectory = Path.Combine(safeDirectory, "logs"); + var sourceVariable = CdidxConfigFile.ConfigSourceEnvironmentVariablePrefix + "CDIDX_GLOBAL_TOOL_LOG_DIR"; + using var environment = EnvironmentVariableScope.Capture("CDIDX_GLOBAL_TOOL_LOG_DIR", sourceVariable); + environment.Set("CDIDX_GLOBAL_TOOL_LOG_DIR", null); + environment.Set(sourceVariable, null); + try + { + Directory.CreateDirectory(logDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "global_tool_log_dir": "safe/logs" }"""); + var config = CdidxConfigFile.Load(workspace); + Assert.True(config.Loaded); + var swapped = false; + RepositoryOutputPathBoundary.BeforeMutationForTesting = (operation, _) => + { + if (swapped || operation != "write_private_text") + return; + + swapped = true; + Directory.Move(safeDirectory, originalDirectory); + Directory.CreateSymbolicLink(safeDirectory, outside); + }; + + using var scopedConfig = CdidxEnvironment.Push(config.Settings, config.Sources); + var persisted = LastFailureEventStore.TryPersist( + ["search"], + "test", + CommandExitCodes.UnhandledException, + new InvalidOperationException("test failure"), + DateTimeOffset.UtcNow, + dbPathForTesting: Path.Combine(workspace, ".cdidx", "codeindex.db"), + workspacePathForTesting: workspace); + + Assert.False(persisted); + Assert.True(swapped); + Assert.False(File.Exists(Path.Combine(outside, LastFailureEventStore.FileName))); + } + finally + { + RepositoryOutputPathBoundary.BeforeMutationForTesting = null; + DeleteDirectoryLink(safeDirectory); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + [Fact] public void PrivateLogFile_HardenExisting_CapsBestEffortWork_Issue3027() { From 22a37e97836039cde4ba830a7417a58a29c5e4d2 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 26 Aug 2026 08:16:25 +0900 Subject: [PATCH 3/6] Reject same-device output mount aliases (#5181) --- DEVELOPER_GUIDE.md | 20 +- changelog.d/unreleased/5181.security.md | 4 +- src/CodeIndex/Cli/CdidxConfigFile.cs | 2 +- .../Cli/RepositoryOutputPathBoundary.cs | 212 ++++++++++++++++-- tests/CodeIndex.Tests/CdidxConfigFileTests.cs | 134 +++++++++++ 5 files changed, 347 insertions(+), 25 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 32da278f7b..ec2f6179b0 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -2951,11 +2951,13 @@ commits to it, so read-only state/cache/runtime mounts fall through to the next candidate instead of losing the first log write. Repository-configured `metrics_path` and `global_tool_log_dir` values from `.cdidxrc.json` or `.cdidx/config.json` use a stricter boundary: every existing component below -the config workspace is rejected when it is a symbolic link, junction, +the config workspace is rejected when it is a symbolic link, junction, bind or cross-device mount point, reparse point, device, or dangling link. The boundary -is revalidated before each mutation; on POSIX, directory creation, append, -permission changes, rotation, and deletion are additionally anchored to the -workspace directory handle with no-follow relative operations. The same +is revalidated before each mutation; on Linux, path and opened-handle mount IDs +also reject same-device bind mounts. On POSIX, directory creation, append, +permission changes, rotation, replacement, and deletion are additionally anchored +to the workspace directory handle with no-follow relative operations, and guarded +renames fsync their already-open destination parent before reporting success. The same `global_tool_log_dir` guard covers lifecycle logs, file query traces, and the bounded `last-failure.json` diagnostic. An unsafe value fails config validation with the bounded `unsafe_output_path` diagnostic and does not create, append, @@ -6966,11 +6968,13 @@ read-only な state/cache/runtime mount は最初の log write を失うので 次の candidate へ fall through します。`.cdidxrc.json` または `.cdidx/config.json` の repository config に由来する `metrics_path` と `global_tool_log_dir` には、より厳格な境界を適用します。config workspace -配下の既存 component が symbolic link、junction、cross-device mount point、 +配下の既存 component が symbolic link、junction、bind mount / cross-device mount point、 reparse point、device、dangling link のいずれかであれば拒否し、各 mutation の -直前にも境界を再検証します。POSIX ではさらに directory 作成、append、permission -変更、rotation、delete を workspace directory handle 起点の no-follow relative -operation へ固定します。同じ `global_tool_log_dir` guard を lifecycle log、file +直前にも境界を再検証します。Linux では path と open 済み handle の mount ID も +比較して同一 device の bind mount を拒否します。POSIX ではさらに directory 作成、 +append、permission 変更、rotation、置換、delete を workspace directory handle 起点の +no-follow relative operation へ固定し、guarded rename は成功を返す前に open 済みの +destination parent を fsync します。同じ `global_tool_log_dir` guard を lifecycle log、file query trace、上限付きの `last-failure.json` 診断にも適用します。安全でない値は 上限付きの `unsafe_output_path` 診断で config validation に失敗し、外部 target の 作成、追記、rotation、置換、削除、chmod は行いません。明示的な CLI と process diff --git a/changelog.d/unreleased/5181.security.md b/changelog.d/unreleased/5181.security.md index a688d25ef6..f2f409450d 100644 --- a/changelog.d/unreleased/5181.security.md +++ b/changelog.d/unreleased/5181.security.md @@ -18,8 +18,8 @@ affected: ## English -- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, cross-device mount, reparse-point, device, and dangling-link components below the config workspace. Mutations are revalidated, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle; the global-log guard also covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. +- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, bind-mount, cross-device-mount, reparse-point, device, and dangling-link components below the config workspace. Mutations are revalidated, Linux path/open-handle mount IDs reject same-device bind mounts, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle; guarded renames also fsync the destination parent before returning success. The global-log guard covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. ## 日本語 -- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、cross-device mount、reparse point、device、dangling link を拒否します。mutation 前の再検証に加え、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を workspace directory handle に固定します。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 +- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、bind mount、cross-device mount、reparse point、device、dangling link を拒否します。mutation 前の再検証に加え、Linux では path / open 済み handle の mount ID で同一 device の bind mount も拒否し、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を workspace directory handle に固定します。guarded rename は成功を返す前に destination parent を fsync します。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 diff --git a/src/CodeIndex/Cli/CdidxConfigFile.cs b/src/CodeIndex/Cli/CdidxConfigFile.cs index b9e873a9f5..39392b9029 100644 --- a/src/CodeIndex/Cli/CdidxConfigFile.cs +++ b/src/CodeIndex/Cli/CdidxConfigFile.cs @@ -1016,7 +1016,7 @@ private static bool TryReadWorkspaceOutputPath(JsonElement element, string key, error = pathFailure == "outside_workspace" ? $"{FormatConfigDiagnosticPrefix(path)} `{key}` must resolve inside the config workspace root `{FormatConfigDiagnosticPath(workspaceRoot)}`." : pathFailure == RepositoryOutputPathBoundary.UnsafeReason - ? $"{FormatConfigDiagnosticPrefix(path)} `{key}` is unsafe ({RepositoryOutputPathBoundary.UnsafeReason}); symbolic links, junctions, cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root." + ? $"{FormatConfigDiagnosticPrefix(path)} `{key}` is unsafe ({RepositoryOutputPathBoundary.UnsafeReason}); symbolic links, junctions, bind or cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root." : $"{FormatConfigDiagnosticPrefix(path)} `{key}` path is invalid (invalid_path)."; return false; } diff --git a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs index a815f6ee19..4dcde9c5e4 100644 --- a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs +++ b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs @@ -14,9 +14,12 @@ internal static class RepositoryOutputPathBoundary { internal const string UnsafeReason = "unsafe_output_path"; private const string UnsafeMessage = - "symbolic links, junctions, cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root"; + "symbolic links, junctions, bind or cross-device mount points, reparse points, devices, and dangling links are not allowed below the config workspace root"; private static readonly AsyncLocal?> BeforeMutation = new(); private static readonly AsyncLocal?> ContainsPath = new(); + private static readonly AsyncLocal?> UnixMountId = new(); + private static readonly AsyncLocal?> UnixHandleMountId = new(); + private static readonly AsyncLocal?> UnixDirectoryFsync = new(); internal static Action? BeforeMutationForTesting { @@ -30,6 +33,24 @@ internal static Func? ContainsPathForTesting set => ContainsPath.Value = value; } + internal static Func? UnixMountIdForTesting + { + get => UnixMountId.Value; + set => UnixMountId.Value = value; + } + + internal static Func? UnixHandleMountIdForTesting + { + get => UnixHandleMountId.Value; + set => UnixHandleMountId.Value = value; + } + + internal static Func? UnixDirectoryFsyncForTesting + { + get => UnixDirectoryFsync.Value; + set => UnixDirectoryFsync.Value = value; + } + internal static bool TryResolveConfiguredPath( string rawPath, string workspaceRoot, @@ -143,6 +164,14 @@ internal static void ValidatePathComponents( } var rootDevice = TryGetUnixDevice(normalizedRoot, out var device) ? device : (long?)null; + ulong? rootMountId = null; + if (RequiresUnixMountIdentity) + { + var canonicalRoot = ResolveCanonicalWorkspaceRoot(normalizedRoot); + if (!TryGetUnixMountId(canonicalRoot, out var resolvedRootMountId)) + throw CreateException("output path", UnsafeReason); + rootMountId = resolvedRootMountId; + } var relative = Path.GetRelativePath(normalizedRoot, normalizedPath); if (relative == ".") return; @@ -184,6 +213,12 @@ internal static void ValidatePathComponents( { throw CreateException("output path", UnsafeReason); } + if (rootMountId.HasValue + && (!TryGetUnixMountId(current, out var currentMountId) + || currentMountId != rootMountId.Value)) + { + throw CreateException("output path", UnsafeReason); + } } } @@ -235,6 +270,105 @@ private static bool TryGetUnixDevice(string path, out long device) } } + internal static bool RequiresUnixMountIdentity => + OperatingSystem.IsLinux() + || UnixMountIdForTesting is not null + || UnixHandleMountIdForTesting is not null; + + internal static bool TryGetUnixMountId(string path, out ulong mountId) + { + mountId = 0; + if (UnixMountIdForTesting is { } provider) + { + var value = provider(path); + if (value.HasValue) + { + mountId = value.Value; + return true; + } + return false; + } + + if (!OperatingSystem.IsLinux()) + return false; + + try + { + if (UnixStatX( + UnixCurrentWorkingDirectory, + path, + UnixAtSymlinkNoFollow, + UnixStatXMountId, + out var status) != 0 + || (status.Mask & UnixStatXMountId) == 0) + { + return false; + } + + mountId = status.MountId; + return true; + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + return false; + } + } + + internal static bool TryGetUnixMountId(SafeFileHandle handle, out ulong mountId) + { + mountId = 0; + var descriptor = handle.DangerousGetHandle().ToInt32(); + if (UnixHandleMountIdForTesting is { } provider) + { + var value = provider(descriptor); + if (value.HasValue) + { + mountId = value.Value; + return true; + } + return false; + } + + if (!OperatingSystem.IsLinux()) + return false; + + try + { + if (UnixStatX( + descriptor, + string.Empty, + UnixAtEmptyPath | UnixAtSymlinkNoFollow, + UnixStatXMountId, + out var status) != 0 + || (status.Mask & UnixStatXMountId) == 0) + { + return false; + } + + mountId = status.MountId; + return true; + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + return false; + } + } + + internal static int FsyncUnixDirectory(SafeFileHandle handle) + { + var descriptor = handle.DangerousGetHandle().ToInt32(); + if (UnixDirectoryFsyncForTesting is { } fsync) + return fsync(descriptor); + + int result; + do + { + result = UnixFsync(descriptor); + } + while (result != 0 && Marshal.GetLastWin32Error() == 4); + return result; + } + private static bool IsPathException(Exception ex) => ex is ArgumentException or IOException @@ -255,6 +389,32 @@ internal static bool IsPathEqualOrParent(string parent, string child) [DllImport("libc", EntryPoint = "free")] private static extern void UnixFree(IntPtr pointer); + [DllImport("libc", EntryPoint = "statx", SetLastError = true)] + private static extern int UnixStatX( + int directory, + string path, + int flags, + uint mask, + out UnixStatXStatus status); + + [DllImport("libc", EntryPoint = "fsync", SetLastError = true)] + private static extern int UnixFsync(int descriptor); + + private const int UnixCurrentWorkingDirectory = -100; + private const int UnixAtSymlinkNoFollow = 0x100; + private const int UnixAtEmptyPath = 0x1000; + private const uint UnixStatXMountId = 0x1000; + + [StructLayout(LayoutKind.Explicit, Size = 256)] + private struct UnixStatXStatus + { + [FieldOffset(0)] + internal uint Mask; + + [FieldOffset(144)] + internal ulong MountId; + } + [StructLayout(LayoutKind.Sequential)] private struct UnixFileStatus { @@ -393,6 +553,12 @@ internal void MoveReplacingUnix(string source, string destination) { throw CreateNativeIOException(); } + if (RepositoryOutputPathBoundary.FsyncUnixDirectory(destinationParent) != 0) + { + throw new IOException( + "Guarded replace completed, but the target file was already replaced " + + "and its parent directory could not be flushed."); + } } internal void ValidateRelatedPath(string path, bool expectsDirectory = false) @@ -459,7 +625,7 @@ private void CreateSensitiveDirectoriesUnix(string directory) { var normalizedDirectory = PathCasing.NormalizeBoundaryPath(directory); var components = GetRelativeComponents(normalizedDirectory); - using var root = OpenUnixRootDirectory(out var rootDevice); + using var root = OpenUnixRootDirectory(out var rootIdentity); SafeFileHandle current = root; try { @@ -488,7 +654,7 @@ private void CreateSensitiveDirectoriesUnix(string directory) created = true; } - EnsureUnixObject(next, rootDevice, expectedType: UnixDirectoryType); + EnsureUnixObject(next, rootIdentity, expectedType: UnixDirectoryType); if (created || index == components.Length - 1) { PrepareMutation("set_directory_permissions", currentPath, expectsDirectory: true); @@ -521,7 +687,7 @@ private void CreateSensitiveDirectoriesUnix(string directory) private SafeFileHandle OpenUnixFile(string path, bool create, bool append, bool truncate = false) { - using var parent = OpenUnixParentDirectory(path, out var rootDevice); + using var parent = OpenUnixParentDirectory(path, out var rootIdentity); var flags = UnixWriteOnly | UnixCloseOnExec | UnixNoFollow; if (append) flags |= UnixAppend; @@ -540,7 +706,7 @@ private SafeFileHandle OpenUnixFile(string path, bool create, bool append, bool var handle = new SafeFileHandle(new IntPtr(descriptor), ownsHandle: true); try { - EnsureUnixObject(handle, rootDevice, expectedType: UnixRegularFileType); + EnsureUnixObject(handle, rootIdentity, expectedType: UnixRegularFileType); if (create && UnixFChmod(handle.DangerousGetHandle().ToInt32(), (uint)PrivateLogFile.PrivateFileMode) != 0) { @@ -558,17 +724,17 @@ private SafeFileHandle OpenUnixFile(string path, bool create, bool append, bool private SafeFileHandle OpenUnixParentDirectory(string path) => OpenUnixParentDirectory(path, out _); - private SafeFileHandle OpenUnixParentDirectory(string path, out long rootDevice) + private SafeFileHandle OpenUnixParentDirectory(string path, out UnixBoundaryIdentity rootIdentity) { var parentPath = Path.GetDirectoryName(Path.GetFullPath(path)) ?? throw RepositoryOutputPathBoundary.CreateException(FieldName, "invalid_path"); - return OpenUnixDirectory(parentPath, out rootDevice); + return OpenUnixDirectory(parentPath, out rootIdentity); } - private SafeFileHandle OpenUnixDirectory(string directory, out long rootDevice) + private SafeFileHandle OpenUnixDirectory(string directory, out UnixBoundaryIdentity rootIdentity) { var components = GetRelativeComponents(PathCasing.NormalizeBoundaryPath(directory)); - var current = OpenUnixRootDirectory(out rootDevice); + var current = OpenUnixRootDirectory(out rootIdentity); try { foreach (var component in components) @@ -576,7 +742,7 @@ private SafeFileHandle OpenUnixDirectory(string directory, out long rootDevice) var next = TryOpenUnixDirectoryAt(current, component) ?? throw CreateNativeIOException(); try { - EnsureUnixObject(next, rootDevice, expectedType: UnixDirectoryType); + EnsureUnixObject(next, rootIdentity, expectedType: UnixDirectoryType); } catch { @@ -595,7 +761,7 @@ private SafeFileHandle OpenUnixDirectory(string directory, out long rootDevice) } } - private SafeFileHandle OpenUnixRootDirectory(out long rootDevice) + private SafeFileHandle OpenUnixRootDirectory(out UnixBoundaryIdentity rootIdentity) { var descriptor = UnixOpen( CanonicalWorkspaceRoot, @@ -611,7 +777,14 @@ private SafeFileHandle OpenUnixRootDirectory(out long rootDevice) { throw CreateNativeIOException(); } - rootDevice = status.Device; + ulong? mountId = null; + if (RepositoryOutputPathBoundary.RequiresUnixMountIdentity) + { + if (!RepositoryOutputPathBoundary.TryGetUnixMountId(handle, out var resolvedMountId)) + throw CreateNativeIOException(); + mountId = resolvedMountId; + } + rootIdentity = new UnixBoundaryIdentity(status.Device, mountId); return handle; } catch @@ -635,11 +808,20 @@ private SafeFileHandle OpenUnixRootDirectory(out long rootDevice) throw CreateNativeIOException(); } - private static void EnsureUnixObject(SafeFileHandle handle, long rootDevice, int expectedType) + private static void EnsureUnixObject( + SafeFileHandle handle, + UnixBoundaryIdentity rootIdentity, + int expectedType) { if (UnixFStat(handle.DangerousGetHandle(), out var status) != 0 || (status.Mode & UnixFileTypeMask) != expectedType - || status.Device != rootDevice) + || status.Device != rootIdentity.Device) + { + throw CreateNativeIOException(); + } + if (rootIdentity.MountId.HasValue + && (!RepositoryOutputPathBoundary.TryGetUnixMountId(handle, out var mountId) + || mountId != rootIdentity.MountId.Value)) { throw CreateNativeIOException(); } @@ -709,6 +891,8 @@ private bool Allows(string path, bool expectsDirectory) private const int UnixDirectoryType = 0x4000; private const int UnixRegularFileType = 0x8000; + private readonly record struct UnixBoundaryIdentity(long Device, ulong? MountId); + private static int UnixCloseOnExec => OperatingSystem.IsMacOS() ? 0x01000000 : 0x00080000; private static int UnixNoFollow => OperatingSystem.IsMacOS() ? 0x00000100 : 0x00020000; private static int UnixDirectory => OperatingSystem.IsMacOS() ? 0x00100000 : 0x00010000; diff --git a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs index ad728e9789..ba306685dc 100644 --- a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs +++ b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs @@ -483,6 +483,140 @@ bool IgnoreCase(string path) } } + [Fact] + public void OutputBoundary_RejectsSameDeviceDistinctUnixMountIdentity_Issue5181Review() + { + if (OperatingSystem.IsWindows()) + return; + + var workspace = CreateTempDir(); + var mountedDirectory = Path.Combine(workspace, "mounted"); + try + { + Directory.CreateDirectory(mountedDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "mounted/new/metrics.jsonl" }"""); + RepositoryOutputPathBoundary.UnixMountIdForTesting = path => + string.Equals( + Path.GetFullPath(path), + mountedDirectory, + StringComparison.Ordinal) + ? 2UL + : 1UL; + + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + + Assert.True(result.Failed); + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, result.Error, StringComparison.Ordinal); + Assert.False(Directory.Exists(Path.Combine(mountedDirectory, "new"))); + Assert.Empty(result.Settings); + } + finally + { + RepositoryOutputPathBoundary.UnixMountIdForTesting = null; + TestProjectHelper.DeleteDirectory(workspace); + } + } + + [Fact] + public void OutputBoundary_RejectsDistinctMountIdentityFromOpenedAncestor_Issue5181Review() + { + if (OperatingSystem.IsWindows()) + return; + + var workspace = CreateTempDir(); + var safeDirectory = Path.Combine(workspace, "safe"); + var metricsPath = Path.Combine(safeDirectory, "metrics.jsonl"); + try + { + Directory.CreateDirectory(safeDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "safe/metrics.jsonl" }"""); + RepositoryOutputPathBoundary.UnixMountIdForTesting = _ => 1UL; + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + Assert.True(result.Loaded); + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + metricsPath, + destinationIsDirectory: false); + Assert.NotNull(boundary); + var handleProbeCount = 0; + RepositoryOutputPathBoundary.UnixHandleMountIdForTesting = _ => + Interlocked.Increment(ref handleProbeCount) == 1 ? 1UL : 2UL; + + var exception = Assert.Throws( + () => PrivateLogFile.OpenAppend(metricsPath, boundary: boundary)); + + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, exception.Message, StringComparison.Ordinal); + Assert.Equal(2, handleProbeCount); + Assert.False(File.Exists(metricsPath)); + } + finally + { + RepositoryOutputPathBoundary.UnixHandleMountIdForTesting = null; + RepositoryOutputPathBoundary.UnixMountIdForTesting = null; + TestProjectHelper.DeleteDirectory(workspace); + } + } + + [Fact] + public void OutputBoundary_GuardedRenameSurfacesParentFsyncFailure_Issue5181Review() + { + if (OperatingSystem.IsWindows()) + return; + + var workspace = CreateTempDir(); + var metricsPath = Path.Combine(workspace, "logs", "metrics.jsonl"); + Exception? rotationFailure = null; + try + { + Directory.CreateDirectory(Path.GetDirectoryName(metricsPath)!); + File.WriteAllText(metricsPath, "current"); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "logs/metrics.jsonl" }"""); + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + Assert.True(result.Loaded); + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + metricsPath, + destinationIsDirectory: false); + Assert.NotNull(boundary); + var fsyncCount = 0; + RepositoryOutputPathBoundary.UnixDirectoryFsyncForTesting = _ => + { + fsyncCount++; + return -1; + }; + + var rotated = PrivateLogFile.TryRotateSlots( + metricsPath, + retainedFileCount: 2, + onFailure: ex => rotationFailure = ex, + boundary: boundary); + + Assert.False(rotated); + var durabilityFailure = Assert.IsType(rotationFailure); + Assert.IsNotType(durabilityFailure); + Assert.Contains("already replaced", durabilityFailure.Message, StringComparison.Ordinal); + Assert.Contains("parent directory could not be flushed", durabilityFailure.Message, StringComparison.Ordinal); + Assert.Equal(1, fsyncCount); + Assert.False(File.Exists(metricsPath)); + Assert.Equal("current", File.ReadAllText(metricsPath + ".1")); + } + finally + { + RepositoryOutputPathBoundary.UnixDirectoryFsyncForTesting = null; + TestProjectHelper.DeleteDirectory(workspace); + } + } + [Fact] public void RepositoryOutputBoundary_RevalidatesAfterInjectedAncestorSwap_Issue5181() { From 5abae0b314d2379fa5be3719352b98c6ea324c67 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 26 Aug 2026 08:52:07 +0900 Subject: [PATCH 4/6] Close output mutation race windows (#5181) --- DEVELOPER_GUIDE.md | 15 +- changelog.d/unreleased/5181.security.md | 4 +- src/CodeIndex/Cli/GlobalToolLog.cs | 2 +- src/CodeIndex/Cli/PrivateLogFile.cs | 46 +- .../Cli/RepositoryOutputPathBoundary.cs | 469 ++++++++++++++++-- tests/CodeIndex.Tests/CdidxConfigFileTests.cs | 149 ++++++ 6 files changed, 621 insertions(+), 64 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index ec2f6179b0..5813aeb02f 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -2956,8 +2956,12 @@ cross-device mount point, reparse point, device, or dangling link. The boundary is revalidated before each mutation; on Linux, path and opened-handle mount IDs also reject same-device bind mounts. On POSIX, directory creation, append, permission changes, rotation, replacement, and deletion are additionally anchored -to the workspace directory handle with no-follow relative operations, and guarded -renames fsync their already-open destination parent before reporting success. The same +to the workspace directory handle with no-follow relative operations. Directory +parents are rebound from that root after mutation-time validation so a +moved ancestor is not reused. Guarded renames fsync their already-open destination +parent before reporting success. On Windows, root-handle-relative native opens reject +reparse points during name resolution, and guarded replacement and deletion remain +handle/root relative. The same `global_tool_log_dir` guard covers lifecycle logs, file query traces, and the bounded `last-failure.json` diagnostic. An unsafe value fails config validation with the bounded `unsafe_output_path` diagnostic and does not create, append, @@ -6973,8 +6977,11 @@ reparse point、device、dangling link のいずれかであれば拒否し、 直前にも境界を再検証します。Linux では path と open 済み handle の mount ID も 比較して同一 device の bind mount を拒否します。POSIX ではさらに directory 作成、 append、permission 変更、rotation、置換、delete を workspace directory handle 起点の -no-follow relative operation へ固定し、guarded rename は成功を返す前に open 済みの -destination parent を fsync します。同じ `global_tool_log_dir` guard を lifecycle log、file +no-follow relative operation へ固定し、mutation 時の検証後に directory parent を root +から再取得するため、外部へ移動済みの ancestor handle を再利用しません。guarded rename は +成功を返す前に open 済みの destination parent を fsync します。Windows では root handle +相対の native open が name resolution 中の reparse point を拒否し、guarded replacement / +delete も handle / root 相対で実行します。同じ `global_tool_log_dir` guard を lifecycle log、file query trace、上限付きの `last-failure.json` 診断にも適用します。安全でない値は 上限付きの `unsafe_output_path` 診断で config validation に失敗し、外部 target の 作成、追記、rotation、置換、削除、chmod は行いません。明示的な CLI と process diff --git a/changelog.d/unreleased/5181.security.md b/changelog.d/unreleased/5181.security.md index f2f409450d..6774f0cb3b 100644 --- a/changelog.d/unreleased/5181.security.md +++ b/changelog.d/unreleased/5181.security.md @@ -18,8 +18,8 @@ affected: ## English -- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, bind-mount, cross-device-mount, reparse-point, device, and dangling-link components below the config workspace. Mutations are revalidated, Linux path/open-handle mount IDs reject same-device bind mounts, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle; guarded renames also fsync the destination parent before returning success. The global-log guard covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. +- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, bind-mount, cross-device-mount, reparse-point, device/special-file, and dangling-link components below the config workspace. Mutations are revalidated, Linux path/open-handle mount IDs reject same-device bind mounts, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle with mutation-time parent rebinding; guarded renames also fsync the destination parent before returning success. Windows mutations use root-handle-relative no-reparse native operations. The global-log guard covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. ## 日本語 -- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、bind mount、cross-device mount、reparse point、device、dangling link を拒否します。mutation 前の再検証に加え、Linux では path / open 済み handle の mount ID で同一 device の bind mount も拒否し、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を workspace directory handle に固定します。guarded rename は成功を返す前に destination parent を fsync します。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 +- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、bind mount、cross-device mount、reparse point、device / special file、dangling link を拒否します。mutation 前の再検証に加え、Linux では path / open 済み handle の mount ID で同一 device の bind mount も拒否し、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を mutation 時の parent 再取得を伴う workspace directory handle に固定します。guarded rename は成功を返す前に destination parent を fsync します。Windows mutation は root handle 相対の no-reparse native operation を使います。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 diff --git a/src/CodeIndex/Cli/GlobalToolLog.cs b/src/CodeIndex/Cli/GlobalToolLog.cs index 70835ed1c0..dd435fefae 100644 --- a/src/CodeIndex/Cli/GlobalToolLog.cs +++ b/src/CodeIndex/Cli/GlobalToolLog.cs @@ -403,7 +403,7 @@ private static bool CanWriteProbe(string directory, RepositoryOutputPathGuard? b } boundary.PrepareMutation("write_probe_delete", probePath); if (OperatingSystem.IsWindows()) - File.Delete(LongPath.EnsureWindowsPrefix(probePath)); + boundary.DeleteFileWindows(probePath); else boundary.DeleteFileUnix(probePath); boundary.CompleteMutation(probePath); diff --git a/src/CodeIndex/Cli/PrivateLogFile.cs b/src/CodeIndex/Cli/PrivateLogFile.cs index d01fc97271..0679d3736e 100644 --- a/src/CodeIndex/Cli/PrivateLogFile.cs +++ b/src/CodeIndex/Cli/PrivateLogFile.cs @@ -18,9 +18,11 @@ internal static Stream OpenAppend( RejectUnsafeTarget(path); Stream stream; - if (boundary is not null && !OperatingSystem.IsWindows()) + if (boundary is not null) { - stream = boundary.OpenAppendUnix(path); + stream = OperatingSystem.IsWindows() + ? boundary.OpenAppendWindows(path, share) + : boundary.OpenAppendUnix(path); } else if (OperatingSystem.IsWindows()) { @@ -73,7 +75,7 @@ internal static void WritePrivateTextReplacing( boundary.PrepareMutation("write_private_text_stage", stagedPath); RejectUnsafeTarget(stagedPath); using (var stream = OperatingSystem.IsWindows() - ? new FileStream(stagedPath, FileMode.CreateNew, FileAccess.Write, FileShare.None) + ? boundary.OpenReplacingWindows(stagedPath, createNew: true) : boundary.OpenReplacingUnix(stagedPath)) { var encoded = Encoding.UTF8.GetBytes(contents); @@ -86,7 +88,7 @@ internal static void WritePrivateTextReplacing( boundary.PrepareMutation("write_private_text_publish_source", stagedPath); boundary.PrepareMutation("write_private_text_publish_destination", path); if (OperatingSystem.IsWindows()) - AtomicFileWriter.MoveReplacing(stagedPath, path); + boundary.MoveReplacingWindows(stagedPath, path); else boundary.MoveReplacingUnix(stagedPath, path); boundary.CompleteMutation(path); @@ -103,7 +105,7 @@ private static void TryDeleteStagedFile(string path, RepositoryOutputPathGuard b { boundary.PrepareMutation("write_private_text_cleanup", path); if (OperatingSystem.IsWindows()) - File.Delete(path); + boundary.DeleteFileWindows(path); else boundary.DeleteFileUnix(path); boundary.CompleteMutation(path); @@ -228,9 +230,12 @@ internal static void PruneOldFiles( if (ShouldPruneFile(file, retainedPaths, retainedFiles, retainedFileCount)) { boundary?.PrepareMutation("prune_old_file", file.FullName); - if (boundary is not null && !OperatingSystem.IsWindows()) + if (boundary is not null) { - boundary.DeleteFileUnix(file.FullName); + if (OperatingSystem.IsWindows()) + boundary.DeleteFileWindows(file.FullName); + else + boundary.DeleteFileUnix(file.FullName); } else { @@ -313,8 +318,13 @@ internal static bool TryRotateSlots( { var lastSlot = SlotPath(path, retainedFileCount - 1); boundary?.PrepareMutation("rotate_delete", lastSlot); - if (boundary is not null && !OperatingSystem.IsWindows()) - boundary.DeleteFileUnix(lastSlot); + if (boundary is not null) + { + if (OperatingSystem.IsWindows()) + boundary.DeleteFileWindows(lastSlot); + else + boundary.DeleteFileUnix(lastSlot); + } else AtomicFileWriter.TryDeleteFile(lastSlot, onCleanupFailure); boundary?.CompleteMutation(lastSlot); @@ -327,8 +337,13 @@ internal static bool TryRotateSlots( continue; boundary?.PrepareMutation("rotate_source", current); boundary?.PrepareMutation("rotate_destination", next); - if (boundary is not null && !OperatingSystem.IsWindows()) - boundary.MoveReplacingUnix(current, next); + if (boundary is not null) + { + if (OperatingSystem.IsWindows()) + boundary.MoveReplacingWindows(current, next); + else + boundary.MoveReplacingUnix(current, next); + } else AtomicFileWriter.MoveReplacing(current, next); boundary?.CompleteMutation(current); @@ -341,8 +356,13 @@ internal static bool TryRotateSlots( var first = SlotPath(path, 1); boundary?.PrepareMutation("rotate_source", path); boundary?.PrepareMutation("rotate_destination", first); - if (boundary is not null && !OperatingSystem.IsWindows()) - boundary.MoveReplacingUnix(path, first); + if (boundary is not null) + { + if (OperatingSystem.IsWindows()) + boundary.MoveReplacingWindows(path, first); + else + boundary.MoveReplacingUnix(path, first); + } else AtomicFileWriter.MoveReplacing(path, first); boundary?.CompleteMutation(path); diff --git a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs index 4dcde9c5e4..d64920c53d 100644 --- a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs +++ b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs @@ -1,4 +1,5 @@ using System.Runtime.InteropServices; +using System.Text; using CodeIndex.Indexer; using Microsoft.Win32.SafeHandles; @@ -20,6 +21,7 @@ internal static class RepositoryOutputPathBoundary private static readonly AsyncLocal?> UnixMountId = new(); private static readonly AsyncLocal?> UnixHandleMountId = new(); private static readonly AsyncLocal?> UnixDirectoryFsync = new(); + private static readonly AsyncLocal?> BeforeWindowsNativeMutation = new(); internal static Action? BeforeMutationForTesting { @@ -51,6 +53,12 @@ internal static Func? UnixDirectoryFsyncForTesting set => UnixDirectoryFsync.Value = value; } + internal static Action? BeforeWindowsNativeMutationForTesting + { + get => BeforeWindowsNativeMutation.Value; + set => BeforeWindowsNativeMutation.Value = value; + } + internal static bool TryResolveConfiguredPath( string rawPath, string workspaceRoot, @@ -163,7 +171,16 @@ internal static void ValidatePathComponents( throw CreateException("output path", UnsafeReason); } - var rootDevice = TryGetUnixDevice(normalizedRoot, out var device) ? device : (long?)null; + long? rootDevice = null; + if (!OperatingSystem.IsWindows()) + { + if (!TryGetUnixStatus(normalizedRoot, out var rootStatus) + || (rootStatus.Mode & UnixFileTypeMask) != UnixDirectoryType) + { + throw CreateException("output path", UnsafeReason); + } + rootDevice = rootStatus.Device; + } ulong? rootMountId = null; if (RequiresUnixMountIdentity) { @@ -208,8 +225,10 @@ internal static void ValidatePathComponents( throw CreateException("output path", UnsafeReason); if (rootDevice.HasValue - && TryGetUnixDevice(current, out var currentDevice) - && currentDevice != rootDevice.Value) + && (!TryGetUnixStatus(current, out var currentStatus) + || currentStatus.Device != rootDevice.Value + || (currentStatus.Mode & UnixFileTypeMask) + != (isDirectory ? UnixDirectoryType : UnixRegularFileType))) { throw CreateException("output path", UnsafeReason); } @@ -251,18 +270,15 @@ private static bool IsLinkOrReparseEntry(string path) } } - private static bool TryGetUnixDevice(string path, out long device) + private static bool TryGetUnixStatus(string path, out UnixFileStatus status) { - device = 0; + status = default; if (OperatingSystem.IsWindows()) return false; try { - if (UnixStat(LongPath.EnsureWindowsPrefix(path), out var status) != 0) - return false; - device = status.Device; - return true; + return UnixStat(LongPath.EnsureWindowsPrefix(path), out status) == 0; } catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) { @@ -404,6 +420,9 @@ private static extern int UnixStatX( private const int UnixAtSymlinkNoFollow = 0x100; private const int UnixAtEmptyPath = 0x1000; private const uint UnixStatXMountId = 0x1000; + private const int UnixFileTypeMask = 0xF000; + private const int UnixDirectoryType = 0x4000; + private const int UnixRegularFileType = 0x8000; [StructLayout(LayoutKind.Explicit, Size = 256)] private struct UnixStatXStatus @@ -497,6 +516,29 @@ internal Stream OpenAppendUnix(string path) } } + internal Stream OpenAppendWindows(string path, FileShare share) + { + if (!OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + var handle = OpenWindowsPath( + path, + WindowsFileAppendData | WindowsFileReadAttributes | WindowsFileWriteAttributes | WindowsSynchronize, + (uint)share, + WindowsFileOpenIf, + WindowsFileNormal, + WindowsFileNonDirectory | WindowsFileSynchronousIoNonAlert); + try + { + return new FileStream(handle, FileAccess.Write, bufferSize: 4096, isAsync: false); + } + catch + { + handle.Dispose(); + throw; + } + } + internal FileStream OpenReplacingUnix(string path) { if (OperatingSystem.IsWindows()) @@ -514,6 +556,29 @@ internal FileStream OpenReplacingUnix(string path) } } + internal FileStream OpenReplacingWindows(string path, bool createNew) + { + if (!OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + var handle = OpenWindowsPath( + path, + WindowsFileWriteData | WindowsFileReadAttributes | WindowsFileWriteAttributes | WindowsSynchronize, + shareAccess: 0, + createNew ? WindowsFileCreate : WindowsFileOverwriteIf, + WindowsFileNormal, + WindowsFileNonDirectory | WindowsFileSynchronousIoNonAlert); + try + { + return new FileStream(handle, FileAccess.Write, bufferSize: 4096, isAsync: false); + } + catch + { + handle.Dispose(); + throw; + } + } + internal void SetPrivateFileModeUnix(string path) { if (OperatingSystem.IsWindows()) @@ -538,6 +603,23 @@ internal bool DeleteFileUnix(string path) throw CreateNativeIOException(); } + internal bool DeleteFileWindows(string path) + { + if (!OperatingSystem.IsWindows()) + return false; + + using var root = OpenWindowsRootDirectory(); + var relativePath = GetWindowsRelativePath(path); + using var objectName = new WindowsObjectName(root, relativePath); + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting?.Invoke("delete", path); + var status = WindowsNtDeleteFile(ref objectName.Attributes); + if (status >= 0) + return true; + if (status is WindowsStatusObjectNameNotFound or WindowsStatusObjectPathNotFound) + return false; + throw CreateNativeIOException(); + } + internal void MoveReplacingUnix(string source, string destination) { if (OperatingSystem.IsWindows()) @@ -561,6 +643,65 @@ internal void MoveReplacingUnix(string source, string destination) } } + internal void MoveReplacingWindows(string source, string destination) + { + if (!OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + using var sourceHandle = OpenWindowsPath( + source, + WindowsDelete | WindowsFileReadAttributes | WindowsSynchronize, + WindowsShareRead | WindowsShareWrite | WindowsShareDelete, + WindowsFileOpen, + WindowsFileNormal, + WindowsFileNonDirectory | WindowsFileSynchronousIoNonAlert); + var destinationParentPath = Path.GetDirectoryName(Path.GetFullPath(destination)) + ?? throw RepositoryOutputPathBoundary.CreateException(FieldName, "invalid_path"); + using var destinationParent = PathCasing.PathsEqualByDirectoryNamespace( + WorkspaceRoot, + destinationParentPath) + ? OpenWindowsRootDirectory() + : OpenWindowsPath( + destinationParentPath, + WindowsFileListDirectory | WindowsFileReadAttributes | WindowsSynchronize, + WindowsShareRead | WindowsShareWrite | WindowsShareDelete, + WindowsFileOpen, + WindowsFileDirectory, + WindowsFileDirectoryOption | WindowsFileSynchronousIoNonAlert); + + var destinationName = Path.GetFileName(destination); + var encodedName = Encoding.Unicode.GetBytes(destinationName); + var rootOffset = IntPtr.Size; + var lengthOffset = rootOffset + IntPtr.Size; + var nameOffset = lengthOffset + sizeof(uint); + var buffer = Marshal.AllocHGlobal(nameOffset + encodedName.Length); + try + { + for (var index = 0; index < nameOffset; index++) + Marshal.WriteByte(buffer, index, 0); + Marshal.WriteByte(buffer, 0, 1); + Marshal.WriteIntPtr(buffer, rootOffset, destinationParent.DangerousGetHandle()); + Marshal.WriteInt32(buffer, lengthOffset, encodedName.Length); + Marshal.Copy(encodedName, 0, IntPtr.Add(buffer, nameOffset), encodedName.Length); + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting?.Invoke("rename", source); + using var verificationRoot = OpenWindowsRootDirectory(); + EnsureWindowsHandleUnderRoot(verificationRoot, sourceHandle); + EnsureWindowsHandleUnderRoot(verificationRoot, destinationParent); + var status = WindowsNtSetInformationFile( + sourceHandle, + out _, + buffer, + (uint)(nameOffset + encodedName.Length), + WindowsFileRenameInformation); + if (status < 0) + throw CreateNativeIOException(); + } + finally + { + Marshal.FreeHGlobal(buffer); + } + } + internal void ValidateRelatedPath(string path, bool expectsDirectory = false) { var fullPath = Path.GetFullPath(path); @@ -577,47 +718,38 @@ internal void ValidateRelatedPath(string path, bool expectsDirectory = false) private void CreateSensitiveDirectories(string directory) { - if (!OperatingSystem.IsWindows()) + if (OperatingSystem.IsWindows()) { - CreateSensitiveDirectoriesUnix(directory); + CreateSensitiveDirectoriesWindows(directory); return; } + CreateSensitiveDirectoriesUnix(directory); + } + + private void CreateSensitiveDirectoriesWindows(string directory) + { var normalizedDirectory = PathCasing.NormalizeBoundaryPath(directory); - var relative = Path.GetRelativePath(WorkspaceRoot, normalizedDirectory); - if (relative == ".") + var components = GetRelativeComponents(normalizedDirectory); + if (components.Length == 0) return; - var components = relative.Split( - [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], - StringSplitOptions.RemoveEmptyEntries); - var current = WorkspaceRoot; - foreach (var component in components) + for (var index = 0; index < components.Length; index++) { - current = Path.Combine(current, component); - RepositoryOutputPathBoundary.ValidatePathComponents(WorkspaceRoot, current, destinationIsDirectory: true); - var created = false; - if (!Directory.Exists(LongPath.EnsureWindowsPrefix(current))) - { - PrepareMutation("create_directory", current, expectsDirectory: true); - if (OperatingSystem.IsWindows()) - Directory.CreateDirectory(LongPath.EnsureWindowsPrefix(current)); - else - Directory.CreateDirectory(LongPath.EnsureWindowsPrefix(current), DataDirectorySecurity.PrivateDirectoryMode); - CompleteMutation(current, expectsDirectory: true); - created = true; - } - - if (!OperatingSystem.IsWindows() - && (created || string.Equals( - PathCasing.NormalizeBoundaryPath(current), - normalizedDirectory, - PathCasing.ComparisonFor(normalizedDirectory)))) - { - PrepareMutation("set_directory_permissions", current, expectsDirectory: true); - File.SetUnixFileMode(LongPath.EnsureWindowsPrefix(current), DataDirectorySecurity.PrivateDirectoryMode); - CompleteMutation(current, expectsDirectory: true); - } + var currentPath = Path.Combine(WorkspaceRoot, Path.Combine(components[..(index + 1)])); + PrepareMutation("create_directory", currentPath, expectsDirectory: true); + using var handle = OpenWindowsPath( + currentPath, + WindowsFileListDirectory + | WindowsFileAddSubdirectory + | WindowsFileReadAttributes + | WindowsFileWriteAttributes + | WindowsSynchronize, + WindowsShareRead | WindowsShareWrite | WindowsShareDelete, + WindowsFileOpenIf, + WindowsFileDirectory, + WindowsFileDirectoryOption | WindowsFileSynchronousIoNonAlert); + CompleteMutation(currentPath, expectsDirectory: true); } } @@ -641,15 +773,21 @@ private void CreateSensitiveDirectoriesUnix(string directory) if (next is null) { PrepareMutation("create_directory", currentPath, expectsDirectory: true); + var parentPath = index == 0 + ? WorkspaceRoot + : Path.Combine(WorkspaceRoot, Path.Combine(components[..index])); + using var reboundParent = OpenUnixDirectory(parentPath, out var reboundIdentity); + if (reboundIdentity != rootIdentity) + throw CreateNativeIOException(); if (UnixMkdirAt( - current.DangerousGetHandle().ToInt32(), + reboundParent.DangerousGetHandle().ToInt32(), component, (uint)DataDirectorySecurity.PrivateDirectoryMode) != 0 && Marshal.GetLastWin32Error() != 17) { throw CreateNativeIOException(); } - next = TryOpenUnixDirectoryAt(current, component) ?? throw CreateNativeIOException(); + next = TryOpenUnixDirectoryAt(reboundParent, component) ?? throw CreateNativeIOException(); CompleteMutation(currentPath, expectsDirectory: true); created = true; } @@ -658,6 +796,10 @@ private void CreateSensitiveDirectoriesUnix(string directory) if (created || index == components.Length - 1) { PrepareMutation("set_directory_permissions", currentPath, expectsDirectory: true); + next.Dispose(); + next = OpenUnixDirectory(currentPath, out var reboundIdentity); + if (reboundIdentity != rootIdentity) + throw CreateNativeIOException(); if (UnixFChmod( next.DangerousGetHandle().ToInt32(), (uint)DataDirectorySecurity.PrivateDirectoryMode) != 0) @@ -885,6 +1027,109 @@ private bool Allows(string path, bool expectsDirectory) out _); } + private SafeFileHandle OpenWindowsPath( + string path, + uint desiredAccess, + uint shareAccess, + uint disposition, + uint attributes, + uint options) + { + using var root = OpenWindowsRootDirectory(); + var relativePath = GetWindowsRelativePath(path); + using var objectName = new WindowsObjectName(root, relativePath); + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting?.Invoke("open", path); + var status = WindowsNtCreateFile( + out var handle, + desiredAccess, + ref objectName.Attributes, + out _, + IntPtr.Zero, + attributes, + shareAccess, + disposition, + options, + IntPtr.Zero, + 0); + if (status < 0) + { + handle?.Dispose(); + throw CreateNativeIOException(); + } + + try + { + EnsureWindowsHandleUnderRoot(root, handle); + return handle; + } + catch + { + handle.Dispose(); + throw; + } + } + + private SafeFileHandle OpenWindowsRootDirectory() + { + var handle = WindowsCreateFile( + LongPath.EnsureWindowsPrefix(CanonicalWorkspaceRoot), + WindowsFileListDirectory | WindowsFileReadAttributes | WindowsSynchronize, + WindowsShareRead | WindowsShareWrite | WindowsShareDelete, + IntPtr.Zero, + WindowsOpenExisting, + WindowsFileFlagBackupSemantics, + IntPtr.Zero); + if (handle.IsInvalid) + { + handle.Dispose(); + throw CreateNativeIOException(); + } + return handle; + } + + private string GetWindowsRelativePath(string path) + { + var components = GetRelativeComponents(PathCasing.NormalizeBoundaryPath(path)); + if (components.Length == 0) + throw RepositoryOutputPathBoundary.CreateException(FieldName, "invalid_path"); + return string.Join('\\', components); + } + + private static void EnsureWindowsHandleUnderRoot(SafeFileHandle root, SafeFileHandle handle) + { + var rootPath = GetWindowsFinalPath(root); + var handlePath = GetWindowsFinalPath(handle); + if (!RepositoryOutputPathBoundary.IsPathEqualOrParent(rootPath, handlePath)) + throw CreateNativeIOException(); + } + + private static string GetWindowsFinalPath(SafeFileHandle handle) + { + var capacity = 512; + while (capacity <= short.MaxValue) + { + var buffer = new StringBuilder(capacity); + var length = WindowsGetFinalPathNameByHandle(handle, buffer, (uint)buffer.Capacity, flags: 0); + if (length == 0) + throw CreateNativeIOException(); + if (length < buffer.Capacity) + return NormalizeWindowsFinalPath(buffer.ToString()); + capacity = checked((int)length + 1); + } + throw CreateNativeIOException(); + } + + private static string NormalizeWindowsFinalPath(string path) + { + const string extendedUncPrefix = @"\\?\UNC\"; + const string extendedPrefix = @"\\?\"; + if (path.StartsWith(extendedUncPrefix, StringComparison.OrdinalIgnoreCase)) + return @"\\" + path[extendedUncPrefix.Length..]; + if (path.StartsWith(extendedPrefix, StringComparison.OrdinalIgnoreCase)) + return path[extendedPrefix.Length..]; + return path; + } + private const int UnixReadOnly = 0; private const int UnixWriteOnly = 1; private const int UnixFileTypeMask = 0xF000; @@ -900,6 +1145,34 @@ private bool Allows(string path, bool expectsDirectory) private static int UnixAppend => OperatingSystem.IsMacOS() ? 0x00000008 : 0x00000400; private static int UnixTruncate => OperatingSystem.IsMacOS() ? 0x00000400 : 0x00000200; + private const uint WindowsFileListDirectory = 0x00000001; + private const uint WindowsFileWriteData = 0x00000002; + private const uint WindowsFileAddSubdirectory = 0x00000004; + private const uint WindowsFileAppendData = 0x00000004; + private const uint WindowsFileReadAttributes = 0x00000080; + private const uint WindowsFileWriteAttributes = 0x00000100; + private const uint WindowsDelete = 0x00010000; + private const uint WindowsSynchronize = 0x00100000; + private const uint WindowsShareRead = 0x00000001; + private const uint WindowsShareWrite = 0x00000002; + private const uint WindowsShareDelete = 0x00000004; + private const uint WindowsFileDirectory = 0x00000010; + private const uint WindowsFileNormal = 0x00000080; + private const uint WindowsFileOpen = 0x00000001; + private const uint WindowsFileCreate = 0x00000002; + private const uint WindowsFileOpenIf = 0x00000003; + private const uint WindowsFileOverwriteIf = 0x00000005; + private const uint WindowsFileDirectoryOption = 0x00000001; + private const uint WindowsFileSynchronousIoNonAlert = 0x00000020; + private const uint WindowsFileNonDirectory = 0x00000040; + private const uint WindowsOpenExisting = 3; + private const uint WindowsFileFlagBackupSemantics = 0x02000000; + private const uint WindowsObjectCaseInsensitive = 0x00000040; + private const uint WindowsObjectDontReparse = 0x00001000; + private const int WindowsFileRenameInformation = 10; + private const int WindowsStatusObjectNameNotFound = unchecked((int)0xC0000034); + private const int WindowsStatusObjectPathNotFound = unchecked((int)0xC000003A); + [DllImport("libc", EntryPoint = "open", SetLastError = true)] private static extern int UnixOpen(string path, int flags); @@ -925,6 +1198,114 @@ private static extern int UnixRenameAt( [DllImport("libSystem.Native", EntryPoint = "SystemNative_FStat", SetLastError = true)] private static extern int UnixFStat(IntPtr descriptor, out UnixFileStatus status); + [DllImport("kernel32.dll", EntryPoint = "CreateFileW", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern SafeFileHandle WindowsCreateFile( + string path, + uint desiredAccess, + uint shareMode, + IntPtr securityAttributes, + uint creationDisposition, + uint flagsAndAttributes, + IntPtr templateFile); + + [DllImport("kernel32.dll", EntryPoint = "GetFinalPathNameByHandleW", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern uint WindowsGetFinalPathNameByHandle( + SafeFileHandle handle, + StringBuilder path, + uint pathLength, + uint flags); + + [DllImport("ntdll.dll", EntryPoint = "NtCreateFile")] + private static extern int WindowsNtCreateFile( + out SafeFileHandle fileHandle, + uint desiredAccess, + ref WindowsObjectAttributes objectAttributes, + out WindowsIoStatusBlock ioStatusBlock, + IntPtr allocationSize, + uint fileAttributes, + uint shareAccess, + uint createDisposition, + uint createOptions, + IntPtr extendedAttributes, + uint extendedAttributesLength); + + [DllImport("ntdll.dll", EntryPoint = "NtDeleteFile")] + private static extern int WindowsNtDeleteFile(ref WindowsObjectAttributes objectAttributes); + + [DllImport("ntdll.dll", EntryPoint = "NtSetInformationFile")] + private static extern int WindowsNtSetInformationFile( + SafeFileHandle fileHandle, + out WindowsIoStatusBlock ioStatusBlock, + IntPtr fileInformation, + uint length, + int fileInformationClass); + + [StructLayout(LayoutKind.Sequential)] + private struct WindowsUnicodeString + { + internal ushort Length; + internal ushort MaximumLength; + internal IntPtr Buffer; + } + + [StructLayout(LayoutKind.Sequential)] + private struct WindowsObjectAttributes + { + internal uint Length; + internal IntPtr RootDirectory; + internal IntPtr ObjectName; + internal uint Attributes; + internal IntPtr SecurityDescriptor; + internal IntPtr SecurityQualityOfService; + } + + [StructLayout(LayoutKind.Sequential)] + private struct WindowsIoStatusBlock + { + internal IntPtr Status; + internal UIntPtr Information; + } + + private sealed class WindowsObjectName : IDisposable + { + private readonly IntPtr _nameBuffer; + private readonly IntPtr _unicodeString; + + internal WindowsObjectName(SafeFileHandle root, string relativePath) + { + var byteLength = checked(relativePath.Length * sizeof(char)); + if (byteLength > ushort.MaxValue - sizeof(char)) + throw RepositoryOutputPathBoundary.CreateException("output path", "invalid_path"); + + _nameBuffer = Marshal.StringToHGlobalUni(relativePath); + _unicodeString = Marshal.AllocHGlobal(Marshal.SizeOf()); + Marshal.StructureToPtr( + new WindowsUnicodeString + { + Length = (ushort)byteLength, + MaximumLength = (ushort)(byteLength + sizeof(char)), + Buffer = _nameBuffer, + }, + _unicodeString, + fDeleteOld: false); + Attributes = new WindowsObjectAttributes + { + Length = (uint)Marshal.SizeOf(), + RootDirectory = root.DangerousGetHandle(), + ObjectName = _unicodeString, + Attributes = WindowsObjectCaseInsensitive | WindowsObjectDontReparse, + }; + } + + internal WindowsObjectAttributes Attributes; + + public void Dispose() + { + Marshal.FreeHGlobal(_unicodeString); + Marshal.FreeHGlobal(_nameBuffer); + } + } + [StructLayout(LayoutKind.Sequential)] private struct UnixFileStatus { diff --git a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs index ba306685dc..87333c6d6a 100644 --- a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs +++ b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs @@ -617,6 +617,152 @@ public void OutputBoundary_GuardedRenameSurfacesParentFsyncFailure_Issue5181Revi } } + [Fact] + public void OutputBoundary_RebindsParentAfterAncestorRenameAndReplacement_Issue5181Review() + { + if (OperatingSystem.IsWindows()) + return; + + var workspace = CreateTempDir(); + var outside = CreateTempDir(); + var safeDirectory = Path.Combine(workspace, "safe"); + var movedDirectory = Path.Combine(outside, "moved"); + var metricsPath = Path.Combine(safeDirectory, "new", "metrics.jsonl"); + try + { + Directory.CreateDirectory(safeDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "safe/new/metrics.jsonl" }"""); + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + Assert.True(result.Loaded); + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + metricsPath, + destinationIsDirectory: false); + Assert.NotNull(boundary); + var swapped = false; + RepositoryOutputPathBoundary.BeforeMutationForTesting = (operation, _) => + { + if (swapped || operation != "create_directory") + return; + swapped = true; + Directory.Move(safeDirectory, movedDirectory); + Directory.CreateDirectory(safeDirectory); + }; + + boundary!.CreateSensitiveDestinationDirectory(); + + Assert.True(swapped); + Assert.True(Directory.Exists(Path.Combine(safeDirectory, "new"))); + Assert.False(Directory.Exists(Path.Combine(movedDirectory, "new"))); + } + finally + { + RepositoryOutputPathBoundary.BeforeMutationForTesting = null; + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + + [Fact] + public void OutputBoundary_WindowsNativeOpenRejectsJunctionSwapAfterPrepare_Issue5181Review() + { + if (!OperatingSystem.IsWindows()) + return; + + var workspace = CreateTempDir(); + var outside = CreateTempDir(); + var safeDirectory = Path.Combine(workspace, "safe"); + var originalDirectory = Path.Combine(workspace, "safe-original"); + var metricsPath = Path.Combine(safeDirectory, "new", "metrics.jsonl"); + var probeLink = Path.Combine(workspace, "junction-probe"); + try + { + if (!TryCreateDirectoryLink(probeLink, outside)) + return; + DeleteLinkEntry(probeLink); + Directory.CreateDirectory(safeDirectory); + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "safe/new/metrics.jsonl" }"""); + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + Assert.True(result.Loaded); + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + metricsPath, + destinationIsDirectory: false); + Assert.NotNull(boundary); + var swapped = false; + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting = (operation, path) => + { + if (swapped + || operation != "open" + || !PathCasing.PathsEqualByDirectoryNamespace(path, safeDirectory)) + { + return; + } + Directory.Move(safeDirectory, originalDirectory); + Directory.CreateSymbolicLink(safeDirectory, outside); + swapped = true; + }; + + var exception = Assert.Throws( + () => boundary!.CreateSensitiveDestinationDirectory()); + + Assert.True(swapped); + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, exception.Message, StringComparison.Ordinal); + Assert.False(Directory.Exists(Path.Combine(outside, "new"))); + Assert.False(File.Exists(Path.Combine(outside, "new", "metrics.jsonl"))); + } + finally + { + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting = null; + DeleteLinkEntry(safeDirectory); + TestProjectHelper.DeleteDirectory(workspace); + TestProjectHelper.DeleteDirectory(outside); + } + } + + [Fact] + public void LoadAndApply_OutputPathAtUnixFifoIsRejectedBeforeOpen_Issue5181Review() + { + if (OperatingSystem.IsWindows()) + return; + + var workspace = CreateTempDir(); + var fifoPath = Path.Combine(workspace, "metrics.jsonl"); + try + { + try + { + if (Mkfifo(fifoPath, 0x180) != 0) + return; + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + return; + } + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "metrics.jsonl" }"""); + + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + + Assert.True(result.Failed); + Assert.Contains(RepositoryOutputPathBoundary.UnsafeReason, result.Error, StringComparison.Ordinal); + Assert.Empty(result.Settings); + } + finally + { + TestProjectHelper.DeleteDirectory(workspace); + } + } + [Fact] public void RepositoryOutputBoundary_RevalidatesAfterInjectedAncestorSwap_Issue5181() { @@ -1522,6 +1668,9 @@ private static void DeleteLinkEntry(string path) private static (int ExitCode, string Stdout, string Stderr) CaptureConsole(Func action) => ConsoleCapture.Capture(action); + [System.Runtime.InteropServices.DllImport("libc", EntryPoint = "mkfifo", SetLastError = true)] + private static extern int Mkfifo(string path, uint mode); + private sealed class TestEnvironment { private readonly Dictionary _env; From b80a37ddcb452870f01c6de181b20e57ed71220f Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 26 Aug 2026 09:24:12 +0900 Subject: [PATCH 5/6] Anchor Windows workspace root aliases (#5181) --- DEVELOPER_GUIDE.md | 14 +++-- changelog.d/unreleased/5181.security.md | 4 +- .../Cli/RepositoryOutputPathBoundary.cs | 21 ++++++- tests/CodeIndex.Tests/CdidxConfigFileTests.cs | 55 +++++++++++++++++++ 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 5813aeb02f..9b35710344 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -2959,9 +2959,10 @@ permission changes, rotation, replacement, and deletion are additionally anchore to the workspace directory handle with no-follow relative operations. Directory parents are rebound from that root after mutation-time validation so a moved ancestor is not reused. Guarded renames fsync their already-open destination -parent before reporting success. On Windows, root-handle-relative native opens reject -reparse points during name resolution, and guarded replacement and deletion remain -handle/root relative. The same +parent before reporting success. On Windows, the guard retains the workspace root's +final physical path so a retargeted root alias is not followed; root-handle-relative +native opens reject reparse points during name resolution, and guarded replacement +and deletion remain handle/root relative. The same `global_tool_log_dir` guard covers lifecycle logs, file query traces, and the bounded `last-failure.json` diagnostic. An unsafe value fails config validation with the bounded `unsafe_output_path` diagnostic and does not create, append, @@ -6979,9 +6980,10 @@ reparse point、device、dangling link のいずれかであれば拒否し、 append、permission 変更、rotation、置換、delete を workspace directory handle 起点の no-follow relative operation へ固定し、mutation 時の検証後に directory parent を root から再取得するため、外部へ移動済みの ancestor handle を再利用しません。guarded rename は -成功を返す前に open 済みの destination parent を fsync します。Windows では root handle -相対の native open が name resolution 中の reparse point を拒否し、guarded replacement / -delete も handle / root 相対で実行します。同じ `global_tool_log_dir` guard を lifecycle log、file +成功を返す前に open 済みの destination parent を fsync します。Windows では workspace +root の最終的な物理 path を guard が保持するため、差し替えられた root alias を再追跡しません。 +root handle 相対の native open は name resolution 中の reparse point を拒否し、guarded +replacement / delete も handle / root 相対で実行します。同じ `global_tool_log_dir` guard を lifecycle log、file query trace、上限付きの `last-failure.json` 診断にも適用します。安全でない値は 上限付きの `unsafe_output_path` 診断で config validation に失敗し、外部 target の 作成、追記、rotation、置換、削除、chmod は行いません。明示的な CLI と process diff --git a/changelog.d/unreleased/5181.security.md b/changelog.d/unreleased/5181.security.md index 6774f0cb3b..86002b3a57 100644 --- a/changelog.d/unreleased/5181.security.md +++ b/changelog.d/unreleased/5181.security.md @@ -18,8 +18,8 @@ affected: ## English -- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, bind-mount, cross-device-mount, reparse-point, device/special-file, and dangling-link components below the config workspace. Mutations are revalidated, Linux path/open-handle mount IDs reject same-device bind mounts, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle with mutation-time parent rebinding; guarded renames also fsync the destination parent before returning success. Windows mutations use root-handle-relative no-reparse native operations. The global-log guard covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. +- **Repository-configured metrics and global-log paths can no longer escape through filesystem aliases (#5181)** — `metrics_path` and `global_tool_log_dir` now reject symbolic-link, junction, bind-mount, cross-device-mount, reparse-point, device/special-file, and dangling-link components below the config workspace. Mutations are revalidated, Linux path/open-handle mount IDs reject same-device bind mounts, and POSIX writes, directory creation, permission changes, rotation, replacement, and deletion are anchored to the workspace directory handle with mutation-time parent rebinding; guarded renames also fsync the destination parent before returning success. Windows guards retain the workspace root's final physical path and use root-handle-relative no-reparse native operations, so retargeting a root alias cannot redirect later mutations. The global-log guard covers query traces and `last-failure.json`, so an untrusted checkout cannot redirect them to an external target. ## 日本語 -- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、bind mount、cross-device mount、reparse point、device / special file、dangling link を拒否します。mutation 前の再検証に加え、Linux では path / open 済み handle の mount ID で同一 device の bind mount も拒否し、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を mutation 時の parent 再取得を伴う workspace directory handle に固定します。guarded rename は成功を返す前に destination parent を fsync します。Windows mutation は root handle 相対の no-reparse native operation を使います。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 +- **repository config の metrics / global-log path が filesystem alias を経由して外部へ逸脱できないようになりました (#5181)** — `metrics_path` と `global_tool_log_dir` は config workspace 配下の symbolic link、junction、bind mount、cross-device mount、reparse point、device / special file、dangling link を拒否します。mutation 前の再検証に加え、Linux では path / open 済み handle の mount ID で同一 device の bind mount も拒否し、POSIX の書き込み、directory 作成、permission 変更、rotation、置換、削除を mutation 時の parent 再取得を伴う workspace directory handle に固定します。guarded rename は成功を返す前に destination parent を fsync します。Windows guard は workspace root の最終的な物理 path を保持し、root handle 相対の no-reparse native operation を使うため、root alias を差し替えても後続 mutation を外部へリダイレクトできません。global-log guard は query trace と `last-failure.json` にも適用し、信頼できない checkout が外部 target へリダイレクトできないようにしました。 diff --git a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs index d64920c53d..3affddd325 100644 --- a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs +++ b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs @@ -131,7 +131,7 @@ internal static RepositoryOutputPathBoundaryException CreateException(string fie internal static string ResolveCanonicalWorkspaceRoot(string workspaceRoot) { if (OperatingSystem.IsWindows()) - return PathCasing.NormalizeBoundaryPath(workspaceRoot); + return RepositoryOutputPathGuard.ResolveCanonicalWindowsDirectory(workspaceRoot); IntPtr pointer = IntPtr.Zero; try @@ -478,6 +478,25 @@ internal RepositoryOutputPathGuard( internal string DestinationPath { get; } internal bool DestinationIsDirectory { get; } + internal static string ResolveCanonicalWindowsDirectory(string path) + { + if (!OperatingSystem.IsWindows()) + throw new PlatformNotSupportedException(); + + using var handle = WindowsCreateFile( + LongPath.EnsureWindowsPrefix(path), + WindowsFileListDirectory | WindowsFileReadAttributes | WindowsSynchronize, + WindowsShareRead | WindowsShareWrite | WindowsShareDelete, + IntPtr.Zero, + WindowsOpenExisting, + WindowsFileFlagBackupSemantics, + IntPtr.Zero); + if (handle.IsInvalid) + throw CreateNativeIOException(); + + return PathCasing.NormalizeBoundaryPath(GetWindowsFinalPath(handle)); + } + internal void CreateSensitiveDestinationDirectory() { var directory = DestinationIsDirectory diff --git a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs index 87333c6d6a..e48ddf6588 100644 --- a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs +++ b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs @@ -431,6 +431,61 @@ public void LoadAndApply_WorkspaceRootAliasStillAllowsOrdinaryContainedOutput_Is } } + [Fact] + public void OutputBoundary_WindowsWorkspaceRootAliasRetargetUsesOriginalTarget_Issue5181Review() + { + if (!OperatingSystem.IsWindows()) + return; + + var container = CreateTempDir(); + var physicalWorkspace = Path.Combine(container, "physical"); + var workspaceAlias = Path.Combine(container, "workspace-alias"); + var outside = CreateTempDir(); + try + { + Directory.CreateDirectory(physicalWorkspace); + if (!TryCreateDirectoryLink(workspaceAlias, physicalWorkspace)) + return; + + File.WriteAllText( + Path.Combine(physicalWorkspace, CdidxConfigFile.FileName), + """{ "metrics_path": "safe/metrics.jsonl" }"""); + var result = CdidxConfigFile.Load(workspaceAlias, new TestEnvironment().Read); + Assert.True(result.Loaded); + using var environment = CdidxEnvironment.Push(result.Settings, result.Sources); + var metricsPath = result.Settings[MetricsSink.EnvVarName]; + var boundary = RepositoryOutputPathBoundary.CreateGuardForConfigSource( + MetricsSink.EnvVarName, + "metrics_path", + metricsPath, + destinationIsDirectory: false); + Assert.NotNull(boundary); + var swapped = false; + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting = (operation, _) => + { + if (swapped || operation != "open") + return; + + DeleteLinkEntry(workspaceAlias); + Assert.True(TryCreateDirectoryLink(workspaceAlias, outside)); + swapped = true; + }; + + boundary!.CreateSensitiveDestinationDirectory(); + + Assert.True(swapped); + Assert.True(Directory.Exists(Path.Combine(physicalWorkspace, "safe"))); + Assert.False(Directory.Exists(Path.Combine(outside, "safe"))); + } + finally + { + RepositoryOutputPathBoundary.BeforeWindowsNativeMutationForTesting = null; + DeleteLinkEntry(workspaceAlias); + TestProjectHelper.DeleteDirectory(container); + TestProjectHelper.DeleteDirectory(outside); + } + } + [Fact] public void OutputBoundary_RejectsCaseOnlySiblingInDistinctParentNamespace_Issue5181Review() { From af02574c61b4a2a72b9d0de1021912038be8d654 Mon Sep 17 00:00:00 2001 From: Widthdom Date: Wed, 26 Aug 2026 09:53:28 +0900 Subject: [PATCH 6/6] Add Linux mount identity fallback (#5181) --- DEVELOPER_GUIDE.md | 6 +- USER_GUIDE.md | 14 ++++ .../Cli/RepositoryOutputPathBoundary.cs | 81 +++++++++++++++---- tests/CodeIndex.Tests/CdidxConfigFileTests.cs | 55 +++++++++++++ 4 files changed, 139 insertions(+), 17 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 9b35710344..26e19041c2 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -2954,7 +2954,8 @@ next candidate instead of losing the first log write. Repository-configured the config workspace is rejected when it is a symbolic link, junction, bind or cross-device mount point, reparse point, device, or dangling link. The boundary is revalidated before each mutation; on Linux, path and opened-handle mount IDs -also reject same-device bind mounts. On POSIX, directory creation, append, +from `statx` or the `/proc/self/fdinfo` fallback also reject same-device bind +mounts. On POSIX, directory creation, append, permission changes, rotation, replacement, and deletion are additionally anchored to the workspace directory handle with no-follow relative operations. Directory parents are rebound from that root after mutation-time validation so a @@ -6975,7 +6976,8 @@ read-only な state/cache/runtime mount は最初の log write を失うので `global_tool_log_dir` には、より厳格な境界を適用します。config workspace 配下の既存 component が symbolic link、junction、bind mount / cross-device mount point、 reparse point、device、dangling link のいずれかであれば拒否し、各 mutation の -直前にも境界を再検証します。Linux では path と open 済み handle の mount ID も +直前にも境界を再検証します。Linux では `statx` または +`/proc/self/fdinfo` fallback から得た path と open 済み handle の mount ID も 比較して同一 device の bind mount を拒否します。POSIX ではさらに directory 作成、 append、permission 変更、rotation、置換、delete を workspace directory handle 起点の no-follow relative operation へ固定し、mutation 時の検証後に directory parent を root diff --git a/USER_GUIDE.md b/USER_GUIDE.md index b0d581692e..a2fdbe6836 100644 --- a/USER_GUIDE.md +++ b/USER_GUIDE.md @@ -2706,6 +2706,13 @@ Run `cdidx status --log-path` to print the active log directory without opening ### Project-local configuration file (`.cdidx/config.json` / `.cdidxrc.json`) +Repository-configured `metrics_path` and `global_tool_log_dir` values must remain +inside the config workspace. Existing components below that workspace must not be +symbolic links, junctions, bind or cross-device mount points, reparse points, +devices or special files, or dangling links; unsafe values fail validation with +`unsafe_output_path`. Use the CLI flag or a real environment variable when an +outside destination is intentional. + You can check a `.cdidx/config.json` or `.cdidxrc.json` file into a repository to set per-project defaults instead of relying on shell-profile or CI env vars (#1571). Before a config-dependent command runs, `cdidx` walks upward from the current working directory looking for the first project config file, validates its schema, and materializes recognized keys as scoped environment settings — so every existing env-var consumer picks them up without process-global mutation. Static commands that do not consume project settings (`license`, `--version`, help forms, shell completions, and any command's `--help`) skip config discovery and remain usable even when an unrelated project config is malformed. Discovery stops after checking a directory that contains `.git`, `.hg`, `.svn`, `cdidx.workspace.json`, or `.cdidx-workspace.json`, so a child workspace does not inherit a config file from an unrelated parent. Precedence is **CLI flag > environment variable > config file > built-in default**. A config-file value is applied only when the matching env var is not already set in the process, so a value the user already exported in the shell or CI always wins. Config JSON is bounded to 64 KiB and a conservative nesting depth before schema validation. For config-dependent commands, a malformed file (invalid JSON, unknown key, wrong type, or excessive nesting) is a hard error: cdidx exits `1` with the file path and all detected offending fields. JSON mode returns the versioned command-error envelope with `error_code: "E024_CONFIG_INVALID"` and `category: "configuration"` instead of writing human-only text to stderr. Set `CDIDX_DISABLE_CONFIG_FILE=1` to bypass the file entirely. @@ -6308,6 +6315,13 @@ MCP のレスポンスサイズ上限は、環境変数 override で guard が ### プロジェクト固有の設定ファイル (`.cdidx/config.json` / `.cdidxrc.json`) +repository config の `metrics_path` と `global_tool_log_dir` は config workspace +内に収まる必要があります。workspace 配下の既存 component には symbolic link、 +junction、bind mount または cross-device mount、reparse point、device / special +file、dangling link を含めることはできず、安全でない値は +`unsafe_output_path` で検証に失敗します。意図的に外部の出力先を使う場合は CLI +flag または実際の環境変数を使ってください。 + シェルプロファイルや CI の環境変数に頼らず、プロジェクトごとの既定値を `.cdidx/config.json` または `.cdidxrc.json` ファイルとしてリポジトリにチェックインできます (#1571)。config に依存する command の実行前に、`cdidx` はカレントディレクトリから上方向に最初のプロジェクト設定ファイルを探索し、スキーマを検証してから既知のキーを scoped environment setting として注入します。これにより、process-global な環境変数を変更せずに、既存の環境変数コンシューマが同じ値を受け取れます。プロジェクト設定を使用しない static command（`license`、`--version`、help 形式、shell completion、および各 command の `--help`）は config 探索を省略するため、無関係なプロジェクト設定が不正でも利用できます。探索は `.git`、`.hg`、`.svn`、`cdidx.workspace.json`、`.cdidx-workspace.json` を含むディレクトリを確認した後で停止するため、子 workspace が無関係な親ディレクトリの設定ファイルを継承しません。 優先順位は **CLI フラグ > 環境変数 > 設定ファイル > 組み込み既定値** です。設定ファイル由来の値は、対応する環境変数がプロセスで未設定の場合にのみ適用されるため、シェルや CI で既に export されている値が常に優先されます。設定 JSON はスキーマ検証前に 64 KiB と保守的なネスト深度の上限で検査されます。config に依存する command では、不正なファイル（無効な JSON、未知のキー、型違い、過度なネスト）は hard error として扱われ、cdidx はファイルパスと検出できた該当フィールドすべてを示して終了コード `1` で終了します。JSON mode では human-only text を stderr に書く代わりに、`error_code: "E024_CONFIG_INVALID"` と `category: "configuration"` を持つ versioned command-error envelope を返します。完全にバイパスしたい場合は `CDIDX_DISABLE_CONFIG_FILE=1` を設定してください。 diff --git a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs index 3affddd325..527ca9e8d8 100644 --- a/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs +++ b/src/CodeIndex/Cli/RepositoryOutputPathBoundary.cs @@ -302,7 +302,9 @@ internal static bool TryGetUnixMountId(string path, out ulong mountId) mountId = value.Value; return true; } - return false; + + return OperatingSystem.IsLinux() + && TryGetLinuxMountIdFromPathDescriptor(path, out mountId); } if (!OperatingSystem.IsLinux()) @@ -315,19 +317,18 @@ internal static bool TryGetUnixMountId(string path, out ulong mountId) path, UnixAtSymlinkNoFollow, UnixStatXMountId, - out var status) != 0 - || (status.Mask & UnixStatXMountId) == 0) + out var status) == 0 + && (status.Mask & UnixStatXMountId) != 0) { - return false; + mountId = status.MountId; + return true; } - - mountId = status.MountId; - return true; } catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) { - return false; } + + return TryGetLinuxMountIdFromPathDescriptor(path, out mountId); } internal static bool TryGetUnixMountId(SafeFileHandle handle, out ulong mountId) @@ -342,7 +343,9 @@ internal static bool TryGetUnixMountId(SafeFileHandle handle, out ulong mountId) mountId = value.Value; return true; } - return false; + + return OperatingSystem.IsLinux() + && TryGetLinuxMountIdFromFdInfo(descriptor, out mountId); } if (!OperatingSystem.IsLinux()) @@ -355,16 +358,58 @@ internal static bool TryGetUnixMountId(SafeFileHandle handle, out ulong mountId) string.Empty, UnixAtEmptyPath | UnixAtSymlinkNoFollow, UnixStatXMountId, - out var status) != 0 - || (status.Mask & UnixStatXMountId) == 0) + out var status) == 0 + && (status.Mask & UnixStatXMountId) != 0) { - return false; + mountId = status.MountId; + return true; } - - mountId = status.MountId; - return true; } catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + } + + return TryGetLinuxMountIdFromFdInfo(descriptor, out mountId); + } + + internal static bool TryParseLinuxFdInfoMountId(string fdInfo, out ulong mountId) + { + mountId = 0; + using var reader = new StringReader(fdInfo); + while (reader.ReadLine() is { } line) + { + const string Prefix = "mnt_id:"; + if (!line.StartsWith(Prefix, StringComparison.Ordinal)) + continue; + + return ulong.TryParse(line.AsSpan(Prefix.Length).Trim(), out mountId); + } + + return false; + } + + private static bool TryGetLinuxMountIdFromPathDescriptor(string path, out ulong mountId) + { + mountId = 0; + var descriptor = UnixOpenMountProbe( + path, + UnixPathOnly | UnixOpenCloseOnExec | UnixOpenNoFollow); + if (descriptor < 0) + return false; + + using var handle = new SafeFileHandle(new IntPtr(descriptor), ownsHandle: true); + return TryGetLinuxMountIdFromFdInfo(descriptor, out mountId); + } + + private static bool TryGetLinuxMountIdFromFdInfo(int descriptor, out ulong mountId) + { + mountId = 0; + try + { + var fdInfo = File.ReadAllText($"/proc/self/fdinfo/{descriptor}"); + return TryParseLinuxFdInfoMountId(fdInfo, out mountId); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return false; } @@ -416,9 +461,15 @@ private static extern int UnixStatX( [DllImport("libc", EntryPoint = "fsync", SetLastError = true)] private static extern int UnixFsync(int descriptor); + [DllImport("libc", EntryPoint = "open", SetLastError = true)] + private static extern int UnixOpenMountProbe(string path, int flags); + private const int UnixCurrentWorkingDirectory = -100; private const int UnixAtSymlinkNoFollow = 0x100; private const int UnixAtEmptyPath = 0x1000; + private const int UnixPathOnly = 0x200000; + private const int UnixOpenCloseOnExec = 0x80000; + private const int UnixOpenNoFollow = 0x20000; private const uint UnixStatXMountId = 0x1000; private const int UnixFileTypeMask = 0xF000; private const int UnixDirectoryType = 0x4000; diff --git a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs index e48ddf6588..cca52ead2a 100644 --- a/tests/CodeIndex.Tests/CdidxConfigFileTests.cs +++ b/tests/CodeIndex.Tests/CdidxConfigFileTests.cs @@ -574,6 +574,61 @@ public void OutputBoundary_RejectsSameDeviceDistinctUnixMountIdentity_Issue5181R } } + [Theory] + [InlineData("pos:\t0\nflags:\t0100000\nmnt_id:\t42\nino:\t123\n", 42UL)] + [InlineData("mnt_id: 18446744073709551615\n", ulong.MaxValue)] + public void OutputBoundary_ParsesLinuxFdInfoMountIdentityFallback_Issue5181Review( + string fdInfo, + ulong expectedMountId) + { + var parsed = RepositoryOutputPathBoundary.TryParseLinuxFdInfoMountId( + fdInfo, + out var mountId); + + Assert.True(parsed); + Assert.Equal(expectedMountId, mountId); + } + + [Theory] + [InlineData("")] + [InlineData("pos:\t0\nino:\t123\n")] + [InlineData("mnt_id:\tnot-a-number\n")] + [InlineData("mnt_id:\t18446744073709551616\n")] + public void OutputBoundary_RejectsInvalidLinuxFdInfoMountIdentityFallback_Issue5181Review( + string fdInfo) + { + Assert.False( + RepositoryOutputPathBoundary.TryParseLinuxFdInfoMountId(fdInfo, out _)); + } + + [Fact] + public void OutputBoundary_UsesLinuxFdInfoWhenStatxMountIdentityIsUnavailable_Issue5181Review() + { + if (!OperatingSystem.IsLinux()) + return; + + var workspace = CreateTempDir(); + try + { + File.WriteAllText( + Path.Combine(workspace, CdidxConfigFile.FileName), + """{ "metrics_path": "logs/metrics.jsonl" }"""); + RepositoryOutputPathBoundary.UnixMountIdForTesting = _ => null; + + var result = CdidxConfigFile.Load(workspace, new TestEnvironment().Read); + + Assert.True(result.Loaded); + Assert.Equal( + Path.Combine(workspace, "logs", "metrics.jsonl"), + result.Settings[MetricsSink.EnvVarName]); + } + finally + { + RepositoryOutputPathBoundary.UnixMountIdForTesting = null; + TestProjectHelper.DeleteDirectory(workspace); + } + } + [Fact] public void OutputBoundary_RejectsDistinctMountIdentityFromOpenedAncestor_Issue5181Review() {