MonkeyCanCode commented on PR #4772: URL: https://github.com/apache/polaris/pull/4772#issuecomment-4714519304
Hello @venkateshwaracholan , Thanks for flagging this and for contributing the fix. The `--debug` mode was initially introduced in https://github.com/apache/polaris/pull/2154 with the assumption it was mainly for local developer use. As the CLI has matured, it's now used in more environments, including CI, so the credential exposure needs to be addressed. I agree we should prevent leaking `client_secret` in the OAuth body and the `Authorization Bearer token` on management API calls. However, the `log_sanitizer.py` module feels disproportionate to the problem. The actual leak involves three specific patterns based on my understanding: 1. `client_secret` in OAuth token request bodies 2. `access_token/refresh_token` in OAuth token responses 3. `Authorization header` on management API calls Roughly half the entries in `SENSITIVE_KEYS` aren't actual Polaris API field names and they are speculative. I do like the the newly added module been robust and future-proof, but I am wondering if we should start with a simpler approach instead: 1. Redact known sensitive header names 2. For `/oauth/tokens` URLs, fully redact the body 3. Walk request/response bodies and redact only the sensitive field names that actually appear in the Polaris API The three patterns above can be handled with straightforward inline redaction in the urllib3 wrapper, which keeps the fix focused on the actual leak. What do you think? Thanks, Yong -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
