CalvinKirs opened a new pull request, #66483:
URL: https://github.com/apache/doris/pull/66483
### What problem does this PR solve?
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 ran neither.**
`iceberg.catalog.type=jdbc` and
`paimon.catalog.type=jdbc` reach the same class-loading sink through
`jdbc.driver_url`, but only
the jdbc catalog validated it.
**None of the three 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 configuration
did not actually
protect the catalogs it was meant to protect.
What this PR does:
- Extracts the mandatory rule into `JdbcDriverUrlSecurity` (fe-connector-spi
— the one module all
three connectors depend on), deleting the jdbc-local copy, and calls it
from each provider's
`validateProperties`. fe-core runs that hook on CREATE **and** on ALTER,
and never on replay, so
existing catalogs and follower startup are unaffected.
- Adds `ConnectorProvider.driverUrlsToValidate(properties)`: the connector
names the values it would
hand to a class loader, and `PluginDrivenExternalCatalog` applies the
fe.conf gate to them on
ALTER. Only the connector knows which property is a jar; only the engine
knows the fe.conf policy.
The declaration is flavor-gated, so a stray `driver_url` left on a
REST/HMS/filesystem catalog
does not turn a previously-accepted catalog into a CREATE/ALTER failure.
- Bumps `<connector.plugin.api.version>` `3.0` -> `4.0` together with the
refreshed
`connector-plugin-surface.txt`, per the rule recorded in
`ConnectorPluginSurfaceTest`: a 3.x
plugin inherits the empty default of the new method, so its `driver_url`
would skip the operator's
policy without any error — it has to be refused at load time rather than
silently under-enforced.
- `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.
### Release note
Fixed `driver_url` validation for jdbc-flavored catalogs: the Iceberg and
Paimon JDBC catalogs now
apply the same driver-jar rules as the JDBC catalog, and `ALTER CATALOG` now
applies
`jdbc_driver_secure_path` / `jdbc_driver_url_white_list` instead of only
`CREATE CATALOG` doing so.
### Check List (For Author)
- Test
- [x] Unit Test
New: `JdbcDriverUrlSecurityTest` (rule semantics, moved with the class),
`IcebergJdbcDriverUrlSecurityTest`, `PaimonJdbcDriverUrlSecurityTest`
(rule reachable on both the
provider and the `preCreateValidation` path; skipped on non-jdbc flavors),
`PluginDrivenExternalCatalogDriverUrlGateTest` (ALTER honours the operator
allow-list; a connector
that declares no jar is untouched).
Each new test was mutation-checked: removing the corresponding production
line turns it red.
Full runs: fe-connector-spi/jdbc/iceberg/paimon `Tests run: 1174,
Failures: 0, Errors: 0`;
fe-core plugin/API-version/gate tests `Tests run: 33, Failures: 0, Errors:
0`; 0 Checkstyle
violations.
- Behavior changed:
- [x] Yes.
A `driver_url` containing a `..` segment, or a bare file name outside
`[A-Za-z0-9._-]+\.jar`, is
now rejected on the Iceberg/Paimon JDBC catalogs as it already was on the
JDBC catalog, and on
ALTER as well as CREATE. `jdbc_driver_secure_path` /
`jdbc_driver_url_white_list` now also apply
on ALTER. The default posture is unchanged: `jdbc_driver_secure_path`
still defaults to `*`, so a
remote driver jar is still accepted unless the operator narrows the config
— what changes is that
narrowing it now holds on every DDL path. Validation never runs on replay,
so no existing catalog
and no follower startup can be broken by this.
- Does this need documentation?
- [ ] No.
--
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]
