Fix: CLI's "--debug" mode leaks credentials to stderr (sanitizer misses camelCase keys on the wire) - #5289
Fix: CLI's "--debug" mode leaks credentials to stderr (sanitizer misses camelCase keys on the wire)#5289XD-DENG wants to merge 3 commits into
Conversation
…es camelCase keys on the wire) "polaris --debug" mirrors HTTP request and response bodies to stderr. Bodies are supposed to be redacted before writing, which is the stated purpose of log_sanitizer.py However, the sanitizer redacts only client_secret / access_token / refresh_token (snake_case). The Polaris management API serializes bodies by alias — camelCase — so clientSecret, bearerToken, etc. flow through unredacted. Every real management-plane request and response leaks credentials under "--debug". The snake_case keys only appear in the /oauth/tokens form body, which is already handled by a URL-based special case, so the sanitizer's key list is effectively dead for real traffic.
|
Will take a look tomorrow. |
| # The Polaris management API serializes bodies by alias (camelCase — e.g. | ||
| # ``clientSecret``, ``bearerToken``), while the OAuth token endpoint uses | ||
| # snake_case. Matching on the normalized key covers both. | ||
| SENSITIVE_BODY_KEYS = frozenset( |
There was a problem hiding this comment.
I would preferred we be more specific on this as all sensitive keys should be known. Also, this doesn't cover case for s3, gcs, and adls as they can be present in the body during debug mode as well. I would change to following instead:
"client_secret",
"clientSecret",
"access_token",
"accessToken",
"refresh_token",
"refreshToken",
"bearerToken",
"token",
"password",
"secret",
"s3.secret-access-key",
"s3.session-token",
"gcs.oauth2.token",
"adls.sas-token",
|
|
||
|
|
||
| def _is_sensitive_key(key: Any) -> bool: | ||
| return ( |
There was a problem hiding this comment.
For this, we will need to change to following to captured known keys as well as adls specific where they can put adls.sas-token as prefix (reference: https://github.com/apache/polaris/blob/main/site/content/in-dev/unreleased/configuration/config-sections/storage-azure.md?plain=1):
return key in SENSITIVE_BODY_KEYS or (
isinstance(key, str) and key.startswith("adls.sas-token")
)
|
Hi @MonkeyCanCode , thanks for the review. |
Problem
CLI
polaris --debugmirrors HTTP request and response bodies to stderr. Bodies are supposed to be redacted before writing, which is the stated purpose oflog_sanitizer.py.However, the sanitizer redacts only
client_secret/access_token/refresh_token(snake_case). The Polaris management API serializes bodies by alias (camelCase) soclientSecret,bearerToken, etc. flow through unredacted. Every real management-plane request and response leaks credentials under--debug. The snake_case keys only appear in the/oauth/tokensform body, which is already handled by a URL-based special case, so the sanitizer's key list is effectively dead for real traffic.How to reproduce the issue (no server required)
Example A —
principals reset(top-levelclientSecret)mainBody: {"clientId": "NEWCID", "clientSecret": "LEAK-ME-IF-YOU-CAN"}Body: {"clientId": "NEWCID", "clientSecret": "***REDACTED***"}Example B —
catalogs create --type externalwith OAuth (nestedclientSecret)Stderr
Body:excerpt from theconnectionConfigInfo.authenticationParametersblock:main"clientSecret": "OAUTH-SECRET-LEAK""clientSecret": "***REDACTED***"Example C —
catalogs create --type externalwith Bearer auth (bearerToken)Demonstrates the previously-unknown-to-the-sanitizer
bearerTokenkey.main"bearerToken": "BEARER-TOKEN-LEAK""bearerToken": "***REDACTED***"Fix
Two changes in
log_sanitizer.py:SENSITIVE_BODY_KEYSbecomes a normalized set (case-folded,_/-stripped):{clientsecret, accesstoken, refreshtoken, bearertoken, token, password, secret}. A new_is_sensitive_key(key)helper normalizes each incoming key before lookup, used by bothsanitize_dataand_sanitize_form_body. CoversclientSecret,client-secret,CLIENT_SECRET,ClientSecret, and the previous snake_case cases in one rule.clientId,tokenType, and other non-credential lookalikes are deliberately preserved (existing tests assert this).Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)