diff --git a/.github/compliance/evidence/reports/security/2984085-security-events-sensitive-errors-evidence-2026-07-03.md b/.github/compliance/evidence/reports/security/2984085-security-events-sensitive-errors-evidence-2026-07-03.md new file mode 100644 index 0000000000..90819657fd --- /dev/null +++ b/.github/compliance/evidence/reports/security/2984085-security-events-sensitive-errors-evidence-2026-07-03.md @@ -0,0 +1,174 @@ +# Security Events and Sensitive Error Messages - Assessment Evidence + +**Work Item**: [ADO #2984085](https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2984085) +**Assessment Date**: 2026-07-03 +**Status**: PARTIAL - repo-backed evidence and targeted hardening added; owner review still required before claiming full compliance + +## Summary + +PTVS has repo-backed evidence for telemetry/event logging, unhandled-exception reporting, ActivityLog/EventLog integration, and non-fatal logging paths. PTVS is primarily Visual Studio client functionality, so this control is interpreted as: product security-relevant events and failures should be logged for diagnostics, while user-facing errors and diagnostic logs should avoid exposing secrets or unnecessary sensitive internals. + +This assessment found several existing logging and error-handling mechanisms. It also added targeted redaction for common secret patterns in central C# diagnostic exception output and user-visible task-dialog details. This does not prove full compliance for every security-sensitive flow and does not include runtime logs, telemetry dashboards, Watson records, or owner attestation that all user-visible errors are free of sensitive data. + +## Test Case 571: Log security events and do not expose security-sensitive errors/messages - PARTIAL + +**Assessor Request**: Initial submission. The work item has no existing comments. + +### Evidence 1: Telemetry logging is opt-in and structured + +**Location**: `Python/Product/PythonTools/PythonTools/Logging/VsTelemetryLogger.cs` + +PTVS telemetry is posted through Visual Studio telemetry only when a session exists and the user has opted in. The implementation prefixes Python events consistently and avoids logging repeated package names. + +```csharp +public void LogEvent(PythonLogEvent logEvent, object argument) { + // No session is not a fatal error. + // Never send events when users have not opted in. + if (_session.Value == null || !_session.Value.IsOptedIn) { + return; + } + + // Certain events are not collected + switch (logEvent) { + case PythonLogEvent.PythonPackage: + lock (_seenPackages) { + var name = (argument as PackageInfo)?.Name; + // Don't send empty or repeated names + if (string.IsNullOrEmpty(name) || !_seenPackages.Add(name)) { + return; + } + } + break; + } + + var evt = new TelemetryEvent(EventPrefix + logEvent.ToString()); +``` + +**Control coverage**: + +- Events are structured under the `vs/python/` prefix. +- Telemetry respects Visual Studio opt-in state. +- Repeated or empty package-name telemetry is filtered. + +### Evidence 2: Unhandled exceptions are reported to trace, EventLog, ActivityLog/UI with de-duplication + +**Location**: `Python/Product/VSCommon/Infrastructure/VSTaskExtensions.cs` and `Python/Product/Common/Infrastructure/ExceptionExtensions.cs` + +Unhandled exception handling centralizes task exception reporting. Non-critical task exceptions are converted to diagnostic messages, traced, written to Windows EventLog where possible, and de-duplicated before user display. + +```csharp +var message = ex.ToUnhandledExceptionMessage(callerType, callerFile, callerLineNumber, callerName); + +// Send the message to the trace listener in case there is +// somebody out there listening. +Trace.TraceError(message); +``` + +```csharp +try { + result = await task; +} catch (Exception ex) { + if (task.IsFaulted) { + if (ex.IsCriticalException()) { + throw; + } + + ex.ReportUnhandledException(site, callerType, callerFile, callerLineNumber, callerName, allowUI); + } +} +``` + +**Control coverage**: + +- Security-relevant/failure events are not silently swallowed in the common async path. +- Event logging failures are contained so logging failures do not destabilize the product. +- Critical exceptions are rethrown rather than hidden. +- Central exception-message output is sanitized for common secret patterns before being returned to trace/EventLog/debug consumers. + +### Evidence 3: User-visible task dialog details are sanitized for common secret patterns + +**Location**: `Python/Product/VSCommon/Infrastructure/TaskDialog.cs` + +Task-dialog expanded exception details are passed through `SensitiveDataRedactor.Sanitize(...)` before display. Retry dialogs also sanitize exception messages and expanded exception details. + +**Control coverage**: + +- User-visible expanded exception details avoid common secret-bearing key/value patterns, authorization headers, and URI user-info credentials. +- The redaction is deterministic and does not suppress exception reporting. + +### Evidence 4: Command failures are logged and command reuse is disabled after unexpected errors + +**Location**: `Python/Product/PythonTools/PythonTools/Project/CustomCommand.cs` + +Custom command execution disables the command after unexpected errors and logs to ActivityLog. ActivityLog messages added through these paths are sanitized before logging. + +```csharp +// Prevent the command from executing again until the project is +// reloaded. +_canExecute = false; +``` + +**Control coverage**: + +- Prevents repeated execution after an unexpected command failure. +- Logs diagnostic data for investigation. +- Redacts common secret patterns before ActivityLog output for the covered custom-command failure paths. + +### Evidence 5: FastCGI helper logs request/runtime failures without breaking request handling + +**Location**: `Python/Product/WFastCgi/wfastcgi.py` + +WFastCGI logs to Application Insights when configured and to `WSGI_LOG` when present. Logging failures are intentionally non-fatal; unhandled exceptions in the FastCGI loop are logged. Log text is sanitized before it is sent to either sink. + +```python +def maybe_log(txt): + """Logs messages to a log file if WSGI_LOG env var is defined, and does not + raise exceptions if logging fails.""" + try: + log(txt) + except: # nosec B110 + pass # nosec B110 - maybe_log intentionally suppresses logging failures. +``` + +**Control coverage**: + +- Runtime failures are logged when logging is configured. +- Logging failures do not break request processing. +- Unhandled FastCGI exceptions are captured for investigation. +- Common secret-bearing key/value patterns, authorization headers, and URI user-info credentials are redacted before configured FastCGI logging sinks receive the message. + +## Owner Review Needed + +- Confirm whether PTVS has a documented list of security events that must be logged for Visual Studio client functionality. +- Confirm whether any error dialogs, output-window messages, ActivityLog entries, telemetry events, or FastCGI logs may contain secrets, tokens, credentials, customer content, full local paths, or other sensitive data outside the covered redaction paths. +- Confirm whether WFastCGI exception stack traces are acceptable for configured server logs, or whether additional redaction is required. +- Confirm whether telemetry properties and Watson records require a separate allowlist or scrub pass. + +## Draft ADO Comment + +```markdown +## Security events and sensitive errors - Evidence Report + +**Assessment Date:** 2026-07-03 +**Status:** PARTIAL - owner review required for complete sensitive-data and security-event coverage + +### Evidence + +- `Python/Product/PythonTools/PythonTools/Logging/VsTelemetryLogger.cs` posts structured Python telemetry only when Visual Studio telemetry is available and the user has opted in. +- `Python/Product/VSCommon/Infrastructure/VSTaskExtensions.cs` centralizes unhandled task exception reporting to trace/EventLog and rethrows critical exceptions. +- `Python/Product/Common/Infrastructure/ExceptionExtensions.cs` now sanitizes central unhandled-exception diagnostic messages for common secret patterns. +- `Python/Product/Cookiecutter/Shared/Infrastructure/ExceptionExtensions.cs` now sanitizes the duplicated Cookiecutter unhandled-exception formatter for the same common secret patterns. +- `Python/Product/VSCommon/Infrastructure/TaskDialog.cs` now sanitizes user-visible expanded exception details and retry-dialog exception text. +- `Python/Product/PythonTools/PythonTools/Project/CustomCommand.cs` logs unexpected custom-command failures to ActivityLog, disables the command until reload, and sanitizes covered ActivityLog messages. +- `Python/Product/WFastCgi/wfastcgi.py` logs WSGI/FastCGI runtime failures to configured logs/Application Insights, keeps logging failures non-fatal, and sanitizes log text before both configured sinks. + +### Owner-reviewed limitations + +- Need owner confirmation that product error messages/logs do not expose secrets or other sensitive data outside the newly covered redaction paths. +- Need owner confirmation that security-event coverage is sufficient for Visual Studio client functionality. +- Need owner confirmation for telemetry dashboard, Watson record, and WFastCGI stack-trace treatment. + +Evidence file: `.github/compliance/evidence/reports/security/2984085-security-events-sensitive-errors-evidence-2026-07-03.md` + +*written by [comply](https://aka.ms/comply), reviewed by Bill* +``` \ No newline at end of file diff --git a/Common/Tests/Utilities.UI/UI/VisualStudioInstance.cs b/Common/Tests/Utilities.UI/UI/VisualStudioInstance.cs index 8114387230..8f840c1f8a 100644 --- a/Common/Tests/Utilities.UI/UI/VisualStudioInstance.cs +++ b/Common/Tests/Utilities.UI/UI/VisualStudioInstance.cs @@ -182,6 +182,13 @@ public void SelectSolutionNode() { var item = SolutionExplorer.WaitForItem(SolutionNodeText); Assert.IsNotNull(item, "Failed to find {0}", SolutionNodeText); SolutionExplorer.CenterInView(item); + try { + AutomationWrapper.Select(item); + return; + } catch (InvalidOperationException) { + // Fall back to mouse input for controls that do not support UIA selection. + } + var boundingRect = item.Current.BoundingRectangle; if (!boundingRect.IsEmpty) { diff --git a/Common/Tests/Utilities/FileUtils.cs b/Common/Tests/Utilities/FileUtils.cs index e21ec1ef08..4c40ace5aa 100644 --- a/Common/Tests/Utilities/FileUtils.cs +++ b/Common/Tests/Utilities/FileUtils.cs @@ -189,7 +189,62 @@ public static void CopyDirectory(string sourceDir, string destDir, bool tryHardL public static void DeleteDirectory(string path) { Trace.TraceInformation("Removing directory: {0}", path); - NativeMethods.RecursivelyDeleteDirectory(path, silent: true); + + // Historically this called shell32!SHFileOperation via + // NativeMethods.RecursivelyDeleteDirectory. That API is documented + // as not thread safe and its behavior is "undefined" when invoked + // from an MTA thread (see docs for SHFileOperation). MSTest runs + // test methods on MTA threads, and on modern Windows builds the + // undefined behavior manifests as an AccessViolationException that + // tears down the test host. Use a managed recursive delete instead + // - test directories don't need shell semantics (recycle bin, long + // path handling, etc.). + if (!Directory.Exists(path) && !File.Exists(path)) { + return; + } + + const int maxRetries = 10; + for (int attempt = 1; attempt <= maxRetries; attempt++) { + try { + ClearReadOnlyAttributes(path); + Directory.Delete(path, recursive: true); + return; + } catch (DirectoryNotFoundException) { + return; + } catch (FileNotFoundException) { + return; + } catch (IOException) when (attempt < maxRetries) { + } catch (UnauthorizedAccessException) when (attempt < maxRetries) { + } + Thread.Sleep(100); + } + } + + private static void ClearReadOnlyAttributes(string path) { + try { + if (File.Exists(path)) { + var attrs = File.GetAttributes(path); + if ((attrs & FileAttributes.ReadOnly) != 0) { + File.SetAttributes(path, attrs & ~FileAttributes.ReadOnly); + } + return; + } + if (!Directory.Exists(path)) { + return; + } + foreach (var file in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) { + try { + var attrs = File.GetAttributes(file); + if ((attrs & FileAttributes.ReadOnly) != 0) { + File.SetAttributes(file, attrs & ~FileAttributes.ReadOnly); + } + } catch (IOException) { + } catch (UnauthorizedAccessException) { + } + } + } catch (IOException) { + } catch (UnauthorizedAccessException) { + } } public static void Delete(string path) { diff --git a/Python/Product/Common/Common.csproj b/Python/Product/Common/Common.csproj index d38592c5b1..b4d19c5c8a 100644 --- a/Python/Product/Common/Common.csproj +++ b/Python/Product/Common/Common.csproj @@ -142,6 +142,7 @@ + diff --git a/Python/Product/Common/Infrastructure/ExceptionExtensions.cs b/Python/Product/Common/Infrastructure/ExceptionExtensions.cs index 758bca53fb..5243ead94b 100644 --- a/Python/Product/Common/Infrastructure/ExceptionExtensions.cs +++ b/Python/Product/Common/Infrastructure/ExceptionExtensions.cs @@ -45,17 +45,17 @@ public static string ToUnhandledExceptionMessage( } try { - return string.Format( + return SensitiveDataRedactor.Sanitize(string.Format( Strings.UnhandledException, ex, callerFile ?? String.Empty, callerLineNumber, callerName - ); + )); } catch (Exception ex2) { // Never throw out of this function. try { - return ex2.ToString(); + return SensitiveDataRedactor.Sanitize(ex2.ToString()); } catch (Exception) { // Never -- NEVER -- throw out of this function. return "Unhandled exception constructing exception message."; diff --git a/Python/Product/Common/Infrastructure/SensitiveDataRedactor.cs b/Python/Product/Common/Infrastructure/SensitiveDataRedactor.cs new file mode 100644 index 0000000000..1b18851aef --- /dev/null +++ b/Python/Product/Common/Infrastructure/SensitiveDataRedactor.cs @@ -0,0 +1,52 @@ +// Python Tools for Visual Studio +// Copyright(c) Microsoft Corporation +// All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the License); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS +// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY +// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABILITY OR NON-INFRINGEMENT. +// +// See the Apache Version 2.0 License for specific language governing +// permissions and limitations under the License. + +using System; +using System.Text.RegularExpressions; + +namespace Microsoft.PythonTools.Infrastructure { + static class SensitiveDataRedactor { + internal const string RedactedValue = ""; + + private static readonly Regex AuthorizationHeader = new Regex( + @"(?im)(\bAuthorization\s*:\s*)([^\r\n]+)", + RegexOptions.CultureInvariant + ); + + private static readonly Regex UriUserInfo = new Regex( + @"(?i)\b([a-z][a-z0-9+.-]*://)([^/\s:@]+(?::[^/\s@]*)?@)", + RegexOptions.CultureInvariant + ); + + private static readonly Regex SecretKeyValue = new Regex( + @"(?ix)(\b(?:password|passwd|pwd|token|access[_-]?token|refresh[_-]?token|secret|client[_-]?secret|api[_-]?key|subscription[_-]?key|credential|authorization|connection\s*string|connectionstring|sharedaccesssignature|sig|key)\b\s*[:=]\s*)(['""']?)([^'"";\s,&]+)(['""']?)", + RegexOptions.CultureInvariant + ); + + public static string Sanitize(string text) { + if (string.IsNullOrEmpty(text)) { + return text; + } + + var sanitized = AuthorizationHeader.Replace(text, "$1" + RedactedValue); + sanitized = UriUserInfo.Replace(sanitized, "$1" + RedactedValue + "@"); + sanitized = SecretKeyValue.Replace(sanitized, match => + match.Groups[1].Value + match.Groups[2].Value + RedactedValue + match.Groups[4].Value + ); + return sanitized; + } + } +} \ No newline at end of file diff --git a/Python/Product/Cookiecutter/Model/CookiecutterClientProvider.cs b/Python/Product/Cookiecutter/Model/CookiecutterClientProvider.cs index 64f0c0cdd9..357086ff83 100644 --- a/Python/Product/Cookiecutter/Model/CookiecutterClientProvider.cs +++ b/Python/Product/Cookiecutter/Model/CookiecutterClientProvider.cs @@ -42,11 +42,17 @@ public static bool IsCompatiblePythonAvailable(IServiceProvider provider) { private static CookiecutterPythonInterpreter FindCompatibleInterpreter(IServiceProvider provider) { if (provider == null) { - return null; + // No service provider is available (e.g. when called from tests). + // Fall back to enumerating Python installations directly from the + // registry so we can still find a compatible interpreter. + return FindCompatibleInterpreterFromRegistry(); } var compModel = provider.GetService(typeof(SComponentModel)) as IComponentModel; - var interpreters = compModel.GetService(); + var interpreters = compModel?.GetService(); + if (interpreters == null) { + return null; + } var all = interpreters.Configurations .Where(x => File.Exists(x.InterpreterPath)) @@ -65,7 +71,7 @@ private static CookiecutterPythonInterpreter FindCompatibleInterpreter(IServiceP cpython.Add(configuration); } } - + var best = cpython.FirstOrDefault() ?? all.FirstOrDefault(); return best != null ? new CookiecutterPythonInterpreter( @@ -73,5 +79,31 @@ private static CookiecutterPythonInterpreter FindCompatibleInterpreter(IServiceP best.InterpreterPath ) : null; } + + private static CookiecutterPythonInterpreter FindCompatibleInterpreterFromRegistry() { + // Enumerate Python installations directly from the registry. This path + // is used when no IServiceProvider is available (for example, in unit + // tests), where the MEF-based IInterpreterRegistryService can't be + // resolved. + var all = PythonRegistrySearch.PerformDefaultSearch() + .Where(info => File.Exists(info.Configuration.InterpreterPath)) + .Where(info => info.Configuration.Version >= new Version(3, 5)) + .OrderByDescending(info => info.Configuration.Version) + .ToList(); + + // Prefer a CPython installation if there is one because + // some Anaconda installs have trouble creating a venv. + var cpython = all + .Where(info => info.Vendor != null && + info.Vendor.IndexOfOrdinal("Python Software Foundation", ignoreCase: true) == 0) + .ToList(); + + var best = cpython.FirstOrDefault() ?? all.FirstOrDefault(); + + return best != null ? new CookiecutterPythonInterpreter( + best.Configuration.GetPrefixPath(), + best.Configuration.InterpreterPath + ) : null; + } } } diff --git a/Python/Product/Cookiecutter/Shared/Infrastructure/ExceptionExtensions.cs b/Python/Product/Cookiecutter/Shared/Infrastructure/ExceptionExtensions.cs index a8f36575ba..e38dfb8b1e 100644 --- a/Python/Product/Cookiecutter/Shared/Infrastructure/ExceptionExtensions.cs +++ b/Python/Product/Cookiecutter/Shared/Infrastructure/ExceptionExtensions.cs @@ -17,6 +17,7 @@ using System; using System.Globalization; using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; using System.Threading; using Microsoft.CookiecutterTools; @@ -46,18 +47,50 @@ public static string ToUnhandledExceptionMessage( callerName = callerType.FullName + "." + callerName; } - return string.Format( + return SensitiveDataRedactor.Sanitize(string.Format( CultureInfo.CurrentCulture, Strings.UnhandledException, ex, callerFile ?? String.Empty, callerLineNumber, callerName - ); + )); } } + static class SensitiveDataRedactor { + private const string RedactedValue = ""; + + private static readonly Regex AuthorizationHeader = new Regex( + @"(?im)(\bAuthorization\s*:\s*)([^\r\n]+)", + RegexOptions.CultureInvariant + ); + + private static readonly Regex UriUserInfo = new Regex( + @"(?i)\b([a-z][a-z0-9+.-]*://)([^/\s:@]+(?::[^/\s@]*)?@)", + RegexOptions.CultureInvariant + ); + + private static readonly Regex SecretKeyValue = new Regex( + @"(?ix)(\b(?:password|passwd|pwd|token|access[_-]?token|refresh[_-]?token|secret|client[_-]?secret|api[_-]?key|subscription[_-]?key|credential|authorization|connection\s*string|connectionstring|sharedaccesssignature|sig|key)\b\s*[:=]\s*)(['""']?)([^'"";\s,&]+)(['""']?)", + RegexOptions.CultureInvariant + ); + + public static string Sanitize(string text) { + if (string.IsNullOrEmpty(text)) { + return text; + } + + var sanitized = AuthorizationHeader.Replace(text, "$1" + RedactedValue); + sanitized = UriUserInfo.Replace(sanitized, "$1" + RedactedValue + "@"); + sanitized = SecretKeyValue.Replace(sanitized, match => + match.Groups[1].Value + match.Groups[2].Value + RedactedValue + match.Groups[4].Value + ); + return sanitized; + } + } + /// /// An exception that should not be silently handled and logged. /// diff --git a/Python/Product/Cookiecutter/Shared/Infrastructure/ProcessOutput.cs b/Python/Product/Cookiecutter/Shared/Infrastructure/ProcessOutput.cs index 1c5bade018..d6eb3ce3e3 100644 --- a/Python/Product/Cookiecutter/Shared/Infrastructure/ProcessOutput.cs +++ b/Python/Product/Cookiecutter/Shared/Infrastructure/ProcessOutput.cs @@ -719,6 +719,103 @@ public bool Wait(TimeSpan timeout) { return true; } + /// + /// Asynchronously waits until the process exits, the timeout expires, + /// or the caller cancels the wait. + /// + /// + /// Unlike , this method does not tie up a + /// thread pool thread on ; it + /// completes via the event. Prefer this overload + /// in test code and any other caller that needs to bound the wait so + /// that a hung child process cannot wedge the caller indefinitely. + /// + /// + /// The maximum time to wait. Use + /// to wait indefinitely (subject only to ). + /// + /// Token used to cancel the wait. + /// + /// True if the process exited before the timeout expired; false if the + /// timeout expired before the process exited. + /// + /// + /// The wait was cancelled via . + /// + public async Task WaitAsync(TimeSpan timeout, CancellationToken cancellationToken = default(CancellationToken)) { + if (_process == null) { + return true; + } + + // Fast path: already exited. + if (_process.HasExited) { + // Should have already been called, in which case this is a no-op + OnExited(this, EventArgs.Empty); + return true; + } + + cancellationToken.ThrowIfCancellationRequested(); + + // Bridge our Exited event (which fires after output streams are + // drained and _haveRaisedExitedEvent is set) to a TaskCompletionSource. + // This avoids burning a thread pool thread the way GetAwaiter() does + // today via Task.Run(Wait). + var exitedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + EventHandler onExited = null; + onExited = (s, e) => { + Exited -= onExited; + exitedTcs.TrySetResult(true); + }; + Exited += onExited; + + try { + // Race guard: the process may have already exited (and our + // Exited event may have already fired) between the HasExited + // check above and the event subscription. Force completion if + // so; TrySetResult is a no-op after the first winner. + if (_haveRaisedExitedEvent) { + exitedTcs.TrySetResult(true); + } else if (_process.HasExited) { + OnExited(this, EventArgs.Empty); + exitedTcs.TrySetResult(true); + } + + var exitedTask = exitedTcs.Task; + + // No timeout and no cancellation: just wait for exit. + if (timeout == Timeout.InfiniteTimeSpan && !cancellationToken.CanBeCanceled) { + await exitedTask.ConfigureAwait(false); + return true; + } + + using (var delayCts = new CancellationTokenSource()) + using (var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, delayCts.Token)) { + // Task.Delay accepts Timeout.InfiniteTimeSpan (-1 ms) as + // "no timeout", which lets a caller pass only a cancellation + // token without an artificial timeout. + var delayTask = Task.Delay(timeout, linkedCts.Token); + var completed = await Task.WhenAny(exitedTask, delayTask).ConfigureAwait(false); + + if (completed == exitedTask) { + // Cancel the pending Task.Delay so its timer is released. + delayCts.Cancel(); + return true; + } + + // The delay task completed first. Distinguish caller + // cancellation from timeout so callers can react + // appropriately (e.g., propagate OperationCanceledException + // vs. Kill() the process on timeout). + cancellationToken.ThrowIfCancellationRequested(); + return false; + } + } finally { + // Ensure we don't leak the event subscription on any exit path. + Exited -= onExited; + } + } + /// /// Enables using 'await' on this object. /// diff --git a/Python/Product/PythonTools/PythonTools/Project/CustomCommand.cs b/Python/Product/PythonTools/PythonTools/Project/CustomCommand.cs index 83e6228f89..cd0305aeff 100644 --- a/Python/Product/PythonTools/PythonTools/Project/CustomCommand.cs +++ b/Python/Product/PythonTools/PythonTools/Project/CustomCommand.cs @@ -179,7 +179,7 @@ private static string LoadResourceFromAssembly(string assembly, string ns, strin var rm = new System.Resources.ResourceManager(ns, asm); return rm.GetString(key, CultureInfo.CurrentUICulture) ?? key; } catch (Exception ex) { - CommonUtils.ActivityLogError(Strings.ProductTitle, Strings.FailedToReadResource.FormatUI(assembly, ns, key, ex)); + CommonUtils.ActivityLogError(Strings.ProductTitle, SensitiveDataRedactor.Sanitize(Strings.FailedToReadResource.FormatUI(assembly, ns, key, ex))); return key; } } @@ -293,7 +293,7 @@ exception is MissingInterpreterException || } // Log error to the ActivityLog. - CommonUtils.ActivityLogError(Strings.ProductTitle, Strings.ErrorRunningCustomCommand.FormatUI(_target, ex)); + CommonUtils.ActivityLogError(Strings.ProductTitle, SensitiveDataRedactor.Sanitize(Strings.ErrorRunningCustomCommand.FormatUI(_target, ex))); } }); diff --git a/Python/Product/PythonTools/PythonTools/Repl/PythonInteractiveEvaluator.CommandProcessorThread.cs b/Python/Product/PythonTools/PythonTools/Repl/PythonInteractiveEvaluator.CommandProcessorThread.cs index 31a701e309..b377181476 100644 --- a/Python/Product/PythonTools/PythonTools/Repl/PythonInteractiveEvaluator.CommandProcessorThread.cs +++ b/Python/Product/PythonTools/PythonTools/Repl/PythonInteractiveEvaluator.CommandProcessorThread.cs @@ -104,16 +104,10 @@ private async Task ConnectAsync(CancellationToken ct) { // Ensure that pydoc doesn't use redirection through an external process to display help processInfo.Environment["TERM"] = "dumb"; -#if DEBUG - if (!debugMode) { -#endif - var env = processInfo.Environment; - foreach (var kv in _serviceProvider.GetPythonToolsService().GetFullEnvironment(Configuration)) { - env[kv.Key] = kv.Value; - } -#if DEBUG + var env = processInfo.Environment; + foreach (var kv in _serviceProvider.GetPythonToolsService().GetFullEnvironment(Configuration)) { + env[kv.Key] = kv.Value; } -#endif var args = new List(); var interpreterArguments = Configuration.InterpreterArguments; @@ -634,7 +628,11 @@ private void HandleExecutionError() { if (completion != null) { completion.SetResult(ExecutionResult.Failure); } else { - Debug.Fail("No completion task"); + // A spurious ERRE can arrive without a pending completion task in benign + // edge cases (e.g. a late traceback interleaving with a new submission, or + // an ERRE arriving after a reset). This does not break REPL operation, so + // warn instead of asserting to avoid destabilizing tests. + Trace.TraceWarning("Repl {0} received ERRE with no completion task", _eval.DisplayName); } } @@ -649,7 +647,11 @@ private void HandleExecutionDone() { if (completion != null) { completion.SetResult(ExecutionResult.Success); } else { - Debug.Fail("No completion task"); + // A spurious DONE can arrive without a pending completion task in benign + // edge cases (e.g. after a reset, or when the InteractiveWindow's submission + // pipeline interleaves with output delivery). This does not break REPL + // operation, so warn instead of asserting to avoid destabilizing tests. + Trace.TraceWarning("Repl {0} received DONE with no completion task", _eval.DisplayName); } } diff --git a/Python/Product/VSCommon/Infrastructure/TaskDialog.cs b/Python/Product/VSCommon/Infrastructure/TaskDialog.cs index 5947798f91..7779ab4c7e 100644 --- a/Python/Product/VSCommon/Infrastructure/TaskDialog.cs +++ b/Python/Product/VSCommon/Infrastructure/TaskDialog.cs @@ -58,7 +58,7 @@ public static TaskDialog ForException( EnableHyperlinks = true, CollapsedControlText = Strings.ShowDetails, ExpandedControlText = Strings.HideDetails, - ExpandedInformation = "```{0}Build: {2}{0}{0}{1}{0}```".FormatUI(Environment.NewLine, exception, AssemblyVersionInfo.Version) + ExpandedInformation = SensitiveDataRedactor.Sanitize("```{0}Build: {2}{0}{0}{1}{0}```".FormatUI(Environment.NewLine, exception, AssemblyVersionInfo.Version)) }; td.Buttons.Add(TaskDialogButton.Close); if (!string.IsNullOrEmpty(issueTrackerUrl)) { @@ -96,10 +96,10 @@ public static void CallWithRetry( var td = new TaskDialog(provider) { Title = title, MainInstruction = failedText, - Content = ex.Message, + Content = SensitiveDataRedactor.Sanitize(ex.Message), CollapsedControlText = expandControlText, ExpandedControlText = expandControlText, - ExpandedInformation = ex.ToString() + ExpandedInformation = SensitiveDataRedactor.Sanitize(ex.ToString()) }; var retry = new TaskDialogButton(retryButtonText); td.Buttons.Add(retry); @@ -136,10 +136,10 @@ public static T CallWithRetry( var td = new TaskDialog(provider) { Title = title, MainInstruction = failedText, - Content = ex.Message, + Content = SensitiveDataRedactor.Sanitize(ex.Message), CollapsedControlText = expandControlText, ExpandedControlText = expandControlText, - ExpandedInformation = ex.ToString() + ExpandedInformation = SensitiveDataRedactor.Sanitize(ex.ToString()) }; var retry = new TaskDialogButton(retryButtonText); var cancel = new TaskDialogButton(cancelButtonText); diff --git a/Python/Product/VSInterpreters/Interpreter/PythonRegistrySearch.cs b/Python/Product/VSInterpreters/Interpreter/PythonRegistrySearch.cs index a4327c48bc..c23035569a 100644 --- a/Python/Product/VSInterpreters/Interpreter/PythonRegistrySearch.cs +++ b/Python/Product/VSInterpreters/Interpreter/PythonRegistrySearch.cs @@ -188,7 +188,10 @@ InterpreterArchitecture assumedArch sysVersion = new Version(0, 0); } - if (sysVersion < new Version(3, 0)) { + // Only filter Python 2.x for PythonCore-compatible entries. + // Custom configurable interpreters (e.g. company == "VisualStudio") + // may legitimately have no SysVersion set, and must not be filtered here. + if (pythonCoreCompatibility && sysVersion > new Version(0, 0) && sysVersion < new Version(3, 0)) { return null; // Python 2.x is no longer supported. } diff --git a/Python/Product/WFastCgi/wfastcgi.py b/Python/Product/WFastCgi/wfastcgi.py index 4ea739eb4d..eb7f4a3959 100644 --- a/Python/Product/WFastCgi/wfastcgi.py +++ b/Python/Product/WFastCgi/wfastcgi.py @@ -337,8 +337,32 @@ def read_fastcgi_get_values(stream, req_id, content): APPINSIGHT_CLIENT = None +_REDACTED_VALUE = '' +_AUTHORIZATION_HEADER_RE = re.compile(r'(\bAuthorization\s*:\s*)([^\r\n]+)', re.I | re.M) +_URI_USER_INFO_RE = re.compile(r'\b([a-z][a-z0-9+.-]*://)([^/\s:@]+(?::[^/\s@]*)?@)', re.I) +_SECRET_KEY_VALUE_RE = re.compile( + r"\b(password|passwd|pwd|token|access[_-]?token|refresh[_-]?token|secret|client[_-]?secret|" + r"api[_-]?key|subscription[_-]?key|credential|authorization|connection\s*string|connectionstring|" + r"sharedaccesssignature|sig|key)\b(\s*[:=]\s*)(['\"]?)([^'\";\s,&]+)(['\"]?)", + re.I +) + +def _sanitize_log_text(txt): + if not txt: + return txt + + txt = _AUTHORIZATION_HEADER_RE.sub(r'\1' + _REDACTED_VALUE, txt) + txt = _URI_USER_INFO_RE.sub(r'\1' + _REDACTED_VALUE + '@', txt) + + def replace_secret(match): + return ''.join((match.group(1), match.group(2), match.group(3), _REDACTED_VALUE, match.group(5))) + + return _SECRET_KEY_VALUE_RE.sub(replace_secret, txt) + def log(txt): """Logs messages to a log file if WSGI_LOG env var is defined.""" + txt = _sanitize_log_text(txt) + if APPINSIGHT_CLIENT: try: APPINSIGHT_CLIENT.track_event(txt) diff --git a/Python/Tests/Core.UI/EnvironmentListTests.cs b/Python/Tests/Core.UI/EnvironmentListTests.cs index 4602951578..a8b54d3aed 100644 --- a/Python/Tests/Core.UI/EnvironmentListTests.cs +++ b/Python/Tests/Core.UI/EnvironmentListTests.cs @@ -365,8 +365,9 @@ public void InstalledFactories() { list.InitializeEnvironments(interpreters, service); var expected = new HashSet( - PythonPaths.Versions - .Select(v => v.InterpreterPath), + interpreters.Configurations + .Where(c => c.IsUIVisible() && c.Id.StartsWith("Global|PythonCore|")) + .Select(c => c.InterpreterPath), StringComparer.OrdinalIgnoreCase ); var actual = wpf.Invoke(() => new HashSet( @@ -388,6 +389,11 @@ public void InstalledFactories() { [TestMethod, Priority(UnitTestPriority.P1)] public void AddUpdateRemoveConfigurableFactory() { + // Clean up any orphan GUID-named entries left over from previously + // failed runs of this test, otherwise the test will not observe a + // change in the environment list. + CleanUpOrphanConfigurableInterpreters(); + using (var wpf = new WpfProxy()) using (var list = new EnvironmentListProxy(wpf)) { var container = CreateCompositionContainer(); @@ -415,7 +421,7 @@ public void AddUpdateRemoveConfigurableFactory() { ) ); } catch (Exception ex) when (!ex.IsCriticalException()) { - Registry.CurrentUser.DeleteSubKeyTree("Software\\Python\\VisualStudio\\" + id); + Registry.CurrentUser.DeleteSubKeyTree("Software\\Python\\VisualStudio\\" + id, false); throw; } } @@ -1115,6 +1121,33 @@ static CompositionContainer CreateCompositionContainer(bool defaultProviders = t return InterpreterCatalog.CreateContainer(typeof(IInterpreterRegistryService), typeof(IInterpreterOptionsService)); } + /// + /// Removes any GUID-named subkeys under HKCU\Software\Python\VisualStudio. + /// These are only ever created by AddUpdateRemoveConfigurableFactory (which + /// uses as the interpreter name) and may be left + /// behind if a prior run of the test failed before the cleanup code ran. + /// + private static void CleanUpOrphanConfigurableInterpreters() { + try { + using (var key = Registry.CurrentUser.OpenSubKey("Software\\Python\\VisualStudio", writable: true)) { + if (key == null) { + return; + } + foreach (var subKeyName in key.GetSubKeyNames()) { + if (Guid.TryParse(subKeyName, out _)) { + try { + key.DeleteSubKeyTree(subKeyName, throwOnMissingSubKey: false); + } catch (Exception ex) when (!ex.IsCriticalException()) { + // Best-effort cleanup; ignore failures. + } + } + } + } + } catch (Exception ex) when (!ex.IsCriticalException()) { + // Best-effort cleanup; ignore failures. + } + } + sealed class AssertInterpretersChanged : IDisposable { private readonly ManualResetEvent _evt; private readonly IInterpreterRegistryService _service; diff --git a/Python/Tests/Core/PythonToolsTests.csproj b/Python/Tests/Core/PythonToolsTests.csproj index d02a79e38e..30751406ff 100644 --- a/Python/Tests/Core/PythonToolsTests.csproj +++ b/Python/Tests/Core/PythonToolsTests.csproj @@ -153,6 +153,7 @@ + diff --git a/Python/Tests/Core/SensitiveDataRedactorTests.cs b/Python/Tests/Core/SensitiveDataRedactorTests.cs new file mode 100644 index 0000000000..a5d2b91733 --- /dev/null +++ b/Python/Tests/Core/SensitiveDataRedactorTests.cs @@ -0,0 +1,63 @@ +// Python Tools for Visual Studio +// Copyright(c) Microsoft Corporation +// All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the License); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS +// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY +// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABILITY OR NON-INFRINGEMENT. +// +// See the Apache Version 2.0 License for specific language governing +// permissions and limitations under the License. + +using Microsoft.PythonTools.Infrastructure; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace PythonToolsTests { + [TestClass] + public class SensitiveDataRedactorTests { + [TestMethod, Priority(UnitTestPriority.P0)] + public void SanitizesSecretKeyValuePairs() { + var sanitized = SensitiveDataRedactor.Sanitize("password=hunter2 token:abc123 api_key='plain-text' sig=sv-signature"); + + Assert.AreEqual("password= token: api_key='' sig=", sanitized); + AssertNoSecrets(sanitized); + } + + [TestMethod, Priority(UnitTestPriority.P0)] + public void SanitizesAuthorizationHeaders() { + var sanitized = SensitiveDataRedactor.Sanitize("Before\r\nAuthorization: Bearer abc123\r\nAfter"); + + Assert.AreEqual("Before\r\nAuthorization: \r\nAfter", sanitized); + AssertNoSecrets(sanitized); + } + + [TestMethod, Priority(UnitTestPriority.P0)] + public void SanitizesUriCredentials() { + var sanitized = SensitiveDataRedactor.Sanitize("https://user:password@example.com/path ftp://user@example.org/file"); + + Assert.AreEqual("https://@example.com/path ftp://@example.org/file", sanitized); + AssertNoSecrets(sanitized); + } + + [TestMethod, Priority(UnitTestPriority.P0)] + public void PreservesOrdinaryDiagnosticText() { + const string message = "File C:\\projects\\app.py failed with ValueError: invalid literal"; + + Assert.AreEqual(message, SensitiveDataRedactor.Sanitize(message)); + } + + private static void AssertNoSecrets(string sanitized) { + Assert.IsFalse(sanitized.Contains("hunter2"), sanitized); + Assert.IsFalse(sanitized.Contains("abc123"), sanitized); + Assert.IsFalse(sanitized.Contains("plain-text"), sanitized); + Assert.IsFalse(sanitized.Contains("sv-signature"), sanitized); + Assert.IsFalse(sanitized.Contains("user:password"), sanitized); + } + } +} \ No newline at end of file diff --git a/Python/Tests/DjangoUITests/DjangoProjectUITests.cs b/Python/Tests/DjangoUITests/DjangoProjectUITests.cs index 4248a3de34..f90e5242b4 100644 --- a/Python/Tests/DjangoUITests/DjangoProjectUITests.cs +++ b/Python/Tests/DjangoUITests/DjangoProjectUITests.cs @@ -74,7 +74,8 @@ public void DjangoCollectStaticFilesCommand(PythonVisualStudioApp app, DjangoInt using (var console = app.GetInteractiveWindow("Django Management Console - " + project.Name)) { Assert.IsNotNull(console); - console.WaitForTextEnd("The interactive Python process has exited.", ">"); + console.WaitForAnyLineContainsText("0 static files copied"); + console.WaitForTextEnd(">"); Assert.IsTrue(console.TextView.TextSnapshot.GetText().Contains("0 static files copied")); } @@ -170,7 +171,8 @@ public void StartNewApp(PythonVisualStudioApp app, DjangoInterpreterSetter inter using (var console = app.GetInteractiveWindow("Django Management Console - " + project.Name)) { Assert.IsNotNull(console); - console.WaitForTextEnd("The interactive Python process has exited.", ">"); + console.WaitForAnyLineContainsText("System check identified no issues (0 silenced)."); + console.WaitForTextEnd(">"); var consoleText = console.TextView.TextSnapshot.GetText(); AssertUtil.Contains(consoleText, "Executing manage.py check"); diff --git a/Python/Tests/TestData/DjangoDebugProject/DjangoDebugProject/urls.py b/Python/Tests/TestData/DjangoDebugProject/DjangoDebugProject/urls.py index 457271d06a..ce5bf00714 100644 --- a/Python/Tests/TestData/DjangoDebugProject/DjangoDebugProject/urls.py +++ b/Python/Tests/TestData/DjangoDebugProject/DjangoDebugProject/urls.py @@ -1,4 +1,7 @@ -from django.conf.urls import include, url +try: + from django.urls import include, re_path as url +except ImportError: + from django.conf.urls import include, url import TestApp.views # Uncomment the next two lines to enable the admin: