test(sysadmin): cover author-only CAS, audit rollback, and HTTP gates

Verify concurrent revision conflicts, null/string rejection, plain-user
denials, and PG lock/read paths for the new policy key.

Co-authored-by: Cursor <cursoragent@cursor.com>

internal/server/admin_system_handlers_test.go

@@ -12,34 +12,68 @@ import (
 
 	"chat-agent-system/internal/auth"
 	"chat-agent-system/internal/sysadmin"
+
+	"github.com/jackc/pgx/v5"
 )
 
 type fakeSettingsStore struct {
 	mu    sync.Mutex
-	items map[string]json.RawMessage
+	items map[string]struct {
+		value    json.RawMessage
+		revision int64
+	}
 }
 
 func newFakeSettingsStore() *fakeSettingsStore {
-	return &fakeSettingsStore{items: map[string]json.RawMessage{}}
+	return &fakeSettingsStore{items: map[string]struct {
+		value    json.RawMessage
+		revision int64
+	}{}}
 }
 
-func (f *fakeSettingsStore) Get(_ context.Context, key string) (json.RawMessage, *int64, *time.Time, error) {
+func (f *fakeSettingsStore) Get(_ context.Context, key string) (json.RawMessage, *int64, *time.Time, int64, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	raw, ok := f.items[key]
+	entry, ok := f.items[key]
 	if !ok {
-		return nil, nil, nil, sysadmin.ErrSettingNotFound
+		return nil, nil, nil, 0, sysadmin.ErrSettingNotFound
 	}
 	at := time.Now()
 	by := int64(1)
-	return raw, &by, &at, nil
+	return append(json.RawMessage(nil), entry.value...), &by, &at, entry.revision, nil
 }
 
-func (f *fakeSettingsStore) Upsert(_ context.Context, key string, value json.RawMessage, _ int64) (time.Time, error) {
+func (f *fakeSettingsStore) Upsert(_ context.Context, key string, value json.RawMessage, _ int64) (time.Time, int64, error) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	f.items[key] = append(json.RawMessage(nil), value...)
-	return time.Now(), nil
+	prev := f.items[key]
+	f.items[key] = struct {
+		value    json.RawMessage
+		revision int64
+	}{value: append(json.RawMessage(nil), value...), revision: prev.revision}
+	return time.Now(), prev.revision, nil
+}
+
+func (f *fakeSettingsStore) UpsertCAS(_ context.Context, key string, value json.RawMessage, _, expectedRevision int64, _ string) (time.Time, int64, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	entry, ok := f.items[key]
+	if !ok {
+		return time.Time{}, 0, sysadmin.ErrSettingNotFound
+	}
+	if entry.revision != expectedRevision {
+		return time.Time{}, 0, sysadmin.ErrSettingRevisionConflict
+	}
+	newRev := entry.revision + 1
+	f.items[key] = struct {
+		value    json.RawMessage
+		revision int64
+	}{value: append(json.RawMessage(nil), value...), revision: newRev}
+	return time.Now(), newRev, nil
+}
+
+func (f *fakeSettingsStore) GetForUpdate(ctx context.Context, _ pgx.Tx, key string) (json.RawMessage, *int64, *time.Time, int64, error) {
+	return f.Get(ctx, key)
 }
 
 // registerTestUser 注册并登录一个普通用户，返回其 Bearer Token。
@@ -242,3 +276,50 @@ func TestAdminSystemSettings_UpdateWithoutStoreReturns503(t *testing.T) {
 		t.Fatalf("expected 503 without settings store, got %d %s", w.Code, w.Body.String())
 	}
 }
+
+func TestAdminSystemSettings_AuthorOnlyPolicyCAS(t *testing.T) {
+	srv := newTestServer(t)
+	srv.cfg.AllowInMemory = false
+	store := newFakeSettingsStore()
+	// 模拟迁移种子行
+	_, _, _ = store.Upsert(context.Background(), sysadmin.SettingKeyAllowAuthorOnlyApproval, json.RawMessage(`false`), 0)
+	srv.sysadmin = sysadmin.NewService(srv.cfg, sysadmin.Options{Store: store})
+	token := loginTestAdmin(t, srv)
+	plainToken := registerTestUser(t, srv, "plain_author_only_probe")
+
+	path := "/api/v1/admin/system/settings/" + sysadmin.SettingKeyAllowAuthorOnlyApproval
+
+	// 非管理员禁止
+	if w := doSysadminRequest(t, srv, http.MethodPut, path, plainToken, `{"value":true,"expected_revision":0}`); w.Code != http.StatusForbidden && w.Code != http.StatusUnauthorized {
+		t.Fatalf("plain user: expected 401/403, got %d %s", w.Code, w.Body.String())
+	}
+
+	// 缺少 expected_revision → 400
+	if w := doSysadminRequest(t, srv, http.MethodPut, path, token, `{"value":true}`); w.Code != http.StatusBadRequest {
+		t.Fatalf("missing revision: expected 400, got %d %s", w.Code, w.Body.String())
+	}
+
+	// null / 字符串布尔 → 400
+	for _, body := range []string{`{"value":null,"expected_revision":0}`, `{"value":"true","expected_revision":0}`} {
+		if w := doSysadminRequest(t, srv, http.MethodPut, path, token, body); w.Code != http.StatusBadRequest {
+			t.Fatalf("invalid bool %s: expected 400, got %d %s", body, w.Code, w.Body.String())
+		}
+	}
+
+	w := doSysadminRequest(t, srv, http.MethodPut, path, token, `{"value":true,"expected_revision":0}`)
+	if w.Code != http.StatusOK {
+		t.Fatalf("enable: expected 200, got %d %s", w.Code, w.Body.String())
+	}
+	var enabled sysadmin.Setting
+	if err := json.Unmarshal(w.Body.Bytes(), &enabled); err != nil {
+		t.Fatalf("decode: %v", err)
+	}
+	if enabled.Value != true || enabled.Revision != 1 {
+		t.Fatalf("expected true@1, got %+v", enabled)
+	}
+
+	// 过期 revision → 409
+	if w := doSysadminRequest(t, srv, http.MethodPut, path, token, `{"value":false,"expected_revision":0}`); w.Code != http.StatusConflict {
+		t.Fatalf("stale revision: expected 409, got %d %s", w.Code, w.Body.String())
+	}
+}

internal/sysadmin/policy_test.go

@@ -0,0 +1,176 @@
+package sysadmin
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"sync"
+	"testing"
+)
+
+func seedAuthorOnlyPolicy(m *memStore, allowed bool, revision int64) {
+	raw := json.RawMessage(`false`)
+	if allowed {
+		raw = json.RawMessage(`true`)
+	}
+	uid := int64(0)
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.items[SettingKeyAllowAuthorOnlyApproval] = memEntry{
+		value:     append(json.RawMessage(nil), raw...),
+		updatedBy: &uid,
+		updatedAt: m.items[SettingKeyAllowAuthorOnlyApproval].updatedAt,
+		revision:  revision,
+	}
+}
+
+func TestAuthorOnlyPolicy_DefaultClosedWhenMissing(t *testing.T) {
+	svc := NewService(nil, Options{Store: newMemStore()})
+	pol, err := svc.GetAuthorOnlyApprovalPolicy(context.Background())
+	if err != nil {
+		t.Fatalf("GetAuthorOnlyApprovalPolicy: %v", err)
+	}
+	if pol.Allowed || pol.Revision != 0 || pol.Key != SettingKeyAllowAuthorOnlyApproval {
+		t.Fatalf("expected closed default, got %+v", pol)
+	}
+}
+
+func TestAuthorOnlyPolicy_StoreUnavailable(t *testing.T) {
+	svc := NewService(nil, Options{})
+	_, err := svc.GetAuthorOnlyApprovalPolicy(context.Background())
+	if !errors.Is(err, ErrStoreUnavailable) {
+		t.Fatalf("expected ErrStoreUnavailable, got %v", err)
+	}
+}
+
+func TestUpdateAuthorOnly_RequiresExpectedRevision(t *testing.T) {
+	store := newMemStore()
+	seedAuthorOnlyPolicy(store, false, 0)
+	svc := NewService(nil, Options{Store: store})
+	_, err := svc.UpdateSetting(context.Background(), SettingKeyAllowAuthorOnlyApproval, json.RawMessage(`true`), 1)
+	if !errors.Is(err, ErrExpectedRevisionRequired) {
+		t.Fatalf("expected ErrExpectedRevisionRequired, got %v", err)
+	}
+}
+
+func TestUpdateAuthorOnly_RejectsNullAndStringBool(t *testing.T) {
+	store := newMemStore()
+	seedAuthorOnlyPolicy(store, false, 0)
+	svc := NewService(nil, Options{Store: store})
+	zero := int64(0)
+	for _, raw := range []string{`null`, `"true"`, `1`, `yes`} {
+		_, err := svc.UpdateSettingEx(context.Background(), SettingUpdate{
+			Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(raw),
+			UpdatedBy: 1, ExpectedRevision: &zero,
+		})
+		if !errors.Is(err, ErrInvalidSettingValue) {
+			t.Fatalf("%s: expected ErrInvalidSettingValue, got %v", raw, err)
+		}
+	}
+}
+
+func TestUpdateAuthorOnly_CASSuccessAndConflict(t *testing.T) {
+	store := newMemStore()
+	seedAuthorOnlyPolicy(store, false, 0)
+	svc := NewService(nil, Options{Store: store})
+	zero := int64(0)
+	setting, err := svc.UpdateSettingEx(context.Background(), SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`true`),
+		UpdatedBy: 9, ExpectedRevision: &zero, RequestID: "req-1",
+	})
+	if err != nil {
+		t.Fatalf("first CAS: %v", err)
+	}
+	if setting.Value != true || setting.Revision != 1 {
+		t.Fatalf("expected true@rev1, got value=%v rev=%d", setting.Value, setting.Revision)
+	}
+	if len(store.audits) != 1 || store.audits[0].afterRev != 1 || store.audits[0].requestID != "req-1" {
+		t.Fatalf("expected one audit with request id, got %+v", store.audits)
+	}
+
+	pol, err := svc.GetAuthorOnlyApprovalPolicy(context.Background())
+	if err != nil || !pol.Allowed || pol.Revision != 1 {
+		t.Fatalf("policy after enable: %+v err=%v", pol, err)
+	}
+
+	_, err = svc.UpdateSettingEx(context.Background(), SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`false`),
+		UpdatedBy: 9, ExpectedRevision: &zero,
+	})
+	if !errors.Is(err, ErrSettingRevisionConflict) {
+		t.Fatalf("expected conflict on stale revision, got %v", err)
+	}
+
+	one := int64(1)
+	setting, err = svc.UpdateSettingEx(context.Background(), SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`false`),
+		UpdatedBy: 10, ExpectedRevision: &one,
+	})
+	if err != nil {
+		t.Fatalf("second CAS: %v", err)
+	}
+	if setting.Value != false || setting.Revision != 2 {
+		t.Fatalf("expected false@rev2, got value=%v rev=%d", setting.Value, setting.Revision)
+	}
+}
+
+func TestUpdateAuthorOnly_AuditFailureRollsBack(t *testing.T) {
+	store := newMemStore()
+	seedAuthorOnlyPolicy(store, false, 0)
+	store.failAudit = true
+	svc := NewService(nil, Options{Store: store})
+	zero := int64(0)
+	_, err := svc.UpdateSettingEx(context.Background(), SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`true`),
+		UpdatedBy: 1, ExpectedRevision: &zero,
+	})
+	if err == nil {
+		t.Fatal("expected audit failure")
+	}
+	pol, err := svc.GetAuthorOnlyApprovalPolicy(context.Background())
+	if err != nil || pol.Allowed || pol.Revision != 0 {
+		t.Fatalf("setting must remain closed after audit failure: %+v err=%v", pol, err)
+	}
+}
+
+func TestUpdateAuthorOnly_ConcurrentCASOnlyOneWins(t *testing.T) {
+	store := newMemStore()
+	seedAuthorOnlyPolicy(store, false, 0)
+	svc := NewService(nil, Options{Store: store})
+	zero := int64(0)
+
+	var wg sync.WaitGroup
+	results := make(chan error, 2)
+	for i := 0; i < 2; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			_, err := svc.UpdateSettingEx(context.Background(), SettingUpdate{
+				Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`true`),
+				UpdatedBy: 1, ExpectedRevision: &zero,
+			})
+			results <- err
+		}()
+	}
+	wg.Wait()
+	close(results)
+
+	var okCount, conflictCount int
+	for err := range results {
+		switch {
+		case err == nil:
+			okCount++
+		case errors.Is(err, ErrSettingRevisionConflict):
+			conflictCount++
+		default:
+			t.Fatalf("unexpected error: %v", err)
+		}
+	}
+	if okCount != 1 || conflictCount != 1 {
+		t.Fatalf("expected 1 success + 1 conflict, got ok=%d conflict=%d", okCount, conflictCount)
+	}
+	pol, err := svc.GetAuthorOnlyApprovalPolicy(context.Background())
+	if err != nil || !pol.Allowed || pol.Revision != 1 {
+		t.Fatalf("expected single winner true@1, got %+v err=%v", pol, err)
+	}
+}

internal/sysadmin/settings_test.go

@@ -7,41 +7,89 @@ import (
 	"sync"
 	"testing"
 	"time"
+
+	"github.com/jackc/pgx/v5"
 )
 
 // memStore 进程内 SettingsStore，用于读优先级与写路径测试。
 type memStore struct {
-	mu    sync.Mutex
-	items map[string]memEntry
+	mu        sync.Mutex
+	items     map[string]memEntry
+	audits    []memAudit
+	failAudit bool
 }
 
 type memEntry struct {
 	value     json.RawMessage
 	updatedBy *int64
 	updatedAt time.Time
+	revision  int64
+}
+
+type memAudit struct {
+	key              string
+	before, after    json.RawMessage
+	beforeRev, afterRev int64
+	actor            int64
+	requestID        string
 }
 
 func newMemStore() *memStore {
 	return &memStore{items: map[string]memEntry{}}
 }
 
-func (m *memStore) Get(_ context.Context, key string) (json.RawMessage, *int64, *time.Time, error) {
+func (m *memStore) Get(_ context.Context, key string) (json.RawMessage, *int64, *time.Time, int64, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	entry, ok := m.items[key]
 	if !ok {
-		return nil, nil, nil, ErrSettingNotFound
+		return nil, nil, nil, 0, ErrSettingNotFound
 	}
 	at := entry.updatedAt
-	return entry.value, entry.updatedBy, &at, nil
+	return append(json.RawMessage(nil), entry.value...), entry.updatedBy, &at, entry.revision, nil
 }
 
-func (m *memStore) Upsert(_ context.Context, key string, value json.RawMessage, updatedBy int64) (time.Time, error) {
+func (m *memStore) Upsert(_ context.Context, key string, value json.RawMessage, updatedBy int64) (time.Time, int64, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	at := time.Now()
-	m.items[key] = memEntry{value: append(json.RawMessage(nil), value...), updatedBy: &updatedBy, updatedAt: at}
-	return at, nil
+	prev := m.items[key]
+	m.items[key] = memEntry{
+		value:     append(json.RawMessage(nil), value...),
+		updatedBy: &updatedBy,
+		updatedAt: at,
+		revision:  prev.revision,
+	}
+	return at, prev.revision, nil
+}
+
+func (m *memStore) UpsertCAS(_ context.Context, key string, value json.RawMessage, updatedBy, expectedRevision int64, requestID string) (time.Time, int64, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	entry, ok := m.items[key]
+	if !ok {
+		return time.Time{}, 0, ErrSettingNotFound
+	}
+	if entry.revision != expectedRevision {
+		return time.Time{}, 0, ErrSettingRevisionConflict
+	}
+	if m.failAudit {
+		return time.Time{}, 0, errors.New("sysadmin: audit write failed")
+	}
+	at := time.Now()
+	newRev := entry.revision + 1
+	before := append(json.RawMessage(nil), entry.value...)
+	after := append(json.RawMessage(nil), value...)
+	m.items[key] = memEntry{value: after, updatedBy: &updatedBy, updatedAt: at, revision: newRev}
+	m.audits = append(m.audits, memAudit{
+		key: key, before: before, after: after,
+		beforeRev: entry.revision, afterRev: newRev, actor: updatedBy, requestID: requestID,
+	})
+	return at, newRev, nil
+}
+
+func (m *memStore) GetForUpdate(ctx context.Context, _ pgx.Tx, key string) (json.RawMessage, *int64, *time.Time, int64, error) {
+	return m.Get(ctx, key)
 }
 
 func findSetting(t *testing.T, settings []Setting, key string) Setting {
@@ -79,7 +127,7 @@ func TestListSettings_FallsBackToDefaultWithoutStore(t *testing.T) {
 
 func TestListSettings_DBOverridesDefault(t *testing.T) {
 	store := newMemStore()
-	if _, err := store.Upsert(context.Background(), "ui.maintenance_notice", json.RawMessage(`"停机维护中"`), 7); err != nil {
+	if _, _, err := store.Upsert(context.Background(), "ui.maintenance_notice", json.RawMessage(`"停机维护中"`), 7); err != nil {
 		t.Fatalf("seed store: %v", err)
 	}
 	svc := NewService(nil, Options{Store: store})
@@ -120,7 +168,7 @@ func TestListSettings_EnvLayerBetweenDBAndDefault(t *testing.T) {
 
 	// DB 优先于 env
 	store := newMemStore()
-	if _, err := store.Upsert(context.Background(), "test.env_flag", json.RawMessage(`false`), 1); err != nil {
+	if _, _, err := store.Upsert(context.Background(), "test.env_flag", json.RawMessage(`false`), 1); err != nil {
 		t.Fatalf("seed store: %v", err)
 	}
 	svc = &Service{specs: specs, store: store}

internal/sysadmin/store_pg_test.go

@@ -3,6 +3,7 @@ package sysadmin
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"testing"
 
 	"chat-agent-system/internal/db"
@@ -25,13 +26,13 @@ func TestPGSettingsStore_RoundTrip(t *testing.T) {
 
 	store := newPGSettingsStore(pool)
 	key := "ui.maintenance_notice"
-	if _, _, _, err := store.Get(ctx, key); err != ErrSettingNotFound {
+	if _, _, _, _, err := store.Get(ctx, key); err != ErrSettingNotFound {
 		t.Fatalf("expected ErrSettingNotFound, got %v", err)
 	}
-	if _, err := store.Upsert(ctx, key, json.RawMessage(`"维护中"`), 1); err != nil {
+	if _, _, err := store.Upsert(ctx, key, json.RawMessage(`"维护中"`), 1); err != nil {
 		t.Fatalf("upsert: %v", err)
 	}
-	raw, updatedBy, updatedAt, err := store.Get(ctx, key)
+	raw, updatedBy, updatedAt, revision, err := store.Get(ctx, key)
 	if err != nil {
 		t.Fatalf("get: %v", err)
 	}
@@ -44,10 +45,13 @@ func TestPGSettingsStore_RoundTrip(t *testing.T) {
 	if updatedAt == nil {
 		t.Fatal("expected updatedAt")
 	}
-	if _, err := store.Upsert(ctx, key, json.RawMessage(`"恢复"`), 2); err != nil {
+	if revision != 0 {
+		t.Fatalf("plain upsert must not bump revision, got %d", revision)
+	}
+	if _, _, err := store.Upsert(ctx, key, json.RawMessage(`"恢复"`), 2); err != nil {
 		t.Fatalf("second upsert: %v", err)
 	}
-	raw, updatedBy, _, err = store.Get(ctx, key)
+	raw, updatedBy, _, _, err = store.Get(ctx, key)
 	if err != nil {
 		t.Fatalf("get after overwrite: %v", err)
 	}
@@ -55,3 +59,84 @@ func TestPGSettingsStore_RoundTrip(t *testing.T) {
 		t.Fatalf("expected overwrite, got %s by %+v", raw, updatedBy)
 	}
 }
+
+func TestPGSettingsStore_AuthorOnlyCASAndAudit(t *testing.T) {
+	dsn := db.SkipIfNoTestDatabaseURL(t)
+	db.RefuseSharedEnvDSN(t, dsn)
+	ctx := context.Background()
+	pool, err := pgxpool.New(ctx, dsn)
+	if err != nil {
+		t.Fatalf("connect: %v", err)
+	}
+	defer pool.Close()
+	if err := db.RunMigrationsOnceForTest(ctx, pool, dsn); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+
+	store := newPGSettingsStore(pool)
+	svc := NewService(nil, Options{Store: store})
+
+	pol, err := svc.GetAuthorOnlyApprovalPolicy(ctx)
+	if err != nil {
+		t.Fatalf("get policy: %v", err)
+	}
+	if pol.Allowed {
+		t.Fatalf("seeded policy must default closed, got %+v", pol)
+	}
+
+	rev := pol.Revision
+	setting, err := svc.UpdateSettingEx(ctx, SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`true`),
+		UpdatedBy: 11, ExpectedRevision: &rev, RequestID: "pg-cas-1",
+	})
+	if err != nil {
+		t.Fatalf("enable: %v", err)
+	}
+	if setting.Value != true || setting.Revision != rev+1 {
+		t.Fatalf("expected true@%d, got value=%v rev=%d", rev+1, setting.Value, setting.Revision)
+	}
+
+	_, err = svc.UpdateSettingEx(ctx, SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`false`),
+		UpdatedBy: 11, ExpectedRevision: &rev,
+	})
+	if !errors.Is(err, ErrSettingRevisionConflict) {
+		t.Fatalf("expected conflict, got %v", err)
+	}
+
+	var auditCount int
+	if err := pool.QueryRow(ctx, `
+		SELECT COUNT(*) FROM system_setting_policy_audits
+		WHERE setting_key = $1 AND request_id = 'pg-cas-1'
+	`, SettingKeyAllowAuthorOnlyApproval).Scan(&auditCount); err != nil {
+		t.Fatalf("count audits: %v", err)
+	}
+	if auditCount != 1 {
+		t.Fatalf("expected 1 audit row, got %d", auditCount)
+	}
+
+	tx, err := pool.Begin(ctx)
+	if err != nil {
+		t.Fatalf("begin: %v", err)
+	}
+	defer tx.Rollback(ctx)
+	locked, err := svc.LockAuthorOnlyApprovalPolicy(ctx, tx)
+	if err != nil {
+		t.Fatalf("lock: %v", err)
+	}
+	if !locked.Allowed || locked.Revision != rev+1 {
+		t.Fatalf("locked snapshot mismatch: %+v", locked)
+	}
+	if err := tx.Commit(ctx); err != nil {
+		t.Fatalf("commit: %v", err)
+	}
+
+	// 复位为关闭，避免污染同库其它测试对默认态的假设
+	next := setting.Revision
+	if _, err := svc.UpdateSettingEx(ctx, SettingUpdate{
+		Key: SettingKeyAllowAuthorOnlyApproval, Value: json.RawMessage(`false`),
+		UpdatedBy: 11, ExpectedRevision: &next,
+	}); err != nil {
+		t.Fatalf("reset closed: %v", err)
+	}
+}