[vishwasvaidya-cloudsufi]

What's this about

A VRP report found a SQL injection in the Cloud SQL PostgreSQL connector's sample path. A table path containing a " could break out of the quoted name and run extra statements, like an UPDATE, using the connection's saved credential. The same issue existed in every connector.

What changed

• Connectors: table names and the strata column are now properly escaped, so a quote inside a name can't break out anymore.
• Source plugins: Import Query and Bounding Query are now parsed (JSqlParser 4.9) and must be a single SELECT. Anything else is rejected before it ever reaches the database. Use the Database action plugins for SET, EXEC, UPDATE and similar.
• SQL Server [bracket] names keep working.

Heads up

Existing pipelines with a non-SELECT Import Query can fail after this and need to move that logic to an action.

Testing

Added unit tests for escaping in every connector, including the payload from the report, plus validator tests and end-to-end pipeline tests confirming bad queries are blocked and no data is changed. Not yet tested against real database instances.

[gemini-code-assist]

Code Review

This pull request introduces SQL identifier quoting to prevent SQL injection vulnerabilities across various database connectors, and adds query validation using jsqlparser to ensure that import and bounding queries are read-only SELECT statements. Feedback on these changes highlights several potential NullPointerException risks when database or schema parameters are null during identifier quoting, as well as a recommendation to use Locale.ROOT with toLowerCase() to avoid locale-sensitive string comparison issues.

[gemini-code-assist]

If schema is null, calling quoteIdentifier(schema, '"') will throw a NullPointerException because quoteIdentifier calls replace on the passed string. Since schema can be null when not specified, we should handle the null case gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("\"%s\".\"%s\"", schema, table);
	return quoteIdentifier(schema, '"') + "." + quoteIdentifier(table, '"');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	return (schema == null ? "" : quoteIdentifier(schema, '"') + ".") + quoteIdentifier(table, '"');
	}

[gemini-code-assist]

If database is null, calling quoteIdentifier(database, '')will throw aNullPointerException. Since database` can be null, we should handle the null case gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("`%s`.`%s`", database, table);
	return quoteIdentifier(database, '`') + "." + quoteIdentifier(table, '`');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	return (database == null ? "" : quoteIdentifier(database, '`') + ".") + quoteIdentifier(table, '`');
	}

[gemini-code-assist]

If schema is null, calling quoteIdentifier(schema, '"') will throw a NullPointerException. Since schema can be null, we should handle the null case gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("\"%s\".\"%s\"", schema, table);
	return quoteIdentifier(schema, '"') + "." + quoteIdentifier(table, '"');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	return (schema == null ? "" : quoteIdentifier(schema, '"') + ".") + quoteIdentifier(table, '"');
	}

[gemini-code-assist]

If database is null, calling quoteIdentifier(database, '"') will throw a NullPointerException. We should make this method null-safe by checking if database or schema are null before quoting and concatenating them.

  protected String getTableName(String database, String schema, String table) {
    StringBuilder sb = new StringBuilder();
    if (database != null) {
      sb.append(quoteIdentifier(database, '"')).append(".");
    }
    if (schema != null) {
      sb.append(quoteIdentifier(schema, '"')).append(".");
    }
    sb.append(quoteIdentifier(table, '"'));
    return sb.toString();
  }

[gemini-code-assist]

If database or schema is null, calling quoteIdentifier on them will throw a NullPointerException. We should handle null values gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("\"%s\".\"%s\".\"%s\"", database, schema, table);
	return quoteIdentifier(database, '"') + "." + quoteIdentifier(schema, '"') + "."
	+ quoteIdentifier(table, '"');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	StringBuilder sb = new StringBuilder();
	if (database != null) {
	sb.append(quoteIdentifier(database, '"')).append(".");
	}
	if (schema != null) {
	sb.append(quoteIdentifier(schema, '"')).append(".");
	}
	sb.append(quoteIdentifier(table, '"'));
	return sb.toString();
	}

[gemini-code-assist]

If database is null, calling quoteIdentifier(database, '')will throw aNullPointerException`. We should handle the null case gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("`%s`.`%s`", database, table);
	return quoteIdentifier(database, '`') + "." + quoteIdentifier(table, '`');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	return (database == null ? "" : quoteIdentifier(database, '`') + ".") + quoteIdentifier(table, '`');
	}

[gemini-code-assist]

If schema is null, calling quoteIdentifier(schema, '"') will throw a NullPointerException. We should handle the null case gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("\"%s\".\"%s\"", schema, table);
	return quoteIdentifier(schema, '"') + "." + quoteIdentifier(table, '"');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	return (schema == null ? "" : quoteIdentifier(schema, '"') + ".") + quoteIdentifier(table, '"');
	}

[gemini-code-assist]

If schema is null, calling quoteIdentifier(schema, '"') will throw a NullPointerException. We should handle the null case gracefully to avoid runtime exceptions.

Suggested change

	protected String getTableName(String database, String schema, String table) {
	return String.format("\"%s\".\"%s\"", schema, table);
	return quoteIdentifier(schema, '"') + "." + quoteIdentifier(table, '"');
	}
	@Override
	protected String getTableName(String database, String schema, String table) {
	return (schema == null ? "" : quoteIdentifier(schema, '"') + ".") + quoteIdentifier(table, '"');
	}

[gemini-code-assist]

If identifier is null, calling identifier.replace will throw a NullPointerException. Adding a null check makes this utility method robust against null inputs.

  public static String quoteIdentifier(String identifier, char quote) {
    if (identifier == null) {
      return null;
    }
    String quoteString = String.valueOf(quote);
    return quoteString + identifier.replace(quoteString, quoteString + quoteString) + quoteString;
  }

[gemini-code-assist]

Using toLowerCase() without specifying a locale can cause unexpected behavior in environments with certain default locales (such as Turkish). It is safer to use toLowerCase(Locale.ROOT).

Suggested change

	return connectionString != null && (connectionString.toLowerCase().startsWith("jdbc:sqlserver:")
	|| connectionString.toLowerCase().startsWith("jdbc:jtds:sqlserver:"));
	return connectionString != null && (connectionString.toLowerCase(java.util.Locale.ROOT).startsWith("jdbc:sqlserver:")
	|| connectionString.toLowerCase(java.util.Locale.ROOT).startsWith("jdbc:jtds:sqlserver:"));