From 9e7e524e257384f305f13566c4445152ae000943 Mon Sep 17 00:00:00 2001 From: anderson-joyle Date: Fri, 10 Jul 2026 10:53:41 -0500 Subject: [PATCH] Redact PII (file paths, URIs, GUIDs, component names) from LSP telemetry Route language-server log call sites that leaked full file URIs/paths, environment/bot GUIDs, and agent/component file names into telemetry through the existing LogSensitiveInformation mechanism (or in-place redaction where that API isn't available). Geo information is unaffected (it derives from App Insights IP geolocation, not log messages). - LanguageServer: "Failed to get language" now logs a URI-free Error plus the full URI via LogSensitiveInformation (debug-only). - LspUriFactory: unsupported-scheme sample redacted to scheme-only in telemetry. - LspExceptionHandler: File/DirectoryNotFound log a path-free Error; full message still returned to the client and surfaced locally. - DidChangeWatchedFilesHandler: file-name log lines redacted (Info lines via LogSensitiveInformation; the "file does not exist" warning drops the name, preserved in debug). - LoggingHttpHandler: GUIDs in request paths replaced with {id}. - HostApplicationBuilderExtensions: IPC startup logs transport kind only. Updated ChangeMethodTests (per-file message now surfaces at Info) and added a GUID-redaction test for LoggingHttpHandler. Full unit suite: 873 passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f6d50381-bfe9-4a08-9a2d-d650cd7f5b42 --- .../HostApplicationBuilderExtensions.cs | 4 ++-- .../Handlers/DidChangeWatchedFilesHandler.cs | 22 ++++++++++++++----- .../Impl.Core/LSP/LanguageServer.cs | 5 ++++- .../Impl.Core/LSP/Uris/LspUriFactory.cs | 5 ++++- .../Impl.PullAgent/LoggingHttpHandler.cs | 8 ++++++- .../Impl.PullAgent/LspExceptionHandler.cs | 17 ++++++++++++-- .../Impl.Core/ChangeMethodTests.cs | 4 +++- .../Impl.PullAgent/LoggingHttpHandlerTests.cs | 14 ++++++++++++ 8 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/LanguageServers/PowerPlatformLS/Impl.Core/DependencyInjection/HostApplicationBuilderExtensions.cs b/src/LanguageServers/PowerPlatformLS/Impl.Core/DependencyInjection/HostApplicationBuilderExtensions.cs index dab07b69..3dae94c8 100644 --- a/src/LanguageServers/PowerPlatformLS/Impl.Core/DependencyInjection/HostApplicationBuilderExtensions.cs +++ b/src/LanguageServers/PowerPlatformLS/Impl.Core/DependencyInjection/HostApplicationBuilderExtensions.cs @@ -108,12 +108,12 @@ private static ILspTransport CreateIpcTransport(IServiceProvider serviceProvider if (useNamedPipes) { - logger.LogInformation($"Creating IPC: namedpipe={namedPipeInfo}"); + logger.LogInformation("Creating IPC: namedpipe"); transport = new NamedPipeIpc(namedPipeInfo, logger); } else { - logger.LogInformation($"Creating IPC: unixdomainsockets={namedPipeInfo}"); + logger.LogInformation("Creating IPC: unixdomainsockets"); transport = new UnixDomainSocketsIPC(namedPipeInfo, logger); } } diff --git a/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Handlers/DidChangeWatchedFilesHandler.cs b/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Handlers/DidChangeWatchedFilesHandler.cs index 229a4103..fc54748a 100644 --- a/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Handlers/DidChangeWatchedFilesHandler.cs +++ b/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Handlers/DidChangeWatchedFilesHandler.cs @@ -57,15 +57,20 @@ public async Task HandleNotificationAsync(DidChangeWatchedFilesParams request, R if (!WorkspacePath.TryGetLanguageType(filePath, out _)) { - _logger.LogInformation( - $"Client notified '{changeFileEvent.Type}' event on watched files that has no language definition: {filePath.FileName}. Change won't be tracked."); + // File name may be an agent/component name (EUII); redact it from telemetry. + _logger.LogSensitiveInformation( + $"Client notified '{changeFileEvent.Type}' event on watched files that has no language definition: {filePath.FileName}. Change won't be tracked.", + $"Client notified '{changeFileEvent.Type}' event on watched files that has no language definition. Change won't be tracked."); continue; } var context = _contextResolver.Resolve(new TextDocumentIdentifier { Uri = changeFileEvent.Uri }); if (context.IsInvalid) { - _logger.LogInformation($"File is not tracked. File is not found in workspace: '{filePath.FileName}'"); + // File name may be an agent/component name (EUII); redact it from telemetry. + _logger.LogSensitiveInformation( + $"File is not tracked. File is not found in workspace: '{filePath.FileName}'", + "File is not tracked. File is not found in workspace."); } else { @@ -81,8 +86,10 @@ public async Task HandleNotificationAsync(DidChangeWatchedFilesParams request, R if (!WorkspacePath.TryGetLanguageType(filePath, out _)) { - _logger.LogInformation( - $"Client notified '{changeFileEvent.Type}' event on watched files that has no language definition: {filePath.FileName}. Change won't be tracked."); + // File name may be an agent/component name (EUII); redact it from telemetry. + _logger.LogSensitiveInformation( + $"Client notified '{changeFileEvent.Type}' event on watched files that has no language definition: {filePath.FileName}. Change won't be tracked.", + $"Client notified '{changeFileEvent.Type}' event on watched files that has no language definition. Change won't be tracked."); continue; } @@ -91,7 +98,12 @@ public async Task HandleNotificationAsync(DidChangeWatchedFilesParams request, R var fileInfo = _fileProvider.GetFileInfo(filePath); if (!fileInfo.Exists) { + // File name may be an agent/component name (EUII); keep it out of telemetry + // (preserved for local debugging via LogSensitiveInformation). _logger.LogWarning( + $"Can't process '{changeFileEvent.Type}' event for a watched file: " + + "The file does not exist."); + _logger.LogSensitiveInformation( $"Can't process '{changeFileEvent.Type}' event for '{filePath.FileName}': " + "The file does not exist."); continue; diff --git a/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/LanguageServer.cs b/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/LanguageServer.cs index ab6c581c..2f32ecf5 100644 --- a/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/LanguageServer.cs +++ b/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/LanguageServer.cs @@ -64,7 +64,10 @@ public override bool TryGetLanguageForRequest(string methodName, object? seriali if (!lspWorkspaceManager.TryGetLanguageForDocument(typedLspUri, out var languageObject)) { - Logger.LogError($"Failed to get language for {typedLspUri.Raw}"); + // The raw URI is a full file path (EUII); keep it out of telemetry while + // preserving the full detail for local debugging. + Logger.LogError("Failed to get language for document."); + Logger.LogSensitiveInformation($"Failed to get language for {typedLspUri.Raw}"); language = null; return false; } diff --git a/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Uris/LspUriFactory.cs b/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Uris/LspUriFactory.cs index 683b89a5..cd733fad 100644 --- a/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Uris/LspUriFactory.cs +++ b/src/LanguageServers/PowerPlatformLS/Impl.Core/LSP/Uris/LspUriFactory.cs @@ -141,7 +141,10 @@ private static void LogUnsupportedSchemeOnce(string scheme, string sampleRaw, IL { // Truncate sample for logging var truncatedSample = sampleRaw.Length > 100 ? sampleRaw[..97] + "..." : sampleRaw; - logger.LogInformation($"UnsupportedSchemeObserved: scheme='{scheme}', sample='{truncatedSample}'"); + // The sample may be a full file path or URI (EUII); only the scheme is safe for telemetry. + logger.LogSensitiveInformation( + $"UnsupportedSchemeObserved: scheme='{scheme}', sample='{truncatedSample}'", + $"UnsupportedSchemeObserved: scheme='{scheme}'"); } } } diff --git a/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LoggingHttpHandler.cs b/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LoggingHttpHandler.cs index 21919579..22f21b3b 100644 --- a/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LoggingHttpHandler.cs +++ b/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LoggingHttpHandler.cs @@ -4,6 +4,7 @@ using Microsoft.PowerPlatformLS.Contracts.Internal.Common; using System; using System.Diagnostics; + using System.Text.RegularExpressions; using System.Threading.Tasks; /// @@ -20,6 +21,11 @@ public LoggingHttpHandler(ILogger logger) private static int _requestId; + // Environment/bot identifiers (GUIDs) in request paths are PII; redact them from telemetry. + private static readonly Regex GuidPattern = new( + "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}", + RegexOptions.Compiled); + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { int httpId = Interlocked.Increment(ref _requestId); @@ -45,7 +51,7 @@ protected override async Task SendAsync(HttpRequestMessage private static string GetPathAndQuery(Uri? uri) { if (uri == null) return string.Empty; - return uri.PathAndQuery; + return GuidPattern.Replace(uri.PathAndQuery, "{id}"); } } } diff --git a/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LspExceptionHandler.cs b/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LspExceptionHandler.cs index 60e49975..052d6a08 100644 --- a/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LspExceptionHandler.cs +++ b/src/LanguageServers/PowerPlatformLS/Impl.PullAgent/LspExceptionHandler.cs @@ -28,11 +28,12 @@ public static (int Code, string Message) Handle(Exception ex, ILspLogger logger, { // User-recoverable: the user can fix this by performing an action (resync, reclone, etc.). // Log at Error (message only, no stack trace) to match failure indicators in the UI. + // The exception message embeds a filesystem path (EUII); keep it out of telemetry. FileNotFoundException fnf => - LogErrorMessage(logger, 400, fnf.Message), + LogSensitiveErrorMessage(logger, 400, fnf.Message, "File not found."), DirectoryNotFoundException dnf => - LogErrorMessage(logger, 400, dnf.Message), + LogSensitiveErrorMessage(logger, 400, dnf.Message, "Directory not found."), // User validation: caller explicitly threw to signal bad input. InvalidOperationException ioe => @@ -69,6 +70,18 @@ private static (int Code, string Message) LogErrorMessage(ILspLogger logger, int return (code, message); } + /// + /// Logs a PII-free message at Error level for telemetry while routing the full + /// (path-bearing) message through + /// so it is only surfaced locally. The full message is still returned to the client. + /// + private static (int Code, string Message) LogSensitiveErrorMessage(ILspLogger logger, int code, string fullMessage, string safeMessage) + { + logger.LogError(safeMessage); + logger.LogSensitiveInformation(fullMessage); + return (code, fullMessage); + } + private static (int Code, string Message) LogErrorWithTrace(ILspLogger logger, Exception ex) { logger.LogException(ex); diff --git a/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.Core/ChangeMethodTests.cs b/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.Core/ChangeMethodTests.cs index e19c86d5..9e8b89f6 100644 --- a/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.Core/ChangeMethodTests.cs +++ b/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.Core/ChangeMethodTests.cs @@ -70,7 +70,9 @@ public async Task FileFilter_OnDidChangeWatchedFiles_Async() } else { - Assert.Single(logs.Warning.Where(x => x == $"Can't process 'Changed' event for '{entry.filepath.Split('/')[^1]}': The file does not exist.")); + // The full message (with file name) is routed through LogSensitiveInformation, + // which surfaces at Info level in Debug/test builds; the Warning is now file-name-free. + Assert.Single(logs.Info.Where(x => x == $"Can't process 'Changed' event for '{entry.filepath.Split('/')[^1]}': The file does not exist.")); } } } diff --git a/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.PullAgent/LoggingHttpHandlerTests.cs b/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.PullAgent/LoggingHttpHandlerTests.cs index 8cb052e5..5cb23508 100644 --- a/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.PullAgent/LoggingHttpHandlerTests.cs +++ b/src/LanguageServers/PowerPlatformLS/UnitTests/PowerPlatformLS.UnitTests/Impl.PullAgent/LoggingHttpHandlerTests.cs @@ -94,6 +94,20 @@ public void GetPathAndQuery_Returns_Empty_For_Null_Uri() Assert.Equal(string.Empty, result); } + [Fact] + public void GetPathAndQuery_Redacts_Guids_In_Path() + { + var method = typeof(LoggingHttpHandler).GetMethod("GetPathAndQuery", BindingFlags.NonPublic | BindingFlags.Static); + + Assert.NotNull(method); + var uri = new Uri("https://contoso.api.crm.dynamics.com/api/botmanagement/v1/environments/00b0eda3-d061-ea9f-b86f-0fd3f5628590/bots/09031e3d-436d-f111-ab0e-7ced8dfef2b5/content/botcomponents"); + var result = (string)method!.Invoke(null, [uri])!; + + Assert.Equal("/api/botmanagement/v1/environments/{id}/bots/{id}/content/botcomponents", result); + Assert.DoesNotContain("00b0eda3", result); + Assert.DoesNotContain("09031e3d", result); + } + [Fact] public async Task SendAsync_Includes_RequestId_From_LspRequestContext() {