Skip to content

Conversation

@LinuxSuRen
Copy link
Owner

We highly recommend you read the contributor's documentation before starting the review process especially since this is your first contribution to this project.

It was updated on 2024/5/27

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

@LinuxSuRen LinuxSuRen requested a review from yuluo-yx as a code owner September 2, 2025 08:01
if proxy.Echo {
fmt.Println("Original request header:")
for k, v := range req.Header {
fmt.Println(k, ":", v)

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 about 2 months ago

To fix the problem, we should prevent logging sensitive HTTP headers. The best practice is to either omit logging headers entirely, or to redact/obfuscate sensitive header values before logging them. When logging headers, we should maintain a denylist of sensitive header names (e.g., Authorization, Cookie, Set-Cookie, Proxy-Authorization) and mask their values if they're present.
Specifically, in pkg/mock/in_memory.go, within the loop starting at line 183, we should update the logging so that before printing each header, we check if it is sensitive; if so, print its value as <redacted>, else print its actual value. This can be done by a simple helper function that matches header names case-insensitively.
Minimal changes are needed and no functionality should be lost (other than leaking secrets). We'll define the sensitive headers list in this code region and use it for redaction.

Suggested changeset 1
pkg/mock/in_memory.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/pkg/mock/in_memory.go b/pkg/mock/in_memory.go
--- a/pkg/mock/in_memory.go
+++ b/pkg/mock/in_memory.go
@@ -181,7 +181,11 @@
 		if proxy.Echo {
 			fmt.Println("Original request header:")
 			for k, v := range req.Header {
-				fmt.Println(k, ":", v)
+				if isSensitiveHeader(k) {
+					fmt.Println(k, ":", "<redacted>")
+				} else {
+					fmt.Println(k, ":", v)
+				}
 			}
 			fmt.Println("Original request path:", req.URL)
 			fmt.Println("Original request payload:")
@@ -226,6 +230,27 @@
 	})
 }
 
+// isSensitiveHeader returns true if the HTTP header name is considered sensitive.
+func isSensitiveHeader(headerName string) bool {
+	// List of common sensitive headers
+	sensitive := []string{
+		"Authorization",
+		"Cookie",
+		"Set-Cookie",
+		"Proxy-Authorization",
+		"X-Api-Key",
+		"X-Auth-Token",
+		"X-Csrf-Token",
+	}
+	normalized := strings.ToLower(headerName)
+	for _, h := range sensitive {
+		if normalized == strings.ToLower(h) {
+			return true
+		}
+	}
+	return false
+}
+
 func (s *inMemoryServer) tcpProxy(proxy *Proxy) {
 	fmt.Println("start to proxy", proxy.Port)
 	lisener, err := net.Listen("tcp", fmt.Sprintf(":%d", proxy.Port))
EOF
@@ -181,7 +181,11 @@
if proxy.Echo {
fmt.Println("Original request header:")
for k, v := range req.Header {
fmt.Println(k, ":", v)
if isSensitiveHeader(k) {
fmt.Println(k, ":", "<redacted>")
} else {
fmt.Println(k, ":", v)
}
}
fmt.Println("Original request path:", req.URL)
fmt.Println("Original request payload:")
@@ -226,6 +230,27 @@
})
}

// isSensitiveHeader returns true if the HTTP header name is considered sensitive.
func isSensitiveHeader(headerName string) bool {
// List of common sensitive headers
sensitive := []string{
"Authorization",
"Cookie",
"Set-Cookie",
"Proxy-Authorization",
"X-Api-Key",
"X-Auth-Token",
"X-Csrf-Token",
}
normalized := strings.ToLower(headerName)
for _, h := range sensitive {
if normalized == strings.ToLower(h) {
return true
}
}
return false
}

func (s *inMemoryServer) tcpProxy(proxy *Proxy) {
fmt.Println("start to proxy", proxy.Port)
lisener, err := net.Listen("tcp", fmt.Sprintf(":%d", proxy.Port))
Copilot is powered by AI and may make mistakes. Always verify output.
@github-actions
Copy link

github-actions bot commented Sep 2, 2025

There are 1 test cases, failed count 0:

Name Average Max Min Count Error
12.524702ms 15.174516ms 10.137634ms 3 0

Reported by api-testing.

@codacy-production
Copy link

codacy-production bot commented Sep 2, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.07% (target: -1.00%) 15.79% (target: 80.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (5789fae) 21947 8386 38.21%
Head commit (d43820a) 22336 (+389) 8551 (+165) 38.28% (+0.07%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#820) 19 3 15.79%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

}
fmt.Println("Original request path:", req.URL)
fmt.Println("Original request payload:")
fmt.Println(string(requestBody))

Check notice

Code scanning / SonarCloud

Logging should not be vulnerable to injection attacks Low

Change this code to not log user-controlled data. See more on SonarQube Cloud
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
B Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@LinuxSuRen LinuxSuRen merged commit d8c0de4 into master Sep 16, 2025
27 of 32 checks passed
@LinuxSuRen LinuxSuRen deleted the feat/mock-echo branch September 16, 2025 11:04
@LinuxSuRen LinuxSuRen added the enhancement New feature or request label Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants