Skip to content

Conversation

@peterbed
Copy link
Collaborator

No description provided.

}
catch (Exception ex)
{
_logger.LogError(ex, "Error retrieving foster family with GUID: -", guid);

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 3 days ago

To fix the problem, sanitize the user-provided guid before including it in any log entries. For plain-text logs, this typically means removing line breaks and other control characters that could be interpreted as log delimiters, and optionally trimming whitespace. We should also avoid altering the behavior of the application itself, so the original guid value must still be used for the database query and exceptions; only the logged representation should be sanitized.

The best minimal fix here is to introduce a local sanitized variable inside GetFosterFamily, derived from guid by removing \r and \n (and optionally trimming). Then use this sanitized variable in the _logger.LogError call while leaving all other logic untouched. We do not need new imports; string.Replace and string.Trim are part of the BCL. Concretely, inside GetFosterFamily(string guid), right before the try (or at the top of the method), define var safeGuid = guid?.Replace("\r", "").Replace("\n", "").Trim(); and change the logging call on line 84 to use safeGuid instead of guid. No other files strictly need changes; the controller already sanitizes guid in its own logging statement using Replace(Environment.NewLine, "").

Suggested changeset 1
CheckYourEligibility.API/Gateways/FosterFamilyGateway.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/CheckYourEligibility.API/Gateways/FosterFamilyGateway.cs b/CheckYourEligibility.API/Gateways/FosterFamilyGateway.cs
--- a/CheckYourEligibility.API/Gateways/FosterFamilyGateway.cs
+++ b/CheckYourEligibility.API/Gateways/FosterFamilyGateway.cs
@@ -64,6 +64,10 @@
 
     public async Task<FosterFamilyResponse?> GetFosterFamily(string guid)
     {
+        var safeGuid = guid?
+            .Replace("\r", string.Empty)
+            .Replace("\n", string.Empty)
+            .Trim();
 
         try
         {
@@ -81,7 +85,7 @@
         }
         catch (Exception ex)
         {
-            _logger.LogError(ex, "Error retrieving foster family with GUID: -", guid);
+            _logger.LogError(ex, "Error retrieving foster family with GUID: -", safeGuid);
             throw new NotFoundException($"Unable to find foster family: - {guid}, {ex.Message}");
         }
 
EOF
@@ -64,6 +64,10 @@

public async Task<FosterFamilyResponse?> GetFosterFamily(string guid)
{
var safeGuid = guid?
.Replace("\r", string.Empty)
.Replace("\n", string.Empty)
.Trim();

try
{
@@ -81,7 +85,7 @@
}
catch (Exception ex)
{
_logger.LogError(ex, "Error retrieving foster family with GUID: -", guid);
_logger.LogError(ex, "Error retrieving foster family with GUID: -", safeGuid);
throw new NotFoundException($"Unable to find foster family: - {guid}, {ex.Message}");
}

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