Skip to content

Commit 9bd287d

Browse files
srihithgsrihithg
committed
[BugFix] insert files credential redaction (#71245)
Signed-off-by: srihithg <78094568+srihithg@users.noreply.github.com> Made-with: Cursor
1 parent 47c6271 commit 9bd287d

5 files changed

Lines changed: 1568 additions & 80 deletions

File tree

‎fe/fe-core/src/main/java/com/starrocks/common/util/SqlCredentialRedactor.java‎

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,16 @@
1515
package com.starrocks.common.util;
1616

1717
import com.google.common.collect.ImmutableSet;
18+
import com.google.re2j.Matcher;
19+
import com.google.re2j.Pattern;
1820
import com.starrocks.connector.share.credential.CloudConfigurationConstants;
1921
import com.starrocks.fs.hdfs.HdfsFsManager;
2022
import com.starrocks.sql.ast.CreateRoutineLoadStmt;
2123
import com.starrocks.sql.ast.LoadStmt;
2224

25+
import java.util.HashSet;
26+
import java.util.Locale;
2327
import java.util.Set;
24-
import java.util.regex.Matcher;
25-
import java.util.regex.Pattern;
2628

2729
/**
2830
* Utility class to redact sensitive credentials from SQL strings.
@@ -83,7 +85,16 @@ public class SqlCredentialRedactor {
8385
.add("broker.password")
8486
.build();
8587

86-
// Pattern to match key-value pairs in SQL
88+
// Lowercase set for O(1) lookup (case-insensitive matching)
89+
private static final Set<String> CREDENTIAL_KEYS_LOWERCASE = new HashSet<>();
90+
91+
static {
92+
for (String key : CREDENTIAL_KEYS) {
93+
CREDENTIAL_KEYS_LOWERCASE.add(key.toLowerCase(Locale.ROOT));
94+
}
95+
}
96+
97+
// Simplified pattern to match any key-value pair in SQL
8798
// This pattern handles cases like:
8899
// "key"="value"
89100
// 'key'='value'
@@ -97,13 +108,16 @@ public class SqlCredentialRedactor {
97108
// key'='value
98109
// key"="value
99110
// Values can contain spaces and span multiple lines, separated by commas
111+
private static final int MAX_KEY_LENGTH =
112+
CREDENTIAL_KEYS.stream().map(String::length).max(Integer::compareTo).orElse(1);
113+
// NOTE: MAX_KEY_LENGTH is used to avoid matching too many characters of a long string
100114
private static final Pattern KEY_VALUE_PATTERN = Pattern.compile(
101-
"(?:([\"']?)(" + String.join("|", CREDENTIAL_KEYS.stream()
102-
.map(Pattern::quote)
103-
.toArray(String[]::new)) + ")([\"']?))\\s*=\\s*" +
104-
"(?:([\"'])((?:[^\\\\]|\\\\.)*?)\\4|([^,]*?))" +
105-
"(?=\\s*,|\\s*$|\\s*\\)|\\s*\\n)",
106-
Pattern.CASE_INSENSITIVE | Pattern.DOTALL | Pattern.MULTILINE
115+
"([\"'])" + // quote
116+
"([^\"'=\\s,()]{1," + MAX_KEY_LENGTH + "})" + // key
117+
"([\"'])" + // quote
118+
"\\s*=\\s*" + // =
119+
"(?:'((?:[^'\\\\]|\\\\.)*)'|\"((?:[^\"\\\\]|\\\\.)*)\"|([^,()\\n]*))",
120+
Pattern.DOTALL | Pattern.MULTILINE
107121
);
108122

109123
private static final Pattern IDENTIFIED_BY_PATTERN = Pattern.compile(
@@ -130,6 +144,27 @@ public class SqlCredentialRedactor {
130144
private static final String REDACTED_VALUE = "***";
131145
private static final String LDAP_SIMPLE_AUTH_PLUGIN = "AUTHENTICATION_LDAP_SIMPLE";
132146

147+
/**
148+
* Cheap check for whether {@link #redact(String)} might change the SQL. Used on hot paths (e.g. query profiles)
149+
* to skip full redaction when the text has no credential-related markers.
150+
*/
151+
public static boolean mayNeedCredentialRedaction(String sql) {
152+
if (sql == null || sql.isEmpty()) {
153+
return false;
154+
}
155+
String lower = sql.toLowerCase(Locale.ROOT);
156+
for (String key : CREDENTIAL_KEYS_LOWERCASE) {
157+
if (lower.indexOf(key) >= 0) {
158+
return true;
159+
}
160+
}
161+
// "password" is already in CREDENTIAL_KEYS_LOWERCASE, so "set password"
162+
// (with any whitespace) is caught by the loop above. Only "identified"
163+
// needs a separate check — a single keyword avoids false negatives from
164+
// whitespace variations like IDENTIFIED\nBY or IDENTIFIED\tWITH.
165+
return lower.contains("identified");
166+
}
167+
133168
/**
134169
* Redact sensitive credentials from SQL string.
135170
*
@@ -150,28 +185,39 @@ public static String redact(String sql) {
150185

151186
private static String redactKeyValueCredentials(String sql) {
152187
Matcher matcher = KEY_VALUE_PATTERN.matcher(sql);
153-
StringBuffer result = new StringBuffer();
188+
StringBuilder result = new StringBuilder(sql.length() + 100);
154189

190+
int lastEnd = 0;
155191
while (matcher.find()) {
156-
String replacement;
157192
String keyPrefix = matcher.group(1) != null ? matcher.group(1) : "";
158193
String key = matcher.group(2);
159194
String keySuffix = matcher.group(3) != null ? matcher.group(3) : "";
160195

161-
// Determine if value is quoted or unquoted
162-
if (matcher.group(4) != null && matcher.group(5) != null) {
163-
// Quoted value case
164-
String valueQuote = matcher.group(4);
165-
replacement = keyPrefix + key + keySuffix + " = " + valueQuote + REDACTED_VALUE + valueQuote;
196+
// Check if this key should be redacted (case-insensitive)
197+
if (CREDENTIAL_KEYS_LOWERCASE.contains(key.toLowerCase(Locale.ROOT))) {
198+
// Append text before the match
199+
result.append(sql, lastEnd, matcher.start());
200+
201+
// Build replacement for redacted value
202+
String replacement;
203+
if (matcher.group(4) != null && matcher.group(5) != null) {
204+
// Quoted value case
205+
String valueQuote = matcher.group(4);
206+
replacement = keyPrefix + key + keySuffix + " = " + valueQuote + REDACTED_VALUE + valueQuote;
207+
} else {
208+
// Unquoted value case
209+
replacement = keyPrefix + key + keySuffix + " = " + REDACTED_VALUE;
210+
}
211+
result.append(replacement);
166212
} else {
167-
// Unquoted value case
168-
replacement = keyPrefix + key + keySuffix + " = " + REDACTED_VALUE;
213+
// Not a credential key, append original match
214+
result.append(sql, lastEnd, matcher.end());
169215
}
170-
171-
matcher.appendReplacement(result, Matcher.quoteReplacement(replacement));
216+
lastEnd = matcher.end();
172217
}
173218

174-
matcher.appendTail(result);
219+
// Append remaining text
220+
result.append(sql, lastEnd, sql.length());
175221
return result.toString();
176222
}
177223

‎fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java‎

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
package com.starrocks.qe;
3636

37+
import com.google.common.annotations.VisibleForTesting;
3738
import com.google.common.base.Preconditions;
3839
import com.google.common.base.Strings;
3940
import com.google.common.collect.Lists;
@@ -387,9 +388,16 @@ private RuntimeProfile buildTopLevelProfile() {
387388
AstToSQLBuilder.toSQLOrDefault(parsedStmt, originStmt.originStmt);
388389
if (AuditEncryptionChecker.needEncrypt(parsedStmt)) {
389390
summaryProfile.addInfoString(ProfileManager.SQL_STATEMENT,
390-
AstToSQLBuilder.toSQLOrDefault(parsedStmt, originStmt.originStmt));
391+
SqlCredentialRedactor.redact(AstToSQLBuilder.toSQLOrDefault(parsedStmt, originStmt.originStmt)));
392+
} else if (Config.enable_sql_desensitize_in_log) {
393+
String desensitizedSql = AstToSQLBuilder.toSQL(parsedStmt, FormatOptions.allEnable()
394+
.setColumnSimplifyTableName(false)
395+
.setEnableDigest(true))
396+
.orElse("this is a desensitized sql");
397+
summaryProfile.addInfoString(ProfileManager.SQL_STATEMENT,
398+
SqlCredentialRedactor.redact(desensitizedSql));
391399
} else {
392-
summaryProfile.addInfoString(ProfileManager.SQL_STATEMENT, sql);
400+
summaryProfile.addInfoString(ProfileManager.SQL_STATEMENT, SqlCredentialRedactor.redact(sql));
393401
}
394402

395403
// Add some import variables in profile
@@ -659,7 +667,7 @@ private ExecPlan generateExecPlan() throws Exception {
659667
if (e.getType().equals(ErrorType.USER_ERROR)) {
660668
throw e;
661669
} else {
662-
LOG.warn("Planner error: " + originStmt.originStmt, e);
670+
LOG.warn("Planner error: {}", getRedactedOriginStmtInString(), e);
663671
throw e;
664672
}
665673
} catch (Exception e) {
@@ -743,14 +751,29 @@ public void execute() throws Exception {
743751
execPlan = generateExecPlan();
744752
} catch (Exception e) {
745753
LOG.warn("Generate exec plan failed for explain stmt: {}",
746-
parsedStmt.getOrigStmt().originStmt, e);
754+
getRedactedOriginStmtInString(), e);
747755
}
748756
handleExplainExecPlan(execPlan);
749757
return;
750758
}
751759

752-
// execPlan is the output of planner
753-
ExecPlan execPlan = generateExecPlan();
760+
// Register as a planning query so it is visible in current_queries during optimization.
761+
// The planning entry is removed before handleQueryStmt/handleDMLStmt re-registers
762+
// with the real Coordinator, avoiding AlreadyExistsException from putIfAbsent.
763+
ExecPlan execPlan;
764+
context.setPlanning(true);
765+
try {
766+
QeProcessorImpl.INSTANCE.registerQuery(context.getExecutionId(),
767+
QeProcessorImpl.QueryInfo.fromPlanningQuery(context, getRedactedOriginStmtInString()));
768+
} catch (Exception e) {
769+
LOG.warn("Failed to register planning query: {}", DebugUtil.printId(context.getExecutionId()), e);
770+
}
771+
try {
772+
execPlan = generateExecPlan();
773+
} finally {
774+
context.setPlanning(false);
775+
QeProcessorImpl.INSTANCE.unregisterQuery(context.getExecutionId());
776+
}
754777

755778
// no need to execute http query dump request in BE
756779
if (context.isHTTPQueryDump) {
@@ -821,7 +844,7 @@ public void execute() throws Exception {
821844
originStmt = this.originStmt.originStmt;
822845
}
823846
needRetry = true;
824-
LOG.warn("retry {} times. stmt: {}", (i + 1), originStmt);
847+
LOG.warn("retry {} times. stmt: {}", (i + 1), SqlCredentialRedactor.redact(originStmt));
825848
} else {
826849
throw e;
827850
}
@@ -954,7 +977,7 @@ public void execute() throws Exception {
954977
String sql = originStmt != null ? originStmt.originStmt : "";
955978
String truncatedSql = sql.length() > 200 ? sql.substring(0, 200) + "..." : sql;
956979
LOG.error("LargeInPredicate optimization failed, sql: {}, error: {}. Will retry with" +
957-
" enable_large_in_predicate=false.", truncatedSql, e.getMessage());
980+
" enable_large_in_predicate=false.", SqlCredentialRedactor.redact(truncatedSql), e.getMessage());
958981
throw e;
959982
} catch (StarRocksException e) {
960983
String sql = originStmt != null ? originStmt.originStmt : "";
@@ -1006,7 +1029,8 @@ public void execute() throws Exception {
10061029
context.setSessionVariable(sessionVariableBackup);
10071030

10081031
if (shouldMarkIdleCheck && originWarehouseId != null) {
1009-
WarehouseIdleChecker.decreaseRunningSQL(originWarehouseId);
1032+
WarehouseIdleChecker.decreaseRunningSQL(originWarehouseId,
1033+
getRedactedOriginStmtInString());
10101034
}
10111035

10121036
recordExecStatsIntoContext();
@@ -1520,7 +1544,7 @@ private void handleQueryStmt(ExecPlan execPlan) throws Exception {
15201544
}
15211545

15221546
QeProcessorImpl.INSTANCE.registerQuery(context.getExecutionId(),
1523-
new QeProcessorImpl.QueryInfo(context, originStmt.originStmt, coord));
1547+
new QeProcessorImpl.QueryInfo(context, getRedactedOriginStmtInString(), coord));
15241548

15251549
if (isSchedulerExplain) {
15261550
coord.execWithoutDeploy();
@@ -2316,7 +2340,7 @@ private void handleDdlStmt() throws DdlException {
23162340
if (sql == null) {
23172341
sql = originStmt.originStmt;
23182342
}
2319-
LOG.warn("DDL statement (" + sql + ") process failed.", e);
2343+
LOG.warn("DDL statement ({}) process failed.", SqlCredentialRedactor.redact(sql), e);
23202344
}
23212345
context.setState(e.getQueryState());
23222346
} catch (DdlException | AlterJobException e) {
@@ -2329,7 +2353,7 @@ private void handleDdlStmt() throws DdlException {
23292353
if (sql == null || sql.isEmpty()) {
23302354
sql = originStmt.originStmt;
23312355
}
2332-
LOG.warn("DDL statement (" + sql + ") process failed.", e);
2356+
LOG.warn("DDL statement ({}) process failed.", SqlCredentialRedactor.redact(sql), e);
23332357
context.getState().setError(e.getMessage());
23342358
}
23352359
}
@@ -2535,7 +2559,7 @@ public void handleDMLStmtWithProfile(ExecPlan execPlan, DmlStmt stmt) throws Exc
25352559
try {
25362560
handleDMLStmt(execPlan, stmt);
25372561
} catch (Throwable t) {
2538-
LOG.warn("DML statement({}) process failed.", originStmt.originStmt, t);
2562+
LOG.warn("DML statement({}) process failed.", getRedactedOriginStmtInString(), t);
25392563
throw t;
25402564
} finally {
25412565
boolean isAsync = false;
@@ -2589,7 +2613,7 @@ public void handleDMLStmt(ExecPlan execPlan, DmlStmt stmt) throws Exception {
25892613
context.getState().setOk();
25902614
} catch (QueryStateException e) {
25912615
if (e.getQueryState().getStateType() != MysqlStateType.OK) {
2592-
LOG.warn("DDL statement(" + originStmt.originStmt + ") process failed.", e);
2616+
LOG.warn("DDL statement({}) process failed.", getRedactedOriginStmtInString(), e);
25932617
}
25942618
context.setState(e.getQueryState());
25952619
}
@@ -2713,8 +2737,8 @@ public void handleDMLStmt(ExecPlan execPlan, DmlStmt stmt) throws Exception {
27132737

27142738
coord.setLoadJobId(jobId);
27152739
trackingSql = "select tracking_log from information_schema.load_tracking_logs where job_id=" + jobId;
2716-
2717-
QeProcessorImpl.QueryInfo queryInfo = new QeProcessorImpl.QueryInfo(context, originStmt.originStmt, coord);
2740+
QeProcessorImpl.QueryInfo queryInfo =
2741+
new QeProcessorImpl.QueryInfo(context, getRedactedOriginStmtInString(), coord);
27182742
QeProcessorImpl.INSTANCE.registerQuery(context.getExecutionId(), queryInfo);
27192743

27202744
if (isSchedulerExplain) {
@@ -2743,7 +2767,7 @@ public void handleDMLStmt(ExecPlan execPlan, DmlStmt stmt) throws Exception {
27432767
if (Config.log_plan_cancelled_by_crash_be && context.getQueryDetail() == null) {
27442768
LOG.warn("Query cancelled by crash of backends [QueryId={}] [SQL={}] [Plan={}]",
27452769
DebugUtil.printId(context.getExecutionId()),
2746-
originStmt == null ? "" : originStmt.originStmt,
2770+
getRedactedOriginStmtInString(),
27472771
execPlan.getExplainString(TExplainLevel.COSTS));
27482772
}
27492773

@@ -2954,10 +2978,7 @@ public void handleDMLStmt(ExecPlan execPlan, DmlStmt stmt) throws Exception {
29542978
}
29552979
} catch (Throwable t) {
29562980
// if any throwable being thrown during insert operation, first we should abort this txn
2957-
String failedSql = "";
2958-
if (originStmt != null && originStmt.originStmt != null) {
2959-
failedSql = originStmt.originStmt;
2960-
}
2981+
String failedSql = getRedactedOriginStmtInString();
29612982
LOG.warn("failed to handle stmt [{}] label: {}", failedSql, label, t);
29622983
String errMsg = t.getMessage();
29632984
if (errMsg == null) {
@@ -3077,6 +3098,15 @@ public String getOriginStmtInString() {
30773098
return originStmt.originStmt;
30783099
}
30793100

3101+
@VisibleForTesting
3102+
String getRedactedOriginStmtInString() {
3103+
String sql = getOriginStmtInString();
3104+
if (!SqlCredentialRedactor.mayNeedCredentialRedaction(sql)) {
3105+
return sql;
3106+
}
3107+
return SqlCredentialRedactor.redact(sql);
3108+
}
3109+
30803110
public Pair<List<TResultBatch>, Status> executeStmtWithExecPlan(ConnectContext context, ExecPlan plan) {
30813111
List<TResultBatch> sqlResult = Lists.newArrayList();
30823112
try {
@@ -3195,11 +3225,17 @@ public void addRunningQueryDetail(StatementBase parsedStmt) {
31953225
}
31963226

31973227
try {
3198-
String sql;
3199-
if (AuditEncryptionChecker.needEncrypt(parsedStmt)) {
3200-
sql = AstToSQLBuilder.toSQLOrDefault(parsedStmt, parsedStmt.getOrigStmt().originStmt);
3201-
} else {
3202-
sql = parsedStmt.getOrigStmt().originStmt;
3228+
String originSql = getQueryDetailSql(parsedStmt);
3229+
boolean needEncrypt = AuditEncryptionChecker.needEncrypt(parsedStmt);
3230+
String sql = originSql;
3231+
if (needEncrypt || Config.enable_sql_desensitize_in_log) {
3232+
sql = AstToSQLBuilder.toSQL(parsedStmt, FormatOptions.allEnable()
3233+
.setColumnSimplifyTableName(false)
3234+
.setHideCredential(needEncrypt)
3235+
.setEnableDigest(Config.enable_sql_desensitize_in_log))
3236+
.orElse("this is a desensitized sql");
3237+
} else if (SqlCredentialRedactor.mayNeedCredentialRedaction(originSql)) {
3238+
sql = SqlCredentialRedactor.redact(originSql);
32033239
}
32043240

32053241
boolean isQuery = context.isQueryStmt(parsedStmt);

0 commit comments

Comments
 (0)