fix(integrations): mssql guard gaps and Entra query, scope, and output findings

MSSQL read-only screen
- Screen RENAME, documented T-SQL DDL for Azure Synapse dedicated SQL pools and
  Analytics Platform System, which are reachable over TDS with exactly the
  connection fields this block exposes. `SELECT 1 RENAME OBJECT dbo.t TO t2`
  was a schema change passing an operation advertised as read-only.
- Screen the Service Broker family: RECEIVE as a word, and END/MOVE/GET
  CONVERSATION and SEND ON CONVERSATION as two-token phrases, since END closes
  every CASE. RECEIVE is a destructive read and END CONVERSATION WITH CLEANUP
  drops a conversation's messages.

MSSQL routes and block
- Build the insert statement before connecting, matching update and delete, so a
  bad identifier answers 400 instead of burning a TLS+login and returning 500.
- Declare `truncated`/`truncationReason` on the block, which the tools declare
  and the routes emit but the block left unreferenceable.

Microsoft Entra ID
- Pair `$count=true` with `ConsistencyLevel: eventual` conditionally. Graph
  documents `hasMembersWithLicenseErrors`, `isLicenseReconciliationNeeded`, and
  `identities/any(i:i/issuer)` as filterable only *without* advanced query
  parameters, and documents advanced queries as unsupported in Azure AD B2C
  tenants, so the unconditional pair broke filters that previously worked. When
  continuing from a nextLink the pairing is read off the link itself.
- Request `LicenseAssignment.Read.All` instead of `Directory.Read.All`. The
  latter was needed by `GET /subscribedSkus` alone, whose permission table names
  the former as least privileged and does not list the ReadWrite scope we hold.
- Enumerate the block's real output keys instead of a single `response` object
  no tool emits.

Author: waleedlatif1

apps/sim/app/api/tools/mssql/insert/route.ts

@@ -33,11 +33,27 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       `[${requestId}] Inserting data into ${params.table} on ${params.host}:${params.port}/${params.database}`
     )
 
+    /**
+     * Built before connecting so a bad identifier costs no TLS+login round trip
+     * and answers 400 like the update, delete, query, and execute routes, rather
+     * than falling through to the catch-all as a 500.
+     */
+    let built: { query: string; values: unknown[] }
+    try {
+      built = buildInsertQuery(params.table, params.data)
+    } catch (error) {
+      const message = getErrorMessage(error, 'Invalid statement')
+      logger.warn(`[${requestId}] Insert statement rejected: ${message}`)
+      return NextResponse.json(
+        { error: `Microsoft SQL Server insert failed: ${message}` },
+        { status: 400 }
+      )
+    }
+
     const pool = await createMSSQLConnection(params)
 
     try {
-      const { query, values } = buildInsertQuery(params.table, params.data)
-      const result = await executeQuery(pool, query, values)
+      const result = await executeQuery(pool, built.query, built.values)
 
       logger.info(`[${requestId}] Insert executed successfully, ${result.rowCount} row(s) inserted`)
 

apps/sim/app/api/tools/mssql/route-guards.test.ts

@@ -1,7 +1,7 @@
 /**
  * @vitest-environment node
  *
- * The update and delete routes build their statement before opening a
+ * The insert, update, and delete routes build their statement before opening a
  * connection, so a rejected WHERE clause or a bad identifier costs no TLS+login
  * round trip and answers 400 like the query and execute routes do.
  */
@@ -29,6 +29,7 @@ vi.mock('@sim/security/dns', () => ({
 }))
 
 import { POST as DELETE_POST } from '@/app/api/tools/mssql/delete/route'
+import { POST as INSERT_POST } from '@/app/api/tools/mssql/insert/route'
 import { POST as UPDATE_POST } from '@/app/api/tools/mssql/update/route'
 
 const connection = {
@@ -42,7 +43,7 @@ const connection = {
   connectionTimeout: 15000,
 }
 
-describe('MSSQL update and delete guards run before connecting', () => {
+describe('MSSQL insert, update, and delete guards run before connecting', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
@@ -68,6 +69,8 @@ describe('MSSQL update and delete guards run before connecting', () => {
   )
 
   it.each([
+    ['insert', INSERT_POST, { table: 'users-table', data: { a: 1 } }],
+    ['insert column', INSERT_POST, { table: 'users', data: { 'bad-col': 1 } }],
     ['update', UPDATE_POST, { table: 'users-table', data: { a: 1 }, where: 'id = 1' }],
     ['delete', DELETE_POST, { table: 'users-table', where: 'id = 1' }],
   ])('answers 400 for a bad identifier on %s without connecting', async (_op, handler, body) => {
@@ -78,6 +81,7 @@ describe('MSSQL update and delete guards run before connecting', () => {
   })
 
   it.each([
+    ['insert', INSERT_POST, { table: 'users', data: { a: 1 } }],
     ['update', UPDATE_POST, { table: 'users', data: { a: 1 }, where: 'id = 1' }],
     ['delete', DELETE_POST, { table: 'users', where: 'id = 1' }],
   ])('still runs an accepted %s statement', async (_op, handler, body) => {

apps/sim/app/api/tools/mssql/utils.test.ts

@@ -445,6 +445,64 @@ describe('read-only screens cover the rest of the session and transaction family
   })
 })
 
+describe('read-only screens cover RENAME and the Service Broker statement family', () => {
+  /**
+   * `RENAME` is documented T-SQL DDL for Azure Synapse dedicated SQL pools and
+   * Analytics Platform System, both reachable over TDS with the connection
+   * fields this block exposes — so a schema change was passing an operation
+   * advertised as read-only.
+   */
+  it.each([
+    ['RENAME OBJECT', 'SELECT 1 RENAME OBJECT dbo.Customer TO Customer1'],
+    ['RENAME OBJECT COLUMN', 'SELECT 1 RENAME OBJECT dbo.t COLUMN c1 TO c2'],
+    ['RENAME DATABASE', 'SELECT 1 RENAME DATABASE db1 TO db2'],
+    ['RECEIVE', 'SELECT 1 RECEIVE TOP(1) * FROM dbo.MyQueue'],
+    ['END CONVERSATION', "SELECT 1 END CONVERSATION '00000000-0000-0000-0000-000000000000'"],
+    [
+      'MOVE CONVERSATION',
+      "SELECT 1 MOVE CONVERSATION '00000000-0000-0000-0000-000000000000' TO '00000000-0000-0000-0000-000000000001'",
+    ],
+    ['GET CONVERSATION GROUP', 'SELECT 1 GET CONVERSATION GROUP @g FROM dbo.MyQueue'],
+    [
+      'SEND ON CONVERSATION',
+      "SELECT 1 SEND ON CONVERSATION '00000000-0000-0000-0000-000000000000' MESSAGE TYPE [t] ('x')",
+    ],
+  ])('rejects %s in the Query operation', (_label, query) => {
+    expect(validateReadOnlyQuery(query).isValid).toBe(false)
+  })
+
+  it.each([
+    ['RENAME OBJECT', 'id = 1 RENAME OBJECT dbo.t TO t2'],
+    ['RECEIVE', 'id = 1 RECEIVE TOP(1) * FROM dbo.MyQueue'],
+    ['END CONVERSATION', "id = 1 END CONVERSATION '00000000-0000-0000-0000-000000000000'"],
+    ['GET CONVERSATION GROUP', 'id = 1 GET CONVERSATION GROUP @g FROM dbo.MyQueue'],
+  ])('rejects %s in an update or delete WHERE clause', (_label, where) => {
+    expect(() => buildUpdateQuery('t', { a: 1 }, where)).toThrow()
+    expect(() => buildDeleteQuery('t', where)).toThrow()
+  })
+
+  /**
+   * The over-screening guard. `END` closes every `CASE`, and `rename`/`receive`
+   * are the stems of ordinary column names, so neither addition may cost the
+   * plain SELECTs this operation exists to run.
+   */
+  it('still accepts CASE … END and ordinary identifiers built on the new words', () => {
+    const allowed = [
+      "SELECT CASE WHEN status = 1 THEN 'on' ELSE 'off' END FROM dbo.jobs",
+      "SELECT CASE WHEN a = 1 THEN 'x' END AS conversation_state FROM dbo.t",
+      'SELECT renamed_at, rename_log, received_at, receive_queue FROM dbo.audit',
+      'SELECT conversation_id, get_flag, move_order, send_at, end_date FROM dbo.t',
+    ]
+
+    for (const query of allowed) {
+      expect(validateReadOnlyQuery(query)).toEqual({ isValid: true })
+    }
+
+    expect(() => buildUpdateQuery('audit', { a: 1 }, 'renamed_at > 0')).not.toThrow()
+    expect(() => buildDeleteQuery('audit', 'received_at > 0 AND conversation_id = 3')).not.toThrow()
+  })
+})
+
 describe('executeQuery result caps', () => {
   function makeCapPool(recordset: unknown[]) {
     return {

apps/sim/app/api/tools/mssql/utils.ts

@@ -276,14 +276,28 @@ export function toRowsResponseBody(result: MSSQLQueryResult, message: string) {
  * but it introduces a second statement in exactly the same semicolon-less way,
  * which is what this list exists to reject.
  *
+ * `RENAME` is documented T-SQL DDL — it applies to Azure Synapse Analytics
+ * dedicated SQL pools and Analytics Platform System, both of which speak TDS on
+ * port 1433 and are reachable with exactly the connection fields this block
+ * exposes. `SELECT 1 RENAME OBJECT dbo.Customer TO Customer1` is a valid
+ * semicolon-less batch that changes schema through an operation advertised as
+ * read-only, and `RENAME DATABASE` and `RENAME OBJECT … COLUMN … TO …` reach it
+ * the same way.
+ *
+ * `RECEIVE` is the Service Broker read that *removes* the messages it returns,
+ * so it is a write in everything but name. Its siblings — `END`/`MOVE`/`GET`
+ * `CONVERSATION` and `SEND ON CONVERSATION` — open with words that are ordinary
+ * identifiers (`END` closes every `CASE`), so they are screened as phrases in
+ * {@link MSSQL_STATEMENT_PHRASES} instead.
+ *
  * `FETCH` is deliberately **absent**: `OFFSET … FETCH NEXT` is the standard
  * T-SQL paging clause, so screening it would reject the ordinary paged SELECT
  * this operation exists to run. Word boundaries keep the additions off ordinary
  * identifiers — `settled`, `offset_value`, and `begin_date` all match nothing.
  * @see https://learn.microsoft.com/en-us/sql/t-sql/statements/statements
  */
 const MSSQL_STATEMENT_KEYWORDS =
-  /\b(?:insert|update|updatetext|writetext|readtext|delete|merge|drop|create|alter|truncate|disable|enable|set|begin|commit|rollback|grant|revoke|deny|exec|execute|backup|restore|shutdown|reconfigure|dbcc|kill|checkpoint|use|bulk|revert|setuser|openrowset|opendatasource|openquery|openxml|waitfor|into|deallocate)\b/i
+  /\b(?:insert|update|updatetext|writetext|readtext|delete|merge|drop|create|alter|truncate|rename|receive|disable|enable|set|begin|commit|rollback|grant|revoke|deny|exec|execute|backup|restore|shutdown|reconfigure|dbcc|kill|checkpoint|use|bulk|revert|setuser|openrowset|opendatasource|openquery|openxml|waitfor|into|deallocate)\b/i
 
 /**
  * The remaining session, transaction, cursor, and key-management statements,
@@ -297,10 +311,16 @@ const MSSQL_STATEMENT_KEYWORDS =
  * run. `DEALLOCATE` is the one exception and lives in the word list above — it
  * has no ordinary-identifier reading.
  *
- * None of these writes table data or schema, which is why they were missed; they
- * are screened because the file's stated rule is that a second statement is
- * rejected structurally, not by what it happens to do. `RAISERROR ... WITH LOG`
- * writes to the error log and the Windows application log, so it is not inert.
+ * Most of these write neither table data nor schema, which is why they were
+ * missed; they are screened because the file's stated rule is that a second
+ * statement is rejected structurally, not by what it happens to do.
+ * `RAISERROR ... WITH LOG` writes to the error log and the Windows application
+ * log, so it is not inert. The Service Broker conversation statements are not
+ * inert either: `END CONVERSATION ... WITH CLEANUP` drops every message in a
+ * conversation, `MOVE CONVERSATION` reassigns it, and `SEND ON CONVERSATION`
+ * enqueues a message — and the handles they need are enumerable through this
+ * same path, because the catalog screen applies only to WHERE clauses.
+ * @see https://learn.microsoft.com/en-us/sql/t-sql/statements/end-conversation-transact-sql
  * @see https://learn.microsoft.com/en-us/sql/t-sql/statements/statements
  */
 const MSSQL_STATEMENT_PHRASES: readonly RegExp[] = [
@@ -309,6 +329,8 @@ const MSSQL_STATEMENT_PHRASES: readonly RegExp[] = [
   /\bclose\s+(?:all\s+symmetric\s+keys|master\s+key|symmetric\s+key)\b/i,
   /\badd\s+signature\b/i,
   /\braiserror[\s\S]*?\bwith\s+log\b/i,
+  /\b(?:end|move|get)\s+conversation\b/i,
+  /\bsend\s+on\s+conversation\b/i,
 ]
 
 /** Matches the first screened statement phrase, or `null`. */

apps/sim/blocks/blocks/microsoft_ad.test.ts

@@ -2,7 +2,9 @@
  * @vitest-environment node
  */
 import { describe, expect, it } from 'vitest'
+import { SCOPE_DESCRIPTIONS } from '@/lib/oauth/utils'
 import { MicrosoftAdBlock } from '@/blocks/blocks/microsoft_ad'
+import * as microsoftAdTools from '@/tools/microsoft_ad'
 
 /**
  * The tri-state assertions run against `{ ...inputs, ...buildParams(inputs) }`, the shape the
@@ -121,9 +123,22 @@ describe('MicrosoftAdBlock', () => {
   /**
    * `User.ReadWrite.All` is listed on every `/users` read this block performs — list, get,
    * licenseDetails, registeredDevices, and ownedDevices — so `User.Read.All` was pure consent
-   * noise. `Directory.Read.All` and `GroupMember.ReadWrite.All` are deliberately retained:
-   * `GET /subscribedSkus` names neither `LicenseAssignment.ReadWrite.All` nor any scope left in
-   * this list, and `POST /groups/{id}/members/$ref` accepts `GroupMember.ReadWrite.All` only.
+   * noise.
+   *
+   * `Directory.Read.All` was required by exactly one call, `GET /subscribedSkus`, whose
+   * permission table names `LicenseAssignment.Read.All` as least privileged and
+   * `Directory.Read.All` only as a higher-privileged alternative — and does **not** list
+   * `LicenseAssignment.ReadWrite.All`, so the write scope this block already holds does not
+   * cover the read. Every other call is covered by a narrower scope in the list:
+   * directory roles by `RoleManagement.ReadWrite.Directory`, group members by
+   * `Group.ReadWrite.All`/`GroupMember.ReadWrite.All`, devices by `Device.Read.All`, audits by
+   * `AuditLog.Read.All`, service principals by `Application.Read.All`, user app-role
+   * assignments by `AppRoleAssignment.ReadWrite.All`, and CA policies by `Policy.Read.All`.
+   *
+   * `GroupMember.ReadWrite.All` is deliberately retained: `POST /groups/{id}/members/$ref`
+   * accepts it and nothing else in this list for a user member.
+   * @see https://learn.microsoft.com/en-us/graph/api/subscribedsku-list
+   * @see https://learn.microsoft.com/en-us/graph/api/group-post-members
    */
   describe('requested OAuth scopes', () => {
     const requiredScopes =
@@ -135,10 +150,39 @@ describe('MicrosoftAdBlock', () => {
       expect(requiredScopes).not.toContain('User.Read.All')
     })
 
+    it('requests the least-privileged scope for subscribedSkus, not Directory.Read.All', () => {
+      expect(requiredScopes).toContain('LicenseAssignment.Read.All')
+      expect(requiredScopes).not.toContain('Directory.Read.All')
+    })
+
     it('keeps the scopes no retained scope covers', () => {
-      expect(requiredScopes).toEqual(
-        expect.arrayContaining(['Directory.Read.All', 'GroupMember.ReadWrite.All'])
-      )
+      expect(requiredScopes).toEqual(expect.arrayContaining(['GroupMember.ReadWrite.All']))
+    })
+
+    it('describes every scope it requests', () => {
+      const undescribed = requiredScopes.filter((scope) => !SCOPE_DESCRIPTIONS[scope])
+      expect(undescribed).toEqual([])
+    })
+  })
+
+  /**
+   * `getBlockOutputs` derives the referenceable schema from `blockConfig.outputs`, so a single
+   * `response` entry made the tag dropdown offer `<microsoft_ad.response>` — which corresponds to
+   * nothing, since every tool puts its fields at the top level of `output` — and left the real
+   * outputs unreferenceable downstream.
+   */
+  describe('declared outputs', () => {
+    const toolOutputKeys = new Set(
+      Object.values(microsoftAdTools).flatMap((tool) => Object.keys(tool.outputs ?? {}))
+    )
+    const blockOutputKeys = new Set(Object.keys(MicrosoftAdBlock.outputs))
+
+    it('declares every key its tools emit', () => {
+      expect([...toolOutputKeys].filter((key) => !blockOutputKeys.has(key)).sort()).toEqual([])
+    })
+
+    it('declares nothing no tool emits', () => {
+      expect([...blockOutputKeys].filter((key) => !toolOutputKeys.has(key)).sort()).toEqual([])
     })
   })
 })