fix(dbconn): stop interpreting user DDL as an escape format string

[morgo]

Problem

Several call sites build the sqlescape format string by concatenating raw user SQL onto a trusted prefix, e.g. "ALTER TABLE %n ALGORITHM=INSTANT, " + c.stmt.Alter. sqlescape.EscapeSQL interprets %n / %? / %% anywhere in the text — including inside string literals of the user's DDL:

• An --alter containing %n or %? in a literal (e.g. COMMENT '100%new', CHECK (name LIKE 'a%?')) fails escaping with missing arguments. On the default force-kill path this goes through sqlescape.MustEscapeSQL, which panics the process — with the kill timer already armed.
• %% in a literal (e.g. DEFAULT '50%% off') is silently collapsed to a single %, so spirit executes different DDL than the user wrote ('50% off' instead of '50%% off').

Non-ALTER statements (CREATE TABLE / DROP TABLE / RENAME TABLE) were also executed by passing the raw user statement as the format string with zero args.

Fix

User SQL is now always data, never a format string. sqlescape gains a %r verb that splices a string argument into the SQL verbatim — no quoting, no format interpretation. The spliced text is never re-scanned, so %n / %? / %% sequences inside it are inert. (sqlescape is maintained as a hard fork and no longer synced from TiDB, so extending the verb set is safe; its README now documents this.)

• Call sites keep a compile-time-constant format string and pass the user clause as an argument, e.g. dbconn.ForceExec(..., "ALTER TABLE %n ALGORITHM=INSTANT, %r", tableName, alter). Non-ALTER user statements execute via dbconn.Exec(ctx, db, "%r", stmt).
• ForceExec now escapes with the error-returning EscapeSQL before the kill timer is armed — a bad format string returns an error instead of panicking while a timer that kills other connections is pending. Kill-timer/retry semantics are unchanged.
• %r rejects non-string arguments and errors on missing arguments, same as %n / %?.

Call sites audited and fixed (all places where a non-constant format string reached Exec/ForceExec):

• pkg/migration/change.go alterNewTable — ALGORITHM=COPY attempt and the plain retry
• pkg/migration/change.go attemptInstantDDL — ForceExec and Exec variants
• pkg/migration/change.go attemptInplaceDDL — ForceExec and Exec variants
• pkg/migration/runner.go — non-ALTER single-statement execution (stmt.Statement was the format string)

All other Exec / ForceExec / EscapeSQL call sites were audited (re-verified against current main) and use compile-time constant format strings (incl. checkpoint.go's "CREATE TABLE %n " + tableDDL, which concatenates two package constants); they are unchanged. pkg/move / pkg/datasync already execute fetched DDL via plain ExecContext.

Testing

New regression tests (the instant-path test reproduces the pre-fix process panic missing arguments, need 2-th arg, but only got 1 args):

• pkg/dbconn/sqlescape TestEscapeSQLRawVerb: verbatim splice (with %n/%?/%% payloads), composition with %n/%?, missing-argument and non-string-argument errors
• pkg/dbconn TestExecRawVerb (literals reach the server verbatim; the same text placed in the format string still errors), TestForceExecRawVerb (no format interpretation and the MDL-blocker force-kill still works), TestForceExecBadFormatString (bad format returns an error before the kill timer is armed, instead of panicking)
• pkg/migration TestPercentSignsInDDLLiterals: unchanged from the previous revision of this PR — instant path with %n/%? in a comment, instant path with %% in DEFAULT/COMMENT (byte-identical to a plain-client sibling table), copy path via alterNewTable, and the non-ALTER --statement path

Runs against local compose MySQL 8.0:

• full ./pkg/dbconn/... suite: pass
• pkg/migration TestPercentSignsInDDLLiterals: pass
• go build, go vet, gofmt, golangci-lint: clean

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a correctness/safety issue in Spirit’s SQL escaping layer where user-provided DDL was accidentally treated as a sqlescape format string, causing %n / %? / %% sequences inside string literals to either error (including a MustEscapeSQL panic on the ForceExec path) or be rewritten (e.g., %% collapsing to %). It introduces a raw-splice escape verb and updates migration/dbconn execution paths so user SQL is always treated as data, not a format.

Changes:

• Add a new sqlescape verb %r to splice raw SQL strings verbatim (no quoting, no format re-scan) into a trusted, constant format string.
• Update migration execution paths (ALTER clause embedding and non-ALTER statement execution) to pass user SQL via %r instead of concatenating it into the format string.
• Make dbconn.ForceExec escape the statement before arming the force-kill timer, returning a normal error for bad format strings instead of panicking mid-flight, and add regression tests covering these cases.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/migration/runner.go	Executes non-ALTER user statements via dbconn.Exec("%r", stmt) so % in literals isn’t format-interpreted.
pkg/migration/ddl_test.go	Adds end-to-end regression coverage ensuring %n/%?/%% in DDL literals are preserved and don’t crash the instant/copy/non-ALTER paths.
pkg/migration/change.go	Switches ALTER TABLE assembly to constant format strings using %r for user clauses across instant/inplace/copy paths.
pkg/dbconn/sqlescape/utils.go	Implements %r in the core SQL escape/format routine and documents the verb in code comments.
pkg/dbconn/sqlescape/utils_test.go	Adds unit tests validating %r behavior, composition, and error cases.
pkg/dbconn/sqlescape/README.md	Updates documentation to reflect the forked status and documents %r/EscapeIdentifier.
pkg/dbconn/dbconn.go	Escapes in ForceExec prior to arming the kill timer; updates docs to recommend %r for raw user SQL.
pkg/dbconn/dbconn_test.go	Adds integration tests for %r via Exec/ForceExec and verifies bad format strings fail safely.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.