Skip to content

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

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

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

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1914

Summary

  • Harden Oscar metadata, DDL, and DML generation by routing identifiers and SQL literal content through dialect-specific processors and structural guards.
  • Keep ordinary quoteIdentifier conditional while making quoteIdentifierAlways preserve raw text exactly across embedded and boundary double quotes.
  • Preserve structurally valid already-quoted tokens in conditional contexts, validate unknown column-type syntax before it reaches create/alter DDL, and reject indexes whose column list contains no usable column.
  • Quote schema, table, column, and index names across qualified DDL plus update SET and WHERE paths.
  • Add focused identifier, metadata, DDL, DML, unknown-type, boundary-quote, and empty-index regression tests.

Affected surfaces

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

Verification

  • Commands and results:
    • mvn -f chat2db-community-server/pom.xml -pl chat2db-community-plugins/chat2db-community-oscar -am clean test -DskipTests=false -Dmaven.test.skip=false
    • Result: 8-module reactor BUILD SUCCESS; tools 26/26, SPI 90/90, Oscar 26/26; zero failures, errors, or skipped tests in these modules.
    • git diff --check origin/main...HEAD
    • Result: passed with no whitespace errors.
  • Manual verification: N/A; generated SQL contracts are covered by executable unit tests and no Oscar instance is available in this environment.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: No endpoint, public DTO, or stored-data migration changes.
  • Database or driver compatibility: Existing known Oscar types keep their generated syntax. Unknown types are accepted only when their qualified/type-parameter structure is balanced and contain no statement breakout.
  • Network, privacy, or security: Reduces command construction risk from metadata-controlled identifiers, defaults, sort directions, and unknown types. No network or privacy behavior changes.
  • Community / Local / Pro boundary: Community Oscar plugin only; other products inherit it only when built with this Community plugin.
  • Backward compatibility: Valid identifiers, defaults, index definitions, and custom types remain supported. Malformed raw SQL fragments and empty index definitions now fail fast.

Reviewer map

  • Start here: chat2db-community-server/chat2db-community-plugins/chat2db-community-oscar/src/main/java/ai/chat2db/plugin/oscar/identifier/OscarIdentifierProcessor.java, then OscarSqlGuards.java, enums/type/OscarColumnTypeEnum.java, and builder/OscarSqlBuilder.java.
  • Failure condition: Any generated identifier can break its quoted token, always-quote fails exact round-trip, an unknown type can append a statement, or a blank index column list emits malformed DDL.
  • Rollback or disable path: Revert this PR. There is no feature flag or data migration.

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: Maintainer used AI-assisted code review and test development; all resulting changes were source-reviewed and verified with the clean Maven reactor reported above.

HandSonic and others added 6 commits July 29, 2026 20:35
…tainer review (OtterMind#1914)

- strengthen OscarIdentifierProcessor: shared INSTANCE, escapeString with
  single-quote doubling, static escapeIdentifier for quoted-template content
- OscarMetaData call sites use getSQLIdentifierProcessor().escapeString(...)
- non-escapable validation moved to OscarSqlGuards (default expressions,
  length units, sort orders)
- OscarSqlEscapes removed; tests migrated (15 green)
…e for DDL paths (OtterMind#1914)

- quoteIdentifier(String) is conditional again: null/blank pass through,
  valid non-keyword identifiers return unquoted, everything else wraps
  with embedded-quote doubling; versioned overload delegates to it
- new quoteIdentifierAlways(String) carries the old always-quote
  SqlEscapes semantics (null -> null, else wrap with doubling)
- quoteIdentifierIgnoreCase keeps its SPI always-quote, case-preserving
  meaning by delegating to quoteIdentifierAlways, so DDL-generation
  sites (OscarUtils/builers/enums) keep emitting quoted identifiers
- tests cover conditional and always behaviors incl. null passthrough
  (15 green)
…gex alert (OtterMind#1914)

'([^']|'')*' -> '[^']*(?:''[^']*)*' (same language, no backtracking)
…eDoS) (OtterMind#1914)

Regexes for quoted literals and function-call defaults are replaced by
O(n) character scanners; numeric/keyword patterns (simple classes) stay.

@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 on the latest main and verified Oscar identifier round trips, qualified DDL/DML quoting, structural type guards, and empty-index rejection. The clean 8-module reactor passes with Oscar 26/26, SPI 90/90, and tools 26/26.

@openai0229
openai0229 merged commit a6b07c2 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.

Security: 34 SQL Injection Vulnerabilities in Database Plugins

3 participants