fix(analyzer): adopt sqlglot-go v0.21.0 parser fixes, pin every MySQL statement resolution - #89
Merged
Merged
Conversation
A unit test enumerating the MySQL 8.0/8.4 statement kinds a client can send, curated from the reference manual's §15 (broad, not a machine-verified extract). Each runs through the real analyzer and its resolution is asserted, so a statement that stops failing closed breaks the build. Auditing the output found four privileged statements that resolve to a connect-only passthrough instead of failing closed, all parser artifacts: START REPLICA / START SLAVE / START GROUP_REPLICATION parse like START TRANSACTION and classify SESSION (their STOP/RESET counterparts, which do not collide, fail closed); ANALYZE TABLE sits in the analyzer's session-passthrough set (CHECK/OPTIMIZE/REPAIR do not). None exposes row data — the gap is authorization, not masking. TestPrivilegedStatementsAreGated enumerates the four so the set cannot grow silently and closing one is noticed. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01AK83USBkxzao4uFfjxzy8n
…verage audit sqlglot-go v0.21.0 fixes three MySQL parse deviations proxy-monster's enforcement depended on: - START REPLICA / START SLAVE / START GROUP_REPLICATION parsed as a Transaction node (the word after START taken as a transaction mode), indistinguishable from START TRANSACTION. The analyzer classified them SESSION and passed them through with connect only — a principal who could connect could start replication with no grant. They now parse as Command and fail closed. - STOP/FLUSH/UNLOCK INSTANCE/XA/BINLOG/HELP/RESTART/SHUTDOWN were coerced into expression nodes (Alias/Column); they now parse as Command. Already fail-closed here, now robustly so. - TABLE t (the 8.0.19 SELECT * shorthand) parsed as an Alias; it now parses as Select, so a real read is enforced and masked instead of denied. Bump effect, measured by the statement-coverage test: exactly four resolutions change — TABLE gains result.read+sql.select (denied read → masked read), and the three START forms move from a connect-only passthrough to fail-closed. Completing and hardening the audit alongside the bump: - The coverage set gains the replication/admin/SET kinds the audit was missing (SET sql_log_bin, SET PERSIST[_ONLY], SELECT INTO DUMPFILE, SHOW REPLICAS / SLAVE HOSTS / SLAVE STATUS / BINARY LOG STATUS, SHOW CREATE *). Four more pre-existing under-gatings surface and are recorded in knownConnectOnlyGaps next to ANALYZE TABLE: SET sql_log_bin, SHOW MASTER STATUS, SHOW BINARY LOGS, SHOW REPLICAS/SLAVE HOSTS — all the same benign-catch-all passthrough the statement-typing redesign (docs/statement-typing.md) closes by construction. - privilegedNeedingGate is widened to the full privileged set (including the utility-gated rows), so a regression from `… + utility:X` back to a bare passthrough now fails the invariant. - resolve() models decideQuery's short-circuit order (INADMISSIBLE, then the datasource-grant loop, then the unanalyzable gate), removing an unreachable relay label on CALL; its comments no longer overstate it as a decideQuery replica or the list as proven-exhaustive. - go.sum is tidied (stale v0.20.0 checksums dropped) and the analyzer README pin updated to v0.21.0. The c-shared analyzer lib is rebuilt from the new pin by the JVM build; no committed artifact changes. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01AK83USBkxzao4uFfjxzy8n
sjincho
force-pushed
the
sjcho/test/mysql-statement-coverage
branch
from
August 4, 2026 07:07
79430a3 to
6ccc56e
Compare
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopts sqlglot-go v0.21.0 and pins the resolution of every MySQL statement kind
so the two stay honest with each other.
The parser fixes (sqlglot-go v0.21.0, ridi-oss/sqlglot-go#59)
Three MySQL parse deviations proxy-monster's enforcement depended on:
START REPLICA/START SLAVE/START GROUP_REPLICATIONparsed as aTransactionnode — the word afterSTARTtaken as a transaction mode,indistinguishable from
START TRANSACTION. The analyzer classified themSESSION and passed them through with connect only, so a principal who could
connect could start replication with no grant. They now parse as
Commandandfail closed. This is a real authorization gap, closed.
STOP/FLUSH/UNLOCK INSTANCE/XA/BINLOG/HELP/RESTART/SHUTDOWNwere coerced into expression nodes (Alias/Column); nowCommand.Already fail-closed here, now robustly so.
TABLE t(the 8.0.19SELECT *shorthand) parsed asAlias; nowSelect, soa real read is enforced and masked instead of denied.
Verified against the actual PR branch before merge, then re-measured after the
version bump.
The coverage test
analyzer/probe/mysql_statement_coverage_test.goenumerates every MySQL 8.0/8.4statement kind a client can send — the list taken from the reference manual's
§15, not the analyzer's own dispatch — runs each through the real analyzer, and
asserts its resolution. A statement the analyzer never names is caught here, not
silently absent.
TestPrivilegedStatementsAreGatedis the security invariant: aprivileged kind that resolves to a connect-only passthrough fails the build.
docs/mysql-statement-coverage.mdis the human table, generated from the test.Enforcement effect
Exactly four of 173 resolutions change, all intended:
TABLEgainsresult.read + sql.select(denied read → masked read); the threeSTARTformsmove from connect-only passthrough to fail-closed. The other sixteen
Commandconversions keep their resolution.
Known gap left standing
ANALYZE TABLEstill resolves connect-only — but that is a proxy-monster-sideclassification (
KindAnalyzein the analyzer's session-passthrough set), not aparse issue, and it is tracked separately. The coverage test pins it in
knownConnectOnlyGapsso it cannot be forgotten.Verification
mise run verifygreen — the c-shared analyzer lib is rebuilt from the new pinand the full JVM + Go + web suite passes, so the parser bump is safe product-wide.