Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ public void alterResource(String resourceName, Map<String, String> properties) t

// log alter
Env.getCurrentEnv().getEditLog().logAlterResource(resource);
LOG.info("Alter resource success. Resource: {}", resource);
// Only log non-sensitive identifiers here because resource objects may retain credential properties.
LOG.info("Alter resource success. Resource: {}, type: {}", resource.getName(), resource.getType());
}

public void replayAlterResource(Resource resource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class DatasourcePrintableMap<K, V> extends BasicPrintableMap<K, V> {
SENSITIVE_KEY.add("bos_secret_accesskey");
SENSITIVE_KEY.add("jdbc.password");
SENSITIVE_KEY.add("elasticsearch.password");
SENSITIVE_KEY.add("ai.api_key");
SENSITIVE_KEY.addAll(Arrays.asList(
MCProperties.SECRET_KEY));
SENSITIVE_KEY.addAll(ConnectorPropertiesUtils.getSensitiveKeys(S3Properties.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,17 @@ public LogicalPlan visitAlterAuthenticationIntegrationProperties(
return super.visitAlterAuthenticationIntegrationProperties(ctx);
}

@Override
public LogicalPlan visitAlterResource(DorisParser.AlterResourceContext ctx) {
if (ctx.propertyClause() != null) {
DorisParser.PropertyClauseContext propertyClauseContext = ctx.propertyClause();
encryptProperty(visitPropertyClause(propertyClauseContext),
propertyClauseContext.fileProperties.start.getStartIndex(),
propertyClauseContext.fileProperties.stop.getStopIndex());
}
return super.visitAlterResource(ctx);
}

// select from tvf
@Override
public LogicalPlan visitTableValuedFunction(DorisParser.TableValuedFunctionContext ctx) {
Expand Down Expand Up @@ -246,6 +257,17 @@ public LogicalPlan visitAlterJob(DorisParser.AlterJobContext ctx) {
return super.visitAlterJob(ctx);
}

@Override
public LogicalPlan visitCreateResource(DorisParser.CreateResourceContext ctx) {
if (ctx.properties != null) {
DorisParser.PropertyClauseContext propertyClauseContext = ctx.properties;
encryptProperty(visitPropertyClause(propertyClauseContext),
propertyClauseContext.fileProperties.start.getStartIndex(),
propertyClauseContext.fileProperties.stop.getStopIndex());
}
return super.visitCreateResource(ctx);
}

private void encryptProperty(Map<String, String> properties, int start, int stop) {
if (MapUtils.isNotEmpty(properties)) {
DatasourcePrintableMap<String, String> printableMap = new DatasourcePrintableMap<>(properties, "=",
Expand Down
48 changes: 39 additions & 9 deletions fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
import org.apache.doris.nereids.trees.plans.commands.EmptyCommand;
import org.apache.doris.nereids.trees.plans.commands.Forward;
import org.apache.doris.nereids.trees.plans.commands.LoadCommand;
import org.apache.doris.nereids.trees.plans.commands.NeedAuditEncryption;
import org.apache.doris.nereids.trees.plans.commands.PrepareCommand;
import org.apache.doris.nereids.trees.plans.commands.Redirect;
import org.apache.doris.nereids.trees.plans.commands.SupportProfile;
Expand Down Expand Up @@ -588,7 +589,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), originStmt == null ? "null" : originStmt.originStmt);
DebugUtil.printId(queryId), getStmtForLoggingBeforeParse());
}
queryRetry(queryId);
}
Expand Down Expand Up @@ -756,7 +757,7 @@ public void checkBlockRulesByScan(Planner planner) throws AnalysisException {

private void executeByNereids(TUniqueId queryId) throws Exception {
if (LOG.isDebugEnabled()) {
LOG.debug("Nereids start to execute query:\n {}", originStmt.originStmt);
LOG.debug("Nereids start to execute query:\n {}", getStmtForLogging(originStmt.originStmt));
Comment thread
wenzhenghu marked this conversation as resolved.
Outdated
}
context.setQueryId(queryId);
context.setStartTime();
Expand Down Expand Up @@ -837,29 +838,32 @@ private void executeByNereids(TUniqueId queryId) throws Exception {
((Command) logicalPlan).run(context, this);
} catch (QueryStateException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Command({}) process failed.", originStmt.originStmt, e);
LOG.debug("Command({}) process failed.", getStmtForLogging(originStmt.originStmt), e);
}
context.setState(e.getQueryState());
throw new NereidsException("Command(" + originStmt.originStmt + ") process failed",
throw new NereidsException("Command(" + getStmtForLogging(originStmt.originStmt)
+ ") process failed",
new AnalysisException(e.getMessage(), e));
} catch (UserException e) {
// Return message to info client what happened.
if (LOG.isDebugEnabled()) {
LOG.debug("Command({}) process failed.", originStmt.originStmt, e);
LOG.debug("Command({}) process failed.", getStmtForLogging(originStmt.originStmt), e);
}
if (Config.isCloudMode() && SystemInfoService.needRetryWithReplan(e.getDetailMessage())) {
// For errors in SystemInfoService.NEED_REPLAN_ERRORS,
// throw exception directly to trigger a replan retry outside(in StmtExecutor.queryRetry())
throw e;
}
context.getState().setError(e.getMysqlErrorCode(), e.getMessage());
throw new NereidsException("Command (" + originStmt.originStmt + ") process failed",
throw new NereidsException("Command (" + getStmtForLogging(originStmt.originStmt)
+ ") process failed",
new AnalysisException(e.getMessage(), e));
} catch (Exception | Error e) {
// Maybe our bug
LOG.info("Command({}) process failed.", originStmt.originStmt, e);
LOG.info("Command({}) process failed.", getStmtForLogging(originStmt.originStmt), e);
context.getState().setError(ErrorCode.ERR_UNKNOWN_ERROR, e.getMessage());
throw new NereidsException("Command (" + originStmt.originStmt + ") process failed.",
throw new NereidsException("Command (" + getStmtForLogging(originStmt.originStmt)
+ ") process failed.",
new AnalysisException(e.getMessage() == null ? e.toString() : e.getMessage(), e));
}
} else {
Expand Down Expand Up @@ -899,7 +903,7 @@ private void executeByNereids(TUniqueId queryId) throws Exception {
planner.plan(parsedStmt, context.getSessionVariable().toThrift());
checkBlockRulesByScan(planner);
} catch (Exception e) {
LOG.warn("Nereids plan query failed:\n{}", originStmt.originStmt, e);
LOG.warn("Nereids plan query failed:\n{}", getStmtForLogging(originStmt.originStmt), e);
throw new NereidsException(new AnalysisException(e.getMessage(), e));
}
profile.getSummaryProfile().setQueryPlanFinishTime(TimeUtils.getStartTimeMs());
Expand Down Expand Up @@ -2390,6 +2394,32 @@ public String getOriginStmtInString() {
return "";
}

private String getStmtForLogging(String stmt) {
Comment thread
wenzhenghu marked this conversation as resolved.
if (stmt == null || !(parsedStmt instanceof LogicalPlanAdapter)) {
return stmt;
}
LogicalPlan logicalPlan = ((LogicalPlanAdapter) parsedStmt).getLogicalPlan();
if (!(logicalPlan instanceof NeedAuditEncryption)) {
return stmt;
}
return ((NeedAuditEncryption) logicalPlan).geneEncryptionSQL(stmt);
Comment thread
wenzhenghu marked this conversation as resolved.
Outdated
}

private String getStmtForLoggingBeforeParse() {
if (originStmt == null || originStmt.originStmt == null) {
return null;
}
try {
LogicalPlan logicalPlan = new NereidsParser().parseSingle(originStmt.originStmt);
if (!(logicalPlan instanceof NeedAuditEncryption)) {
return originStmt.originStmt;
}
return ((NeedAuditEncryption) logicalPlan).geneEncryptionSQL(originStmt.originStmt);
} catch (Exception e) {
return originStmt.originStmt;
Comment thread
wenzhenghu marked this conversation as resolved.
Outdated
}
}

public List<ByteBuffer> getProxyQueryResultBufList() {
return ((ProxyMysqlChannel) context.getMysqlChannel()).getProxyResultBufferList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,27 @@ public void testAlterRepositoryMasksSecretKey() {
Assertions.assertFalse(masked.contains("SUPERSECRET"), "secret_key must be masked: " + masked);
Assertions.assertTrue(masked.contains("*XXX"), "expected mask token: " + masked);
}

@Test
public void testCreateResourceMasksAiApiKey() {
String sql = "CREATE EXTERNAL RESOURCE \"ai_resource\" PROPERTIES ("
+ "\"type\" = \"ai\", "
+ "\"ai.api_key\" = \"sk-test\", "
+ "\"ai.endpoint\" = \"https://api.test\")";
String masked = encrypt(sql);
Assertions.assertFalse(masked.contains("sk-test"), masked);
Assertions.assertTrue(masked.contains("*XXX"), masked);
Assertions.assertTrue(masked.contains("https://api.test"), masked);
}

@Test
public void testAlterResourceMasksAiApiKey() {
String sql = "ALTER RESOURCE \"ai_resource\" PROPERTIES ("
+ "\"ai.api_key\" = \"sk-test\", "
+ "\"ai.endpoint\" = \"https://api.test\")";
String masked = encrypt(sql);
Assertions.assertFalse(masked.contains("sk-test"), masked);
Assertions.assertTrue(masked.contains("*XXX"), masked);
Assertions.assertTrue(masked.contains("https://api.test"), masked);
}
}
57 changes: 57 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/qe/StmtExecutorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
package org.apache.doris.qe;

import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.InternalSchemaInitializer;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.ResourceMgr;
import org.apache.doris.common.Config;
import org.apache.doris.common.FeConstants;
import org.apache.doris.mysql.MysqlChannel;
import org.apache.doris.mysql.MysqlSerializer;
import org.apache.doris.mysql.authenticate.TestLogAppender;
import org.apache.doris.planner.PlanFragment;
import org.apache.doris.planner.Planner;
import org.apache.doris.planner.ResultFileSink;
Expand All @@ -49,6 +52,15 @@
import java.util.concurrent.atomic.AtomicInteger;

public class StmtExecutorTest extends TestWithFeService {
private static final String CREATE_AI_RESOURCE_SQL = "CREATE EXTERNAL RESOURCE \"ai_resource_log_test\"\n"
+ "PROPERTIES\n"
+ "(\n"
+ " \"type\" = \"ai\",\n"
+ " \"ai.provider_type\" = \"openai\",\n"
+ " \"ai.endpoint\" = \"https://api.test\",\n"
+ " \"ai.model_name\" = \"gpt-test\",\n"
+ " \"ai.api_key\" = \"sk-test-secret\"\n"
+ ");";

@Override
protected void runBeforeAll() throws Exception {
Expand Down Expand Up @@ -472,4 +484,49 @@ public void testShouldDisableCloudVersionCacheOnRetryForE230() {
connectContext.getSessionVariable().cloudTableVersionCacheTtlMs = originalTableTtl;
}
}

@Test
public void testNeedAuditEncryptionStatementLogsMaskedSql() throws Exception {
boolean originalPrintRequest = Config.enable_print_request_before_execution;
Config.enable_print_request_before_execution = true;
try (TestLogAppender appender = TestLogAppender.attach(StmtExecutor.class)) {
connectContext.getState().reset();
StmtExecutor stmtExecutor = new StmtExecutor(connectContext, CREATE_AI_RESOURCE_SQL);
stmtExecutor.execute();

Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.INFO, "sk-test-secret"));
Assertions.assertTrue(appender.contains(org.apache.logging.log4j.Level.INFO, "*XXX"));
} finally {
Config.enable_print_request_before_execution = originalPrintRequest;
}
connectContext.getState().reset();
StmtExecutor showExecutor = new StmtExecutor(connectContext, "");
showExecutor.execute();
Assertions.assertEquals(QueryState.MysqlStateType.OK, connectContext.getState().getStateType());
}

@Test
public void testAlterResourceSuccessLogDoesNotPrintResourceObject() throws Exception {
createResource(CREATE_AI_RESOURCE_SQL);
Comment thread
wenzhenghu marked this conversation as resolved.
Outdated
String alterSql = "ALTER RESOURCE \"ai_resource_log_test\" PROPERTIES ("
+ "\"ai.api_key\" = \"sk-updated-secret\")";
String fullResourceJson = Env.getCurrentEnv().getResourceMgr().getResource("ai_resource_log_test").toString();

try (TestLogAppender appender = TestLogAppender.attach(ResourceMgr.class)) {
connectContext.getState().reset();
StmtExecutor stmtExecutor = new StmtExecutor(connectContext, alterSql);
stmtExecutor.execute();

Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.INFO, "sk-updated-secret"));
Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.INFO, "\"properties\""));
Assertions.assertFalse(appender.contains(org.apache.logging.log4j.Level.INFO, fullResourceJson));
}
}

private void createResource(String sql) throws Exception {
connectContext.getState().reset();
StmtExecutor stmtExecutor = new StmtExecutor(connectContext, sql);
stmtExecutor.execute();
Assertions.assertEquals(QueryState.MysqlStateType.OK, connectContext.getState().getStateType());
}
}
Loading