test✅: fail loudly instead of skipping when the sqlite setup breaks

The privilege-escalation tests skipped themselves when opening the in-memory
database or running AutoMigrate failed. Both depend on nothing outside the
process, so a failure there means the environment is genuinely broken - and a
security regression that quietly does not run is worse than one that is
missing, because CI stays green either way.

Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx

app/admin/apis/sys_user.go

@@ -1,6 +1,7 @@
 package apis
 
 import (
+	"errors"
 	"github.com/gin-gonic/gin/binding"
 	"go-admin/app/admin/models"
 	"golang.org/x/crypto/bcrypt"
@@ -15,6 +16,7 @@ import (
 	"go-admin/app/admin/service"
 	"go-admin/app/admin/service/dto"
 	"go-admin/common/actions"
+	"go-admin/common/middleware"
 )
 
 type SysUser struct {
@@ -149,12 +151,34 @@ func (e SysUser) Update(c *gin.Context) {
 		return
 	}
 
-	req.SetUpdateBy(user.GetUserId(c))
+	callerId := user.GetUserId(c)
+
+	// This route is in CasbinExclude so the personal-center screen can edit
+	// the caller's own record without a policy grant (see settings.go). That
+	// exclusion covers the whole route, not just the caller's own record, and
+	// the request carries the target userId in the body - so without this
+	// check here, any authenticated caller could edit any other user, up to
+	// and including their roleId. When the target is someone else, ask Casbin
+	// directly for the permission AuthCheckRole skipped.
+	if req.UserId != callerId {
+		allowed, err := middleware.EnforceRoleFor(c, c.Request.URL.Path, c.Request.Method)
+		if err != nil {
+			e.Logger.Error(err)
+			e.Error(500, err, err.Error())
+			return
+		}
+		if !allowed {
+			e.Error(http.StatusForbidden, errors.New("无权更新其他用户数据"), "对不起，您没有该接口访问权限，请联系管理员")
+			return
+		}
+	}
+
+	req.SetUpdateBy(callerId)
 
 	//数据权限检查
 	p := actions.GetPermissionFromContext(c)
 
-	err = s.Update(&req, p)
+	err = s.Update(&req, p, callerId)
 	if err != nil {
 		e.Logger.Error(err)
 		return

app/admin/apis/sys_user_privesc_test.go

@@ -0,0 +1,175 @@
+package apis
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/gin-gonic/gin"
+	"github.com/glebarez/sqlite"
+	mycasbin "github.com/go-admin-team/go-admin-core/v2/casbin"
+	jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth"
+	"github.com/go-admin-team/go-admin-core/v2/logger"
+	"github.com/go-admin-team/go-admin-core/v2/sdk"
+	"github.com/go-admin-team/go-admin-core/v2/sdk/pkg"
+	"gorm.io/gorm"
+
+	"go-admin/app/admin/models"
+)
+
+// PUT /api/v1/sys-user is in settings.go's CasbinExclude so the
+// personal-center screen (go-admin-ui's userInfo.vue) can edit the caller's
+// own record without holding a policy grant on this route. AuthCheckRole
+// skips Enforce entirely for an excluded route, so this file's job is to pin
+// what the handler itself now has to hold shut: the target userId comes from
+// the request body, and nothing upstream of the handler ever checked it
+// against the caller.
+
+// setupPrivescDB wires an in-memory database and a Casbin enforcer with an
+// empty policy - the state of a fresh install for any role but admin - under
+// a tenant unique to the calling test, so mycasbin's process-wide enforcer
+// cache can't hand one test's database to another.
+func setupPrivescDB(t *testing.T) (*gorm.DB, string) {
+	t.Helper()
+
+	// Fatalf, not Skipf: this database is in-memory sqlite with no external
+	// dependency, so failing to open or migrate it means the environment is
+	// actually broken. Skipping here would let these two anti-privesc
+	// regression tests silently stop running while CI stays green - a
+	// standing assertion that never fires is worse than no assertion.
+	db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{})
+	if err != nil {
+		t.Fatalf("sqlite unavailable: %v", err)
+	}
+	if err := db.AutoMigrate(&models.SysUser{}); err != nil {
+		t.Fatalf("automigrate: %v", err)
+	}
+
+	tenant := "sys-user-privesc-" + t.Name()
+
+	previousInterval := mycasbin.ReloadInterval
+	mycasbin.ReloadInterval = 0 // opt out of the background reload goroutine; the test never writes a policy
+	t.Cleanup(func() { mycasbin.ReloadInterval = previousInterval })
+
+	e := mycasbin.Setup(db, tenant)
+	previousEnforcer := sdk.Runtime.GetCasbinByTenant(tenant)
+	sdk.Runtime.SetCasbinByTenant(tenant, e)
+	t.Cleanup(func() { sdk.Runtime.SetCasbinByTenant(tenant, previousEnforcer) })
+
+	return db, tenant
+}
+
+// callUpdate drives SysUser.Update the way the router does for an
+// authenticated, non-admin caller: JWT claims already decoded into the
+// context (that is jwtauth's job, not this handler's) and a database - but
+// without AuthCheckRole, since that middleware never runs Enforce for this
+// route at all.
+func callUpdate(t *testing.T, db *gorm.DB, tenant string, callerId int, body map[string]interface{}) *httptest.ResponseRecorder {
+	t.Helper()
+	gin.SetMode(gin.TestMode)
+
+	raw, err := json.Marshal(body)
+	if err != nil {
+		t.Fatalf("marshal request body: %v", err)
+	}
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Request = httptest.NewRequest(http.MethodPut, "/api/v1/sys-user", bytes.NewReader(raw))
+	c.Request.Host = tenant
+	c.Request.Header.Set("Content-Type", "application/json")
+
+	c.Set("db", db)
+	c.Set(pkg.LoggerKey, logger.NewHelper(logger.DefaultLogger))
+	c.Set(jwt.JwtPayloadKey, jwt.MapClaims{
+		"identity": float64(callerId),
+		"rolekey":  "ordinary-role", // holds no Casbin policy anywhere in this test
+	})
+
+	SysUser{}.Update(c)
+	return w
+}
+
+// TestUpdate_CannotEscalatePrivilegeThroughAnotherUsersRecord is the
+// regression for H6. Before the fix, an ordinary authenticated user could PUT
+// a body naming another user's id and change that user's roleId - the route
+// being Casbin-excluded meant no permission check ever ran, and the data
+// permission scope that would otherwise gate this is off by default.
+func TestUpdate_CannotEscalatePrivilegeThroughAnotherUsersRecord(t *testing.T) {
+	db, tenant := setupPrivescDB(t)
+
+	victim := models.SysUser{Username: "bob", NickName: "Bob", RoleId: 2, DeptId: 1, Status: "1"}
+	if err := db.Create(&victim).Error; err != nil {
+		t.Fatal(err)
+	}
+	attacker := models.SysUser{Username: "alice", NickName: "Alice", RoleId: 2, DeptId: 1, Status: "1"}
+	if err := db.Create(&attacker).Error; err != nil {
+		t.Fatal(err)
+	}
+
+	const elevatedRoleId = 1 // a role the attacker does not hold and has no policy for
+
+	callUpdate(t, db, tenant, attacker.UserId, map[string]interface{}{
+		"userId":   victim.UserId,
+		"username": victim.Username,
+		"nickName": "pwned",
+		"phone":    "13800000000",
+		"email":    "bob@example.com",
+		"roleId":   elevatedRoleId,
+		"deptId":   victim.DeptId,
+		"status":   victim.Status,
+	})
+
+	var after models.SysUser
+	if err := db.First(&after, victim.UserId).Error; err != nil {
+		t.Fatal(err)
+	}
+	if after.RoleId == elevatedRoleId {
+		t.Fatalf("an attacker with no Casbin permission on this route escalated the victim's roleId to %d", after.RoleId)
+	}
+	if after.NickName == "pwned" {
+		t.Fatalf("an attacker with no Casbin permission on this route modified another user's record: %+v", after)
+	}
+}
+
+// TestUpdate_SelfEditCannotChangePrivilegedFields covers the case the
+// CasbinExclude entry exists for: the personal-center screen has to keep
+// working for the caller's own record. The fields that screen exposes
+// (nickName/phone/email/sex) must still save, while roleId/deptId/status stay
+// whatever the database already had even if the request carries something
+// else - a compromised or hand-crafted client is the only way that request
+// would ever differ from what the honest form sends.
+func TestUpdate_SelfEditCannotChangePrivilegedFields(t *testing.T) {
+	db, tenant := setupPrivescDB(t)
+
+	self := models.SysUser{Username: "carol", NickName: "Carol", RoleId: 2, DeptId: 1, Status: "1"}
+	if err := db.Create(&self).Error; err != nil {
+		t.Fatal(err)
+	}
+
+	const elevatedRoleId = 1
+
+	callUpdate(t, db, tenant, self.UserId, map[string]interface{}{
+		"userId":   self.UserId,
+		"username": self.Username,
+		"nickName": "Carol Updated",
+		"phone":    "13900000000",
+		"email":    "carol@example.com",
+		"roleId":   elevatedRoleId, // tampered; must not take effect
+		"deptId":   self.DeptId,
+		"status":   self.Status,
+	})
+
+	var after models.SysUser
+	if err := db.First(&after, self.UserId).Error; err != nil {
+		t.Fatal(err)
+	}
+	if after.RoleId == elevatedRoleId {
+		t.Fatalf("a self-edit changed the caller's own roleId to %d", after.RoleId)
+	}
+	if after.NickName != "Carol Updated" {
+		t.Fatalf("the legitimate personal-center edit did not go through: %+v", after)
+	}
+}

app/admin/service/sys_user.go

@@ -84,7 +84,16 @@ func (e *SysUser) Insert(c *dto.SysUserInsertReq) error {
 }
 
 // Update 修改SysUser对象
-func (e *SysUser) Update(c *dto.SysUserUpdateReq, p *actions.DataPermission) error {
+//
+// callerId is who is asking, not who SetUpdateBy recorded - that field only
+// says who to blame, it never constrained who could be edited. When the
+// target is the caller themselves, roleId/deptId/status are kept at whatever
+// the database already has no matter what the request body carries: this is
+// the personal-center screen's route (see CasbinExclude in settings.go, and
+// the check in the API handler ahead of this call), and letting a caller
+// grant themselves a different role or department through it would be a
+// privilege escalation the exclusion was never meant to open.
+func (e *SysUser) Update(c *dto.SysUserUpdateReq, p *actions.DataPermission, callerId int) error {
 	var err error
 	var model models.SysUser
 	db := e.Orm.Scopes(
@@ -98,6 +107,11 @@ func (e *SysUser) Update(c *dto.SysUserUpdateReq, p *actions.DataPermission) err
 		return errors.New("无权更新该数据")
 
 	}
+	if model.UserId == callerId {
+		c.RoleId = model.RoleId
+		c.DeptId = model.DeptId
+		c.Status = model.Status
+	}
 	c.Generate(&model)
 	update := e.Orm.Model(&model).Where("user_id = ?", &model.UserId).Omit("password", "salt").Updates(&model)
 	if err = update.Error; err != nil {

common/middleware/permission.go

@@ -59,6 +59,33 @@ func AuthCheckRole() gin.HandlerFunc {
 	}
 }
 
+// EnforceRoleFor reports whether the caller's role has explicit Casbin
+// permission to act on path with method.
+//
+// AuthCheckRole never calls Enforce for a route CasbinExclude lists - that
+// is the whole point of the list. A handler on such a route can still need
+// the real answer for part of what it does: sys_user.go's Update shares its
+// excluded route between the personal-center screen editing the caller's own
+// record (which is why the route is excluded at all) and an admin editing
+// someone else's, and only the second case is meant to require a policy
+// grant. That handler asks here instead of assuming the middleware already
+// checked.
+func EnforceRoleFor(c *gin.Context, path, method string) (bool, error) {
+	data, ok := c.Get(jwtauth.JwtPayloadKey)
+	if !ok {
+		return false, nil
+	}
+	v, ok := data.(jwtauth.MapClaims)
+	if !ok {
+		return false, nil
+	}
+	if v["rolekey"] == "admin" {
+		return true, nil
+	}
+	e := sdk.Runtime.GetCasbinByTenant(c.Request.Host)
+	return e.Enforce(v["rolekey"], path, method)
+}
+
 // excludedFromCasbin reports whether the route skips the permission check.
 //
 // It runs for every non-admin request, so the order matters: the method rules