Skip to content

Feature request: Public API for MySQL executable comment (/*!...*/) recognition #397

Description

@fujiabao89

Background

WebDB P0-04 SQL safety policy requires dialect-AST-based classification for all SQL before execution (see ADR-007). MySQL executable comments (/*!NNNNN ... */) are a documented MySQL extension that can hide DDL/DML statements inside what appears to be a comment. String prefix or regex scanning is not an acceptable security boundary.

Current limitation (verified against v0.0.0-20260728103305-d2f82de1b468)

  • No exported ECM token type constant — all token type constants in mysql/parser are unexported (tok*/kw*)
  • spliceGaps field in lexer is unexported — ECM boundary information is inaccessible to API consumers
  • Lexer.NextToken() / Tokenize() produce normal keyword/literal tokens for ECM content — no way to distinguish ECM-originated tokens from regular SQL tokens
  • 0/12 true ECM positive cases recognized via public API in our Spike harness

Required API capability

A stable, public, documented mechanism to answer: "Does this SQL contain executable comments?"

The API must distinguish:

  1. Genuine MySQL executable comments (/*!NNNNN ... */)
  2. Ordinary block comments (/* ... */)
  3. /*!...*/ inside string literals (SELECT '/*!50000 harmless*/' AS str)
  4. Invalid or unknown version formats

Suggested implementations (non-exhaustive):

  • Export ECM token type constant and ensure Lexer.NextToken() emits it
  • Export spliceGaps as a public API
  • Add parser AST node/flag marking ECM-originated nodes

Regex, string prefix, or raw text scanning are NOT acceptable as SQL security boundaries.

Minimal reproduction

Using mysql/parser from v0.0.0-20260728103305-d2f82de1b468:

package main

import (
    "fmt"
    myparser "github.com/bytebase/omni/mysql/parser"
)

func main() {
    sqls := []string{
        "/*!50000 DROP TABLE t*/ SELECT 1",   // ECM with DDL
        "/*!99999 SELECT 1*/",                 // ECM with SELECT
        "SELECT /*!80100 42*/ FROM dual",      // ECM with expression
        "SELECT '/*!50000 harmless*/' AS str", // string literal
    }
    for _, sql := range sqls {
        lex := myparser.NewLexer(sql)
        fmt.Printf("SQL: %s\n", sql)
        for {
            tok := lex.NextToken()
            if tok.Type == 0 { break }
            // Is this token from an executable comment?
            // Currently no public API to determine this.
            fmt.Printf("  Token{Type=%d, Str=%q, Loc=%d, End=%d}\n",
                tok.Type, tok.Str, tok.Loc, tok.End)
        }
        fmt.Println()
    }
}

All ECM content tokens appear identical to normal SQL tokens.

Request

  • Confirm whether the maintainers consider this in scope
  • If accepted: commit to API stability and version compatibility guarantees for the exported ECM signal
  • We are prepared to submit a minimal PR if the approach is approved
  • The Spike evidence (0/12 ECM recognition via public API) is fully reproducible with the fixed version above

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions