[fix](fe) Mask sensitive headers in stream load logs#62108
[fix](fe) Mask sensitive headers in stream load logs#62108liaoxin01 wants to merge 1 commit intoapache:masterfrom
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
There was a problem hiding this comment.
Pull request overview
This PR reduces the risk of credential leakage by masking selected sensitive HTTP request headers when FE logs stream load REST requests.
Changes:
- Mask values for a small set of sensitive headers (e.g.,
Authorization,token) ingetAllHeaders(). - Add
isSensitiveHeader()helper to centralize the masking decision.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| String headerValue = isSensitiveHeader(headerName) ? "***MASKED***" : request.getHeader(headerName); | ||
| headers.append(headerName).append(":").append(headerValue).append(", "); | ||
| } | ||
| return headers.toString(); | ||
| } | ||
|
|
||
| private boolean isSensitiveHeader(String headerName) { | ||
| return "Authorization".equalsIgnoreCase(headerName) | ||
| || "Proxy-Authorization".equalsIgnoreCase(headerName) | ||
| || "token".equalsIgnoreCase(headerName) | ||
| || "Auth-Token".equalsIgnoreCase(headerName); | ||
| } |
There was a problem hiding this comment.
getAllHeaders() still logs every header name/value and only masks a small hard-coded set. Since FE auth can fall back to cookies (session-based auth), unmasked Cookie / Set-Cookie headers can leak session identifiers into INFO logs. Consider expanding the sensitive header list to include cookie headers (and ideally using a centralized allowlist/denylist such as a static Set of lowercased names) so future auth headers don’t get missed.
| private String getAllHeaders(HttpServletRequest request) { | ||
| StringBuilder headers = new StringBuilder(); | ||
| Enumeration<String> headerNames = request.getHeaderNames(); | ||
| while (headerNames.hasMoreElements()) { | ||
| String headerName = headerNames.nextElement(); | ||
| String headerValue = request.getHeader(headerName); | ||
| String headerValue = isSensitiveHeader(headerName) ? "***MASKED***" : request.getHeader(headerName); | ||
| headers.append(headerName).append(":").append(headerValue).append(", "); |
There was a problem hiding this comment.
The new masking behavior in getAllHeaders() is security-sensitive but currently untested. There are HTTPv2 REST action tests in this repo (e.g., GetLogFileActionTest uses reflection to test private helpers); adding a small unit test that verifies sensitive headers are masked (and non-sensitive headers are preserved) would help prevent regressions.
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: FE stream load REST logs printed full request headers, which could leak Authorization and token values into INFO logs. ### Release note None ### Check List (For Author) - Test: No need to test (log sanitization only; no completed automated test run in this environment) - Behavior changed: No - Does this need documentation: No
liaoxin01
force-pushed
the
fix-streamload-mask-headers-master
branch
from
c55096e to
012d589
Compare
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: FE stream load REST logs printed full request headers, which could leak Authorization and token values into INFO logs.
Release note
None
Check List (For Author)