Skip to content

Conversation

@MWG-Logan
Copy link
Member

No description provided.

MWG-Logan and others added 13 commits August 7, 2025 18:28
This commit refactors several classes, including `BicepTemplatesFunctions`, `DeploymentsFunctions`, `StatusFunctions`, `BicepTemplateService`, and `DeploymentService`, to utilize constructor injection for services and logging. This change improves dependency management and reduces the reliance on private fields.

Logging statements have been updated to use the injected `logger`, ensuring consistency across the application. The OpenAPI attributes have been modified to use array syntax for tags, aligning with current conventions.

Additionally, `Program.cs` has been updated to configure services with `AddHttpClient()`, and unnecessary commented-out code has been removed. Error handling has been enhanced in various methods, particularly for argument exceptions during deployment creation.

Overall, these changes enhance the maintainability, readability, and performance of the codebase by adopting modern practices in dependency injection and logging.
This commit introduces a new Blazor WebAssembly project (`Mordos.Web`) and integrates user authentication features across the application. Key changes include:

- Updated `Mordos.API.csproj` with Newtonsoft.Json support.
- Enhanced `Program.cs` for Azure serialization and DI.
- Added `LoginDisplay.razor` and `RedirectToLogin.razor` components for user authentication.
- Created `Authentication.razor` for managing login/logout actions.
- Established `appsettings.json` for Azure AD configuration.
- Added service worker files for offline support and caching.
- Included design requirements documentation and updated styles in `app.css`.
- Updated solution file to include the new project.
---
updated-dependencies:
- dependency-name: Aspire.Hosting.AppHost
  dependency-version: 9.4.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
---
updated-dependencies:
- dependency-name: Azure.Identity
  dependency-version: 1.15.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
---
updated-dependencies:
- dependency-name: Microsoft.Extensions.ServiceDiscovery
  dependency-version: 9.4.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
---
updated-dependencies:
- dependency-name: Microsoft.Extensions.Http.Resilience
  dependency-version: 9.8.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…efaults/Microsoft.Extensions.Http.Resilience-9.8.0

Bump Microsoft.Extensions.Http.Resilience from 9.4.0 to 9.8.0
…efaults/Microsoft.Extensions.ServiceDiscovery-9.4.1

Bump Microsoft.Extensions.ServiceDiscovery from 9.3.1 to 9.4.1
…e.Identity-1.15.0

Bump Azure.Identity from 1.14.2 to 1.15.0
…Aspire.Hosting.AppHost-9.4.1

Bump Aspire.Hosting.AppHost from 9.3.1 to 9.4.1
var tagFilter = req.Query["tagFilter"].FirstOrDefault();

_logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);
logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 4 months ago

To fix the log forging vulnerability, we need to sanitize any user input before logging it. Since the log entry is plain text and the risk described involves newlines, we should remove carriage return (\r) and newline (\n) characters from the string before logging. We'll apply this to both nameFilter and tagFilter, since both are coming from user input and logged. The replacement should be for both possible null or empty values. The best way is to sanitize both variables shortly after extraction, before logging (line 36 or before 37). This fix will not alter the filters passed to the main business logic; it only affects what gets logged. No new methods or dependencies are required; a simple .Replace() (or regex if warranted) is enough.


Suggested changeset 1
Mordos.API/Functions/BicepTemplatesFunctions.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Mordos.API/Functions/BicepTemplatesFunctions.cs b/Mordos.API/Functions/BicepTemplatesFunctions.cs
--- a/Mordos.API/Functions/BicepTemplatesFunctions.cs
+++ b/Mordos.API/Functions/BicepTemplatesFunctions.cs
@@ -34,8 +34,11 @@
             var nameFilter = req.Query["nameFilter"].FirstOrDefault();
             var tagFilter = req.Query["tagFilter"].FirstOrDefault();
 
-            logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);
+            var safeNameFilter = nameFilter?.Replace("\r", "").Replace("\n", "");
+            var safeTagFilter = tagFilter?.Replace("\r", "").Replace("\n", "");
 
+            logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", safeNameFilter, safeTagFilter);
+
             var templates = await bicepTemplateService.GetAllTemplatesAsync(nameFilter, tagFilter);
             return new OkObjectResult(templates);
         }
EOF
@@ -34,8 +34,11 @@
var nameFilter = req.Query["nameFilter"].FirstOrDefault();
var tagFilter = req.Query["tagFilter"].FirstOrDefault();

logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);
var safeNameFilter = nameFilter?.Replace("\r", "").Replace("\n", "");
var safeTagFilter = tagFilter?.Replace("\r", "").Replace("\n", "");

logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", safeNameFilter, safeTagFilter);

var templates = await bicepTemplateService.GetAllTemplatesAsync(nameFilter, tagFilter);
return new OkObjectResult(templates);
}
Copilot is powered by AI and may make mistakes. Always verify output.
var tagFilter = req.Query["tagFilter"].FirstOrDefault();

_logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);
logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 4 months ago

The best fix is to sanitize the user-provided input before logging it. For plain text logs, this means stripping or replacing newline (\n) and carriage return (\r) characters from nameFilter and tagFilter before passing them to the logger. This should be done right before logging, ensuring any future changes to log format or usage are covered. We only edit the logging line (line 37) to use sanitized variants of both filters. No new dependencies are required; use standard string replacement.


Suggested changeset 1
Mordos.API/Functions/BicepTemplatesFunctions.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Mordos.API/Functions/BicepTemplatesFunctions.cs b/Mordos.API/Functions/BicepTemplatesFunctions.cs
--- a/Mordos.API/Functions/BicepTemplatesFunctions.cs
+++ b/Mordos.API/Functions/BicepTemplatesFunctions.cs
@@ -34,7 +34,9 @@
             var nameFilter = req.Query["nameFilter"].FirstOrDefault();
             var tagFilter = req.Query["tagFilter"].FirstOrDefault();
 
-            logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);
+            var safeNameFilter = nameFilter?.Replace("\r", "").Replace("\n", "");
+            var safeTagFilter = tagFilter?.Replace("\r", "").Replace("\n", "");
+            logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", safeNameFilter, safeTagFilter);
 
             var templates = await bicepTemplateService.GetAllTemplatesAsync(nameFilter, tagFilter);
             return new OkObjectResult(templates);
EOF
@@ -34,7 +34,9 @@
var nameFilter = req.Query["nameFilter"].FirstOrDefault();
var tagFilter = req.Query["tagFilter"].FirstOrDefault();

logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", nameFilter, tagFilter);
var safeNameFilter = nameFilter?.Replace("\r", "").Replace("\n", "");
var safeTagFilter = tagFilter?.Replace("\r", "").Replace("\n", "");
logger.LogInformation("Getting Bicep templates with filters - Name: {NameFilter}, Tag: {TagFilter}", safeNameFilter, safeTagFilter);

var templates = await bicepTemplateService.GetAllTemplatesAsync(nameFilter, tagFilter);
return new OkObjectResult(templates);
Copilot is powered by AI and may make mistakes. Always verify output.
}

_logger.LogInformation("Getting deployments with filters - TemplateId: {TemplateId}, Status: {Status}", templateIdFilter, status);
logger.LogInformation("Getting deployments with filters - TemplateId: {TemplateId}, Status: {Status}", templateIdFilter, status);

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 4 months ago

To fix this issue, we should sanitize the user input prior to logging. Since the log appears to be plain text (no sign it will be consumed as HTML), new-line characters and carriage returns should be removed from templateIdFilter before it is included in log entries. This can be done using String.Replace or a regex, removing all occurrences of \r and \n.

  • Edit the affected region in GetDeployments where the input is logged.
  • Create a sanitized version of templateIdFilter by removing line breaks, for example:
    var sanitizedTemplateIdFilter = templateIdFilter?.Replace("\r", "").Replace("\n", "");
  • Use this sanitized value in the logging statement.
  • No external dependencies are required for basic string manipulation.

Suggested changeset 1
Mordos.API/Functions/DeploymentsFunctions.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Mordos.API/Functions/DeploymentsFunctions.cs b/Mordos.API/Functions/DeploymentsFunctions.cs
--- a/Mordos.API/Functions/DeploymentsFunctions.cs
+++ b/Mordos.API/Functions/DeploymentsFunctions.cs
@@ -35,13 +35,16 @@
             var templateIdFilter = req.Query["templateId"].FirstOrDefault();
             var statusFilter = req.Query["status"].FirstOrDefault();
 
+            // Sanitize user input to prevent log forging
+            var sanitizedTemplateIdFilter = templateIdFilter?.Replace("\r", "").Replace("\n", "");
+
             DeploymentStatus? status = null;
             if (!string.IsNullOrEmpty(statusFilter) && Enum.TryParse<DeploymentStatus>(statusFilter, true, out var parsedStatus))
             {
                 status = parsedStatus;
             }
 
-            logger.LogInformation("Getting deployments with filters - TemplateId: {TemplateId}, Status: {Status}", templateIdFilter, status);
+            logger.LogInformation("Getting deployments with filters - TemplateId: {TemplateId}, Status: {Status}", sanitizedTemplateIdFilter, status);
 
             var deployments = await deploymentService.GetAllDeploymentsAsync(templateIdFilter, status);
             return new OkObjectResult(deployments);
EOF
@@ -35,13 +35,16 @@
var templateIdFilter = req.Query["templateId"].FirstOrDefault();
var statusFilter = req.Query["status"].FirstOrDefault();

// Sanitize user input to prevent log forging
var sanitizedTemplateIdFilter = templateIdFilter?.Replace("\r", "").Replace("\n", "");

DeploymentStatus? status = null;
if (!string.IsNullOrEmpty(statusFilter) && Enum.TryParse<DeploymentStatus>(statusFilter, true, out var parsedStatus))
{
status = parsedStatus;
}

logger.LogInformation("Getting deployments with filters - TemplateId: {TemplateId}, Status: {Status}", templateIdFilter, status);
logger.LogInformation("Getting deployments with filters - TemplateId: {TemplateId}, Status: {Status}", sanitizedTemplateIdFilter, status);

var deployments = await deploymentService.GetAllDeploymentsAsync(templateIdFilter, status);
return new OkObjectResult(deployments);
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants