fix: fix working with tables with quotes in the names for postgres and cockroachdb

[iskalyakin]

…(#10991)

Fix working with tables with quotes in the names for PostgreSQL

Closes: #10991

Description of change

There were a few changes in PostgresQueryRunner with changing quoting table_schema and table_name from '"' || "table_schema" || '"' to quote_ident("table_schema"), which allows to deal with table names containing quotes.
It's important because tables with these unusual names can exist, while sometimes it's impossible just rename those tables as this is an arbitrary database, the structure of which needs to be obtained.

Case: PostgreSQL DB containg table with the name This is the "Table Name".
Current behaviour: connection.createQueryRunner().getTables() throws an error.
Next behaviour: connection.createQueryRunner().getTables() works

Fixes #10991

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run format to apply prettier formatting
• npm run test passes with this change
• This pull request links relevant issues as Fixes #0000
• There are new or updated unit tests validating the change
• [-] Documentation has been updated to reflect this change N/A
• The new commits follow conventions explained in CONTRIBUTING.md

[iskalyakin]

It looks like the failed tests (oracle and sqlite better-sqlite3 sqljs) are not related to the PR. These failures also appeared in other recent PRs.

[gioboa]

We aren't creating tests for specific GitHub issues any more. Instead, tests should be added to the suites for the related functionality.

Can you please move these tests please?

Thanks 🙏

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Latest suggestions up to 55130c5

Category	Suggestion	Impact
Security	Prevent SQL injection vulnerability Fix a SQL injection vulnerability in the clearDatabase method by using parameterized queries for schema names instead of string concatenation. src/driver/postgres/PostgresQueryRunner.ts [3200-3202] const selectViewDropsQuery = `SELECT 'DROP VIEW IF EXISTS ' || quote_ident(schemaname) || '.' || quote_ident(viewname) || ' CASCADE;' as "query" ` + - `FROM "pg_views" WHERE "schemaname" IN (${schemaNamesString}) AND "viewname" NOT IN ('geography_columns', 'geometry_columns', 'raster_columns', 'raster_overviews')` + `FROM "pg_views" WHERE "schemaname" = ANY($1) AND "viewname" NOT IN ('geography_columns', 'geometry_columns', 'raster_columns', 'raster_overviews')` +// Pass schemas as parameter: await this.query(selectViewDropsQuery, [schemas]) Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a SQL injection vulnerability due to improper escaping of schema names in the clearDatabase method. The proposed solution to use parameterized queries is a robust fix for this security issue.	High
	Prevent SQL injection in table loading Fix a SQL injection vulnerability in the loadTables method by using parameterized queries for table and schema names instead of building the WHERE clause through string concatenation. src/driver/postgres/PostgresQueryRunner.ts [3404-3406] -const tablesSql = - `SELECT "table_schema", "table_name", obj_description((quote_ident("table_schema") || '.' || quote_ident("table_name"))::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables" WHERE ` + - tablesCondition +// Use parameterized queries instead of string concatenation +const params = tableNames.flatMap((tableName) => { + const { schema, tableName: name } = this.driver.parseTableName(tableName) + return [schema || currentSchema, name] +}) +const placeholders = tableNames.map((_, i) => `("table_schema" = $${i*2+1} AND "table_name" = $${i*2+2})`).join(" OR ") +const tablesSql = `SELECT ... WHERE ${placeholders}` Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a SQL injection vulnerability in the loadTables method where tablesCondition is built via unsafe string concatenation. The proposed solution to use parameterized queries is the correct approach to mitigate this risk.	High
More

---

Previous suggestions

Suggestions up to commit d9a8e8b

Category	Suggestion	Impact
Possible issue	Use format function for regclass casting In PostgresQueryRunner.ts, use format('%I.%I', "table_schema", "table_name") instead of concatenating quote_ident results to construct the table identifier for obj_description. This is a safer and more idiomatic way to build qualified identifiers for regclass casting. src/driver/postgres/PostgresQueryRunner.ts [3392] -const tablesSql = `SELECT "table_schema", "table_name", obj_description((quote_ident("table_schema") || '.' || quote_ident("table_name"))::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables"` +const tablesSql = `SELECT "table_schema", "table_name", obj_description(format('%I.%I', "table_schema", "table_name")::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables"` Suggestion importance[1-10]: 5 __ Why: The suggestion to use format('%I.%I', ...) is a valid improvement for constructing qualified SQL identifiers, which is generally considered a best practice over string concatenation. While the existing code using quote_ident is correct and fixes the bug, format is more idiomatic and arguably safer.	Low
	Use format for regclass casting In PostgresQueryRunner.ts, use format('%I.%I.%I', ...) instead of concatenating quote_ident results to construct the three-part table identifier for col_description. This is a safer and more idiomatic way to build qualified identifiers for regclass casting in PostgreSQL. src/driver/postgres/PostgresQueryRunner.ts [3426] -`SELECT columns.*, pg_catalog.col_description((quote_ident(table_catalog) || '.' || quote_ident(table_schema) || '.' || quote_ident(table_name))::regclass::oid, ordinal_position) AS description, ...` +`SELECT columns.*, pg_catalog.col_description(format('%I.%I.%I', table_catalog, table_schema, table_name)::regclass::oid, ordinal_position) AS description, ...` Suggestion importance[1-10]: 5 __ Why: The suggestion to use format('%I.%I.%I', ...) is a valid improvement for constructing qualified SQL identifiers, which is generally considered a best practice over string concatenation. While the existing code using quote_ident is correct, format is more idiomatic and safer for building identifiers.	Low

Suggestions up to commit d612732

Category	Suggestion	Impact
General	Add CASCADE to sequence drops Add IF EXISTS and CASCADE to the DROP SEQUENCE statement in clearDatabase to prevent errors from dependent objects. src/driver/cockroachdb/CockroachQueryRunner.ts [2868] -const selectSequenceDropsQuery = `SELECT 'DROP SEQUENCE ' || quote_ident(sequence_schema) || '.' || quote_ident(sequence_name) || ';' as "query" FROM "information_schema"."sequences" WHERE "sequence_schema" IN (${schemaNamesString})` +const selectSequenceDropsQuery = `SELECT 'DROP SEQUENCE IF EXISTS ' || quote_ident(sequence_schema) || '.' || quote_ident(sequence_name) || ' CASCADE;' as "query" FROM "information_schema"."sequences" WHERE "sequence_schema" IN (${schemaNamesString})` Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that dropping sequences might fail due to dependencies and proposes adding CASCADE, which makes the clearDatabase method more robust.	Low
	Use format() for regclass casting Use format('%I.%I.%I', ...) to construct the qualified name for the regclass cast, as it is the recommended approach. src/driver/postgres/PostgresQueryRunner.ts [3425-3427] const columnsSql = - `SELECT columns.*, pg_catalog.col_description((quote_ident(table_catalog) || '.' || quote_ident(table_schema) || '.' || quote_ident(table_name))::regclass::oid, ordinal_position) AS description, ` + + `SELECT columns.*, pg_catalog.col_description(format('%I.%I.%I', table_catalog, table_schema, table_name)::regclass::oid, ordinal_position) AS description, ` + ... Suggestion importance[1-10]: 4 __ Why: The suggestion proposes using format() which is a valid and often preferred alternative for building identifiers, but the existing code using quote_ident() is also correct and achieves the same goal, making this a minor style/preference improvement.	Low

Suggestions up to commit 747696a

Category	Suggestion	Impact
Possible issue	Use format function for regclass casting Replace quote_ident() with string concatenation for constructing table identifiers with format('%I.%I', "table_schema", "table_name") for improved readability and safety when casting to regclass. src/driver/postgres/PostgresQueryRunner.ts [3392] -const tablesSql = `SELECT "table_schema", "table_name", obj_description((quote_ident("table_schema") || '.' || quote_ident("table_name"))::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables"` +const tablesSql = `SELECT "table_schema", "table_name", obj_description(format('%I.%I', "table_schema", "table_name")::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables"` Suggestion importance[1-10]: 5 __ Why: The suggestion offers a valid and arguably cleaner way to construct quoted identifiers in PostgreSQL using format('%I.%I', ...) instead of concatenating results from quote_ident(). While the PR's change is correct and fixes the bug, this suggestion provides a more idiomatic and readable alternative.	Low

Suggestions up to commit be73001

Category	Suggestion	Impact
Possible issue	Use format function for identifier construction Replace string concatenation of quote_ident() results with the format() function for constructing qualified table names, improving robustness and adhering to best practices. src/driver/postgres/PostgresQueryRunner.ts [3392] -const tablesSql = `SELECT "table_schema", "table_name", obj_description((quote_ident("table_schema") || '.' || quote_ident("table_name"))::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables"` +const tablesSql = `SELECT "table_schema", "table_name", obj_description((format('%I.%I', "table_schema", "table_name"))::regclass, 'pg_class') AS table_comment FROM "information_schema"."tables"` Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies that using format('%I.%I', ...) is a more robust and idiomatic way to construct qualified SQL identifiers in PostgreSQL than concatenating quote_ident() results.	Low

Suggestions up to commit d443ee3

Category	Suggestion	Impact
Security	Use quote_ident for UDT names Replace manual double-quote escaping for udt_schema and udt_name with quote_ident() for consistency and to prevent potential SQL injection. src/driver/postgres/PostgresQueryRunner.ts [3359-3361] const columnsSql = `SELECT columns.*, pg_catalog.col_description((quote_ident(table_catalog) || '.' || quote_ident(table_schema) || '.' || quote_ident(table_name))::regclass::oid, ordinal_position) AS description, ` + - `('"' || "udt_schema" || '"."' || "udt_name" || '"')::"regtype"::text AS "regtype", pg_catalog.format_type("col_attr"."atttypid", "col_attr"."atttypmod") AS "format_type" ` + + `(quote_ident("udt_schema") || '.' || quote_ident("udt_name"))::"regtype"::text AS "regtype", pg_catalog.format_type("col_attr"."atttypid", "col_attr"."atttypmod") AS "format_type" ` + ... Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a missed instance of manual string quoting that should have been updated as part of this PR's goal, which is a potential security and correctness issue.	Medium

[gioboa]

Let's move on with this PR even though tests are in the old folders.

[pkg-pr-new]

• typeorm-sql-js-example

  npm i https://pkg.pr.new/typeorm/typeorm@10993
  

commit: 55130c5

[alumni]

Would be nice if the same fix would be done for CockroachQueryRunner as well :)

[gioboa]

Would be nice if the same fix would be done for CockroachQueryRunner as well :)

Done

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

No code suggestions found for the PR.

[linbrandon4]

Good fix. Using quote_ident() instead of manually doing '"' || table_name || '"' is the right move — it avoids breaking when table/schema names already contain quotes, and it’s the safer way to build identifiers in Postgres. The example case (“This is the "Table Name"”) makes sense, and the change should stop getTables() from blowing up on weird legacy DBs.

Also glad this got extended to CockroachQueryRunner too, since it has the same quoting problem in practice.

A couple things I’d double-check:

• Make sure this is only being used for identifiers (schema/table names) and not values — quote_ident is correct for identifiers, but it shouldn’t accidentally replace value escaping.
• Since the bot raised “possible SQL injection” flags, it’s worth confirming all identifier inputs come from the DB metadata / controlled sources, and not raw user input.
• Tests: I know the repo is moving away from issue-specific test folders, but as long as the tests cover quoted identifiers for both Postgres + Cockroach (schema + table name cases), that’s the important part.