feat: add datatable read and write MCP tools#10001
Conversation
Deploying windmill with
|
| Latest commit: |
d2a682a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9dd51ba6.windmill.pages.dev |
| Branch Preview URL: | https://ruben-win-2143-add-datatable.windmill.pages.dev |
|
Claude finished @rubenfiszel's task in 4m 4s —— View job Mergeable, but should ideally address nits: write endpoints ( cc @rubenfiszel ReviewSolid PR. The SQL construction is careful — identifiers always go through
All are top-level arrays, so the FindingsP2 — write endpoints are member-level, unlike the rest of the datatable admin surface ( P2 — raw AGENTS.md compliance
Test coverage
|
|
I've now reviewed the full diff across all 10 changed files. Here's my assessment. Pi ReviewGood to merge. The implementation is thorough and well-considered. The three new handlers ( The frontend scope selector change integrates cleanly: it lazily loads data table names in custom mode, decodes/encodes the No P0, P1, or P2 findings. Test coverage
Manual verification still neededThe end-to-end item from the PR's own test plan remains unchecked: calling the tools from an MCP client against a live data table, including a restricted token being rejected on an out-of-scope data table. This should be done before merging, as it exercises the actual MCP stack with a real Postgres backend, confirming that the per-datatable enforcement, list filtering, and the read-only toggle all work end-to-end from an MCP client's perspective. |
Codex Reviewcc @rubenfiszel Found 1 issue:
Test coverageBackend: the PR adds unit coverage for identifier quoting and MCP datatable scope parsing, but there is no test that rejects SQL breakout through Frontend: no component tests expected for this selector change. Manual verification still needed: call |
|
Now I have all the information needed for the review. Let me write the complete review. Pi ReviewShould address issues before merging: SQL injection via UNION in cc @rubenfiszel P0 — SQL injection via UNION in
|
Codex Reviewcc @rubenfiszel Found 1 issue:
Test coverageBackend: the PR adds unit coverage for identifier quoting and datatable scope parsing, but there is no regression test proving Frontend: no component tests expected for the Svelte selector change. Manual verification still needed: call |
|
Addressed the review findings in d2a682a: P0 — SQL injection via
Complex boolean predicates (AND/OR/BETWEEN/ILIKE/function calls/ P2 — member-level write authz: P2 — function-call trust boundary ( Minor — Still open (as noted in the test plan): end-to-end exercise from a live MCP client. The injection fix itself is covered by the new unit tests + a psql check that a sanitized predicate returns the expected rows. |
|
Now I have a thorough understanding of the entire diff. The author (rubenfiszel) pushed commit d2a682a to address the P0 SQL injection finding and the P2 auth finding from prior reviews. Let me verify the fix stands. Pi ReviewGood to merge. The prior-review P0 SQL injection finding (raw
The P2 member-level write authz has been addressed with The scope enforcement is layered correctly:
The frontend scope selector correctly:
One minor observation (P2): P2 — Each element of the Test coverage
Manual verification still neededThe end-to-end item from the PR's own test plan remains unchecked: calling the tools from an MCP client against a live data table, including a restricted token being rejected on an out-of-scope data table and a read-only token rejected on write. This exercises the actual MCP stack with a real Postgres backend, confirming that the per-datatable enforcement, list filtering, and the read-only/write toggles all work end-to-end from an MCP client's perspective. |
Codex Reviewcc @rubenfiszel Found 1 issue:
Test coverageBackend: unit coverage was added for identifier quoting, SQL predicate/order sanitization, and datatable MCP scope parsing/containment. I did not see handler-level integration tests for the new query/insert/update endpoints. Frontend: no component tests expected for this Svelte selector change. Manual verification still needed: exercise the generated MCP tools against a live data table with a normal read, an insert/update token, a read-only token attempting write, and a restricted datatable token attempting an out-of-scope table; expected results are success only for in-scope allowed actions and clear denials otherwise. |
Summary
Fixes WIN-2143.
Exposes Windmill data tables as MCP operations. Data table access is a dedicated, opt-in capability in the MCP URL generator — off by default in every mode — with a read/write sub-toggle and an optional restriction to specific data tables. When enabled, the client gets tools to list tables, read rows (with WHERE conditions), and (if write is on) insert/update rows.
New/existing REST endpoints are marked
x-mcp-tool: truein the OpenAPI spec and auto-generated into the Rust tool registry + the frontend endpoint list; the datatable tools are then gated by a dedicatedmcp:datatables:scope rather than the generic endpoint scopes.Tools
Read:
listDataTables,listDataTableTables,getDataTableTableSchema(existing endpoints, now MCP-exposed), and newqueryDataTable(SELECT withwhere_clause/select/order_by/limit/offset, capped at 1000).Write: new
insertDataTable(row insert; omitted columns keep their default) andupdateDataTable(update rows matching a requiredwhere_clause).Data tables access control
A dedicated "Data tables" section (shown in every mode except "All", where
mcp:allalready grants everything):Scope grammar:
mcp:datatables:read:<names|*>/mcp:datatables:write:<names|*>. Absence ⇒ no datatable access.mcp:allgrants read+write on all data tables.Enforcement:
datatable_tool_allowed(read|write)— datatable tools are opt-in regardless of mode; write tools also require a non-read-only token (they're POST).WindmillBackend::call_endpoint(the only place with the original MCP scopes — the JWT the runner mints for the proxied HTTP call is route-scoped and no longer carriesmcp:scopes). Per-calldatatable_nameis rejected if out of scope;listDataTables/listDataTableTablesresponses are filtered so a restricted token never sees other names.Changes
backend/windmill-api-workspaces/src/workspaces.rs:query_datatable/insert_datatable/update_datatablehandlers +connect_datatable/quote_pg_ident/check_datatable_schemahelpers and routes.backend/windmill-api/openapi.yaml: six datatable endpoints markedx-mcp-tool; regeneratedauto_generated_endpoints.rs+mcpEndpointTools.ts.backend/windmill-mcp/src/common/scope.rs:datatables_read/datatables_write/datatablesonMcpScopeConfig; parsemcp:datatables:read|write:;datatable_access_level,datatable_tool_allowed,is_datatable_allowed; OAuth subset (contains); tests.backend/windmill-mcp/src/server/runner.rs: gate datatable tools by the dedicated scope inlist_tools+call_tool.backend/windmill-api/src/mcp/core.rs: per-datatable arg check + list-response filtering incall_endpoint.backend/windmill-api/src/mcp/oauth_server.rs: advertisemcp:datatables:read:*/write:*.frontend/src/lib/components/mcp/McpScopeSelector.svelte: dedicated Data tables section (master + write toggle + restriction picker + warning); datatable tools excluded from the generic endpoint picker.tokio-postgresdep onwindmill-api-workspaces; unit tests forquote_pg_identand the datatable scope logic.Safety
quote_pg_ident(doubles", rejects empty/NUL).$1::jsonbparam viajsonb_populate_record— Postgres coerces types, no value interpolation.where_clause/order_byare validated withsqlparserand re-serialized from the parsed AST before interpolation: a fragment must be a single self-contained predicate / ORDER-BY list with no set operations, no subqueries, and no trailing tokens. This blocks theUNION/subquery-closing breakout (which would otherwise bypass the system-schema guard and LIMIT) and the error-based/IN (SELECT …)cross-table exfiltration. Complex boolean predicates and multi-column ordering still work.pg_sleep, orpg_read_fileif that role is superuser) — scope datatable roles accordingly (not superuser).mcp:datatables:scope; write additionally requires a non-read-only token, and the REST write endpoints reject operators (require_datatable_writer).Design decisions
mcp:all) grants it.Test plan
cargo check(windmill-api, windmill-api-workspaces);cargo test -p windmill-mcp scope::(9 tests) and-p windmill-api-workspaces(quote_pg_ident +where_clause/order_bysanitizer incl. the reported UNION-injection payload)psql(type coercion, serial default,to_jsonb, per-column update, quoted identifiers)npm run check:fast; svelte-autofixermainproduces token scopemcp:favorites mcp:datatables:write:main(verified in the DB)Screenshots
Dedicated "Data tables" section — opt-in master toggle, write sub-toggle, restriction picker, and access warning:

MCP URL generator — datatable read tools shown (read-only token hides insert/update):

🤖 Generated with Claude Code