feat(mssql): add Microsoft SQL Server integration - #6739
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Six authenticated API routes cover query (SELECT-only with layered T-SQL validation), insert/update/delete (parameterized DML with bracket-quoted identifiers and screened WHERE clauses), execute (raw T-SQL with a statement-kind gate), and introspect (schema metadata via The Reviewed by Cursor Bugbot for commit e1b7386. Configure here. |
|
@cursor review |
Greptile SummaryAdds a complete Microsoft SQL Server integration with workflow tools, authenticated API routes, contracts, documentation, and generated registry metadata.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains at the current head. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/tools/mssql/utils.ts | Implements pinned connection creation, SQL validation, parameterized mutation builders, execution helpers, and schema introspection; the previously reported bypasses are addressed. |
| apps/sim/app/api/tools/mssql/query/route.ts | Authenticates and parses Query requests, enforces the read-only validator before connecting, and closes the pool after execution. |
| apps/sim/app/api/tools/mssql/utils.test.ts | Covers the reported read-only and WHERE-screening bypasses, parameter binding, identifier handling, DNS pinning, and connection cleanup. |
| apps/sim/blocks/blocks/mssql.ts | Registers the six MSSQL operations and their connection and operation-specific inputs. |
| apps/sim/lib/api/contracts/tools/databases/mssql.ts | Defines request and response contracts for all six MSSQL API routes. |
| apps/sim/package.json | Adds the MSSQL runtime and type dependencies with corresponding current lockfile resolutions. |
| bun.lock | Contains the MSSQL, type-package, tedious, and transitive dependency resolutions required by the updated workspace manifest. |
Sequence Diagram
sequenceDiagram
participant W as Workflow
participant T as MSSQL Tool
participant API as Authenticated API Route
participant V as SQL Validator/Builder
participant S as SSRF Host Validator
participant DB as SQL Server
W->>T: Invoke MSSQL operation
T->>API: Submit connection and operation inputs
API->>V: Validate query or build parameterized statement
API->>S: Validate host and select pinned IP
S-->>API: Approved address
API->>DB: Connect through pinned socket
API->>DB: Execute validated statement
DB-->>API: Rows and affected count
API-->>T: Structured operation result
Reviews (10): Last reviewed commit: "fix(mssql): screen trigger and state sta..." | Re-trigger Greptile
|
@cursor review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ecb1dac. Configure here.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 340ca6b. Configure here.
|
@cursor review |
c8ddfa5 to
80b94d5
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 80b94d5. Configure here.
Add a Microsoft SQL Server block backed by six tools (query, execute, insert, update, delete, introspect), mirroring the existing PostgreSQL and MySQL integrations. Connections go through the `mssql` (Tedious) driver: `connectionTimeout` is top-level while `encrypt`, `trustServerCertificate`, and `instanceName` live under `options`, and `port` is omitted when a named instance is used. Values are bound as `@paramN` via `request.input()`; no user value is interpolated into SQL. Identifiers are bracket-quoted after validation and WHERE clauses run through the shared injection guard. Introspection reads INFORMATION_SCHEMA plus the `sys.indexes` catalog views for tables, columns, primary keys, foreign keys, and indexes. The icon is a placeholder database cylinder drawn with `currentColor` until the real brand mark lands. Requires `bun install` for the new `mssql` / `@types/mssql` deps.
…g reads Tedious exposes `options.connector`, a hook that replaces its own resolve-and-connect path, so the connection can be pinned to the address `validateDatabaseHost` already approved instead of re-resolving the hostname. `server` stays the hostname because tedious derives the TLS `servername` from it independently of the connector, so SNI and certificate validation survive the pin. This brings MSSQL in line with the PostgreSQL and MySQL tools. Named instances are dropped: tedious resolves them with a UDP SQL Server Browser lookup issued outside the connector, and node-mssql deletes `port` whenever `instanceName` is set, so no configuration leaves a named instance pinned. A named instance is reachable through its static TCP port. Introspection fixes: - index key columns now filter on `key_ordinal > 0`; INCLUDEd columns and partitioning columns both report `0` and were being returned as key columns, ordered ahead of the real ones - foreign keys resolve through `sys.foreign_keys` / `sys.foreign_key_columns` rather than `INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS`, whose join to `TABLE_CONSTRAINTS` has no row when a key references a unique index and so dropped the key entirely - `is_unique` is a `bit`, which tedious maps to a boolean, so it is coerced rather than compared - schemas come from `sys.schemas`, which needs only `public` and carries no metadata-visibility caveat The WHERE-clause guard also covers `WAITFOR TIME`, `OPENQUERY`, `OPENXML`, the legacy `master..sys*` compatibility views, and extended and OLE-automation procedures beyond `xp_cmdshell`. Regenerates the docs and catalog artifacts the icon change left stale.
The block label, tool description, and docs all present Query as SELECT-only while the route ran whatever T-SQL it was given, so an agent picking mssql_query because "it is only a SELECT" could delete rows. Screen the statement for mutating keywords with string literals stripped, which also catches the WITH ... DELETE form that a leading-token check would miss. Also switch the tool barrel to absolute imports per the repo convention.
…ss batch gap The local validateWhereClause re-derived an older copy of the shared patterns and scanned raw text, so it missed a bare 1=1 and false-positived on prose in a quoted value. Delegate to validateSqlWhereClause, which masks string literals first, and keep only the SQL Server surfaces it has no reason to know about. T-SQL needs no statement terminator, so every semicolon-anchored stacked-query check reads straight past `id = 1 DROP TABLE dbo.users`. Screen for a bare statement-introducing keyword to close that; word boundaries leave ordinary column names like updated_at and deleted_at untouched. Export maskSqlStringLiterals so the dialect layer masks the same way the shared guard does rather than carrying a weaker single-quote-only copy.
…d-only path The previous round left two keyword lists maintained separately, and both were short: DBCC, KILL, CHECKPOINT, USE, and DENY were in neither, so `SELECT 1; DBCC SHRINKDATABASE(...)` and `id = 1 DBCC SHRINKDATABASE(...)` both got through. Collapse them into one MSSQL_STATEMENT_KEYWORDS shared by the query and WHERE screens so a keyword cannot be covered in one place and missed in the other, and add the administrative commands. Also reject any second statement after a semicolon in the Query path outright. That closes SELECT 1; <anything> structurally instead of by naming the anything, so the blacklist no longer has to be exhaustive to hold.
A block comment placed inside a keyword splits it as far as a lexical scan is concerned, so keyword coverage cannot settle whether the server rejoins the halves. Refuse comments in the Query path instead of modelling the tokenizer. A SELECT sent through this operation has no need for one, and Execute Raw SQL still accepts them. Masking leaves comment markers intact, so a literal containing -- still passes.
…reads Every T-SQL screen runs over the shared literal masker, which was written for the MySQL dialect. Three ways to desynchronise it let real SQL hide inside what the masker believes is a string, two of which survived the existing even-quote check: - a backslash before a quote. T-SQL has no backslash escape, so the server closes the literal where the masker swallowed the quote and runs the rest as code. `a='x\' DELETE FROM dbo.t WHERE b='y'` holds four quotes and masks the DELETE out of the keyword screen entirely, so the read-only Query operation would run it. - a double quote inside a bracketed identifier, which the bracket rule missed because it only looked for single quotes. - any unbalanced double quote or backtick, which the parity check did not cover. All three now fail closed. Introspection also filters hypothetical and disabled indexes, which were reported as if they were live, and resolves the referenced side of a foreign key through sys.schemas so a cross-schema reference is no longer an ambiguous bare table name. Values bound through request.input are serialized when they are nested JSON, which the driver otherwise rejects with a bare "Invalid string.".
Asserts on the merged `{ ...inputs, ...buildParams(inputs) }` the generic tool
handler forwards rather than the mapper's return, since a key the mapper omits
keeps its raw subBlock value through that merge. Pins the TLS toggles to their
string form end to end — a switch subBlock would serialize `'false'`, which is
truthy, and the route contract would coerce the user's off into on — and checks
that duplicate subBlock ids agree on their seeded default.
…with no trailing space Only a pool handed back to the route reaches its `finally`, so a pool whose connect rejected leaked its tarn resources — one per attempt when a bad credential is retried. It now closes itself, and a failure to close cannot mask the connect error the caller needs. The read-only screen also anchored on `\s` after the opening keyword, which refused valid reads like `SELECT*FROM dbo.users` and `SELECT(1)`. A word boundary accepts those while still refusing `SELECTX`, and cannot loosen the screen — the keyword and batch checks run over the whole statement regardless.
…ter rebase Artifacts rebuilt with the generators rather than hand-merged, so they carry both the mssql entries and the tools that landed on staging in parallel.
… WHERE clause The shared guard recognises `OR 1` but not `OR (1)`, `OR ((1))`, `OR NOT 0`, or `OR NOT (FALSE)` — a parenthesis or a NOT between the operator and the constant hides it. Both patterns require the constant to be the whole parenthesised term, so a real disjunct such as `OR (1 = priority)` is untouched. This narrows the gap rather than closing it, and is not meant to close it: an always-true expression is not lexically decidable in general, which is why the WHERE screen stays documented as defense-in-depth rather than a boundary.
Rebuilt with the generators so the artifacts carry the servicenow and crowdstrike tools that landed on staging alongside the mssql entries.
…nchor on Execute DISABLE and ENABLE were missing from the shared statement list, so `SELECT 1 DISABLE TRIGGER dbo.audit ON dbo.users` passed the read-only screen as a semicolon-less batch and turned auditing off. SET, BEGIN, COMMIT, and ROLLBACK are added with them, since session and transaction state are reachable the same way. FETCH is deliberately left out: OFFSET ... FETCH NEXT is the standard paging clause, and screening it would reject the ordinary paged SELECT. Execute Raw SQL anchored its allowlist on `\s`, which refused `EXEC(@SQL)` — the ordinary form of dynamic SQL, on the one operation meant to run it. It now uses `\b`, matching the read-only screen.
80b94d5 to
e1b7386
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e1b7386. Configure here.

Summary
options.connector, matching how the PostgreSQL and MySQL tools close the DNS-rebinding window.serverstays the original hostname so SNI and certificate validation are unaffectedsys.*catalog views rather thanINFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS, which silently drops any FK that references a unique index instead of a constraint@param1,@param2, …); identifiers are bracket-quoted after a strict character check; WHERE clauses are screened for stacked queries, UNION injection, comment truncation, tautologies,WAITFOR, catalog probes, andxp_/sp_proceduresNamed instances are not supported — please read
This branch deliberately does not support
instanceName, and that is a security decision rather than a gap.Resolving a named instance is a SQL Server Browser UDP/1434 request issued against the hostname, outside the connector. It therefore never goes through the pinned address, and node-mssql additionally deletes
portwheneverinstanceNameis set — so there is no configuration in which a named instance stays pinned to the IP the SSRF guard approved. Supporting it would reintroduce exactly the rebinding window the connector exists to close.Named instances now require a static TCP port. Configure the instance with a fixed port and connect to
host+port.If the team would rather take the exposure than the ergonomic cost, this is a one-commit revert — the restriction is confined to
createMSSQLConnectionand the block's connection subBlocks.Dependencies
mssql(12.7.0) and@types/mssql(12.3.0) were added toapps/sim/package.json, andbun.lockcarries them plus thetedious→@azure/identitytree they pull in (the lockfile diff is entirely that subtree — no unrelated resolutions moved).Anyone checking this branch out needs to run
bun installbefore it type-checks or runs; without itapp/api/tools/mssql/utils.tsreportsCannot find module 'mssql'.Type of Change
Testing
Nothing was executed against a live SQL Server. All verification is doc-derived: every endpoint, catalog view, response field, and driver behavior above is cited against the Microsoft Learn system-view docs and the tedious/node-mssql API docs in TSDoc at the call site. Repo gates pass:
lint,check:api-validation,tool-metadata:check,integration-catalog:check, andgenerate-docsreports no drift.Before merge this wants one pass against a real instance — in particular the introspection result shape on a low-privilege login, where the metadata-visibility filter returns a silently partial result rather than an error.
Checklist