Skip to content

fix(mysql): escape SQL identifiers and literals in metadata/DDL paths (#1914) - #2172

Merged
openai0229 merged 6 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-mysql
Jul 29, 2026
Merged

fix(mysql): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2172
openai0229 merged 6 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-mysql

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Part of the SQL-injection hardening tracked in #1914. This PR covers the MySQL plugin and now builds on the shared forced-quote SPI merged in #2234.

Summary

  • Uses ISQLIdentifierProcessor.quoteIdentifierAlways for MySQL DDL and metadata identifier positions, including embedded-backtick escaping and raw-value round trips.
  • Escapes MySQL string literals and validates structural tokens such as engine, charset, collation, numeric defaults, bit/hex literals, view options, index direction, and fallback column types.
  • Parses ENUM/SET lists before re-escaping so metadata values such as 'can''t' are not escaped twice; malformed lists are rejected.
  • Keeps DEFINER validation independent of backslash escape behavior, including NO_BACKSLASH_ESCAPES sessions.
  • Hardens table copy/truncate/drop, metadata lookups, routine names, export statements, and export comment headers.
  • Fixes qualified DROP VIEW generation while preserving calls that omit the database name.

Table.partition and ModifyView.viewBody remain intentional raw SQL surfaces and are not reinterpreted by this identifier/literal hardening PR.

Affected surfaces

  • Frontend / Web
  • Backend / API / Storage
  • Database plugin / Driver
  • JCEF / Desktop packaging
  • CI / Build / Release
  • Documentation only

Verification

  • Command: mvn -B -f chat2db-community-server/pom.xml -pl chat2db-community-plugins/chat2db-community-mysql -am -Dmaven.test.skip=false -DskipTests=false clean test
  • Result: BUILD SUCCESS; tests run: 653, failures: 0, errors: 0, skipped: 0.
  • Focused escaping tests: 35 passed across the MySQL identifier processor test classes.

Risk and compatibility

  • Public API or stored data: N/A.
  • Database or driver compatibility: ordinary identifiers retain existing output; embedded delimiters, quoted raw identifiers, malformed ENUM/SET lists, ambiguous DEFINER backslash escapes, and invalid structural tokens now follow explicit safe behavior.
  • Network, privacy, or security: closes second-order injection paths in generated MySQL metadata, DDL, DML, and export SQL.
  • Community / Local / Pro boundary: Community MySQL plugin only.
  • Backward compatibility: shared SPI source compatibility is provided by feat(spi): add quoteIdentifierAlways to ISQLIdentifierProcessor #2234; this PR uses the MySQL override for forced backtick quoting.

Reviewer map

  • Start with MysqlIdentifierProcessor, MysqlSqlGuards, and MysqlIdentifierProcessorTest.
  • Then review the builder/metadata/manager call sites and binary/varbinary value processors.
  • Failure condition: an identifier or literal sink covered by this PR still receives an unescaped value, or a legitimate raw identifier fails the forced-quote round trip.
  • Rollback path: revert this PR.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: The fix, review, verification, and PR description were produced with AI coding assistance.

@HandSonic
HandSonic requested a review from openai0229 as a code owner July 26, 2026 09:29
@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as draft July 26, 2026 09:47
@HandSonic

Copy link
Copy Markdown
Contributor Author

Converting to draft: final adversarial review pass is still in progress for this branch. Will mark ready for review once it completes. (process note: PR was opened prematurely by automation)

@openai0229 openai0229 moved this from In Review to In Progress in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as ready for review July 26, 2026 10:28
@openai0229 openai0229 moved this from In Progress to In Review in Chat2DB Community Jul 26, 2026
HandSonic and others added 6 commits July 29, 2026 11:21
…processor (OtterMind#1914)

- MysqlDmlValueTemplate.wrapHex: require hex digits for the 0x template,
  matching wrapBit's requireBitLiteral
- MysqlBinaryProcessor/MysqlVarBinaryProcessor: only pass values through
  raw when they are well-formed 0x hex literals (isHexLiteral), closing the
  0x-prefixed injection passthrough in generated DML
- MysqlIdentifierProcessor.quoteIdentifier: double embedded backticks via
  MysqlSqlEscapes.quoteIdentifierRaw instead of wrapping verbatim
- add regression tests for all three paths
…tainer review (OtterMind#1914)

- MysqlIdentifierProcessor (SPI ISQLIdentifierProcessor): INSTANCE, always-quote
  backtick quoteIdentifier with one-pair strip + doubling, escapeString with
  backslash-first-then-quote doubling, static escapeIdentifier for quoted templates
- MysqlMetaData call sites use getSQLIdentifierProcessor(); builders/managers/enums/
  value processors use MysqlIdentifierProcessor.INSTANCE
- non-escapable validation moved to MysqlSqlGuards (engine/charset/collation names,
  numeric defaults, bit/hex literals, definers, asc/desc, enum constants, enum/set lists)
- MysqlSqlEscapes removed; tests migrated to MysqlIdentifierProcessorTest (630 green)

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto #2234 and verified the final MySQL implementation. Identifier round trips, DEFINER handling, ENUM/SET literals, fallback type rendering, export titles, DROP VIEW, and generated DDL behavior are covered. Local clean reactor result: 653 tests, 0 failures or errors.

@openai0229
openai0229 merged commit 9367029 into OtterMind:main Jul 29, 2026
19 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants