Skip to content

guard: jsqlparser 5.4.15, jsqltranspiler 1.13, single parse, second read-only net #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stbischof merged 1 commit into from
Sep 22, 2026
Merged

guard: jsqlparser 5.4.15, jsqltranspiler 1.13, single parse, second read-only net #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deparser/api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<dependency>
<groupId>com.manticore-projects.jsqlformatter</groupId>
<artifactId>jsqlparser</artifactId>
<version>5.3.167</version>
<version>5.4.15</version>
</dependency>
</dependencies>
</project>
2 changes: 1 addition & 1 deletion deparser/jsqlparser/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
<dependency>
<groupId>com.manticore-projects.jsqlformatter</groupId>
<artifactId>jsqlparser</artifactId>
<version>5.3.167</version>
<version>5.4.15</version>
</dependency>
<dependency>
<groupId>org.osgi</groupId>
Expand Down
4 changes: 2 additions & 2 deletions guard/jsqltranspiler/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@
<dependency>
<groupId>com.manticore-projects.jsqlformatter</groupId>
<artifactId>jsqlparser</artifactId>
<version>5.3.167</version>
<version>5.4.15</version>
</dependency>
<dependency>
<groupId>ai.starlake.jsqltranspiler</groupId>
<artifactId>jsqltranspiler</artifactId>
<version>1.8</version>
<version>1.13</version>
</dependency>
<dependency>
<groupId>org.eclipse.daanse</groupId>
Expand Down
48 changes: 37 additions & 11 deletions ...iler/src/main/java/org/eclipse/daanse/sql/guard/jsqltranspiler/DeparserColumResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import ai.starlake.transpiler.schema.JdbcResultSetMetaData;
import net.sf.jsqlparser.JSQLParserException;
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
import net.sf.jsqlparser.schema.Column;
import net.sf.jsqlparser.schema.Table;
import net.sf.jsqlparser.statement.Statement;
import net.sf.jsqlparser.statement.select.Select;
Expand All @@ -40,33 +41,58 @@ public DeparserColumResolver(JdbcMetaData metaData, Dialect dialect, DialectDepa
this.dialectDeparser = dialectDeparser;
}

@Override
public String getResolvedStatementText(String sqlStr) throws JSQLParserException {
return getResolvedStatementText(CCJSqlParserUtil.parse(sqlStr));
}

Statement st = CCJSqlParserUtil.parse(sqlStr);
/**
* Resolves and deparses an already parsed statement, so a caller that has the AST
* in hand (the guard, which validated it first) does not pay for a second parse.
*/
public String getResolvedStatementText(Statement st) {
if (st instanceof Select) {
Select select = (Select) st;
select.accept((SelectVisitor<JdbcResultSetMetaData>) this, JdbcMetaData.copyOf(metaData));
stripCurrentCatalogQualifier(select);
normaliseQualifiers(select);
}

return dialectDeparser.deparse(st, dialect);
}

// The resolver only ever fills in a catalog qualifier that is missing; it never clears one that
// was already explicit in the query, even when it names the current catalog. The database
// connection is already scoped to that catalog, so only the schema-qualified name may be sent.
private void stripCurrentCatalogQualifier(Select select) {
// The resolvers only ever fill in qualifiers, they never clear them. The connection is
// already scoped to the current catalog, so a table may only be sent schema-qualified.
// Column references keep just the table name or alias: the FROM item carries the schema.
// (JSQLResolver expands "*" with the FROM item's full qualification, JSQLColumResolver
// with the bare name; this normalisation makes both paths emit the same text.)
private void normaliseQualifiers(Select select) {
String currentCatalogName = metaData.getCurrentCatalogName();
if (currentCatalogName == null || currentCatalogName.isEmpty()) {
return;
}
String currentSchemaName = metaData.getCurrentSchemaName();
new TablesNamesFinder<Void>() {
@Override
public <S> Void visit(Table table, S context) {
if (currentCatalogName.equalsIgnoreCase(table.getUnquotedDatabaseName())) {
stripCurrentCatalog(table);
return super.visit(table, context);
}

@Override
public <S> Void visit(Column column, S context) {
Table table = column.getTable();
if (table != null) {
stripCurrentCatalog(table);
if (currentSchemaName != null && !currentSchemaName.isEmpty()
&& currentSchemaName.equalsIgnoreCase(table.getUnquotedSchemaName())) {
table.setSchemaName(null);
}
}
return super.visit(column, context);
}

private void stripCurrentCatalog(Table table) {
if (currentCatalogName != null && !currentCatalogName.isEmpty()
&& currentCatalogName.equalsIgnoreCase(table.getUnquotedDatabaseName())) {
table.setDatabaseName(null);
}
return super.visit(table, context);
}
}.getTables((Statement) select);
}
Expand Down
32 changes: 27 additions & 5 deletions ...src/main/java/org/eclipse/daanse/sql/guard/jsqltranspiler/ReadOnlyStatementValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import net.sf.jsqlparser.statement.ParenthesedStatement;
import net.sf.jsqlparser.statement.Statement;
import net.sf.jsqlparser.statement.select.ForMode;
import net.sf.jsqlparser.statement.select.ParenthesedSelect;
import net.sf.jsqlparser.statement.select.PlainSelect;
import net.sf.jsqlparser.statement.select.Select;
Expand All @@ -37,15 +38,23 @@
* 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>
* <li>{@code SELECT ... INTO newtable} / {@code INTO TEMP newtable} — creates/writes
* a table (PostgreSQL, SQL Server);</li>
* <li>{@code SELECT ... INTO OUTFILE '/path'} / {@code INTO DUMPFILE '/path'} — writes
* a file on the database host, and {@code SELECT ... INTO @var} — assigns session
* variables instead of returning rows (MySQL/MariaDB, parsed since JSqlParser 5.4);</li>
* <li>{@code SELECT ... FOR UPDATE}/{@code FOR SHARE}/{@code FOR NO KEY UPDATE}/
* {@code FOR KEY 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.
* <p>
* This validator is the guard's primary, precisely worded gate. {@code TranspilerSqlGuard}
* additionally consults JSqlParser's own {@code Statement.getFeatures()} classification
* as a second, coarser net for anything not modelled here.
*/
class ReadOnlyStatementValidator extends TablesNamesFinder<Void> {

Expand All @@ -54,6 +63,8 @@ class ReadOnlyStatementValidator extends TablesNamesFinder<Void> {
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 SELECT_INTO_FILE_OR_VARIABLE_IS_NOT_PERMITTED =
"SELECT INTO OUTFILE/DUMPFILE/variable is not permitted.";
static final String ROW_LOCKING_IS_NOT_PERMITTED = "Row locking (FOR UPDATE/SHARE) is not permitted.";

private String violation;
Expand All @@ -75,9 +86,15 @@ private void record(String message) {
}
}

/** A FOR UPDATE / FOR SHARE clause can hang off any Select node. */
/**
* A FOR UPDATE / FOR SHARE clause can hang off any Select node. Every
* {@link ForMode} is rejected, including the harmless {@code FOR READ ONLY}: it is
* not part of the SQL this guard is meant to pass through, and JSqlParser's own
* classification treats any {@code ForMode} as a transaction side effect anyway.
*/
private void checkRowLocking(Select select) {
if (select.getForMode() != null || select.getForUpdateTable() != null) {
if (select.getForMode() != null || select.getForUpdate() != null
|| select.getForUpdateTables() != null && !select.getForUpdateTables().isEmpty()) {
record(ROW_LOCKING_IS_NOT_PERMITTED);
}
}
Expand All @@ -88,6 +105,11 @@ public <S> Void visit(PlainSelect plainSelect, S context) {
|| plainSelect.getIntoTempTable() != null) {
record(SELECT_INTO_IS_NOT_PERMITTED);
}
// MySQL INTO OUTFILE / INTO DUMPFILE write a file on the server, INTO @var
// assigns session variables; none of the three returns rows to the client.
if (plainSelect.getMySqlSelectIntoClause() != null) {
record(SELECT_INTO_FILE_OR_VARIABLE_IS_NOT_PERMITTED);
}
checkRowLocking(plainSelect);
return super.visit(plainSelect, context);
}
Expand Down
77 changes: 56 additions & 21 deletions ...nspiler/src/main/java/org/eclipse/daanse/sql/guard/jsqltranspiler/TranspilerSqlGuard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

import ai.starlake.transpiler.CatalogNotFoundException;
import ai.starlake.transpiler.ColumnNotFoundException;
import ai.starlake.transpiler.JSQLColumResolver;
import ai.starlake.transpiler.JSQLResolver;
import ai.starlake.transpiler.SchemaNotFoundException;
import ai.starlake.transpiler.TableNotDeclaredException;
Expand All @@ -47,8 +46,10 @@
import net.sf.jsqlparser.expression.Expression;
import net.sf.jsqlparser.parser.CCJSqlParserUtil;
import net.sf.jsqlparser.statement.Statement;
import net.sf.jsqlparser.statement.StatementFeatures;
import net.sf.jsqlparser.statement.Statements;
import net.sf.jsqlparser.statement.StmtFeature;
import net.sf.jsqlparser.statement.select.Select;
import net.sf.jsqlparser.util.deparser.StatementDeParser;

public class TranspilerSqlGuard implements SqlGuard {

Expand All @@ -58,6 +59,8 @@ public class TranspilerSqlGuard implements SqlGuard {
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 String ONLY_ONE_STATEMENT_IS_PERMITTED = "Only a single statement is permitted.";
private static final String STATEMENT_HAS_SIDE_EFFECTS = "Statement has side effects or does not return rows.";
private static final Logger LOGGER = LoggerFactory.getLogger(TranspilerSqlGuard.class);
private JdbcMetaData jdbcMetaDataToCopy;
private List<String> whitelistFunctionsPatterns = new ArrayList<String>();
Expand All @@ -75,20 +78,27 @@ public TranspilerSqlGuard(String currentCatalogName, String currentSchemaName, D
@Override
public String guard(String sqlStr) throws GuardException {

LOGGER.atInfo().log("guard incoming: %s", sqlStr);
StringBuilder builder = new StringBuilder();
StatementDeParser deParser = new StatementDeParser(builder);
LOGGER.atInfo().log("guard incoming: {}", sqlStr);

JSQLResolver resolver = new JSQLResolver(jdbcMetaDataToCopy);

// this does not really work, when there are comments
// @todo: apply a Regex for SQL Comments
if (sqlStr == null || sqlStr.trim().isEmpty()) {
throw new EmptyStatementGuardException();
}

try {
Statement st = CCJSqlParserUtil.parse(sqlStr);
// parse the whole input as a script: CCJSqlParserUtil.parse() stops after the
// first statement and would silently drop a trailing "; DROP TABLE ..." (or
// any other statement) from what is validated and rewritten.
Statements statements = CCJSqlParserUtil.parseStatements(sqlStr);
if (statements == null || statements.isEmpty()) {
// e.g. a comment-only input parses to an empty script
throw new EmptyStatementGuardException();
}
if (statements.size() > 1) {
throw new UnallowedStatementTypeGuardException(ONLY_ONE_STATEMENT_IS_PERMITTED);
}
Statement st = statements.get(0);

// we can test for SELECT, though in practise it won't protect us from harmful statements
if (st instanceof Select) {
Expand Down Expand Up @@ -137,14 +147,7 @@ public String guard(String sqlStr) throws GuardException {
List<String> disallowedFunctions = new ArrayList<>();

for (String function : functionNames) {
boolean isAllowed = false;
for (String pattern : whitelistFunctionsPatterns) {
if (Pattern.matches(pattern, function)) {
isAllowed = true;
break;
}
}
if (isAllowed) {
if (isWhitelistedFunction(function)) {
allowedFunctions.add(function);
} else {
disallowedFunctions.add(function);
Expand All @@ -155,16 +158,23 @@ public String guard(String sqlStr) throws GuardException {
throw new GuardException(QUERY_HAS_DISAllOWED_FUNCTIONS);
}

// Second net: JSqlParser's own statement classification (since 5.4). It
// covers side effects this guard does not model explicitly (session or
// transaction changes, opaque calls, statement types added upstream).
// Functions the resolver saw were already proven against the whitelist
// above, so this can only reject for reasons the checks before did not see.
validateFeatures(st, allowedFunctions);

// we can finally resolve for the actually returned columns
JSQLColumResolver columResolver = new DeparserColumResolver(jdbcMetaDataToCopy, dialect,dialectDeparser);
// on the very same AST that was validated above: no second parse
DeparserColumResolver columResolver = new DeparserColumResolver(jdbcMetaDataToCopy, dialect,
dialectDeparser);
columResolver.setCommentFlag(false);
columResolver.setErrorMode(JdbcMetaData.ErrorMode.STRICT);

String rewritten = columResolver.getResolvedStatementText(sqlStr);

deParser.visit((Select) st);// or rewritten
String rewritten = columResolver.getResolvedStatementText(st);

LOGGER.atInfo().log("guard outgoin: %s", rewritten);
LOGGER.atInfo().log("guard outgoing: {}", rewritten);

return rewritten;

Expand All @@ -186,6 +196,31 @@ public String guard(String sqlStr) throws GuardException {

}

private boolean isWhitelistedFunction(String function) {
for (String pattern : whitelistFunctionsPatterns) {
if (Pattern.matches(pattern, function)) {
return true;
}
}
return false;
}

private void validateFeatures(Statement st, List<String> allowedFunctions) throws GuardException {
// the feature visitor reports function names lower-cased; the whitelist patterns
// are case-sensitive, so accept every name the resolver-based check already passed
StatementFeatures features = st.getFeatures(name -> isWhitelistedFunction(name)
|| allowedFunctions.stream().anyMatch(allowed -> allowed.equalsIgnoreCase(name)));
boolean sideEffect = features.may(StmtFeature.MODIFIES_DATA)
|| features.may(StmtFeature.MODIFIES_SCHEMA)
|| features.may(StmtFeature.MODIFIES_SESSION)
|| features.may(StmtFeature.MODIFIES_TRANSACTION)
|| features.may(StmtFeature.OPAQUE);
if (sideEffect || !features.returnsResultSet()) {
LOGGER.atInfo().log("{} features: {}", STATEMENT_HAS_SIDE_EFFECTS, features);
throw new GuardException(STATEMENT_HAS_SIDE_EFFECTS);
}
}

private static void validateReadOnly(Select select) throws GuardException {
Optional<String> violation = new ReadOnlyStatementValidator().firstViolation(select);
if (violation.isPresent()) {
Expand Down
Loading
Loading