address comments

Signed-off-by: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com>

Author: AmoebaProtozoa

pkg/util/sem/v2/restricted_statement.go

@@ -40,6 +40,10 @@ func isRestrictedRole(userName, hostname string) bool {
 
 // IsRestrictedStatement returns a non-nil error when strict SEM forbids stmt.
 // It runs the check unconditionally; callers gate on IsStrictEnabled.
+//
+// The switch is a default-deny allow-list: any StmtNode not matched below is
+// rejected by the trailing notSupported call. When adding a new statement
+// type, decide explicitly whether it should be allowed or rejected.
 func IsRestrictedStatement(stmt ast.Node) error {
 	switch x := stmt.(type) {
 	case *ast.DeallocateStmt,
@@ -62,7 +66,28 @@ func IsRestrictedStatement(stmt ast.Node) error {
 		*ast.CreateBindingStmt,
 		*ast.DropBindingStmt,
 		*ast.SetBindingStmt,
-		*ast.CompactTableStmt:
+		*ast.CompactTableStmt,
+		// Carried over from the previous default-allow fall-through. They are
+		// preserved as allow here to keep behavior identical to before the
+		// switch was tightened to default-deny; revisit each in a follow-up.
+		*ast.AddQueryWatchStmt,
+		*ast.AlterRangeStmt,
+		*ast.CalibrateResourceStmt,
+		*ast.CallStmt,
+		*ast.CancelDistributionJobStmt,
+		*ast.CreateStatisticsStmt,
+		*ast.DistributeTableStmt,
+		*ast.DropProcedureStmt,
+		*ast.DropQueryWatchStmt,
+		*ast.DropStatisticsStmt,
+		*ast.GrantProxyStmt,
+		*ast.HelpStmt,
+		*ast.ImportIntoActionStmt,
+		*ast.ImportIntoStmt,
+		*ast.RecommendIndexStmt,
+		*ast.RefreshStatsStmt,
+		*ast.RestartStmt,
+		*ast.TrafficStmt:
 		return nil
 	case *ast.LoadDataStmt:
 		return verifyLoadData(x)
@@ -87,7 +112,7 @@ func IsRestrictedStatement(stmt ast.Node) error {
 		return notSupported("SPLIT REGION")
 	}
 
-	return nil
+	return notSupported(fmt.Sprintf("Unsupported statement: %T", stmt))
 }
 
 func verifyDDL(stmt ast.DDLNode) error {
@@ -158,7 +183,6 @@ func verifySimple(stmt ast.Node) error {
 		*ast.KillStmt,
 		*ast.BinlogStmt,
 		*ast.DropStatsStmt,
-		*ast.SetDefaultRoleStmt,
 		*ast.AdminStmt,
 		*ast.GrantStmt,
 		*ast.RevokeStmt,
@@ -194,12 +218,50 @@ func verifySimple(stmt ast.Node) error {
 		}
 		return nil
 	case *ast.SetRoleStmt:
+		// SET ROLE DEFAULT|ALL|ALL EXCEPT can implicitly activate a granted
+		// restricted role (cloud_admin), bypassing the RoleList-only check.
+		// Reject those wildcard forms; allow NONE (deactivates everything)
+		// and the regular form after the RoleList contains no restricted role.
+		switch s.SetRoleOpt {
+		case ast.SetRoleNone:
+			return nil
+		case ast.SetRoleDefault:
+			return notSupported("SET ROLE DEFAULT")
+		case ast.SetRoleAll:
+			return notSupported("SET ROLE ALL")
+		case ast.SetRoleAllExcept:
+			return notSupported("SET ROLE ALL EXCEPT")
+		}
 		for _, role := range s.RoleList {
 			if isRestrictedRole(role.Username, role.Hostname) {
 				return notSupported(fmt.Sprintf("SET ROLE %s", role))
 			}
 		}
 		return nil
+	case *ast.SetDefaultRoleStmt:
+		// SET DEFAULT ROLE is the matching write-side: it persists the role
+		// list a user activates via SET ROLE DEFAULT. Block any attempt to
+		// (a) target a restricted account (root, cloud_admin) so its default
+		// role set cannot be tampered with, (b) install a restricted role as
+		// someone else's default, and (c) use ALL, which would silently
+		// include cloud_admin when it has been granted to the target.
+		for _, user := range s.UserList {
+			if isRestrictedUser(user.Username, user.Hostname) {
+				return notSupported(fmt.Sprintf("SET DEFAULT ROLE TO %s", user))
+			}
+		}
+		switch s.SetRoleOpt {
+		case ast.SetRoleNone:
+			return nil
+		case ast.SetRoleAll:
+			return notSupported("SET DEFAULT ROLE ALL")
+		}
+		for _, role := range s.RoleList {
+			if isRestrictedRole(role.Username, role.Hostname) {
+				return notSupported(fmt.Sprintf("SET DEFAULT ROLE %s", role))
+			}
+		}
+		return nil
 	case *ast.AlterInstanceStmt:
 		return notSupported("ALTER INSTANCE")
 	case *ast.ShutdownStmt:

pkg/util/sem/v2/restricted_statement_test.go

@@ -147,6 +147,25 @@ func TestStatement_RestrictedUserOperations(t *testing.T) {
 	mustReject(t, `SET ROLE 'cloud_admin'@'%'`, "SET ROLE")
 	mustPass(t, `SET ROLE 'role1'@'%'`)
 
+	// SET ROLE wildcard forms can implicitly activate cloud_admin if granted,
+	// so they are blocked outright. NONE is safe (deactivates everything).
+	mustReject(t, `SET ROLE DEFAULT`, "SET ROLE DEFAULT")
+	mustReject(t, `SET ROLE ALL`, "SET ROLE ALL")
+	mustReject(t, `SET ROLE ALL EXCEPT 'role1'@'%'`, "SET ROLE ALL EXCEPT")
+	mustPass(t, `SET ROLE NONE`)
+
+	// SET DEFAULT ROLE writes the role list activated by SET ROLE DEFAULT, so
+	// it must enforce the same protection on both ends: no restricted role can
+	// be installed as a default, the target user list cannot include a
+	// restricted account, and ALL is rejected because it would silently include
+	// cloud_admin when granted.
+	mustReject(t, `SET DEFAULT ROLE 'cloud_admin'@'%' TO 'alice'@'%'`, "SET DEFAULT ROLE")
+	mustReject(t, `SET DEFAULT ROLE 'role1'@'%' TO 'root'@'%'`, "SET DEFAULT ROLE TO")
+	mustReject(t, `SET DEFAULT ROLE 'role1'@'%' TO 'cloud_admin'@'%'`, "SET DEFAULT ROLE TO")
+	mustReject(t, `SET DEFAULT ROLE ALL TO 'alice'@'%'`, "SET DEFAULT ROLE ALL")
+	mustPass(t, `SET DEFAULT ROLE 'role1'@'%' TO 'alice'@'%'`)
+	mustPass(t, `SET DEFAULT ROLE NONE TO 'alice'@'%'`)
+
 	mustPass(t, `CREATE USER 'x'@'%' IDENTIFIED BY 'pw'`)
 	mustPass(t, `ALTER USER 'x'@'%' IDENTIFIED BY 'pw2'`)
 	mustPass(t, `SET PASSWORD FOR 'x'@'%' = 'pw3'`)

tests/realtikvtest/pipelineddmltest/pipelineddml_test.go

@@ -345,11 +345,12 @@ func TestPipelinedDMLNegative(t *testing.T) {
 	tk.MustQuery("show warnings").CheckContain("Pipelined DML can not be used when tidb_constraint_check_in_place=ON. Fallback to standard mode")
 	tk.MustExec("set @@tidb_constraint_check_in_place = 0")
 
-	// strict SEM (starter/essential deployments)
+	// strict SEM (starter/essential deployments). Use t.Cleanup so a mid-test
+	// assertion failure cannot leak the global strict flag into later tests.
 	semv2.EnableStrict()
+	t.Cleanup(semv2.DisableStrict)
 	tk.MustExec("insert into t values(12, 12)")
 	tk.MustQuery("show warnings").CheckContain("Pipelined DML is not supported in this deployment. Fallback to standard mode")
-	semv2.DisableStrict()
 }
 
 func compareTables(t *testing.T, tk *testkit.TestKit, t1, t2 string) {