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.

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

feat: enhance DoImageMultiPartUpload with URL encoding and improved l… #270

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

ShocOne
Copy link
Contributor

@ShocOne ShocOne commented Nov 26, 2024

…ogging

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 1bb942c into main Nov 26, 2024
7 of 8 checks passed
zap.String("contentType", req.Header.Get("Content-Type")),
zap.String("accept", req.Header.Get("Accept")))
c.Sugar.Infow("Final request headers",
zap.Any("headers", req.Header),

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

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

Copilot Autofix AI 4 months 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.

The best way to fix this without changing existing functionality is to:

  1. Identify and exclude sensitive headers from being logged.
  2. Optionally, obfuscate the values of sensitive headers if they need to be logged for debugging purposes.

We will modify the logging statements to exclude or obfuscate sensitive headers.

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
@@ -2,2 +2,3 @@
 
+
 import (
@@ -484,5 +485,5 @@
 	c.Sugar.Debugw("Initial headers",
-		zap.Any("headers", req.Header),
 		zap.String("accept", req.Header.Get("accept")),
-		zap.String("content-type", req.Header.Get("content-type")))
+		zap.String("content-type", req.Header.Get("content-type")),
+		zap.String("authorization", obfuscateHeader(req.Header.Get("authorization"))))
 
@@ -500,5 +501,5 @@
 	c.Sugar.Infow("Final request headers",
-		zap.Any("headers", req.Header),
 		zap.String("accept", req.Header.Get("accept")),
-		zap.String("content-type", req.Header.Get("content-type")))
+		zap.String("content-type", req.Header.Get("content-type")),
+		zap.String("authorization", obfuscateHeader(req.Header.Get("authorization"))))
 
EOF
@@ -2,2 +2,3 @@


import (
@@ -484,5 +485,5 @@
c.Sugar.Debugw("Initial headers",
zap.Any("headers", req.Header),
zap.String("accept", req.Header.Get("accept")),
zap.String("content-type", req.Header.Get("content-type")))
zap.String("content-type", req.Header.Get("content-type")),
zap.String("authorization", obfuscateHeader(req.Header.Get("authorization"))))

@@ -500,5 +501,5 @@
c.Sugar.Infow("Final request headers",
zap.Any("headers", req.Header),
zap.String("accept", req.Header.Get("accept")),
zap.String("content-type", req.Header.Get("content-type")))
zap.String("content-type", req.Header.Get("content-type")),
zap.String("authorization", obfuscateHeader(req.Header.Get("authorization"))))

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant