v5.0.0 Update#171
Conversation
- Wiki documentation preparation (all links fixed for GitHub Wiki format) - Created API_HOME landing page - API V2 implementation with complete endpoint coverage - Added Azure Validation feature with Docker support - Updated to .NET 10 with comprehensive migration guide - Added SQLite configuration repository support - UI/UX improvements and Bootstrap 5 modal updates - Added comprehensive unit tests (300+ tests) - Configuration enhancements and cache improvements - Added health checks and middleware - Updated NuGet packages to latest versions - Added .gitignore rules to prevent large file commits - Workflow documentation for fork/PR process Fixes: - Configuration page cache clear hang - Actions Legend modal opacity - Chevron rotation on log pages - Resource delimiter selection issues - API route template syntax for APIM compatibility - Azure Validation settings migration
v5.0.0 Complete Updates - API V2, Azure Validation, .NET 10, Wiki Docs
There was a problem hiding this comment.
Pull request overview
This pull request introduces comprehensive documentation and workflow improvements for the Azure Naming Tool project v5.0.0. The main additions include a detailed Git workflow guide, expanded technical documentation for v5.0.0 features, and thorough release notes outlining new features, upgrade instructions, and bug fixes.
Key Changes
- Added Git workflow guide with branching, syncing, and CI/CD best practices
- Created comprehensive v5.0.0 documentation covering technical guides, migration instructions, and API documentation
- Added detailed release notes documenting major features including .NET 10 upgrade, dashboard redesign, SQLite support, and Azure Tenant Name Validation
Reviewed changes
Copilot reviewed 29 out of 255 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/development/fix-warnings.ps1 | PowerShell script to fix unused exception variable warnings |
| scripts/development/convert-services.ps1 | Script to identify service conversion targets with hardcoded path |
| docs/v5.0.0/wiki/v5.0.0.md | Release notes documenting v5.0.0 major features and improvements |
| docs/v5.0.0/wiki/V5.0.0_MIGRATION_GUIDE.md | Comprehensive migration guide with deployment instructions and backup procedures |
| docs/v5.0.0/wiki/AZURE_NAME_VALIDATION_GUIDE.md | Complete Azure validation feature documentation with setup and configuration |
| docs/v5.0.0/wiki/AZURE_NAME_VALIDATION_DOCKER_WIKI.md | Docker-specific setup guide for Azure validation |
| docs/v5.0.0/wiki/API_V2.md | Version 2.0 API documentation with examples and migration guide |
| docs/v5.0.0/wiki/API_V1.md | Version 1.0 API documentation with endpoint details |
| docs/v5.0.0/wiki/API.md | API documentation overview comparing V1 and V2 |
| docs/v5.0.0/testing/BACKUP_RESTORE.md | Backup and restore procedures for both storage providers |
| docs/v5.0.0/testing/AZURE_VALIDATION_TESTING_GUIDE.md | Comprehensive testing procedures with automated scripts |
| docs/v5.0.0/testing/AZURE_VALIDATION_SECURITY_GUIDE.md | Security best practices and credential management guide |
| docs/v5.0.0/testing/AZURE_VALIDATION_MIGRATION_FIX.md | Documentation of migration fix for Azure validation settings |
| docs/v5.0.0/development/PHASE5_UI_INTEGRATION_SUMMARY.md | Implementation summary for Phase 5 UI integration |
| docs/v5.0.0/development/AZURE_VALIDATION_FEATURE_COMPLETE.md | Complete feature implementation summary with architecture details |
| docs/v5.0.0/RELEASE_NOTES_v5.0.0.md | Release notes for v5.0.0 with major features and upgrade notes |
| docs/v5.0.0/README.md | Documentation index and organization guide |
| README.md | Updated documentation link to be more prominent |
Comments suppressed due to low confidence (1)
docs/v5.0.0/wiki/AZURE_NAME_VALIDATION_GUIDE.md:1
- Documentation inconsistency: Line 169 states Strategy must be numeric (0, 1, or 2), but lines 162-163 show string values ('NotifyOnly', 'ServicePrincipal') as correct. The example on line 881 also uses string 'ServicePrincipal' marked as correct. Clarify which format is actually required.
# Azure Validation Feature
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Script to help identify service conversion targets | ||
| # This helps us understand which services follow similar patterns | ||
|
|
||
| $servicesPath = "c:\Projects\AzureNamingTool-DEV\src\Services" |
There was a problem hiding this comment.
The hardcoded absolute path 'c:\Projects\AzureNamingTool-DEV\src\Services' makes this script non-portable and will fail on other machines or environments. Consider using a relative path or accepting the path as a parameter: param([string]$servicesPath = \"..\\..\\src\\Services\")
| $servicesPath = "c:\Projects\AzureNamingTool-DEV\src\Services" | |
| param([string]$servicesPath = "..\..\src\Services") |
| - **Port already in use**: Change port in appsettings.json or use environment variable: `ASPNETCORE_URLS=http://localhost:8080` | ||
| - **Permission denied (Linux)**: Ensure proper file permissions: `chmod -R 755 /path/to/azurenamingtool` | ||
|
|
||
| > **� CRITICAL:** After deployment completes and the application starts, you **MUST** restart it again before proceeding to Step 5. This ensures all v5.0.0 features are properly initialized. |
There was a problem hiding this comment.
Corrupted emoji character '�' should be replaced with a proper emoji or removed. Consider using '
| > **� CRITICAL:** After deployment completes and the application starts, you **MUST** restart it again before proceeding to Step 5. This ensures all v5.0.0 features are properly initialized. | |
| > **🔴 CRITICAL:** After deployment completes and the application starts, you **MUST** restart it again before proceeding to Step 5. This ensures all v5.0.0 features are properly initialized. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| _logger.LogError(ex, | ||
| "API request failed. Method: {Method}, Path: {Path}, CorrelationId: {CorrelationId}, Duration: {Duration}ms", | ||
| context.Request.Method, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix the problem, all user-provided input that is logged should be sanitized. For plain text logs, newlines and carriage returns—and optionally other unprintable characters—should be removed or replaced. In this snippet, the most obvious spot is the log entry on line 66-70, where {Method} is filled by context.Request.Method. We should sanitize context.Request.Method before logging it. This can be done inline using a simple .Replace() call replacing all \r and \n with empty strings, or by writing a small helper to do so if used in multiple places.
Changes should be made in src/Middleware/ApiLoggingMiddleware.cs. The code at line 67 should be updated to pass a sanitized method instead of the raw value. Additional imports are not required. The code change is local and straightforward.
| @@ -64,7 +64,7 @@ | ||
|
|
||
| _logger.LogError(ex, | ||
| "API request failed. Method: {Method}, Path: {Path}, CorrelationId: {CorrelationId}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Method.Replace("\r", "").Replace("\n", ""), | ||
| context.Request.Path, | ||
| correlationId, | ||
| stopwatch.ElapsedMilliseconds); |
| _logger.LogError(ex, | ||
| "API request failed. Method: {Method}, Path: {Path}, CorrelationId: {CorrelationId}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Path, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix the problem, sanitize context.Request.Path before it's included in log events. Since logs are stored as plain text or structured log files (not HTML), the right mitigation is to remove or normalize newline characters from this value so a maliciously crafted request path cannot inject a forged log entry. The best fix is to replace all line break characters (\r, \n) with an empty string before logging. This can be done by defining a small helper function or using a chain of .Replace() calls directly when passing to the logger. The fix needs to be applied on line 68 in src/Middleware/ApiLoggingMiddleware.cs, likely by wrapping context.Request.Path with a SanitizeForLog() helper, which strips line breaks. If we use a helper method, we should add it to the same class and call it for each relevant user-controlled input.
| @@ -11,7 +11,13 @@ | ||
| private readonly RequestDelegate _next; | ||
| private readonly ILogger<ApiLoggingMiddleware> _logger; | ||
|
|
||
| /// <summary> | ||
| // Helper to sanitize log input by removing newline characters | ||
| private static string SanitizeForLog(string input) | ||
| { | ||
| if (string.IsNullOrEmpty(input)) | ||
| return input; | ||
| return input.Replace("\r", "").Replace("\n", ""); | ||
| } | ||
| /// Initializes a new instance of the <see cref="ApiLoggingMiddleware"/> class. | ||
| /// </summary> | ||
| /// <param name="next">The next middleware in the pipeline.</param> | ||
| @@ -65,7 +71,7 @@ | ||
| _logger.LogError(ex, | ||
| "API request failed. Method: {Method}, Path: {Path}, CorrelationId: {CorrelationId}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Path, | ||
| SanitizeForLog(context.Request.Path), | ||
| correlationId, | ||
| stopwatch.ElapsedMilliseconds); | ||
|
|
| requestDetails.AppendLine($" Body: {bodyAsText}"); | ||
| } | ||
|
|
||
| _logger.LogInformation(requestDetails.ToString()); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix the problem, all user-controlled values that are written to logs should be sanitized to prevent injection of newline characters or control characters. For plain text logs, the recommended approach is to remove or encode line breaks in values such as request.Method, request.Path, request.QueryString, and the request body, before appending to the log entry.
- In
LogRequestAsync, sanitize all relevant user inputs before appending torequestDetails:request.Methodrequest.Pathrequest.QueryString- The API key (though only the first 8 chars are shown, it's safest to sanitize)
- The request body (
bodyAsText, which may include newlines)
The best way is to define a helper method (e.g. SanitizeForLog) that replaces newlines and possibly other control characters in strings. This method should be added to the same class (for example, as a private static method).
Apply the sanitization when adding each value to the log message (in the above regions).
No dependencies are needed; this can be implemented using native .NET string manipulation.
| @@ -87,15 +87,16 @@ | ||
| // Build request details | ||
| var requestDetails = new StringBuilder(); | ||
| requestDetails.AppendLine($"API Request:"); | ||
| requestDetails.AppendLine($" Method: {request.Method}"); | ||
| requestDetails.AppendLine($" Path: {request.Path}{request.QueryString}"); | ||
| requestDetails.AppendLine($" CorrelationId: {correlationId}"); | ||
| requestDetails.AppendLine($" Method: {SanitizeForLog(request.Method)}"); | ||
| requestDetails.AppendLine($" Path: {SanitizeForLog(request.Path.ToString())}{SanitizeForLog(request.QueryString.ToString())}"); | ||
| requestDetails.AppendLine($" CorrelationId: {SanitizeForLog(correlationId)}"); | ||
|
|
||
| // Log API key info (first few characters only for security) | ||
| if (request.Headers.TryGetValue("APIKey", out var apiKey)) | ||
| { | ||
| var maskedKey = apiKey.ToString().Length > 8 | ||
| ? apiKey.ToString().Substring(0, 8) + "..." | ||
| var apiKeySanitized = SanitizeForLog(apiKey.ToString()); | ||
| var maskedKey = apiKeySanitized.Length > 8 | ||
| ? apiKeySanitized.Substring(0, 8) + "..." | ||
| : "***"; | ||
| requestDetails.AppendLine($" APIKey: {maskedKey}"); | ||
| } | ||
| @@ -113,8 +110,7 @@ | ||
| } | ||
| var bodyAsText = Encoding.UTF8.GetString(buffer, 0, totalRead); | ||
| request.Body.Position = 0; // Reset stream position | ||
|
|
||
| requestDetails.AppendLine($" Body: {bodyAsText}"); | ||
| requestDetails.AppendLine($" Body: {SanitizeForLog(bodyAsText)}"); | ||
| } | ||
|
|
||
| _logger.LogInformation(requestDetails.ToString()); | ||
| @@ -189,6 +185,20 @@ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Sanitizes a string for safe logging: removes newlines and control characters. | ||
| /// </summary> | ||
| private static string SanitizeForLog(string input) | ||
| { | ||
| if (string.IsNullOrEmpty(input)) return ""; | ||
| // Remove newlines and carriage returns and other control characters except basic formatting (if desired) | ||
| // Replace with space or remove | ||
| var sanitized = input.Replace("\r", " ").Replace("\n", " "); | ||
| // Optionally strip other control chars | ||
| sanitized = new string(sanitized.Where(c => !char.IsControl(c)).ToArray()); | ||
| return sanitized; | ||
| } | ||
| } | ||
|
|
||
| /// <summary> |
| { | ||
| _logger.LogError( | ||
| "API request completed with error. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
The problem is that context.Request.Method may be set to a value including malicious characters (e.g., newlines or control characters) by an attacker. To fix this, the method string should be sanitized by removing or replacing problematic characters, most importantly line breaks (\r, \n). The fix should wrap all log statements that include user-derived context.Request.Method so that the value is passed through a sanitizer function before logging.
The best fix is to add a small helper (private) method to sanitize and mark user-input for the log, using string.Replace to strip newlines/carriage returns. This can be accomplished entirely in src/Middleware/ApiLoggingMiddleware.cs by:
- Adding a private method on
ApiLoggingMiddlewareto sanitize strings for logging. - Updating the relevant log statements, including those at lines 167, 176, and 185 (as used in log messages), to pass
SanitizeForLog(context.Request.Method)instead of the original value.
No new dependencies are needed. Only changes within src/Middleware/ApiLoggingMiddleware.cs are required, with modest modifications to the logging calls and addition of the sanitizer method.
| @@ -11,6 +11,14 @@ | ||
| private readonly RequestDelegate _next; | ||
| private readonly ILogger<ApiLoggingMiddleware> _logger; | ||
|
|
||
| // Sanitizes user-controlled values for safe logging | ||
| private static string SanitizeForLog(string input) | ||
| { | ||
| if (string.IsNullOrEmpty(input)) return string.Empty; | ||
| // Remove CR, LF and mark as [USER] for clarity | ||
| return "[USER] " + input.Replace("\r", "").Replace("\n", ""); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="ApiLoggingMiddleware"/> class. | ||
| /// </summary> | ||
| @@ -154,7 +162,7 @@ | ||
| using (_logger.BeginScope(new Dictionary<string, object> | ||
| { | ||
| ["CorrelationId"] = correlationId, | ||
| ["Method"] = context.Request.Method, | ||
| ["Method"] = SanitizeForLog(context.Request.Method), | ||
| ["Path"] = context.Request.Path.ToString(), | ||
| ["StatusCode"] = response.StatusCode, | ||
| ["DurationMs"] = durationMs | ||
| @@ -164,7 +172,7 @@ | ||
| { | ||
| _logger.LogError( | ||
| "API request completed with error. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| SanitizeForLog(context.Request.Method), | ||
| context.Request.Path, | ||
| response.StatusCode, | ||
| durationMs); | ||
| @@ -173,7 +181,7 @@ | ||
| { | ||
| _logger.LogWarning( | ||
| "API request completed with client error. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| SanitizeForLog(context.Request.Method), | ||
| context.Request.Path, | ||
| response.StatusCode, | ||
| durationMs); | ||
| @@ -182,7 +190,7 @@ | ||
| { | ||
| _logger.LogWarning( | ||
| "Slow API request detected. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| SanitizeForLog(context.Request.Method), | ||
| context.Request.Path, | ||
| response.StatusCode, | ||
| durationMs); |
| _logger.LogError( | ||
| "API request completed with error. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Path, |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To resolve this issue, ensure that all user-controlled data being logged (including the request path) is sanitized to remove or neutralize special characters that could be used to forge log entries. In this context, the main concern is newline characters (\r, \n) that can break log structure. A common remedy is to replace any instance of \r or \n in the string with an empty string or another safe character (e.g., space).
Specifically:
- On lines where
context.Request.Pathis used in log messages, replace newline characters from its value before logging. - This fix needs to be applied wherever
context.Request.Pathis logged. - Use
.ToString()to get the string representation, and then.Replace("\r", "").Replace("\n", "")to sanitize.
No new method definitions or external libraries are needed; using built-in string methods is sufficient.
| @@ -165,7 +165,7 @@ | ||
| _logger.LogError( | ||
| "API request completed with error. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Path, | ||
| context.Request.Path.ToString().Replace("\r", "").Replace("\n", ""), | ||
| response.StatusCode, | ||
| durationMs); | ||
| } | ||
| @@ -174,7 +174,7 @@ | ||
| _logger.LogWarning( | ||
| "API request completed with client error. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Path, | ||
| context.Request.Path.ToString().Replace("\r", "").Replace("\n", ""), | ||
| response.StatusCode, | ||
| durationMs); | ||
| } | ||
| @@ -183,7 +183,7 @@ | ||
| _logger.LogWarning( | ||
| "Slow API request detected. Method: {Method}, Path: {Path}, StatusCode: {StatusCode}, Duration: {Duration}ms", | ||
| context.Request.Method, | ||
| context.Request.Path, | ||
| context.Request.Path.ToString().Replace("\r", "").Replace("\n", ""), | ||
| response.StatusCode, | ||
| durationMs); | ||
| } |
| // Use appropriate validation method based on scope | ||
| if (isGlobalScope) | ||
| { | ||
| _logger.LogInformation("Using CheckNameAvailability API for globally unique resource: {ResourceType}", resourceType); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
How to fix, in general terms:
Before logging user-provided data, sanitize that data to ensure it cannot inject new lines or formatting sequences that could alter the log's intended structure or meaning. Generally, this means removing or replacing newline characters (\n and \r) from logged string fields. If possible, also consider quoting or other formatting to clearly demarcate user data.
Detailed fix for this alert:
- In
AzureValidationService.cs, every logging invocation that includes{ResourceType}with data from user input should sanitize theresourceTypevalue. - At minimum, replace occurrences of
resourceTypein log statements with a sanitized form, using.Replace("\r", "").Replace("\n", "")(or other similar patterns), e.g.var sanitizedResourceType = resourceType.Replace("\r", "").Replace("\n", "");
- Update both alert locations (
_logger.LogInformation(...)) on lines 102 ("Using CheckNameAvailability API for globally unique resource") and 107 ("Using Resource Graph query for scoped resource") so that only sanitized values are logged. - Optionally, encapsulate this logic in a helper function if appropriate, but do not introduce broad code changes outside what is required.
Additional needs:
- No new imports are necessary.
- No additional type definitions are required.
- Only code regions from the provided snippets in
AzureValidationService.csneed to be changed.
| @@ -99,12 +99,13 @@ | ||
| // Use appropriate validation method based on scope | ||
| if (isGlobalScope) | ||
| { | ||
| _logger.LogInformation("Using CheckNameAvailability API for globally unique resource: {ResourceType}", resourceType); | ||
| var sanitizedResourceType = resourceType.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogInformation("Using CheckNameAvailability API for globally unique resource: {ResourceType}", sanitizedResourceType); | ||
| validationResult = await CheckNameAvailabilityAsync(resourceName, resourceType, settings); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogInformation("Using Resource Graph query for scoped resource: {ResourceType}", resourceType); | ||
| var sanitizedResourceType = resourceType.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogInformation("Using Resource Graph query for scoped resource: {ResourceType}", sanitizedResourceType); | ||
| validationResult = await CheckResourceExistsAsync(resourceName, resourceType, settings); | ||
| } | ||
|
|
| } | ||
| else | ||
| { | ||
| _logger.LogInformation("Using Resource Graph query for scoped resource: {ResourceType}", resourceType); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix the problem, sanitize user input before it reaches any log sinks. In this case, all logging of the field resourceType (on lines 102 and 107, and anywhere else user input goes directly to logs), must have newlines removed (and optionally mark the value as user input). The best way is to perform string replacement to remove \r and \n characters before logging.
Specifically:
- In
AzureValidationService.cs, inValidateNameAsync, before loggingresourceType(lines 102 and 107), produce a sanitized version for logging, e.g.,SanitizeForLog(resourceType). - If used in other logging contexts, also sanitize.
- Implement
SanitizeForLog(string)as a private helper method in the class.
Required in src/Services/AzureValidationService.cs:
- Add a
SanitizeForLog(string)helper method. This method should replace all newlines and carriage returns in the string with e.g. an empty string. Optionally, other confusion-causing characters could be stripped. - Use the sanitized value in all log statements using
resourceType. - No import is needed; uses
string.Replace.
| @@ -56,6 +56,15 @@ | ||
| /// <summary> | ||
| /// Validates a single resource name against Azure tenant | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Sanitizes user input before logging resourceType values to prevent log forgery. | ||
| /// </remarks> | ||
| private static string SanitizeForLog(string input) | ||
| { | ||
| if (string.IsNullOrEmpty(input)) return string.Empty; | ||
| return input.Replace("\r", "").Replace("\n", ""); | ||
| } | ||
|
|
||
| public async Task<AzureValidationMetadata> ValidateNameAsync(string resourceName, string resourceType) | ||
| { | ||
| var metadata = new AzureValidationMetadata | ||
| @@ -99,12 +108,12 @@ | ||
| // Use appropriate validation method based on scope | ||
| if (isGlobalScope) | ||
| { | ||
| _logger.LogInformation("Using CheckNameAvailability API for globally unique resource: {ResourceType}", resourceType); | ||
| _logger.LogInformation("Using CheckNameAvailability API for globally unique resource: {ResourceType}", SanitizeForLog(resourceType)); | ||
| validationResult = await CheckNameAvailabilityAsync(resourceName, resourceType, settings); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogInformation("Using Resource Graph query for scoped resource: {ResourceType}", resourceType); | ||
| _logger.LogInformation("Using Resource Graph query for scoped resource: {ResourceType}", SanitizeForLog(resourceType)); | ||
| validationResult = await CheckResourceExistsAsync(resourceName, resourceType, settings); | ||
| } | ||
|
|
| var resourceTypeInfo = await GetResourceTypeInfoAsync(resourceType); | ||
| if (resourceTypeInfo == null) | ||
| { | ||
| _logger.LogWarning("Could not find resource type info for {ResourceType}", resourceType); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
The correct fix is to sanitize resourceType before logging it, removing carriage returns (\r), newlines (\n), and any other characters that could lead to log ticket manipulation or forging (such as tabs). Appending a visual marker such as "" is good practice, but .NET's logging frameworks already separate variables. The minimal and recommended fix is to replace line breaks or other dangerous characters.
- In src/Services/AzureValidationService.cs, before calling
_logger.LogWarning("Could not find resource type info for {ResourceType}", resourceType);, sanitizeresourceTypeby removing or replacing dangerous characters. - This can be achieved by calling
resourceType.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", ""), or by using a helper method for clarity. - Only the lines related to the potentially dangerous log statements (here, specifically line 583) need to change; method signatures and other usages may remain as they are.
No additional dependencies are needed, as these are pure .NET string operations.
| @@ -580,7 +580,11 @@ | ||
| var resourceTypeInfo = await GetResourceTypeInfoAsync(resourceType); | ||
| if (resourceTypeInfo == null) | ||
| { | ||
| _logger.LogWarning("Could not find resource type info for {ResourceType}", resourceType); | ||
| // Sanitize user input before logging to prevent log forging | ||
| var sanitizedResourceType = resourceType.Replace(Environment.NewLine, "") | ||
| .Replace("\n", "") | ||
| .Replace("\r", ""); | ||
| _logger.LogWarning("Could not find resource type info for {ResourceType}", sanitizedResourceType); | ||
| return (false, new List<string>()); | ||
| } | ||
|
|
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Error getting resource type info for {ResourceType}", resourceTypeShortName); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
To fix this vulnerability, sanitize user-controlled input before including it in log entries. Since logs appear to be plain text, newlines and any other special characters that could alter the log format should be stripped or encoded. The simplest approach is to remove or replace newline characters from the resourceTypeShortName value before passing it to the logger in line 671 of src/Services/AzureValidationService.cs. Use string.Replace(Environment.NewLine, "") and also cover the standard "\r" and "\n" cases to cover different environments. Perform the sanitization inline before logging. Since you cannot change logger configuration or other files, limit your change to the affected log statement.
| @@ -668,7 +668,9 @@ | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning(ex, "Error getting resource type info for {ResourceType}", resourceTypeShortName); | ||
| // Sanitize user input before logging to prevent log forging | ||
| var sanitizedResourceType = resourceTypeShortName.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogWarning(ex, "Error getting resource type info for {ResourceType}", sanitizedResourceType); | ||
| return null; | ||
| } | ||
| } |
| var resourceTypeInfo = await GetResourceTypeInfoAsync(resourceType); | ||
| if (resourceTypeInfo == null) | ||
| { | ||
| _logger.LogWarning("Could not find resource type info for {ResourceType}, falling back to Resource Graph", resourceType); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 months ago
The best way to fix the problem is to sanitize the user input resourceType before logging by removing or escaping newline (and optionally other control) characters. In C#, the common mitigation is to use String.Replace(Environment.NewLine, ""), .Replace("\r", ""), and .Replace("\n", "") to strip carriage returns and newlines. This fix should be applied directly in the logging statements inside CheckNameAvailabilityAsync (at line 690) so that anywhere we log user-controllable values, they are first sanitized.
Specifically, update the _logger.LogWarning call at line 690 to log a sanitized version of resourceType. As the same pattern occurs for two alert variants, we ensure all logs in this method where resourceType may be interpolated are sanitized (also line 691, 712, and possibly others, as a best practice).
No new imports are required.
| @@ -687,7 +687,9 @@ | ||
| var resourceTypeInfo = await GetResourceTypeInfoAsync(resourceType); | ||
| if (resourceTypeInfo == null) | ||
| { | ||
| _logger.LogWarning("Could not find resource type info for {ResourceType}, falling back to Resource Graph", resourceType); | ||
| // Sanitize resourceType to prevent log forging | ||
| var sanitizedResourceType = resourceType?.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogWarning("Could not find resource type info for {ResourceType}, falling back to Resource Graph", sanitizedResourceType); | ||
| return await CheckResourceExistsAsync(resourceName, resourceType, settings); | ||
| } | ||
|
|
|
Closing this PR as it was created with incorrect branch comparison (mspnp:dev → mspnp:main). The correct PR structure should be from my fork to the upstream dev branch:
I'll create a new PR with the correct branch structure that includes all 255 files of v5.0.0 updates (API V2, Azure Validation, .NET 10 migration, unit tests, etc.). This will also allow me to push updates to address the Copilot recommendations. |
- Add SanitizeForLog helper methods to ApiLoggingMiddleware and AzureValidationService - Sanitize all user-controlled input (Method, Path, QueryString, ResourceType) before logging - Prevents log injection attacks (CodeQL alerts) - Fix hardcoded absolute path in convert-services.ps1 - Fix corrupted emoji character in migration guides (� → 🔴) Addresses all Copilot and CodeQL security recommendations from PR Azure#171
v5.0.0 Complete Updates
Wiki Documentation
API V2 Implementation
Azure Validation Feature
.NET 10 Migration
Architecture Improvements
Testing
UI/UX Improvements
Bug Fixes
Repository Hygiene
Testing
✅ Project builds successfully (0 errors, 0 warnings)
✅ All wiki links validated
✅ 300+ unit tests passing
Documentation
docs/v5.0.0/V5.0.0_MIGRATION_GUIDE.mddocs/v5.0.0/RELEASE_NOTES_v5.0.0.md