github-actions[bot] commented on code in PR #69462:
URL: https://github.com/apache/doris/pull/69462#discussion_r4347221246
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderForEncryption.java:
##########
@@ -104,6 +105,20 @@ public SetVarOp
visitSetLdapAdminPassword(DorisParser.SetLdapAdminPasswordContex
return super.visitSetLdapAdminPassword(ctx);
}
+ @Override
+ public LogicalPlan
visitAlterRoutineLoad(DorisParser.AlterRoutineLoadContext ctx) {
+ if (ctx.propertyItemList() != null) {
+ for (DorisParser.PropertyItemContext property :
ctx.propertyItemList().properties) {
+ if
(KafkaPropertiesUtil.isSensitiveKey(parsePropertyKey(property.key))) {
Review Comment:
[P1] Decode audit keys using the original statement's SQL mode
On a master FE, submit one batch under the default escape mode: `SET
sql_mode='NO_BACKSLASH_ESCAPES'; ALTER ROUTINE LOAD FOR db.job FROM KAFKA
('property.sasl.\password' = 'synthetic-secret')`. `ConnectProcessor` parses
the whole batch before execution, so the ALTER command already contains the
valid key `property.sasl.password` and applies it to the job. After the SET
executes, this audit-time `parsePropertyKey` instead preserves the backslash;
the classifier misses `sasl.\password` and leaves the credential plaintext in
audit/statement SQL. The text is valid in both modes, so the logging fallback
does not help. Preserve the original parse mode or the original decoded
key/range information for masking, and add a batch regression that changes SQL
mode before ALTER.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoutineLoadCommand.java:
##########
@@ -353,4 +353,9 @@ public <R, C> R accept(PlanVisitor<R, C> visitor, C
context) {
public StmtType stmtType() {
return StmtType.ALTER;
}
+
+ @Override
+ public boolean needAuditEncryption() {
+ return true;
Review Comment:
[P2] Preserve ALTER audit events when statement splitting disagrees
The new encryption hook assumes its SQL is a single statement, but
`ConnectProcessor` passes the entire request when `SqlUtils.splitMultiStmts`
returns a different count from Nereids. A valid default-mode example is `ALTER
ROUTINE LOAD FOR db.job FROM KAFKA ('property.sasl.password'='secret'); /*
outer /* inner */ x; */ SELECT 1`: Nereids handles the nested comment and
builds two commands, while the separator closes the comment at the inner `*/`
and retains three nonempty fragments. The ALTER still runs, but its new
`geneEncryptionSQL` call reparses the full request with `singleStatement` and
throws; `AuditLogHelper.logAuditLog` catches this before submitting any event,
so the ALTER disappears from audit logs. Ensure masking receives the original
statement's correct range, or safely handles this fallback, and cover the real
multi-statement audit path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderForEncryption.java:
##########
@@ -104,6 +105,20 @@ public SetVarOp
visitSetLdapAdminPassword(DorisParser.SetLdapAdminPasswordContex
return super.visitSetLdapAdminPassword(ctx);
}
+ @Override
+ public LogicalPlan
visitAlterRoutineLoad(DorisParser.AlterRoutineLoadContext ctx) {
+ if (ctx.propertyItemList() != null) {
+ for (DorisParser.PropertyItemContext property :
ctx.propertyItemList().properties) {
+ if
(KafkaPropertiesUtil.isSensitiveKey(parsePropertyKey(property.key))) {
+
indexInSqlToString.put(Pair.of(property.value.start.getStartIndex(),
Review Comment:
[P1] Convert token offsets to UTF-16 before rewriting sensitive values
When the SQL contains supplementary characters,
`CaseInsensitiveStream.fromString` uses ANTLR code-point indices, but
`BaseViewInfo.rewriteSql` interprets these saved indices as Java UTF-16
offsets. For example, `/* 😀😀😀😀😀😀😀😀 */ ALTER ROUTINE LOAD FOR db.job FROM KAFKA
('property.sasl.password' = 'top-secret')` replaces part of the property name
and leaves `-secret` in the output; with enough preceding emoji the whole
password survives. This result does not throw, so both `AuditLogHelper` and FE
statement logging accept the partially masked SQL. Convert both token
boundaries to UTF-16 offsets against the original SQL before storing them, and
cover supplementary characters before and inside sensitive values.
--
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]
