fix: add sp_executesql/xp_cmdshell to mutating keywords, add error logging

Address Copilot review:
- Add sp_executesql and xp_cmdshell to SQL Server mutating keyword
  pattern (callable without EXEC as first statement in a batch)
- Add parameter-aware error logging to executeReadOnly() matching
  the non-readonly path
- Add tests for implicit sp_executesql and xp_cmdshell in CTEs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: tianzhou

src/connectors/sqlserver/index.ts

@@ -764,7 +764,17 @@ export class SQLServerConnector implements Connector {
         });
       }
 
-      const result = await request.query(processedSQL);
+      let result;
+      try {
+        result = await request.query(processedSQL);
+      } catch (error) {
+        console.error(`[SQL Server executeReadOnly] ERROR: ${(error as Error).message}`);
+        console.error(`[SQL Server executeReadOnly] SQL: ${processedSQL}`);
+        if (parameters && parameters.length > 0) {
+          console.error(`[SQL Server executeReadOnly] Parameters: ${JSON.stringify(parameters)}`);
+        }
+        throw error;
+      }
       return {
         rows: result.recordset || [],
         rowCount: result.rowsAffected[0] || 0,

src/utils/__tests__/allowed-keywords.test.ts

@@ -263,6 +263,14 @@ describe("isReadOnlySQL", () => {
       expect(isReadOnlySQL("WITH cte AS (SELECT 1) EXECUTE('DELETE FROM users')", "sqlserver")).toBe(false);
     });
 
+    it("should reject implicit sp_executesql (no EXEC prefix) inside a CTE", () => {
+      expect(isReadOnlySQL("WITH cte AS (SELECT 1) sp_executesql N'DELETE FROM users'", "sqlserver")).toBe(false);
+    });
+
+    it("should reject xp_cmdshell inside a CTE", () => {
+      expect(isReadOnlySQL("WITH cte AS (SELECT 1) xp_cmdshell 'del *.*'", "sqlserver")).toBe(false);
+    });
+
     it("should not reject EXEC/EXECUTE inside string literals", () => {
       expect(isReadOnlySQL("SELECT * FROM users WHERE name = 'EXEC is a keyword'", "sqlserver")).toBe(true);
     });

src/utils/allowed-keywords.ts

@@ -51,13 +51,15 @@ const mutatingPatternWithReplace = new RegExp(
 );
 
 /**
- * Extended pattern for SQL Server: adds EXEC/EXECUTE which are T-SQL dynamic
- * SQL primitives that can run arbitrary (including mutating) statements.
- * sp_executesql and xp_cmdshell are always invoked via EXEC so are covered
- * transitively.
+ * Extended pattern for SQL Server: adds T-SQL dynamic SQL primitives that can
+ * run arbitrary (including mutating) statements.
+ * - EXEC/EXECUTE: direct dynamic SQL execution
+ * - sp_executesql: system proc for parameterized dynamic SQL (callable without
+ *   EXEC as the first statement in a batch)
+ * - xp_cmdshell: OS command execution
  */
 const mutatingPatternSqlServer = new RegExp(
-  `\\b(?:${[...mutatingKeywords, "execute", "exec"].join("|")})\\b`,
+  `\\b(?:${[...mutatingKeywords, "execute", "exec", "sp_executesql", "xp_cmdshell"].join("|")})\\b`,
   "i",
 );