Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

promote #751

Merged
merged 36 commits into from
Jan 29, 2025
Merged

promote #751

merged 36 commits into from
Jan 29, 2025

Conversation

leewrigh
Copy link
Collaborator

No description provided.

leewrigh and others added 30 commits December 2, 2024 13:59
@leewrigh
Allow AUF access
cf0fef2
@leewrigh
App updates
fb9c253
@leewrigh
Model updates
7840a18
@leewrigh
Model updates
3bad1e5
@leewrigh
More commands
ef8bc6c
@leewrigh
model changes
18c8164
@SujithaChandragupthanAG
update admincommandset
6a3e1ad
@leewrigh
WIP
905f6dc
@leewrigh
Merge remote-tracking branch 'origin/BCPSDEMS-2162-pvt' into DIAM-56-…
df5fe95 
…admin
@leewrigh
merge pvt branch
ccca00d
@leewrigh
Merge pull request #740 from bcgov/DIAM-56-admin
da1c074 
Diam 56 admin
@leewrigh
try net 9 build
03aac75
@leewrigh
Merge pull request #743 from bcgov/DIAM-56-admin
0c3b413 
try net 9 build
@leewrigh
nuget and net 9 changes
c77a2a2
@leewrigh
Merge pull request #744 from bcgov/DIAM-56-admin
7d3a1d0 
nuget and net 9 changes
@leewrigh
add mono
9938e17
@leewrigh
Merge pull request #745 from bcgov/DIAM-56-admin
a712776 
add mono
@leewrigh
install mono
9ea5801
@leewrigh
Merge pull request #746 from bcgov/DIAM-56-admin
be70b0b 
install mono
@leewrigh
Model updates
6dbe1ba
@leewrigh
Model changes
5cf8c3d
@leewrigh
WIP
c70dbf2
@leewrigh
wip
41fc8e3
@leewrigh
wip
2f35075
@leewrigh
Model changes
f2126ee
@leewrigh
wip
7613837
@leewrigh
Add field
1cef2b2
@leewrigh
merge
766d90c
@leewrigh
delegate approval updates
bd6c0a6
@leewrigh
wip
950b210
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 29, 2025
View reviewed changes
backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
response.ResponseData.Add("delegateRequestId", "" + newRequest.RequestId);

var msgId = Guid.NewGuid();
logger.LogInformation($"Sending notification to {lawyerParty.Email} for new delgate request {requestor.Email} [{command.StagingRequestId}]");

Check warning

Code scanning / CodeQL

Exposure of private information Medium

Private data returned by
access to local variable email
Loading
is written to an external location.
Private data returned by
access to local variable email
Loading
is written to an external location.

Copilot Autofix AI 9 days ago

To fix the problem, we should avoid logging sensitive information such as email addresses. Instead, we can log less sensitive information or use anonymized data. In this case, we can remove the email addresses from the log message and replace them with anonymized identifiers or other non-sensitive information.

  • Remove the email addresses from the log message on line 86 in SubmitDelegateRequest.cs.
  • Ensure that the log message still provides useful information for debugging or auditing purposes without exposing private data.
Suggested changeset 1
backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.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/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs b/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
--- a/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
+++ b/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
@@ -85,3 +85,3 @@
                         var msgId = Guid.NewGuid();
-                        logger.LogInformation($"Sending notification to {lawyerParty.Email} for new delgate request {requestor.Email} [{command.StagingRequestId}]");
+                        logger.LogInformation($"Sending notification for new delegate request from party {requestor.Id} to lawyer party {lawyerParty.Id} [{command.StagingRequestId}]");
                         // send a notification to the defense counsel that they have a pending request
EOF
@@ -85,3 +85,3 @@
var msgId = Guid.NewGuid();
logger.LogInformation($"Sending notification to {lawyerParty.Email} for new delgate request {requestor.Email} [{command.StagingRequestId}]");
logger.LogInformation($"Sending notification for new delegate request from party {requestor.Id} to lawyer party {lawyerParty.Id} [{command.StagingRequestId}]");
// send a notification to the defense counsel that they have a pending request
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
response.ResponseData.Add("delegateRequestId", "" + newRequest.RequestId);

var msgId = Guid.NewGuid();
logger.LogInformation($"Sending notification to {lawyerParty.Email} for new delgate request {requestor.Email} [{command.StagingRequestId}]");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI 9 days ago

To fix the problem, we need to sanitize the user input before logging it. Specifically, we should remove any new line characters from the email addresses and other user-provided values to prevent log forging. This can be done using the String.Replace method to replace new line characters with an empty string.

Suggested changeset 1
backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.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/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs b/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
--- a/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
+++ b/backend/webapi/Features/DelegateAccess/Commands/SubmitDelegateRequest.cs
@@ -85,3 +85,6 @@
                         var msgId = Guid.NewGuid();
-                        logger.LogInformation($"Sending notification to {lawyerParty.Email} for new delgate request {requestor.Email} [{command.StagingRequestId}]");
+                        var sanitizedLawyerEmail = lawyerParty.Email.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
+                        var sanitizedRequestorEmail = requestor.Email.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
+                        var sanitizedStagingRequestId = command.StagingRequestId.ToString().Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
+                        logger.LogInformation($"Sending notification to {sanitizedLawyerEmail} for new delgate request {sanitizedRequestorEmail} [{sanitizedStagingRequestId}]");
                         // send a notification to the defense counsel that they have a pending request
EOF
@@ -85,3 +85,6 @@
var msgId = Guid.NewGuid();
logger.LogInformation($"Sending notification to {lawyerParty.Email} for new delgate request {requestor.Email} [{command.StagingRequestId}]");
var sanitizedLawyerEmail = lawyerParty.Email.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
var sanitizedRequestorEmail = requestor.Email.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
var sanitizedStagingRequestId = command.StagingRequestId.ToString().Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
logger.LogInformation($"Sending notification to {sanitizedLawyerEmail} for new delgate request {sanitizedRequestorEmail} [{sanitizedStagingRequestId}]");
// send a notification to the defense counsel that they have a pending request
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
Party = requestor,
LawyerPartyId = lawyer.Id
});
logger.LogInformation($"Storing staging for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI 9 days ago

To fix the problem, we need to sanitize the user input before logging it. Since the log entries are plain text, we should remove any new line characters from the user input to prevent log forging. This can be done using the String.Replace method to replace new line characters with an empty string.

We will modify the log message on line 72 in the file backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs to sanitize the requestor.FirstName and requestor.LastName values before logging them.

Suggested changeset 1
backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.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/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs b/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
--- a/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
+++ b/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
@@ -71,3 +71,5 @@
                             });
-                            logger.LogInformation($"Storing staging for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");
+                            var sanitizedFirstName = requestor.FirstName.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
+                            var sanitizedLastName = requestor.LastName.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
+                            logger.LogInformation($"Storing staging for {sanitizedFirstName} {sanitizedLastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");
 
EOF
@@ -71,3 +71,5 @@
});
logger.LogInformation($"Storing staging for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");
var sanitizedFirstName = requestor.FirstName.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
var sanitizedLastName = requestor.LastName.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "");
logger.LogInformation($"Storing staging for {sanitizedFirstName} {sanitizedLastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
}
else
{
logger.LogError($"Failed to store staging record for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName}");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI 9 days ago

To fix the problem, we need to sanitize the user input before logging it. Specifically, we should remove any new line characters from the user-provided values (requestor.FirstName and requestor.LastName) to prevent log forging attacks. This can be achieved using the String.Replace method to replace new line characters with an empty string.

Suggested changeset 1
backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.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/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs b/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
--- a/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
+++ b/backend/webapi/Features/DelegateAccess/Query/DelegateAccessValidationQuery.cs
@@ -71,3 +71,3 @@
                             });
-                            logger.LogInformation($"Storing staging for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");
+                            logger.LogInformation($"Storing staging for {requestor.FirstName.Replace(Environment.NewLine, "")} {requestor.LastName.Replace(Environment.NewLine, "")} to delegate to {lawyer.FirstName.Replace(Environment.NewLine, "")} {lawyer.LastName.Replace(Environment.NewLine, "")} with id {stagingGuid.ToString()}");
 
@@ -81,3 +81,3 @@
                             {
-                                logger.LogError($"Failed to store staging record for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName}");
+                                logger.LogError($"Failed to store staging record for {requestor.FirstName.Replace(Environment.NewLine, "")} {requestor.LastName.Replace(Environment.NewLine, "")} to delegate to {lawyer.FirstName.Replace(Environment.NewLine, "")} {lawyer.LastName.Replace(Environment.NewLine, "")}");
                                 throw new AccessRequestException($"Failed to store staging record");
EOF
@@ -71,3 +71,3 @@
});
logger.LogInformation($"Storing staging for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName} with id {stagingGuid.ToString()}");
logger.LogInformation($"Storing staging for {requestor.FirstName.Replace(Environment.NewLine, "")} {requestor.LastName.Replace(Environment.NewLine, "")} to delegate to {lawyer.FirstName.Replace(Environment.NewLine, "")} {lawyer.LastName.Replace(Environment.NewLine, "")} with id {stagingGuid.ToString()}");

@@ -81,3 +81,3 @@
{
logger.LogError($"Failed to store staging record for {requestor.FirstName} {requestor.LastName} to delegate to {lawyer.FirstName} {lawyer.LastName}");
logger.LogError($"Failed to store staging record for {requestor.FirstName.Replace(Environment.NewLine, "")} {requestor.LastName.Replace(Environment.NewLine, "")} to delegate to {lawyer.FirstName.Replace(Environment.NewLine, "")} {lawyer.LastName.Replace(Environment.NewLine, "")}");
throw new AccessRequestException($"Failed to store staging record");
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@leewrigh leewrigh merged commit 6a47ec2 into test Jan 29, 2025
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@leewrigh @SujithaChandragupthanAG