Repository navigation
fix(mcp-guard): refuse table reads without table access events - #251
Conversation
Found reviewing the code behind #247. The server raises table access events only for SELECT, data changes, ANALYZE and HANDLER READ, so the schema rules (mcp.allowed_schemas, mcp.deny_system_schemas) did not see: - a subquery in SET or DO: SET @x = (SELECT ... FROM mysql.user) returned the data, also through PREPARE/EXECUTE; - a subquery in SHOW ... WHERE or a CALL argument (a yes/no oracle); - CHECK TABLE / CHECKSUM TABLE. Under the schema rules these are now refused (hides_table_reads, scanned in both quoting modes). System schema names compare case-insensitively under lower_case_table_names 1 or 2. Tests: policy unit cases; test-mcp-guard.sh 110 checks on 8.4.11, 9.7.2, 26.10.0 (the unfixed guard fails all 8 new ones); 3 parity rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@coderabbitai review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Detect hidden reads and match schema names src/mcp/guard/mcp_guard_policy.h, src/mcp/guard/test_mcp_guard_policy.cc |
Policy helpers detect subqueries in selected commands and flag CHECK and CHECKSUM. System-schema and mysql_mcp name comparisons follow the configured case sensitivity. Unit tests cover detection and name matching. |
Enforce schema rules and validate coverage src/mcp/guard/component_mcp_guard.cc, scripts/test-mcp-guard.sh, docs/MCP_COMPONENTS.md, CHANGELOG.md |
The callback refuses hidden reads when schema rules are active. Integration checks cover denied reads and allowed commands without subqueries. Documentation and the changelog describe the behavior and updated test count. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related issues
- component_mcp_guard: schema rules don't see reads inside stored functions and triggers #247: This PR covers several event-free read forms described by the issue. Stored-function and trigger reads remain outside the added detection.
Possibly related PRs
- askdba/myvector#248: This PR extends the guard coverage and parity work established by
#248. - askdba/myvector#241: This PR extends the schema policies and table-access enforcement introduced by
#241. - askdba/myvector#244: This PR changes schema-enforcement behavior for the guard packaged and tested by
#244.
Merge Risk: 
831ba
-
No merge-blocking issue was identified. The reported integration results were not independently reproduced.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: refusing table reads that bypass table-access events in mcp-guard. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 4 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks each query’s trail
For hidden reads that might prevail
When schema rules say, “Not this way,”
The guard can stop the read today
And mixed-case names join the play
Action not completed
|
Found while reviewing the guard code behind #247. #247 is the stored-function case: a component can't see those reads at all. This PR fixes a related class it can catch.
The bug
mcp.allowed_schemasandmcp.deny_system_schemasare enforced on table-access events. MySQL raises those only forSELECT, data changes,ANALYZEandHANDLER READ(sql_audit.cc). Several statements on the read-only allowlist read tables without them.Measured on 9.7.2, with an MCP account holding global
SELECT(the caseallowed_schemasexists for),allowed_schemas = 'd1'anddeny_system_schemason:SET @x = (SELECT MAX(i) FROM d2.t); SELECT @xSET @x = (SELECT COUNT(*) FROM mysql.user)DO @x := (SELECT …),SET SESSION v = (SELECT …),PREPARE s FROM 'SET @p = (SELECT …)'; EXECUTE sSHOW DATABASES WHERE (SELECT …) = 42CALL mysql_mcp.list_tables((SELECT …))CHECKSUM TABLE d2.t,CHECK TABLE d2.tNot affected:
SELECT,TABLEandVALUES(events fire);HANDLER(it has its own event;READ … WHERErejects subqueries);KILL(rejects subqueries);SHOW CREATE(metadata, by design).Fix
hides_table_reads()(policy header):CHECK/CHECKSUM, andSET,DO,CALLorSHOW …statements containing a subquery. Every subquery is parenthesized, so the check is(followed bySELECT,TABLE,VALUESorWITH.EXECUTEd statement included), while a schema rule is in effect:deny_system_schemas, on by default, orallowed_schemas. The error names that rule.SET,SHOWandCALLwithout subqueries are unaffected. Neither the tool pack nor mysql-mcp-server uses these forms (checked).lower_case_table_names1 or 2, system schema names now compare case-insensitively, as the server compares them.Verification
NO_BACKSLASH_ESCAPES.scripts/test-mcp-guard.sh: 110/110 on MySQL 8.4.11, 9.7.2 and 26.10.0.42and amysql.userrow count.🤖 Generated with Claude Code
Summary by CodeRabbit
SET,DO,SHOW ... WHERE,CALL, prepared statements, and table-check commands when those reads are not visible to normal access checks.SELECTas an alternative for affected statement forms.