github-actions[bot] commented on code in PR #69462:
URL: https://github.com/apache/doris/pull/69462#discussion_r4356893706
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -701,7 +701,7 @@ public void execute() throws Exception {
TUniqueId queryId = UniqueIdUtils.fastUniqueId();
if (Config.enable_print_request_before_execution) {
LOG.info("begin to execute query {} {}",
- DebugUtil.printId(queryId),
getStmtForLoggingBeforeParse());
+ DebugUtil.printId(queryId), getStmtForLogging());
Review Comment:
[P1] Keep startup logs fail-closed for an unsplit batch
With `enable_print_request_before_execution` enabled, submit `SELECT 1; /*
outer /* inner */ x; */ ALTER ROUTINE LOAD FOR db.job FROM KAFKA
('property.sasl.password'='synthetic-secret')`. Nereids produces two statements
but the nonrecursive separator produces three fragments, so `ConnectProcessor`
gives the SELECT the entire request as its `OriginStatement`. Its
`UnboundResultSink.needAuditEncryption()` is false, making the new
`getStmtForLogging()` return that entire request unchanged and log the password
before the ALTER even executes. The changed DEBUG call at line 943 does the
same when DEBUG is enabled. Previously both startup sites reparsed the supplied
text with `parseSingle` and failed closed on this batch. Ensure the selected
plan owns all text being logged, or retain fail-closed masking for an unsplit
request, and test real SELECT startup logging in a mismatch batch. This is
distinct from thread 4347221252: the ALTER audit event now survives, but
another statement's star
tup log newly exposes its secret.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -9422,7 +9427,41 @@ public LogicalPlan
visitAlterRoutineLoad(DorisParser.AlterRoutineLoadContext ctx
}
}
- return new AlterRoutineLoadCommand(labelNameInfo, loadPropertyMap,
properties, dataSourceMapProperties);
+ return new AlterRoutineLoadCommand(labelNameInfo, loadPropertyMap,
properties, dataSourceMapProperties,
+ auditSql);
+ }
+
+ private String
buildRoutineLoadAuditSql(DorisParser.AlterRoutineLoadContext ctx) {
+ // Capture masking while the property keys still have the same SQL
mode as execution.
+ // Keep only this statement when the request contains multiple
statements: the audit caller
+ // can pass the entire request if its separate statement splitter
disagrees with Nereids.
+ ParserRuleContext statement = ctx;
+ while (statement.getParent() != null
+ && !(statement.getParent() instanceof MultiStatementsContext)
+ && !(statement.getParent() instanceof SingleStatementContext))
{
+ statement = statement.getParent();
+ }
+ int start = 0;
+ int stop = ctx.start.getInputStream().size() - 1;
+ if (statement.getParent() instanceof MultiStatementsContext
+ && ((MultiStatementsContext)
statement.getParent()).statement().size() > 1) {
+ start = statement.start.getStartIndex();
+ stop = statement.stop.getStopIndex();
+ }
+ String sql = ctx.start.getInputStream().getText(new
org.antlr.v4.runtime.misc.Interval(start, stop));
+ TreeMap<Pair<Integer, Integer>, String> replacements = new
TreeMap<>(new Pair.PairComparator<>());
+ if (ctx.propertyItemList() != null) {
+ for (DorisParser.PropertyItemContext property :
ctx.propertyItemList().properties) {
+ if
(KafkaPropertiesUtil.isSensitiveKey(parsePropertyKey(property.key))) {
Review Comment:
[P1] Account for the master's mode when capturing follower audit SQL
On a follower connection initially using `NO_BACKSLASH_ESCAPES`, submit `SET
sql_mode=''; ALTER ROUTINE LOAD FOR db.job FROM KAFKA
('property.sasl.\password'='synthetic-secret')` for a paused Kafka job. The
batch is built before execution, so this classifier sees the literal backslash
and retains the secret in `auditSql`. The SET then runs locally; ALTER is
forwarded before validation, and `FEOpExecutor` sends its original SQL with the
current default SQL mode. The master reparses it as the valid
`property.sasl.password` and applies the credential, while the follower keeps
its original command and `AuditLogHelper` logs that command's unmasked capture.
Its optional startup logs also use that capture. Account for every
interpretation that can execute as a credential key, or preserve the execution
parse mode across forwarding, and add a real follower/master batch regression.
Unlike thread 4347221246's repaired direct-master audit-time reparse, this path
reparses for execution on anot
her FE, so the local initial key is not the executed key.
--
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]
