xiangfu0 commented on code in PR #18430:
URL: https://github.com/apache/pinot/pull/18430#discussion_r3195305984
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -332,6 +338,8 @@ protected BrokerResponse handleRequest(long requestId,
String query, SqlNodeAndO
}
}
Review Comment:
If fingerprint generation failed above, this still hands the raw SQL to the
query logger and the request-handler warning path already logged it once. The
same pattern also exists on other broker error paths that still log `query`
directly, so `literal_values` and especially `full` do not actually guarantee
that SQL stays out of broker logs. We need a shared redaction helper for every
broker-side query log before advertising this as broker SQL redaction.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/querylog/QueryLogger.java:
##########
@@ -48,10 +48,29 @@ public class QueryLogger {
private static final Logger LOGGER =
LoggerFactory.getLogger(QueryLogger.class);
private static final QueryLogEntry[] QUERY_LOG_ENTRY_VALUES =
QueryLogEntry.values();
+ private static final String FINGERPRINT_FAILED_QUERY_REDACTED =
"FINGERPRINT_FAILED_QUERY_REDACTED";
+ private static final String FULLY_REDACTED = "REDACTED";
+
+ public enum SqlRedactionMode {
+ NONE,
+ LITERAL_VALUES,
+ FULL;
+
+ public static SqlRedactionMode fromString(String value) {
+ try {
+ return valueOf(value.toUpperCase());
+ } catch (IllegalArgumentException e) {
+ LOGGER.warn("Invalid SQL redaction mode '{}', defaulting to NONE",
value);
+ return NONE;
Review Comment:
This fails open on misconfiguration. If an operator sets an invalid
`pinot.broker.query.log.sqlRedaction` value, we silently fall back to `NONE`
and start emitting raw SQL, which is the exact unsafe behavior this knob is
supposed to prevent. For a privacy feature, the safer behavior is to reject
startup or fail closed to a redacted mode instead of disabling redaction.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
