VAULT-43451: batching query instead of splitting based on semi-colen in MSSQL while deletion (#13474) (#14418)

* batching query instead of splitting based on semi colen

* added tests

* updated test

* updated test

* updated

* added changelog

* updated the query map

Co-authored-by: suraj-simha <suraj.s@hashicorp.com>

changelog/_13474.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+database/mssql: Fix dynamic secret revocation by executing custom statements as a single batch instead of splitting on semicolons
+```

plugins/database/mssql/mssql.go

@@ -189,20 +189,19 @@ func (m *MSSQL) DeleteUser(ctx context.Context, req dbplugin.DeleteUserRequest)
 
 	merr := &multierror.Error{}
 
-	// Execute each query
+	// Execute each command string as a single batch rather than splitting on
+	// semicolons. Splitting on ";" destroys the batch boundary, causing subsequent
+	// statements (e.g. DROP USER) to run in the wrong database context and fail.
 	for _, stmt := range req.Statements.Commands {
-		for _, query := range strutil.ParseArbitraryStringSlice(stmt, ";") {
-			query = strings.TrimSpace(query)
-			if len(query) == 0 {
-				continue
-			}
+		if strings.TrimSpace(stmt) == "" {
+			continue
+		}
 
-			m := map[string]string{
-				"name": req.Username,
-			}
-			if err := dbtxn.ExecuteDBQueryDirect(ctx, db, m, query); err != nil {
-				merr = multierror.Append(merr, err)
-			}
+		m := map[string]string{
+			"name": req.Username,
+		}
+		if err := dbtxn.ExecuteDBQueryDirect(ctx, db, m, stmt); err != nil {
+			merr = multierror.Append(merr, err)
 		}
 	}
 

plugins/database/mssql/mssql_test.go

@@ -405,6 +405,75 @@ func TestMSSQLDeleteUserContainedDB(t *testing.T) {
 	assertContainedDBCredsDoNotExist(t, connURL, dbUser)
 }
 
+// TestMSSQLDeleteUserWithBatch verifies that USE database context switches work correctly when statements are executed as a single batch.
+func TestMSSQLDeleteUserWithBatch(t *testing.T) {
+	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
+	defer cleanup()
+
+	dbUser := "vaultuser"
+	initPassword := "p4$sw0rd"
+
+	// Create a test database
+	setupDB, err := sql.Open("mssql", connURL)
+	if err != nil {
+		t.Fatalf("Failed to open connection for setup: %s", err)
+	}
+	defer setupDB.Close()
+
+	_, err = setupDB.ExecContext(context.Background(), "CREATE DATABASE testdb")
+	if err != nil {
+		t.Fatalf("Failed to create test database: %s", err)
+	}
+
+	// Create login in master and user in testdb
+	createSQL := `
+CREATE LOGIN [{{name}}] WITH PASSWORD = '{{password}}';
+USE [testdb];
+CREATE USER [{{name}}] FOR LOGIN [{{name}}];`
+
+	err = createTestMSSQLUser(connURL, dbUser, initPassword, createSQL)
+	if err != nil {
+		t.Fatalf("Failed to create user: %s", err)
+	}
+
+	assertCredsExist(t, connURL, dbUser, initPassword)
+
+	initReq := dbplugin.InitializeRequest{
+		Config: map[string]interface{}{
+			"connection_url": connURL,
+		},
+		VerifyConnection: true,
+	}
+
+	db := new()
+	dbtesting.AssertInitializeCircleCiTest(t, db, initReq)
+	defer dbtesting.AssertClose(t, db)
+
+	deleteReq := dbplugin.DeleteUserRequest{
+		Username: dbUser,
+		Statements: dbplugin.Statements{
+			Commands: []string{
+				`USE [testdb]; DROP USER [{{name}}]; USE [master]; DROP LOGIN [{{name}}];`,
+			},
+		},
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
+	defer cancel()
+
+	deleteResp, err := db.DeleteUser(ctx, deleteReq)
+	if err != nil {
+		t.Fatalf("Failed to delete user with batch statement: %s", err)
+	}
+
+	expectedResp := dbplugin.DeleteUserResponse{}
+	if !reflect.DeepEqual(deleteResp, expectedResp) {
+		t.Fatalf("Fields missing from expected response: Actual: %#v", deleteResp)
+	}
+
+	assertCredsDoNotExist(t, connURL, dbUser, initPassword)
+}
+
 func TestMSSQLContainedDBSQLSanitization(t *testing.T) {
 	cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
 	defer cleanup()