Mysql datasource - #276543
Merged
erikcurrin-elastic merged 30 commits intoSep 15, 2026
Merged
Mysql datasource#276543
Conversation
Contributor
🔍 Preview links for changed docs |
Contributor
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed the MySQL connector. The read-only enforcement, identifier backtick-quoting, and pool-caching logic are solid and well-tested. Left two inline comments on correctness gaps where allowed/valid input is mishandled: single-quote doubling corrupting bound LIKE parameters in searchRows, and the derived-table wrapping breaking SHOW/DESCRIBE/EXPLAIN queries that assertReadOnly and the skill text advertise as supported.
Generated by Claude Reviewer for issue #276543 · 110.3 AIC · ⌖ 7.87 AIC · ⊞ 4.2K
erikcurrin-elastic
added
release_note:skip
Ports the MySQL connector spec from elastic#255310 (kderusso/mysql-datasource), squashed into a single commit to take over the PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves credentials from the unencrypted schema config into auth: basic (ctx.secrets) so they are encrypted at rest, adds the missing skill property, bounds free-text Zod inputs, switches the icon to the standard SVG-file + EuiIcon pattern with the official brand mark, fixes docs placement/ordering, and significantly expands test coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eSql action
- Rewrites MySQL connector to use ctx.getClient('mysql') via the new
multi-client architecture framework (ClientTypeSpec), replacing the
previous singleton approach
- Adds MysqlClientTypeSpec in lib/clients/mysql.ts: decodes Basic auth
credentials, calls networkSettings.ensureHostnameAllowed() before
opening a pool, creates a mysql2 connection pool
- Registers mysql client type in lib/clients/index.ts ClientRegistry
- Adds explicit scope to all actions (read-only actions: 'read';
executeSql: 'destroy') so the LLM knows which actions are safe
- Adds executeSql action for unrestricted SQL (INSERT/UPDATE/DELETE/DDL)
- Removes single-quote escaping from escapeLikePattern — prepared
statements handle quoting
- Adds WidgetType.Number and NumberWidget to the form-generator widget
system so numeric config fields (e.g. port) render as <input type="number">
- Fixes form-generator field_builder to coerce string→number before Zod
validation for ZodNumber schemas, and adds a serializer in NumberWidget
to submit numeric values rather than strings
- Updates MySQL public docs to document executeSql (marked preview 9.6+)
and correct stale read-only claims
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Inline LIMIT as an integer in query and searchRows to avoid mysqld_stmt_execute param-count mismatch when LIMIT ? is bound via the binary protocol - Extract runSql helper to remove duplicated pool-lease / execute / cast-rows boilerplate across all action handlers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
erikcurrin-elastic
force-pushed
the
mysql-datasource
branch
from
August 25, 2026 00:09
6a552f6 to
d7aa085
Compare
Share one read-only SQL guard across MySQL, Snowflake, and BigQuery, restrict query to SELECT/WITH, and apply Kibana TLS settings on the MySQL client. Co-authored-by: Cursor <cursoragent@cursor.com>
erikcurrin-elastic
marked this pull request as ready for review
August 25, 2026 19:51
Contributor
|
Dependency Review Bot Analysis 🔍 Found 1 new third-party dependencies:
Self ChecklistTo help with the review, please update the PR description to address the following points for each new third-party dependency listed above:
Thank you for providing this information! |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Reviewed the MySQL connector. Two concrete concerns flagged inline: the query action's subquery wrapping breaks on trailing-semicolon input (common in agent-generated SQL, currently uncovered by tests), and the shared read-only guard refactor silently tightens Snowflake/BigQuery runQuery by applying WRITE_PATTERN where they previously used prefix-only checks. Identifier quoting, LIKE escaping, parameterization, allowedHosts enforcement, and TLS handling all look sound.
Generated by Claude Reviewer for #276543 · opus · 186 AIC · ⌖ 14.5 AIC · ⊞ 5.1K
TamerlanG
approved these changes
Aug 26, 2026
Remove unused `_opts` parameter from the mysql.test.ts mock to fix the @typescript-eslint/no-unused-vars lint error, and add mysql2 to renovate.json packageRules so the dependency_ownership check finds an owner. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
tara-elastic
reviewed
Sep 4, 2026
…host lookup, and pool.query for parameterless statements
- WRITE_PATTERN: broaden UPDATE arm from UPDATE\s+\w+\s+SET to UPDATE\s+ and
drop the trailing \b so schema-qualified (db.table) and backtick-quoted
identifiers are no longer able to bypass the read-only guard; add test cases
for both evasions
- clients/mysql: remove dead getCustomHostSettings('mysql://...') call —
resolveCustomHosts only stores https: and smtp: URLs, so the lookup always
returned undefined; simplify toMysqlSslOptions signature accordingly
- mysql spec: use pool.query() for parameterless SQL (SHOW, DESCRIBE, bare
SELECT/INSERT in executeSql) and reserve pool.execute() for parameterized
searchRows calls to avoid unnecessary prepared-statement caching
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
One inline note on the shared read-only SQL guard: the DELETE alternative in WRITE_PATTERN misses MySQL's multi-table delete form, which could let the read-scoped query action perform a delete when the DB user has write privileges. Non-blocking, defense-in-depth. The rest of the connector (identifier quoting, bound LIKE params, TLS defaults, host allowlisting, prior review feedback) looks solid.
Generated by Claude Reviewer for #276543 · opus · 253.5 AIC · ⌖ 14.4 AIC · ⊞ 5.1K
…ic_db_connector.ts Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
jcger
approved these changes
Sep 8, 2026
| const schema = z.number(); | ||
|
|
||
| expect(() => getWidgetComponent(schema)).toThrow( | ||
| /No widget found for schema type: ZodNumber/ |
Contributor
There was a problem hiding this comment.
i think we should use another example now that we have the number, but still check that if it's a non implemented type, it throws as we expect it to throw
Contributor
Author
There was a problem hiding this comment.
We have some checks already that cover this
… to isTool:false isReadOnlySql now matches the original Snowflake/BigQuery prefix-only semantics — WRITE_PATTERN and MySQL executable-comment rejection remain exclusively in assertReadOnly. executeSql is isTool:false to keep destructive SQL out of agent tool calls, matching the established Snowflake/BigQuery pattern. Updates tests to reflect both changes, and adds a non-implemented-type throw assertion to the registry describe block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
erikcurrin-elastic
requested review from
emilioalvap,
opauloh and
weltenwort
September 14, 2026 18:54
botelastic
Bot
added
the
Team:One Workflow
upstream/main migrated from yarn to pnpm and added pnpm-lock.yaml; CI does a test merge so it finds the lockfile from main but our package.json declares mysql2@3.24.3 which was absent, causing ERR_PNPM_OUTDATED_LOCKFILE on every job. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
erikcurrin-elastic
force-pushed
the
mysql-datasource
branch
from
September 14, 2026 20:17
955fede to
d884c55
Compare
erikcurrin-elastic
removed request for
a team,
emilioalvap,
opauloh and
weltenwort
September 14, 2026 20:18
erikcurrin-elastic
enabled auto-merge (squash)
September 15, 2026 13:25
# Conflicts: # yarn.lock
Contributor
💛 Build succeeded, but was flaky
Failed CI Steps
Metrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
shared async chunks total size
shared chunk count
shared chunks total size
Total ESLint disabled count
total optimizer output size
Test Failures
History
|
tara-elastic
approved these changes
Sep 15, 2026
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
8 participants
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.
Summary
Implement a MySQL fetcher
Dependencies
New: mysql2 library (current version 3.20.0)
Purpose: Used to plug into the MySQL binary protocol
Justification: Absolutely. We do not want to reverse engineer the protocol
Alternatives explored: The only alternative is the Http bridge, but it is not maintained actively and not viable
Existing dependencies: There are other libraries that we use for Kibana connectors.