This is an automated email from the ASF dual-hosted git repository.
rubenada pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 56471514aa [CALCITE-7740] Harden ModelHandler against
content-dependent errors
56471514aa is described below
commit 56471514aad24f98de054fc0e2fd1f7475f239eb
Author: Ruben Quesada Lopez <[email protected]>
AuthorDate: Tue Aug 25 12:24:32 2026 +0100
[CALCITE-7740] Harden ModelHandler against content-dependent errors
---
.../calcite/config/CalciteSystemProperty.java | 17 +++
.../org/apache/calcite/model/ModelHandler.java | 55 ++++++++-
.../org/apache/calcite/model/ModelHandlerTest.java | 124 +++++++++++++++++++++
site/_docs/security_threat_model.md | 5 +-
4 files changed, 199 insertions(+), 2 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
index 0d14fed45b..7bae4774b0 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
@@ -492,6 +492,23 @@ public final class CalciteSystemProperty<T> {
public static final CalciteSystemProperty<String> MODEL_CLASSES_DENIED =
stringProperty("calcite.model.classes.denied", "");
+ /**
+ * Base directory that file-based connection models
+ * ({@code model=<path>}) must reside in.
+ *
+ * <p>The {@code model} connection property is settable by a query
+ * author, and a non-{@code inline:} value is read from the local
+ * filesystem before any schema is created. When this property is empty
+ * (the default), any local path is accepted. When it is set, relative
+ * model paths resolve under the configured directory and any model path
+ * that resolves outside it is rejected. The check is lexical and does
+ * not follow symbolic links.
+ *
+ * @see org.apache.calcite.model.ModelHandler
+ */
+ public static final CalciteSystemProperty<String> MODEL_BASE_DIRECTORY =
+ stringProperty("calcite.model.baseDirectory", "");
+
/**
* Maximum number of decimal digits that the plain-notation expansion of a
{@code DECIMAL}
* literal may contain.
diff --git a/core/src/main/java/org/apache/calcite/model/ModelHandler.java
b/core/src/main/java/org/apache/calcite/model/ModelHandler.java
index 46661065cc..c30cfc2815 100644
--- a/core/src/main/java/org/apache/calcite/model/ModelHandler.java
+++ b/core/src/main/java/org/apache/calcite/model/ModelHandler.java
@@ -18,6 +18,7 @@
import org.apache.calcite.adapter.jdbc.JdbcSchema;
import org.apache.calcite.avatica.AvaticaUtils;
+import org.apache.calcite.config.CalciteSystemProperty;
import org.apache.calcite.jdbc.CalciteConnection;
import org.apache.calcite.jdbc.CalciteSchema;
import org.apache.calcite.materialize.Lattice;
@@ -53,9 +54,13 @@
import com.google.common.collect.ImmutableMap;
import org.checkerframework.checker.nullness.qual.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.sql.SQLException;
import java.util.ArrayDeque;
import java.util.Collections;
@@ -77,6 +82,11 @@ public class ModelHandler {
.configure(JsonParser.Feature.ALLOW_COMMENTS, true);
private static final ObjectMapper YAML_MAPPER = new YAMLMapper();
+ /** Receives the detail suppressed from the client-facing message when a
+ * non-inline model cannot be read or parsed. */
+ private static final Logger LOGGER =
+ LoggerFactory.getLogger(ModelHandler.class);
+
private final SchemaPlus rootSchema;
private final @Nullable String defaultSchemaName;
private final Deque<Pair<? extends @Nullable String, SchemaPlus>>
schemaStack =
@@ -114,12 +124,55 @@ public ModelHandler(SchemaPlus rootSchema, String uri,
root = mapper.readValue(inline, JsonRoot.class);
} else {
mapper = uri.endsWith(".yaml") || uri.endsWith(".yml") ? YAML_MAPPER :
JSON_MAPPER;
- root = mapper.readValue(new File(uri), JsonRoot.class);
+ try {
+ root = mapper.readValue(modelFile(uri), JsonRoot.class);
+ } catch (IOException e) {
+ // The client-facing message for a non-inline model must not depend
+ // on the file's contents or on whether the file exists. Collapse
everything
+ // to one path-only summary; keep the detail in the operator log.
+ LOGGER.warn("Unable to read model file '{}'", uri, e);
+ throw new IOException("Unable to read model file '" + uri
+ + "'; see log of " + ModelHandler.class.getName()
+ + " for details");
+ } catch (RuntimeException e) {
+ // Same handling for RuntimeException, just in case some custom
+ // deserializers throw it.
+ LOGGER.warn("Unable to read model file '{}'", uri, e);
+ throw new RuntimeException("Unable to read model file '" + uri
+ + "'; see log of " + ModelHandler.class.getName()
+ + " for details");
+ }
}
visit(root);
this.defaultSchemaName = root.defaultSchema;
}
+ /** Resolves a non-{@code inline:} model URI to a file, enforcing
+ * {@link CalciteSystemProperty#MODEL_BASE_DIRECTORY} when it is set. */
+ private static File modelFile(String uri) {
+ return modelFile(CalciteSystemProperty.MODEL_BASE_DIRECTORY.value(), uri);
+ }
+
+ /** As {@link #modelFile(String)}, but with an explicit base directory;
+ * package-private for tests.
+ *
+ * <p>An empty base directory means "no restriction": the URI is used as
+ * given. Otherwise a relative URI resolves under the base directory,
+ * and any URI that resolves (lexically) outside it is rejected. */
+ static File modelFile(String baseDirectory, String uri) {
+ if (baseDirectory.isEmpty()) {
+ return new File(uri);
+ }
+ final Path basePath =
Paths.get(baseDirectory).toAbsolutePath().normalize();
+ final Path path = basePath.resolve(uri).normalize();
+ if (!path.startsWith(basePath)) {
+ throw new SecurityException("Model file '" + uri + "' resolves"
+ + " outside the configured base directory (system property '"
+ + "calcite.model.baseDirectory')");
+ }
+ return path.toFile();
+ }
+
@Deprecated // to be removed before 2.0
public ModelHandler(CalciteConnection connection, String uri) throws
IOException {
this(connection.getRootSchema(), uri);
diff --git a/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
b/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
index 438ec79c3c..7d0e00eda2 100644
--- a/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
+++ b/core/src/test/java/org/apache/calcite/model/ModelHandlerTest.java
@@ -25,8 +25,13 @@
import com.google.common.collect.ImmutableSet;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import java.io.File;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.sql.Connection;
import java.sql.DriverManager;
import java.util.Properties;
@@ -189,4 +194,123 @@ public class ModelHandlerTest {
assertThat(ClassNameFilter.append("", ""), is(""));
}
+ // ----- Non-inline model-file handling ---------------------------------
+ //
+ // The following tests cover two related aspects of the non-inline branch
+ // of the ModelHandler constructor:
+ //
+ // 1. The client-facing IOException on a bad model file carries only
+ // the path, not the file's contents and not whether the file
+ // exists. Legitimate operators find the detail in the log.
+ // 2. When calcite.model.baseDirectory is set, model paths that
+ // resolve outside it are rejected.
+ //
+ // The inline: branch is intentionally untouched: inline text is
+ // supplied by the caller, and detailed parse messages remain
+ // load-bearing for model authors.
+
+ @Test void testModelFileNoRestrictionWhenBaseDirectoryUnset() {
+ assertThat(ModelHandler.modelFile("", "anywhere/model.json").getPath(),
+ is(new File("anywhere/model.json").getPath()));
+ }
+
+ @Test void testModelFileRelativePathResolvesUnderBaseDirectory(
+ @TempDir Path tempDir) {
+ final File file =
+ ModelHandler.modelFile(tempDir.toString(), "sub/model.json");
+ assertThat(file.toPath().startsWith(tempDir), is(true));
+ assertThat(file.getName(), is("model.json"));
+ }
+
+ @Test void testModelFileRelativeEscapeRejected(@TempDir Path tempDir) {
+ SecurityException e =
+ assertThrows(SecurityException.class, () ->
+ ModelHandler.modelFile(tempDir.resolve("base").toString(),
+ "../escape/model.json"));
+ assertThat(e.getMessage(),
+ containsString("calcite.model.baseDirectory"));
+ }
+
+ @Test void testModelFileAbsolutePathOutsideBaseDirectoryRejected(
+ @TempDir Path tempDir) {
+ final Path base = tempDir.resolve("base");
+ final Path outside = tempDir.resolve("outside/model.json");
+ assertThrows(SecurityException.class, () ->
+ ModelHandler.modelFile(base.toString(), outside.toString()));
+ }
+
+ @Test void testModelFileAbsolutePathInsideBaseDirectoryAllowed(
+ @TempDir Path tempDir) {
+ final Path inside = tempDir.resolve("model.json");
+ final File file =
+ ModelHandler.modelFile(tempDir.toString(), inside.toString());
+ assertThat(file.toPath(), is(inside));
+ }
+
+ /** A malformed model file surfaces a path-only message; source-byte
+ * fragments Jackson would otherwise quote in the parse error do not
+ * reach the client through the IOException chain. */
+ @Test void testMalformedModelFileReportsPathOnly(@TempDir Path tempDir) {
+ final String marker = "distinct-first-line-marker";
+ Path modelFile = tempDir.resolve("scratch.json");
+ try {
+ Files.write(modelFile, (marker + " not json
content").getBytes(StandardCharsets.UTF_8));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ SchemaPlus root = CalciteSchema.createRootSchema(false, false).plus();
+ IOException e =
+ assertThrows(IOException.class,
+ () -> new ModelHandler(root, modelFile.toString()));
+ assertThat(e.getMessage(), containsString(modelFile.toString()));
+ assertThat(e.getMessage(), containsString("see log of")); // Full info in
log
+ // Neither the top-level message nor any cause in the chain names the
+ // marker string from the file
+ Throwable t = e;
+ while (t != null) {
+ assertThat(String.valueOf(t.getMessage()),
+ not(containsString(marker)));
+ t = t.getCause();
+ }
+ }
+
+ /** A missing model file produces the same shape of message as a
+ * malformed one: the client cannot distinguish "does not exist" from
+ * "exists but unreadable" from "exists but invalid" via the error
+ * string. */
+ @Test void testMissingModelFileReportsGenericPathOnlyError(
+ @TempDir Path tempDir) {
+ final Path modelFile = tempDir.resolve("no-such-model.json");
+ SchemaPlus root = CalciteSchema.createRootSchema(false, false).plus();
+ IOException e =
+ assertThrows(IOException.class,
+ () -> new ModelHandler(root, modelFile.toString()));
+ assertThat(e.getMessage(), containsString(modelFile.toString()));
+ assertThat(e.getMessage(), containsString("see log of")); // Full info in
log
+ // The message must not carry the "FileNotFound"/"No such file"
+ // signatures that would otherwise distinguish existence
+ Throwable t = e;
+ while (t != null) {
+ assertThat(String.valueOf(t.getMessage()),
+ not(containsString("FileNotFound")));
+ assertThat(String.valueOf(t.getMessage()),
+ not(containsString("No such file")));
+ t = t.getCause();
+ }
+ }
+
+ /** Inline-model errors keep the detailed Jackson message: inline text
+ * comes from the caller, so echoing it back leaks nothing. */
+ @Test void testInlineModelErrorsKeepDetail() {
+ SchemaPlus root = CalciteSchema.createRootSchema(false, false).plus();
+ IOException e =
+ assertThrows(IOException.class,
+ () -> new ModelHandler(root, "inline:{ not valid json"));
+ // The inline branch throws the underlying Jackson IOException
+ // unchanged (or a wrapped one whose chain still names the parser).
+ // Assert only that we did not swap in the path-only message.
+ assertThat(String.valueOf(e.getMessage()),
+ not(containsString("see log of")));
+ }
+
}
diff --git a/site/_docs/security_threat_model.md
b/site/_docs/security_threat_model.md
index d9936197d4..3ddcd89cfa 100644
--- a/site/_docs/security_threat_model.md
+++ b/site/_docs/security_threat_model.md
@@ -76,7 +76,7 @@ ## Inputs
| `tableFactory` and function classes | `model` | a class loaded through a
Calcite table or function SPI | Surprising vs unsurprising class loading (P1) |
| `dataSource`, `jdbcDriver` | connection property or `model` | a class loaded
through a standard-Java SPI (`javax.sql.DataSource`, `java.sql.Driver`) |
Surprising vs unsurprising class loading (P1); the host it then dials is P3 |
| `fun` | connection property | selects built-in function libraries by name |
no class loading; ordinary SQL semantics under P1–P4 |
-| `model` — inline JSON, a `file:` path, or a URL | connection property |
schema/table factories and adapter operands | P1 (factories via SPI), P2
(local-file operands), P3 (a URL model, or a URL-fetching adapter) |
+| `model` — inline JSON, a `file:` path, or a URL | connection property |
schema/table factories and adapter operands | P1 (factories via SPI), P2
(local-file operands), P3 (a URL model, or a URL-fetching adapter). Reading the
model URI's own bytes is the property's documented semantics; the principal who
set the connection property authorises that read. |
| A serialized RelNode plan (`RelJson`) — types and operators | any path that
reconstructs a plan from attacker input | type and operator class resolution |
Surprising vs unsurprising class loading (P1) |
| Adapter operands — e.g. a file/CSV/JSON path, or the os-adapter | `model` or
SQL | the adapter's configured resource | P2 for a configured local path
(opt-in ⇒ not a vulnerability); the os-adapter is opt-in (not a vulnerability) |
@@ -120,6 +120,9 @@ ## Not a vulnerability
must add it on purpose.
* A file, CSV, or JSON adapter reading the local path it was configured
with. Opt-in, by the same reasoning as the os-adapter.
+* Reading the file or URL named by `model=<uri>`. The property names a
+ resource the model handler reads on connection; letting an untrusted
+ principal set connection properties authorises that read.
* Anything that needs a changed system property or classpath. Both are
outside the attacker's reach by assumption.
* The behavior of a third-party driver once Calcite has connected to the
