harden(guard): block side-effecting selects and document the contract

guard/README.md

@@ -0,0 +1,88 @@
+# Daanse SQL Guard
+
+`SqlGuard` narrows an **untrusted** SQL string down to a **read-only query over an
+explicitly whitelisted catalog**. It is the trust boundary between caller-supplied
+SQL (for example a drill-through / pass-through query from an analytics client) and
+the database.
+
+- **API:** `guard/api` — `SqlGuard`, `SqlGuardFactory`, the `elements` catalog model,
+  and the `GuardException` hierarchy.
+- **Implementation:** `guard/jsqltranspiler` — `TranspilerSqlGuard`, built on JSqlParser
+  and JSQLTranspiler.
+
+## What the guard guarantees
+
+`SqlGuard.guard(String)` either returns an equivalent read-only `SELECT` — with every
+table and column fully qualified against the whitelisted catalog and rendered in the
+configured dialect — or throws a `GuardException`. It rejects, by construction:
+
+- any non-`SELECT` statement (INSERT / UPDATE / DELETE / DDL / …);
+- a **data-modifying CTE**, e.g. `WITH x AS (UPDATE foo …) SELECT …`;
+- `SELECT … INTO` (writes a table) and `SELECT … FOR UPDATE` / `FOR SHARE` (row locks);
+- any reference to a table or column **outside the whitelisted catalog**;
+- any **function not permitted by the function whitelist** (see below);
+- empty or unparsable input.
+
+The guard **fails closed**: any unexpected error during validation results in a
+`GuardException`, never an approved statement.
+
+## The function whitelist
+
+The whitelist is the control for which functions a query may call. It is a
+`List<String>` of **full-match, case-sensitive regular expressions** (evaluated with
+`java.util.regex.Pattern.matches`). Every function name appearing anywhere in the
+query — SELECT list, `WHERE`, `HAVING`, subqueries, and so on — must be matched in
+full by at least one pattern, or the whole statement is rejected with
+`GuardException("Query has disallowed functions.")`.
+
+```java
+// allow three aggregate functions, nothing else
+SqlGuard guard = factory.create("", "sch", catalog,
+        List.of("sum", "count", "avg"), dialect);
+```
+
+Key points:
+
+- **An empty list rejects every function** — `List.of()` is the secure default, and
+  the guard is safe out of the box.
+- Matching is **case-sensitive and anchored**: `"sum"` matches only `sum`, not `SUM`.
+  Use `"(?i)sum"`, or list both spellings, when you need case-insensitivity.
+- `List.of(".*")` allows **every** function and is strongly discouraged — see below.
+
+## Keep the whitelist tight
+
+Allow only the specific aggregate/scalar functions your use case needs. In particular,
+**never** allow function families that have side effects or enable denial of service or
+data exfiltration, for example:
+
+| Category | Examples |
+|---|---|
+| Sequence writes | `nextval`, `setval` |
+| Denial of service | `pg_sleep`, `benchmark` |
+| File / network I/O, RCE | `dblink`, `lo_import`, `lo_export`, `pg_read_file`, `pg_ls_dir`, `xp_cmdshell`, `load_file` |
+| Backend control | `pg_terminate_backend`, `pg_cancel_backend` |
+
+A broad pattern such as `.*` re-enables all of these at once. Prefer an explicit,
+enumerated list over any catch-all.
+
+## Why there is no built-in denylist
+
+The guard does **not** ship a hardcoded list of "dangerous" functions to block. Such a
+denylist is:
+
+- **inherently incomplete** — the set of side-effecting functions differs by database,
+  extension, and version, and grows over time;
+- a source of **false confidence** — a denylist that looks comprehensive invites callers
+  to relax the whitelist, which is exactly the wrong direction.
+
+The whitelist is the authoritative control, and it is already default-deny. The correct
+lever is keeping that whitelist tight, not maintaining a denylist that can never be
+complete.
+
+## Defense in depth (outside this library)
+
+Treat the guard as one layer, not the only one. Execute guarded SQL under a **read-only
+database role** and/or inside a **read-only transaction** (for example
+`SET TRANSACTION READ ONLY`, or `default_transaction_read_only` on PostgreSQL). Then even
+a future bypass — a construct no one anticipated — still cannot write. The parser-level
+guard should never be the sole barrier between untrusted SQL and the database.

guard/api/src/main/java/org/eclipse/daanse/sql/guard/api/SqlGuard.java

@@ -15,8 +15,42 @@
 
 import org.eclipse.daanse.sql.guard.api.exception.GuardException;
 
+/**
+ * Narrows an untrusted SQL string to a read-only query over an explicitly
+ * whitelisted catalog. The guard is the trust boundary between caller-supplied
+ * SQL and the database.
+ */
 public interface SqlGuard {
 
+    /**
+     * Validates {@code sql} and, if it is accepted, returns an equivalent read-only
+     * {@code SELECT} with every table and column fully qualified against the
+     * whitelisted catalog and rendered in the configured dialect.
+     * <p>
+     * The statement is rejected with a {@link GuardException} unless it is a plain,
+     * read-only {@code SELECT}. In particular the guard rejects:
+     * <ul>
+     * <li>any non-{@code SELECT} statement (INSERT/UPDATE/DELETE/DDL/…) —
+     * {@link org.eclipse.daanse.sql.guard.api.exception.UnallowedStatementTypeGuardException};</li>
+     * <li>a data-modifying CTE, e.g. {@code WITH x AS (UPDATE foo ...) SELECT ...};</li>
+     * <li>{@code SELECT ... INTO} (writes a table) and {@code SELECT ... FOR UPDATE}
+     * / {@code FOR SHARE} (row locks);</li>
+     * <li>any reference to a table or column outside the whitelisted catalog —
+     * {@link org.eclipse.daanse.sql.guard.api.exception.UnresolvableObjectsGuardException};</li>
+     * <li>any function not permitted by the configured function whitelist (see
+     * {@link SqlGuardFactory#create});</li>
+     * <li>empty input
+     * ({@link org.eclipse.daanse.sql.guard.api.exception.EmptyStatementGuardException})
+     * and unparsable input
+     * ({@link org.eclipse.daanse.sql.guard.api.exception.UnparsableStatementGuardException}).</li>
+     * </ul>
+     * The guard fails closed: any unexpected error during validation results in a
+     * {@link GuardException} rather than an approved statement.
+     *
+     * @param sql the untrusted SQL to validate.
+     * @return the rewritten, fully qualified read-only statement.
+     * @throws GuardException if the statement is not an allowed read-only query.
+     */
     String guard(String sql) throws GuardException;
 
 }

guard/api/src/main/java/org/eclipse/daanse/sql/guard/api/SqlGuardFactory.java

@@ -20,5 +20,38 @@
 
 public interface SqlGuardFactory {
 
+    /**
+     * Creates a {@link SqlGuard} bound to a whitelisted catalog and a function
+     * whitelist.
+     *
+     * @param currentCatalogName      the catalog assumed for unqualified names.
+     * @param currentSchemaName       the schema assumed for unqualified names.
+     * @param databaseCatalog         the catalog, schemas, tables and columns that
+     *                                queries are allowed to reference; anything not
+     *                                described here is rejected.
+     * @param whitelistFunctionsPatterns the functions a query may call, as a list of
+     *            <strong>full-match, case-sensitive</strong> regular expressions
+     *            (evaluated with {@link java.util.regex.Pattern#matches}). A pattern
+     *            is tested against every function name appearing anywhere in the
+     *            query. A function passes only if at least one pattern matches its
+     *            whole name; a single unmatched function rejects the statement.
+     *            <ul>
+     *            <li>An <strong>empty list rejects every function</strong> — this is
+     *            the secure default.</li>
+     *            <li>{@code "sum"} matches only {@code sum}, not {@code SUM}; use
+     *            {@code "(?i)sum"} or list both spellings for case-insensitivity.</li>
+     *            <li>{@code List.of(".*")} allows <em>every</em> function and is
+     *            strongly discouraged: it re-enables side-effecting and
+     *            denial-of-service functions (e.g. {@code nextval}, {@code setval},
+     *            {@code pg_sleep}, {@code dblink}, {@code pg_read_file},
+     *            {@code xp_cmdshell}).</li>
+     *            </ul>
+     *            Keep this list as tight as the use case allows. See
+     *            {@code guard/README.md} for the rationale and guidance.
+     * @param dialect                 the SQL dialect used to render the rewritten,
+     *                                fully qualified output.
+     * @return a guard that enforces the read-only contract described on
+     *         {@link SqlGuard#guard(String)}.
+     */
     SqlGuard create(String currentCatalogName, String currentSchemaName, DatabaseCatalog databaseCatalog, List<String> whitelistFunctionsPatterns, Dialect dialect);
 }

guard/jsqltranspiler/src/main/java/org/eclipse/daanse/sql/guard/jsqltranspiler/ReadOnlyStatementValidator.java

@@ -19,35 +19,42 @@
 import net.sf.jsqlparser.statement.ParenthesedStatement;
 import net.sf.jsqlparser.statement.Statement;
 import net.sf.jsqlparser.statement.select.ParenthesedSelect;
+import net.sf.jsqlparser.statement.select.PlainSelect;
 import net.sf.jsqlparser.statement.select.Select;
+import net.sf.jsqlparser.statement.select.SetOperationList;
 import net.sf.jsqlparser.statement.select.WithItem;
 import net.sf.jsqlparser.statement.delete.ParenthesedDelete;
 import net.sf.jsqlparser.statement.insert.ParenthesedInsert;
 import net.sf.jsqlparser.statement.update.ParenthesedUpdate;
 import net.sf.jsqlparser.util.TablesNamesFinder;
 
 /**
- * Walks a parsed {@link Select} tree and detects data-modifying statements that
- * are wrapped in a {@code WITH} item (a data-modifying CTE), such as
- * {@code WITH x AS (UPDATE foo ...) SELECT ...}.
- * <p>
- * Such statements parse as a {@code Select} and therefore pass the top-level
- * {@code instanceof Select} gate, yet on PostgreSQL/MariaDB they execute the
- * wrapped {@code UPDATE}/{@code INSERT}/{@code DELETE}. The upstream resolver
- * does not populate its update/insert column lists for a {@code WithItem}, so it
- * cannot be relied on to detect this case.
- * <p>
- * This validator reuses {@link TablesNamesFinder}'s full recursive traversal
- * (subqueries, joins, set operations, nested {@code WITH} clauses) and inspects
- * every {@code WithItem}. A {@code WithItem} is only permitted to wrap a
- * {@link ParenthesedSelect}; anything else is a violation.
+ * Walks a parsed {@link Select} tree and rejects statements that parse as a
+ * {@code Select} yet carry a side effect. Several SQL constructs pass the naive
+ * {@code instanceof Select} gate but are not read-only:
+ * <ul>
+ * <li>a data-modifying CTE, e.g. {@code WITH x AS (UPDATE foo ...) SELECT ...} —
+ * on PostgreSQL/MariaDB the wrapped {@code UPDATE}/{@code INSERT}/{@code DELETE}
+ * executes. The upstream resolver does not populate its update/insert column
+ * lists for a {@code WithItem}, so it cannot be relied on to detect this;</li>
+ * <li>{@code SELECT ... INTO newtable} — creates/writes a table
+ * (PostgreSQL, SQL Server);</li>
+ * <li>{@code SELECT ... FOR UPDATE}/{@code FOR SHARE} — takes row locks.</li>
+ * </ul>
+ * The validator reuses {@link TablesNamesFinder}'s full recursive traversal
+ * (subqueries, joins, set operations, nested {@code WITH} clauses). A
+ * {@code WithItem} is only permitted to wrap a {@link ParenthesedSelect}; a
+ * {@code SELECT INTO} or a {@code FOR UPDATE}/{@code FOR SHARE} clause anywhere
+ * in the tree is a violation.
  */
 class ReadOnlyStatementValidator extends TablesNamesFinder<Void> {
 
     static final String DELETE_IS_NOT_PERMITTED = "DELETE is not permitted.";
     static final String UPDATE_IS_NOT_PERMITTED = "UPDATE is not permitted.";
     static final String INSERT_IS_NOT_PERMITTED = "INSERT is not permitted.";
     static final String STATEMENT_IS_NOT_PERMITTED = "Statement is not permitted.";
+    static final String SELECT_INTO_IS_NOT_PERMITTED = "SELECT INTO is not permitted.";
+    static final String ROW_LOCKING_IS_NOT_PERMITTED = "Row locking (FOR UPDATE/SHARE) is not permitted.";
 
     private String violation;
 
@@ -62,6 +69,35 @@ Optional<String> firstViolation(Select select) {
         return Optional.ofNullable(violation);
     }
 
+    private void record(String message) {
+        if (violation == null) {
+            violation = message;
+        }
+    }
+
+    /** A FOR UPDATE / FOR SHARE clause can hang off any Select node. */
+    private void checkRowLocking(Select select) {
+        if (select.getForMode() != null || select.getForUpdateTable() != null) {
+            record(ROW_LOCKING_IS_NOT_PERMITTED);
+        }
+    }
+
+    @Override
+    public <S> Void visit(PlainSelect plainSelect, S context) {
+        if (plainSelect.getIntoTables() != null && !plainSelect.getIntoTables().isEmpty()
+                || plainSelect.getIntoTempTable() != null) {
+            record(SELECT_INTO_IS_NOT_PERMITTED);
+        }
+        checkRowLocking(plainSelect);
+        return super.visit(plainSelect, context);
+    }
+
+    @Override
+    public <S> Void visit(SetOperationList setOpList, S context) {
+        checkRowLocking(setOpList);
+        return super.visit(setOpList, context);
+    }
+
     @Override
     public <S> Void visit(WithItem<?> withItem, S context) {
         ParenthesedStatement statement = withItem.getParenthesedStatement();

guard/jsqltranspiler/src/main/java/org/eclipse/daanse/sql/guard/jsqltranspiler/TranspilerSqlGuard.java

@@ -57,6 +57,7 @@ public class TranspilerSqlGuard implements SqlGuard {
     private static final String INSERT_IS_NOT_PERMITTED = "INSERT is not permitted.";
     private static final String NOTHING_WAS_SELECTED = "Nothing was selected.";
     private static final String QUERY_HAS_DISAllOWED_FUNCTIONS = "Query has disallowed functions.";
+    private static final String COULD_NOT_VALIDATE = "Statement could not be validated.";
     private static final Logger LOGGER = LoggerFactory.getLogger(TranspilerSqlGuard.class);
     private JdbcMetaData jdbcMetaDataToCopy;
     private List<String> whitelistFunctionsPatterns = new ArrayList<String>();
@@ -176,6 +177,11 @@ public String guard(String sqlStr) throws GuardException {
         } catch (CatalogNotFoundException | ColumnNotFoundException | SchemaNotFoundException
             | TableNotDeclaredException | TableNotFoundException ex) {
             throw new UnresolvableObjectsGuardException(ex.getMessage());
+        } catch (RuntimeException ex) {
+            // Fail closed: any unexpected error while validating untrusted SQL must
+            // deny the statement, never let it through or leak an internal exception.
+            LOGGER.atWarn().setCause(ex).log(COULD_NOT_VALIDATE);
+            throw new GuardException(COULD_NOT_VALIDATE);
         }
 
     }

guard/jsqltranspiler/src/test/java/org/eclipse/daanse/sql/guard/jsqltranspiler/integration/SqlGuardTest.java

@@ -269,6 +269,19 @@ y AS (UPDATE foo SET name='b' RETURNING id)
     private static final String INSERT_IS_NOT_PERMITTED = "INSERT is not permitted.";
 
     private static final String DELETE_IS_NOT_PERMITTED = "DELETE is not permitted.";
+
+    private static final String SQL_SELECT_INTO = "SELECT foo.id INTO bar FROM foo";
+
+    private static final String SQL_SELECT_FOR_UPDATE = "SELECT foo.id FROM foo FOR UPDATE";
+
+    private static final String SQL_SELECT_FOR_SHARE = "SELECT foo.id FROM foo FOR SHARE";
+
+    private static final String SQL_UNION_FOR_UPDATE = """
+            SELECT foo.id FROM foo UNION SELECT fooFact.id FROM fooFact FOR UPDATE""";
+
+    private static final String SELECT_INTO_IS_NOT_PERMITTED = "SELECT INTO is not permitted.";
+
+    private static final String ROW_LOCKING_IS_NOT_PERMITTED = "Row locking (FOR UPDATE/SHARE) is not permitted.";
     private static final String SQL_LEFT_JOIN_ON_WRONG_COLUMN = """
             SELECT foo.id FROM foo LEFT JOIN fooFact ON foo.wrongCol = fooFact.id""";
 
@@ -1089,6 +1102,50 @@ void testValidCteStillPasses(@InjectService SqlGuardFactory sqlGuardFactory) thr
         }
     }
 
+    @Nested
+    @DisplayName("Side-Effecting Select Security Tests (SELECT INTO / FOR UPDATE)")
+    class SideEffectingSelectSecurityTests {
+
+        // These parse as a Select but are not read-only: SELECT INTO writes a table,
+        // FOR UPDATE/SHARE takes row locks. They must be rejected by the read-only gate.
+
+        @Test
+        void testSelectInto(@InjectService SqlGuardFactory sqlGuardFactory) throws Exception {
+            DatabaseCatalog databaseCatalog = schemaWithTwoTableTwoCol();
+            SqlGuard guard = sqlGuardFactory.create("", SCH, databaseCatalog, List.of(), dialect);
+            assertThatThrownBy(() -> guard.guard(SQL_SELECT_INTO))
+                    .isInstanceOf(GuardException.class)
+                    .hasMessage(SELECT_INTO_IS_NOT_PERMITTED);
+        }
+
+        @Test
+        void testSelectForUpdate(@InjectService SqlGuardFactory sqlGuardFactory) throws Exception {
+            DatabaseCatalog databaseCatalog = schemaWithTwoTableTwoCol();
+            SqlGuard guard = sqlGuardFactory.create("", SCH, databaseCatalog, List.of(), dialect);
+            assertThatThrownBy(() -> guard.guard(SQL_SELECT_FOR_UPDATE))
+                    .isInstanceOf(GuardException.class)
+                    .hasMessage(ROW_LOCKING_IS_NOT_PERMITTED);
+        }
+
+        @Test
+        void testSelectForShare(@InjectService SqlGuardFactory sqlGuardFactory) throws Exception {
+            DatabaseCatalog databaseCatalog = schemaWithTwoTableTwoCol();
+            SqlGuard guard = sqlGuardFactory.create("", SCH, databaseCatalog, List.of(), dialect);
+            assertThatThrownBy(() -> guard.guard(SQL_SELECT_FOR_SHARE))
+                    .isInstanceOf(GuardException.class)
+                    .hasMessage(ROW_LOCKING_IS_NOT_PERMITTED);
+        }
+
+        @Test
+        void testUnionForUpdate(@InjectService SqlGuardFactory sqlGuardFactory) throws Exception {
+            DatabaseCatalog databaseCatalog = schemaWithTwoTableTwoCol();
+            SqlGuard guard = sqlGuardFactory.create("", SCH, databaseCatalog, List.of(), dialect);
+            assertThatThrownBy(() -> guard.guard(SQL_UNION_FOR_UPDATE))
+                    .isInstanceOf(GuardException.class)
+                    .hasMessage(ROW_LOCKING_IS_NOT_PERMITTED);
+        }
+    }
+
     @Nested
     @DisplayName("JOIN Security Tests")
     class JoinSecurityTests {