Bug Report: ALTER/REVERT VITESS_MIGRATION derive no table ACL permissions, so any authenticated caller can control Online DDL migrations

Overview of the Issue

Under strict table ACL, vttablet checks a statement against the tables its planner derives for it. ALTER VITESS_MIGRATION ... and REVERT VITESS_MIGRATION ... derive none:

// go/vt/vttablet/tabletserver/planbuilder/permission.go
case
    *sqlparser.AlterMigration,
    *sqlparser.RevertMigration,
    ...
    permissions = []Permission{} // TODO(shlomi) what are the correct permissions here? Table is unknown

checkPermissions does run for PlanAlterMigration and PlanRevertMigration, but with no permissions the per-table loop has nothing to check, so any authenticated caller — one with no table grants at all — can drive Online DDL on a keyspace: COMPLETE, CANCEL, CLEANUP, RETRY, LAUNCH, THROTTLE, FORCE_CUTOVER, POSTPONE_COMPLETE, and REVERT. The ... ALL forms need no UUID, so ALTER VITESS_MIGRATION CANCEL ALL cancels every pending migration in the keyspace.

This was raised on #21053 (https://github.com/vitessio/vitess/pull/21053#discussion_r3969694879), which fails closed on statements whose tables the parser discards (DO, CALL, REPAIR, OPTIMIZE, LOAD DATA). The migration statements were deliberately left out of that change:

• They are a different class. The five statements in #21053 name or reach a table and the parser throws it away; the migration statements name a migration UUID, and their effect on a table is indirect and asynchronous, carried out by the Online DDL executor as the tablet itself. It is unauthorized operational control of migrations rather than an ACL data bypass.
• Failing closed would break operators. vtgate plans these as an engine.Send to the shard primaries (go/vt/vtgate/planbuilder/migration.go), which is the documented way to drive Online DDL from a MySQL client. Denying them to every non-exempt caller under strict table ACL — in a security backport — would break in-flight cutovers on a patch upgrade.
• Blanket denial is probably not the right fix anyway. For a specific UUID the affected table is recoverable: Executor.readMigration (go/vt/vttablet/onlineddl/executor.go) loads the _vt.schema_migrations row, which carries mysql_table. Deriving ADMIN on that table (the same role a direct ALTER TABLE requires) matches the intent of the standing TODO(shlomi), and lets an operator with ADMIN on the table manage its migrations while denying everyone else.

Proposed direction

• For a statement that names a UUID: look up the migration and require ADMIN on its mysql_table. This has to happen at execution time (the planner has no access to _vt.schema_migrations), so it needs a permission hook in execAlterMigration / execRevertMigration rather than in BuildPermissions; an unknown UUID should be denied rather than allowed through.
• For the ... ALL forms (which name no UUID): require ADMIN on every table with a pending migration in the keyspace, or treat them as undetermined and fail closed for non-exempt callers — this needs a maintainer decision, since the second option denies CANCEL ALL / COMPLETE ALL to everyone outside the exempt ACL.
• SHOW VITESS_MIGRATIONS, SHOW VITESS_MIGRATION ... LOGS, SHOW VITESS_THROTTLED_APPS, SHOW VITESS_THROTTLER STATUS are in the same arm and derive nothing either; they are reads of Vitess metadata rather than of user tables and are probably fine to leave as they are, but should be considered in the same pass.

Whether this should be handled as a security advisory in its own right is for the maintainers to decide.

Reproduction Steps

1. Start a keyspace with --queryserver-config-strict-table-acl and a table ACL that grants user u2 nothing.
2. As an operator, submit an Online DDL migration on a table: alter vitess_migration ... — e.g. set @@ddl_strategy='vitess'; alter table t add column c int; — and note its UUID.
3. As u2, through vtgate: alter vitess_migration '<uuid>' cancel; (or alter vitess_migration cancel all;).
4. Expected: ... command denied to user 'u2' .... Actual: the migration is cancelled.

Binary Version

vttablet --version
Version: 25.0.0-SNAPSHOT (Git revision 1cf37324b4 branch 'main')

Operating System and Environment details

Any. Verified against the planner on macOS arm64.

Log Fragments

(none: the statement is not logged as denied because no permission is derived for it)

Related

This is one of a set of changes that came out of one investigation into what a stored procedure, or any statement whose tables the planner never sees, can do on a vttablet. They reference each other so the whole picture is reachable from any one of them.

Table ACL — GHSA-w6mx-2f8x-pqf4

• #21053 — fail closed under strict table ACL for statements whose tables the parser discards (DO, CALL, REPAIR, OPTIMIZE, LOAD DATA); this issue was split out of its review
• #21139 — stacked on #21053: check the reads embedded in statements the planner does parse (CREATE TABLE ... AS SELECT, EXPLAIN ANALYZE, SHOW ... WHERE, SET), reject connection settings that carry a subquery, and have a targeted vtgate session store a SET's value rather than its expression
• #21134 — still open: a stored function invoked inside an expression runs its body outside the table ACL
• #21138 — still open: parse the CREATE TABLE source forms the grammar only partially accepts, which #21139 has to deny outright today

Stored-procedure session residue

• #21046 — procedure-body session state (SET SESSION, temp tables) persists on pooled connections after CALL
• #21062 — fixes #21046 by discarding the pooled connection after CALL
• #21063 — still open: the same residue on a transaction connection recycled at commit
• #21066 — still open: the pooled-connection baseline differs between fresh and settings-reset connections on servers using init_connect