Hardening: enforce simple JDBC named query filenames

docs/administering/configuring/jdbc-connector/jdbc_cfg.md

@@ -328,6 +328,8 @@ You may optionally provide the ending semicolon (`;`) for the SQL statement.
 
 The Apache Cloudberry user references a named query by specifying the query file name without the extension. For example, if you define a query in a file named `report.sql`, the name of that query is `report`.
 
+The query name must be a file name without directory components. Forward slashes (`/`), backslashes (`\`), colons (`:`), and null characters are not allowed. The query file must reside directly in the selected JDBC server configuration directory. Symbolic links to query files outside that directory are not allowed.
+
 Named queries are associated with a specific JDBC server configuration. You will provide the available query names to the Apache Cloudberry users that you allow to create external tables using the server configuration.
 
 #### Referencing a Named Query

docs/upgrade/upgrade_pxf.md

@@ -89,3 +89,11 @@ After you install the new version of PXF, perform the following procedure:
     ``` shell
     gpadmin@coordinator$ pxf cluster start
     ```
+
+## Backward-Incompatible Changes
+
+### JDBC Named Queries
+
+JDBC named query files must now reside directly in `$PXF_BASE/servers/<server_name>/`. Query names containing `/`, `\`, `:`, or null characters, and symbolic links to files outside that directory, are rejected.
+
+To migrate, copy affected SQL files into that directory, rename them as needed, and update external table `LOCATION` clauses or foreign table `resource` options to use `query:<query_name>` without the `.sql` extension or directory components. Run `pxf cluster sync` before restarting PXF. See [JDBC Named Query Configuration](../administering/configuring/jdbc-connector/jdbc_cfg.md#jdbc-named-query-configuration).

docs/upgrade/upgrade_pxf_rpm.md

@@ -6,6 +6,8 @@ sidebar_position: 4
 
 If you installed PXF from the `rpm` or `deb` package and have configured and are using PXF in your current Apache Cloudberry installation, you must perform some upgrade actions when you install a new version of PXF.
 
+Before upgrading, review the [backward-incompatible changes](./upgrade_pxf.md#backward-incompatible-changes).
+
 The PXF upgrade procedure has two parts. You perform one procedure before, and one procedure after, you install the new version:
 
 -   [Step 1: Complete the PXF Pre-Upgrade Actions](#step-1-complete-the-pxf-pre-upgrade-actions)

server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcAccessor.java

@@ -33,9 +33,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.File;
 import java.io.IOException;
 import java.nio.charset.Charset;
+import java.nio.file.Path;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
@@ -354,11 +354,16 @@ private String getQueryText() {
 
         String queryText;
         try {
-            File queryFile = new File(serverDirectory, queryName + ".sql");
+            Path serverDirectoryPath = Path.of(serverDirectory).toRealPath();
+            Path queryFile = serverDirectoryPath.resolve(queryName + ".sql").toRealPath();
+            // Resolve symbolic links and compare directory objects, not string prefixes.
+            if (!serverDirectoryPath.equals(queryFile.getParent())) {
+                throw new IllegalArgumentException("Query file must reside in the server configuration directory");
+            }
             if (LOG.isDebugEnabled()) {
-                LOG.debug("Reading text of query={} from {}", queryName, queryFile.getCanonicalPath());
+                LOG.debug("Reading text of query={} from {}", queryName, queryFile);
             }
-            queryText = FileUtils.readFileToString(queryFile, Charset.defaultCharset());
+            queryText = FileUtils.readFileToString(queryFile.toFile(), Charset.defaultCharset());
         } catch (IOException e) {
             throw new RuntimeException(String.format("Failed to read text of query %s : %s", queryName, e.getMessage()), e);
         }

server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePlugin.java

@@ -221,6 +221,11 @@ public void afterPropertiesSet() {
             if (StringUtils.isBlank(queryName)) {
                 throw new IllegalArgumentException(String.format("Query name is not provided in data source [%s]", dataSource));
             }
+            // A named query is a file name within the selected server configuration, not a path.
+            // Check both Unix and Windows syntax, including drive-relative paths and NTFS streams.
+            if (StringUtils.containsAny(queryName, "/\\:\0")) {
+                throw new IllegalArgumentException("Invalid query name: " + queryName);
+            }
             LOG.debug("Query name is {}", queryName);
         } else {
             tableName = dataSource;

server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcAccessorTest.java

@@ -9,12 +9,17 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockedStatic;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.io.File;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
 import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
@@ -23,17 +28,22 @@
 import java.sql.Statement;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.mockStatic;
 
 @ExtendWith(MockitoExtension.class)
 public class JdbcAccessorTest {
 
+    @TempDir
+    Path tempDirectory;
+
     private Configuration configuration;
     private JdbcAccessor accessor;
     private RequestContext context;
@@ -103,22 +113,25 @@ public void testReadFromQueryFailsWhenServerDirectoryDoesNotExist() throws SQLEx
             accessor.afterPropertiesSet();
             Exception e = assertThrows(RuntimeException.class,
                     () -> accessor.openForRead());
-            assertEquals("Failed to read text of query foo : File '/non-existing-directory/foo.sql' does not exist", e.getMessage());
+            assertInstanceOf(NoSuchFileException.class, e.getCause().getCause());
+            assertEquals("Failed to read text of query foo : /non-existing-directory", e.getMessage());
             jdbcBasePluginMockedStatic.verify(() -> JdbcBasePlugin.closeConnection(mockConnection));
         }
     }
 
     @Test
-    public void testReadFromQueryFailsWhenQueryFileIsNotFoundInExistingDirectory() throws SQLException {
+    public void testReadFromQueryFailsWhenQueryFileIsNotFoundInExistingDirectory() throws Exception {
         try (MockedStatic<JdbcBasePlugin> jdbcBasePluginMockedStatic = mockStatic(JdbcBasePlugin.class)) {
             wireMocksForRead();
-            configuration.set("pxf.config.server.directory", "/tmp/");
+            configuration.set("pxf.config.server.directory", tempDirectory.toString());
             context.setDataSource("query:foo");
             accessor.setRequestContext(context);
             accessor.afterPropertiesSet();
             Exception e = assertThrows(RuntimeException.class,
                     () -> accessor.openForRead());
-            assertEquals("Failed to read text of query foo : File '/tmp/foo.sql' does not exist", e.getMessage());
+            assertInstanceOf(NoSuchFileException.class, e.getCause().getCause());
+            assertEquals("Failed to read text of query foo : " +
+                    tempDirectory.toRealPath().resolve("foo.sql"), e.getMessage());
             jdbcBasePluginMockedStatic.verify(() -> JdbcBasePlugin.closeConnection(mockConnection));
         }
     }
@@ -139,6 +152,47 @@ public void testReadFromQueryFailsWhenQueryFileIsEmpty() throws Exception {
         }
     }
 
+    @Test
+    public void testReadFromQueryFailsWhenSymbolicLinkEscapesServerDirectory() throws Exception {
+        Path serverDirectory = Files.createDirectory(tempDirectory.resolve("server"));
+        Path otherDirectory = Files.createDirectory(tempDirectory.resolve("server-other"));
+        Path outsideQuery = Files.write(otherDirectory.resolve("report.sql"),
+                "SELECT secret FROM private_table".getBytes(StandardCharsets.UTF_8));
+        Files.createSymbolicLink(serverDirectory.resolve("report.sql"), outsideQuery);
+        configuration.set("pxf.config.server.directory", serverDirectory.toString());
+        context.setDataSource("query:report");
+        accessor.setRequestContext(context);
+        accessor.afterPropertiesSet();
+
+        try (MockedStatic<JdbcBasePlugin> jdbcBasePluginMockedStatic = mockStatic(JdbcBasePlugin.class)) {
+            wireMocksForRead();
+
+            Exception e = assertThrows(PxfRuntimeException.class, () -> accessor.openForRead());
+            assertEquals("Query file must reside in the server configuration directory", e.getMessage());
+            verify(mockConnection, never()).createStatement();
+            jdbcBasePluginMockedStatic.verify(() -> JdbcBasePlugin.closeConnection(mockConnection));
+        }
+    }
+
+    @Test
+    public void testReadFromQueryWithSymbolicLinksWithinServerDirectory() throws Exception {
+        Path serverDirectory = Files.createDirectory(tempDirectory.resolve("server"));
+        Path queryFile = Files.write(serverDirectory.resolve("report.sql"),
+                "SELECT 1".getBytes(StandardCharsets.UTF_8));
+        Files.createSymbolicLink(serverDirectory.resolve("alias.sql"), queryFile);
+        Path serverLink = Files.createSymbolicLink(tempDirectory.resolve("server-link"), serverDirectory);
+        configuration.set("pxf.config.server.directory", serverLink.toString());
+        context.setDataSource("query:alias");
+        wireMocksForReadWithCreateStatement();
+        when(mockStatement.executeQuery(any())).thenReturn(mockResultSet);
+
+        accessor.setRequestContext(context);
+        accessor.afterPropertiesSet();
+        accessor.openForRead();
+
+        verify(mockStatement).executeQuery("SELECT  FROM (SELECT 1) pxfsubquery");
+    }
+
     @Test
     public void testReadFromQuery() throws Exception {
         String serversDirectory = new File(this.getClass().getClassLoader().getResource("servers").toURI()).getCanonicalPath();

server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePluginTestInitialize.java

@@ -31,6 +31,8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.internal.util.reflection.FieldReader;
 import org.mockito.junit.jupiter.MockitoExtension;
 
@@ -592,6 +594,30 @@ public void testDatasourceIsQuery() throws Exception {
         assertNull(getInternalState(plugin, "tableName"));
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {"report_2026-11", "report.v2", "monthly report", "résumé", "..report"})
+    public void testDatasourceIsQueryWithValidFileName(String queryName) throws Exception {
+        plugin.setRequestContext(makeContextWithDataSource(makeConfiguration(), "query:" + queryName));
+        plugin.afterPropertiesSet();
+
+        assertEquals(queryName, getInternalState(plugin, "queryName"));
+        assertNull(getInternalState(plugin, "tableName"));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = {
+            "../other-server/report", "../../report", "nested/report", "./report", "/tmp/report",
+            "..\\other-server\\report", "nested\\report", "C:\\queries\\report", "\\\\host\\queries\\report",
+            "C:report", "C:/queries/report", "\\report", "\\\\?\\C:\\queries\\report",
+            "\\\\.\\C:\\queries\\report", "report:stream", "report.sql::$DATA", "report\0ignored"
+    })
+    public void testInitializationFailsWhenQueryNameContainsPathCharacters(String queryName) {
+        plugin.setRequestContext(makeContextWithDataSource(makeConfiguration(), "query:" + queryName));
+
+        Exception ex = assertThrows(IllegalArgumentException.class, plugin::afterPropertiesSet);
+        assertEquals("Invalid query name: " + queryName, ex.getMessage());
+    }
+
     @Test
     public void testInitializationFailsWhenDatasourceIsEmptyQuery() {
         plugin.setRequestContext(makeContextWithDataSource(makeConfiguration(), "query:"));