Skip to content

Commit 70eafd8

Browse files
address comments
Signed-off-by: AmoebaProtozoa <8039876+AmoebaProtozoa@users.noreply.github.com>
1 parent f4b0816 commit 70eafd8

3 files changed

Lines changed: 28 additions & 9 deletions

File tree

cmd/tidb-server/main.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,9 +1200,11 @@ func setupSEM() {
12001200
cfg := config.GetGlobalConfig()
12011201

12021202
// Strict SEM is gated on the Starter deployment mode (next-gen only;
1203-
// deploymode.IsStarter already returns false on classic builds), and it
1204-
// layers on top of an enabled SEM, so it still requires
1205-
// security.enable-sem=true.
1203+
// deploymode.IsStarter already returns false on classic builds). It
1204+
// layers on top of an enabled semv2 (not classic sem), so it requires
1205+
// both security.enable-sem=true and a non-empty security.sem-config —
1206+
// the hint allow-list reads sysvar visibility via semv2 helpers, which
1207+
// silently return false when semv2 is uninitialized.
12061208
strictSEM := deploymode.IsStarter()
12071209

12081210
if !cfg.Security.EnableSEM {
@@ -1218,6 +1220,10 @@ func setupSEM() {
12181220
logutil.BgLogger().Fatal("failed to enable SEM", zap.Error(err))
12191221
}
12201222
} else {
1223+
if strictSEM {
1224+
logutil.BgLogger().Warn("Starter deployment mode requires security.sem-config to enable strict SEM; ignoring")
1225+
strictSEM = false
1226+
}
12211227
sem.Enable()
12221228
}
12231229

pkg/util/sem/v2/restricted_statement.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,12 @@ import (
2222
"github.com/pingcap/tidb/pkg/util/dbterror/plannererrors"
2323
)
2424

25-
// Literal identifiers protected from DROP/RENAME/role-change operations.
26-
// tidb-cse drives these off a keyspace username policy; upstream has no such
27-
// concept, so we match the raw names at host '%'.
25+
// Literal identifiers protected from account modification: restrictedUsers
26+
// cannot be dropped, renamed, or have their default-role list rewritten;
27+
// restrictedRoles cannot be granted, revoked, activated via SET ROLE, or
28+
// installed as anyone's default. tidb-cse drives these off a keyspace
29+
// username policy; upstream has no such concept, so we match the raw names
30+
// at host '%'.
2831
var (
2932
restrictedUsers = []string{"cloud_admin", "root"}
3033
restrictedRoles = []string{"cloud_admin"}
@@ -183,7 +186,6 @@ func verifySimple(stmt ast.Node) error {
183186
*ast.KillStmt,
184187
*ast.BinlogStmt,
185188
*ast.DropStatsStmt,
186-
*ast.AdminStmt,
187189
*ast.GrantStmt,
188190
*ast.RevokeStmt,
189191
*ast.NonTransactionalDMLStmt,
@@ -271,7 +273,7 @@ func verifySimple(stmt ast.Node) error {
271273
case *ast.SetResourceGroupStmt:
272274
return notSupported("SET RESOURCE GROUP")
273275
}
274-
return notSupported(fmt.Sprintf("Unsupported Executor %T", stmt))
276+
return notSupported(fmt.Sprintf("Unsupported statement: %T", stmt))
275277
}
276278

277279
func verifyBRIE(stmt *ast.BRIEStmt) error {
@@ -399,7 +401,7 @@ func verifyAdmin(stmt *ast.AdminStmt) error {
399401
case ast.AdminShowSlow:
400402
return notSupported("ADMIN SHOW SLOW")
401403
}
402-
return notSupported(fmt.Sprintf("Unsupported statement: %T", stmt))
404+
return notSupported(fmt.Sprintf("Unsupported ADMIN type %d", stmt.Tp))
403405
}
404406

405407
func notSupported(feature string) error {

pkg/util/sem/v2/restricted_statement_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,14 @@ func TestStatement_Transactional(t *testing.T) {
250250
mustPass(t, sql)
251251
}
252252
}
253+
254+
// TestStatement_DefaultDeny pins the allow-list semantics: an unrecognized
255+
// StmtNode must be rejected. SearchCaseStmt is a real StmtNode that is only
256+
// valid inside stored-procedure bodies, so the top-level dispatcher never
257+
// lists it — this is the closest stand-in for "some future stmt type the
258+
// dispatcher does not know about yet."
259+
func TestStatement_DefaultDeny(t *testing.T) {
260+
err := IsRestrictedStatement(&ast.SearchCaseStmt{})
261+
require.Error(t, err, "default-deny: unknown StmtNode must be rejected")
262+
require.Contains(t, err.Error(), "Unsupported statement")
263+
}

0 commit comments

Comments
 (0)