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

feat: enhance DoImageMultiPartUpload with detailed logging and bounda… #268

Merged
merged 1 commit into from
Nov 26, 2024
Merged

feat: enhance DoImageMultiPartUpload with detailed logging and bounda… #268

merged 1 commit into from
Nov 26, 2024

Conversation

ShocOne
Copy link
Contributor

@ShocOne ShocOne commented Nov 26, 2024

…ry handling

Change

Thank you for your contribution !
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.

Type of Change

Please DELETE options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update (Wiki)
  • Refactor (refactoring code, removing code, changing code structure)

Checklist

  • I'm sure there are no other open Pull Requests for the same update/change
  • My corresponding pipelines / checks run clean and green without any errors or warnings
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (readme)
  • I did format my code

@ShocOne ShocOne merged commit 8ace545 into main Nov 26, 2024
7 of 8 checks passed
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 26, 2024
View reviewed changes
httpclient/multipartrequest.go
req.Header.Add("Content-Type", contentTypeHeader)

c.Sugar.Debugw("Request headers before auth",
zap.Any("headers", req.Header),

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
Loading
flows to a logging call.

Copilot Autofix AI about 1 month ago

To fix the problem, we should avoid logging the entire HTTP request headers directly. Instead, we can selectively log only the non-sensitive parts of the headers or obfuscate sensitive information before logging. In this case, we will remove the logging of the entire headers and only log the content type header, which is less likely to contain sensitive information.

  • Remove the logging of the entire request headers.
  • Log only the content type header, which is less likely to contain sensitive information.
  • Ensure that no sensitive information is logged in clear text.
Suggested changeset 1
httpclient/multipartrequest.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/httpclient/multipartrequest.go b/httpclient/multipartrequest.go
--- a/httpclient/multipartrequest.go
+++ b/httpclient/multipartrequest.go
@@ -487,4 +487,3 @@
 
-	c.Sugar.Debugw("Request headers before auth",
-		zap.Any("headers", req.Header),
+	c.Sugar.Debugw("Request content type before auth",
 		zap.String("contentType", contentTypeHeader))
@@ -493,4 +492,4 @@
 
-	c.Sugar.Debugw("Request headers after auth",
-		zap.Any("headers", req.Header))
+	c.Sugar.Debugw("Request content type after auth",
+		zap.String("contentType", req.Header.Get("Content-Type")))
 
EOF
@@ -487,4 +487,3 @@

c.Sugar.Debugw("Request headers before auth",
zap.Any("headers", req.Header),
c.Sugar.Debugw("Request content type before auth",
zap.String("contentType", contentTypeHeader))
@@ -493,4 +492,4 @@

c.Sugar.Debugw("Request headers after auth",
zap.Any("headers", req.Header))
c.Sugar.Debugw("Request content type after auth",
zap.String("contentType", req.Header.Get("Content-Type")))

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
httpclient/multipartrequest.go

(*c.Integration).PrepRequestParamsAndAuth(req)

c.Sugar.Debugw("Request headers after auth",
zap.Any("headers", req.Header))

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
Loading
flows to a logging call.

Copilot Autofix AI about 1 month ago

To fix the problem, we should avoid logging sensitive information contained in the headers. Instead of logging all headers, we can selectively log non-sensitive headers or obfuscate sensitive values before logging. This ensures that sensitive information is not exposed in the logs.

  • Identify and filter out sensitive headers before logging.
  • Obfuscate or mask sensitive values if they need to be logged for debugging purposes.
  • Update the logging statements to reflect these changes.
Suggested changeset 1
httpclient/multipartrequest.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/httpclient/multipartrequest.go b/httpclient/multipartrequest.go
--- a/httpclient/multipartrequest.go
+++ b/httpclient/multipartrequest.go
@@ -493,4 +493,13 @@
 
+	// Filter out sensitive headers before logging
+	safeHeaders := make(http.Header)
+	for k, v := range req.Header {
+		if strings.ToLower(k) == "authorization" || strings.ToLower(k) == "cookie" {
+			safeHeaders[k] = []string{"[REDACTED]"}
+		} else {
+			safeHeaders[k] = v
+		}
+	}
 	c.Sugar.Debugw("Request headers after auth",
-		zap.Any("headers", req.Header))
+		zap.Any("headers", safeHeaders))
 
EOF
@@ -493,4 +493,13 @@

// Filter out sensitive headers before logging
safeHeaders := make(http.Header)
for k, v := range req.Header {
if strings.ToLower(k) == "authorization" || strings.ToLower(k) == "cookie" {
safeHeaders[k] = []string{"[REDACTED]"}
} else {
safeHeaders[k] = v
}
}
c.Sugar.Debugw("Request headers after auth",
zap.Any("headers", req.Header))
zap.Any("headers", safeHeaders))

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
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.
1 participant
@ShocOne