Skip to content

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

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

openai0229 merged 8 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-xugudb

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1914

Summary

Completes the XuguDB database-plugin portion of the identifier and literal escaping work.

  • Adds XugudbIdentifierProcessor with conditional SPI quoting, DDL-only always quoting, locale-independent reserved-word matching, null preservation, and embedded/boundary double-quote round trips.
  • Replaces regex-based default handling with quote-aware balanced scanners that preserve nested functions, casts, sequence expressions, quoted literals, and parenthesized expressions while rejecting comments, statement boundaries, unbalanced input, top-level commas, and column-clause injection.
  • Quotes qualified table paths and update/primary-key columns across inherited builder, manager, metadata, create/alter/index, drop, truncate, copy, and template paths.
  • Preserves case-only table/column renames, validates unknown type expressions in real builder paths, normalizes index sort direction, and keeps primary-key drop behavior case independent.
  • Consolidates XuguDB string-literal serialization in one processor.

Affected surfaces

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

Verification

  • Commands and results:
    • mvn -B -f chat2db-community-server/pom.xml -pl chat2db-community-plugins/chat2db-community-xugudb -am clean test -DskipTests=false -Dmaven.test.skip=false -Dsurefire.failIfNoSpecifiedTests=false
    • Result: BUILD SUCCESS across 9 reactor modules; 195 tests passed in tested modules, including XuguDB 35/35, Oracle 44/44, SPI 90/90, and tools 26/26.
    • git diff --check origin/main...HEAD: passed.
  • Manual verification: Source-level generated-SQL review completed. No live XuguDB instance was used.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: No public API or stored-data schema change. Uses the existing shared ISQLIdentifierProcessor contract.
  • Database or driver compatibility: Conditional lookup/completion quoting is preserved; generated DDL always quotes raw metadata names. Nested legal defaults and user-defined/parameterized types are covered by regression tests.
  • Network, privacy, or security: No network or privacy change. Reduces malformed SQL and identifier/literal injection risk in metadata-driven DDL, export, and DML paths.
  • Community / Local / Pro boundary: Community XuguDB plugin only; Local and Pro consume the Community plugin behavior without product-specific code changes.
  • Backward compatibility: Ordinary identifiers and values retain their emitted form; mixed-case names, embedded delimiters, reserved words, and case-only renames now preserve object identity.

Reviewer map

  • Start here: XugudbIdentifierProcessor, XugudbSqlGuards, then XUGUDBSqlBuilder, XUGUDBMetaData, and the type/index enums.
  • Failure condition: Quote/remove loses raw text, legal nested defaults/types are rewritten, generated SQL contains unquoted metadata identifiers, or update/primary-key column names bypass quoting.
  • Rollback or disable path: Revert this PR; no migration or persisted-data rollback is required.

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: Substantial AI-assisted maintainer revision, multi-agent review, and test expansion; all final changes were source-reviewed and verified with the commands above.

@HandSonic
HandSonic requested a review from openai0229 as a code owner July 26, 2026 09:57
@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as draft July 26, 2026 10:30
@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 11:05
@openai0229 openai0229 moved this from In Progress to In Review in Chat2DB Community Jul 26, 2026
HandSonic and others added 8 commits July 29, 2026 17:08
…olumn fallback (OtterMind#1914)

- Override quoteIdentifier/quoteQualifiedIdentifier/buildSelectTable/buildTableName/
  buildColumns in XUGUDBSqlBuilder so inherited DefaultSqlBuilder DQL/DML paths
  (exportTableData, insert, update) no longer concatenate raw identifiers
- Replace raw IColumnBuilder.buildDefaultColumn fallback with escaped name +
  anchored type-shape validation (reject otherwise)
- Return trimmed values from default-value/unit validators
- Document raw-identifier contract on XugudbSqlEscapes.escapeIdentifier
- Add attack-string tests: malicious schema/table/column via buildSelectTable and
  buildInsert, comment literal end-to-end, fallback name/type, validator trimming
… tests (OtterMind#1914)

- DEFAULT_VALUE_PATTERN function-call branch now requires balanced
  single-quoted literals as arguments ('([^']|'')*' allowed, bare quotes
  rejected), closing the unbalanced-quote gap in default-value validation
- Add tests: unbalancedQuoteInFunctionDefaultIsRejected,
  balancedQuotedArgsInFunctionDefaultAreAccepted
- Fix extra closing paren typo in the c3 case
…intainer review (OtterMind#1914)

- new XugudbIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier
  with double-quote doubling, escapeString with single-quote doubling
- XUGUDBMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it
- builder/manager/enums use XugudbIdentifierProcessor.INSTANCE
- non-escapable validation moved to XugudbSqlGuards (column defaults, length units)
- XugudbSqlEscapes removed; tests migrated (21 green)
…rough, conditional for SPI, always for DDL) (OtterMind#1914)

Found in re-review: processor was always-quote with null->"" garbage,
breaking SPI completion paths; no quoteIdentifierAlways existed.

@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.

Maintainer review complete. Rebased onto current main, fixed XuguDB identifier/default/type/builder semantics, and verified with the nine-module clean reactor suite (195/195 tests passed).

@openai0229
openai0229 merged commit 81f78f9 into OtterMind:main Jul 29, 2026
19 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 29, 2026
@HandSonic
HandSonic deleted the fix/sqli-xugudb branch August 23, 2026 08:13
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.

Security: 34 SQL Injection Vulnerabilities in Database Plugins

3 participants