diff --git a/backend/internal/handler/image_task_admin_toggle_test.go b/backend/internal/handler/image_task_admin_toggle_test.go index e172adae4..688c943c9 100644 --- a/backend/internal/handler/image_task_admin_toggle_test.go +++ b/backend/internal/handler/image_task_admin_toggle_test.go @@ -66,7 +66,10 @@ func TestAsyncImageEnablesWithoutRestart(t *testing.T) { gin.SetMode(gin.TestMode) repo := &toggleSettingRepo{values: map[string]string{}} - backup := service.NewBackupService(repo, &config.Config{}, passthroughEncryptor{}, nil, nil) + // A fixed encryption key is required to persist a new S3 secret (#4524). + backup := service.NewBackupService(repo, &config.Config{ + Totp: config.TotpConfig{EncryptionKeyConfigured: true}, + }, passthroughEncryptor{}, nil, nil) factory := func(context.Context, *config.ImageStorageConfig) (service.ImageStorage, error) { return noopImageStorage{}, nil } diff --git a/backend/internal/service/backup_service.go b/backend/internal/service/backup_service.go index 2fcf2da89..14080ecde 100644 --- a/backend/internal/service/backup_service.go +++ b/backend/internal/service/backup_service.go @@ -36,6 +36,18 @@ var ( ErrRestoreInProgress = infraerrors.Conflict("RESTORE_IN_PROGRESS", "a restore is already in progress") ErrBackupRecordsCorrupt = infraerrors.InternalServer("BACKUP_RECORDS_CORRUPT", "backup records data is corrupted") ErrBackupS3ConfigCorrupt = infraerrors.InternalServer("BACKUP_S3_CONFIG_CORRUPT", "backup S3 config data is corrupted") + + // ErrSecretEncryptionKeyNotConfigured is returned when an S3 SecretAccessKey + // would be encrypted with an auto-generated (ephemeral) key. That key is + // regenerated on every process start, so the persisted ciphertext becomes + // undecryptable after a restart/upgrade ("cipher: message authentication + // failed"), silently breaking S3 backup/image storage (#4524). Mirrors the + // existing guards for payments (payment.ProvideEncryptionKey) and TOTP + // enablement, which likewise refuse to depend on an auto-generated key. + ErrSecretEncryptionKeyNotConfigured = infraerrors.BadRequest( + "SECRET_ENCRYPTION_KEY_NOT_CONFIGURED", + "cannot store the S3 secret access key: no fixed secret encryption key is configured, so the auto-generated key would change on every restart and make the stored secret undecryptable after a restart or upgrade. Set a fixed TOTP_ENCRYPTION_KEY (e.g. generate one with `openssl rand -hex 32`) and try again", + ) ) // ─── 接口定义 ─── @@ -105,11 +117,16 @@ type BackupRecord struct { // BackupService 数据库备份恢复服务 type BackupService struct { - settingRepo SettingRepository - dbCfg *config.DatabaseConfig - encryptor SecretEncryptor - storeFactory BackupObjectStoreFactory - dumper DBDumper + settingRepo SettingRepository + dbCfg *config.DatabaseConfig + encryptor SecretEncryptor + // encryptionKeyConfigured mirrors cfg.Totp.EncryptionKeyConfigured: false + // means the secret encryption key was auto-generated and does not survive a + // restart. Durable-secret writers must refuse to persist new secrets in that + // mode (#4524). + encryptionKeyConfigured bool + storeFactory BackupObjectStoreFactory + dumper DBDumper opMu sync.Mutex // 保护 backingUp/restoring 标志 backingUp bool @@ -140,13 +157,14 @@ func NewBackupService( ) *BackupService { bgCtx, bgCancel := context.WithCancel(context.Background()) return &BackupService{ - settingRepo: settingRepo, - dbCfg: &cfg.Database, - encryptor: encryptor, - storeFactory: storeFactory, - dumper: dumper, - bgCtx: bgCtx, - bgCancel: bgCancel, + settingRepo: settingRepo, + dbCfg: &cfg.Database, + encryptor: encryptor, + encryptionKeyConfigured: cfg.Totp.EncryptionKeyConfigured, + storeFactory: storeFactory, + dumper: dumper, + bgCtx: bgCtx, + bgCancel: bgCancel, } } @@ -236,6 +254,14 @@ func (s *BackupService) Stop() { // ─── S3 配置管理 ─── +// EncryptionKeyConfigured reports whether a fixed (explicitly configured) secret +// encryption key is in use. When false the key is auto-generated on every start +// and secrets encrypted with it cannot be recovered after a restart, so callers +// that persist durable secrets must refuse to do so (#4524). +func (s *BackupService) EncryptionKeyConfigured() bool { + return s != nil && s.encryptionKeyConfigured +} + func (s *BackupService) GetS3Config(ctx context.Context) (*BackupS3Config, error) { cfg, err := s.loadS3Config(ctx) if err != nil { @@ -257,6 +283,11 @@ func (s *BackupService) UpdateS3Config(ctx context.Context, cfg BackupS3Config) cfg.SecretAccessKey = old.SecretAccessKey } } else { + // 拒绝用自动生成的临时密钥加密：该密钥每次重启都会变化，落库的密文在 + // 重启/升级后无法解密（#4524）。与支付、TOTP 的处理保持一致。 + if !s.encryptionKeyConfigured { + return nil, ErrSecretEncryptionKeyNotConfigured + } // 加密 SecretAccessKey encrypted, err := s.encryptor.Encrypt(cfg.SecretAccessKey) if err != nil { diff --git a/backend/internal/service/backup_service_test.go b/backend/internal/service/backup_service_test.go index b308e6d09..c39fa6b09 100644 --- a/backend/internal/service/backup_service_test.go +++ b/backend/internal/service/backup_service_test.go @@ -211,6 +211,9 @@ func newTestBackupService(repo *mockSettingRepo, dumper DBDumper, store *mockObj User: "test", DBName: "testdb", }, + // A fixed encryption key is the supported production posture: persisting + // an S3 secret requires it (#4524). + Totp: config.TotpConfig{EncryptionKeyConfigured: true}, } factory := func(_ context.Context, _ *BackupS3Config) (BackupObjectStore, error) { return store, nil @@ -218,6 +221,19 @@ func newTestBackupService(repo *mockSettingRepo, dumper DBDumper, store *mockObj return NewBackupService(repo, cfg, &plainEncryptor{}, factory, dumper) } +// newTestBackupServiceEphemeralKey mirrors a deployment that never set +// TOTP_ENCRYPTION_KEY, so the secret encryption key is auto-generated. +func newTestBackupServiceEphemeralKey(repo *mockSettingRepo) *BackupService { + cfg := &config.Config{ + Database: config.DatabaseConfig{Host: "localhost", Port: 5432, User: "test", DBName: "testdb"}, + Totp: config.TotpConfig{EncryptionKeyConfigured: false}, + } + factory := func(_ context.Context, _ *BackupS3Config) (BackupObjectStore, error) { + return newMockObjectStore(), nil + } + return NewBackupService(repo, cfg, &plainEncryptor{}, factory, &mockDumper{}) +} + func seedS3Config(t *testing.T, repo *mockSettingRepo) { t.Helper() cfg := BackupS3Config{ @@ -288,6 +304,42 @@ func TestBackupService_S3ConfigKeepExistingSecret(t *testing.T) { require.Equal(t, "AKID-NEW", internal.AccessKeyID) } +func TestBackupService_UpdateS3Config_RejectsEphemeralKey(t *testing.T) { + repo := newMockSettingRepo() + svc := newTestBackupServiceEphemeralKey(repo) + + // 提供新 secret 但密钥为自动生成 -> 必须拒绝，避免重启后无法解密（#4524）。 + _, err := svc.UpdateS3Config(context.Background(), BackupS3Config{ + Bucket: "my-bucket", + AccessKeyID: "AKID", + SecretAccessKey: "my-secret", + Prefix: "backups", + }) + require.ErrorIs(t, err, ErrSecretEncryptionKeyNotConfigured) + + // 不应写入任何配置。 + raw, _ := repo.GetValue(context.Background(), settingKeyBackupS3Config) + require.Empty(t, raw) +} + +func TestBackupService_UpdateS3Config_NoSecretAllowedWithEphemeralKey(t *testing.T) { + repo := newMockSettingRepo() + svc := newTestBackupServiceEphemeralKey(repo) + + // 不含 secret 的更新（如只改 bucket）不触碰加密路径，应放行。 + _, err := svc.UpdateS3Config(context.Background(), BackupS3Config{ + Bucket: "my-bucket", + AccessKeyID: "AKID", + }) + require.NoError(t, err) +} + +func TestBackupService_EncryptionKeyConfigured(t *testing.T) { + repo := newMockSettingRepo() + require.True(t, newTestBackupService(repo, &mockDumper{}, newMockObjectStore()).EncryptionKeyConfigured()) + require.False(t, newTestBackupServiceEphemeralKey(repo).EncryptionKeyConfigured()) +} + func TestBackupService_SaveRecordConcurrency(t *testing.T) { repo := newMockSettingRepo() svc := newTestBackupService(repo, &mockDumper{}, newMockObjectStore()) diff --git a/backend/internal/service/image_storage_settings.go b/backend/internal/service/image_storage_settings.go index c8ee2c7f3..96080b1b0 100644 --- a/backend/internal/service/image_storage_settings.go +++ b/backend/internal/service/image_storage_settings.go @@ -176,6 +176,11 @@ func (s *ImageStorageSettingService) Update(ctx context.Context, in ImageStorage in.SecretAccessKey = old.SecretAccessKey } } else { + // 拒绝用自动生成的临时密钥加密：重启后密文无法解密（#4524）。 + // 与备份 S3 配置共用同一把密钥，故复用其配置状态判断。 + if s.backup == nil || !s.backup.EncryptionKeyConfigured() { + return nil, ErrSecretEncryptionKeyNotConfigured + } encrypted, err := s.encryptor.Encrypt(in.SecretAccessKey) if err != nil { return nil, fmt.Errorf("encrypt secret: %w", err) diff --git a/backend/internal/service/image_storage_settings_test.go b/backend/internal/service/image_storage_settings_test.go index 3d1d6a2e8..4c23cf387 100644 --- a/backend/internal/service/image_storage_settings_test.go +++ b/backend/internal/service/image_storage_settings_test.go @@ -69,10 +69,16 @@ func (s *recordingStorage) Save(_ context.Context, key, _ string, _ []byte) (str } func newImageStorageFixture(t *testing.T, fallback config.ImageStorageConfig) (*ImageStorageSettingService, *stubSettingRepo, *[]config.ImageStorageConfig) { + return newImageStorageFixtureWithKey(t, fallback, true) +} + +func newImageStorageFixtureWithKey(t *testing.T, fallback config.ImageStorageConfig, encryptionKeyConfigured bool) (*ImageStorageSettingService, *stubSettingRepo, *[]config.ImageStorageConfig) { t.Helper() repo := newStubSettingRepo() encryptor := reversibleEncryptor{} - backup := NewBackupService(repo, &config.Config{}, encryptor, nil, nil) + backup := NewBackupService(repo, &config.Config{ + Totp: config.TotpConfig{EncryptionKeyConfigured: encryptionKeyConfigured}, + }, encryptor, nil, nil) var built []config.ImageStorageConfig factory := func(_ context.Context, cfg *config.ImageStorageConfig) (ImageStorage, error) { @@ -157,7 +163,7 @@ func TestImageStorageSettingsOwnCredentialsAreEncryptedAndMasked(t *testing.T) { saved, err := svc.Update(ctx, ImageStorageSettings{ Enabled: true, Bucket: "my-images", - Endpoint: "https://acct.r2.cloudflarestorage.com", + Endpoint: "https://acct.r2.cloudflarestorage.com", AccessKeyID: "ak", SecretAccessKey: "super-secret", }) require.NoError(t, err) @@ -187,6 +193,34 @@ func TestImageStorageSettingsOwnCredentialsAreEncryptedAndMasked(t *testing.T) { require.Equal(t, "super-secret", (*built)[1].SecretAccessKey) } +// Persisting the service's own S3 secret must be refused when the encryption key +// is auto-generated, otherwise the ciphertext cannot be decrypted after a +// restart (#4524). Reusing the backup credentials stays allowed because it does +// not persist a second copy of the secret. +func TestImageStorageSettingsRejectSecretWithEphemeralKey(t *testing.T) { + svc, repo, built := newImageStorageFixtureWithKey(t, config.ImageStorageConfig{}, false) + ctx := context.Background() + + _, err := svc.Update(ctx, ImageStorageSettings{ + Enabled: true, Bucket: "my-images", + Endpoint: "https://acct.r2.cloudflarestorage.com", + AccessKeyID: "ak", SecretAccessKey: "super-secret", + }) + require.ErrorIs(t, err, ErrSecretEncryptionKeyNotConfigured) + + raw, _ := repo.GetValue(ctx, settingKeyImageStorageConfig) + require.Empty(t, raw, "nothing must be persisted when the secret is rejected") + require.Empty(t, *built) + + // Reusing backup credentials does not persist a secret, so it stays allowed. + seedBackupS3(t, repo, BackupS3Config{ + Endpoint: "https://acct.r2.cloudflarestorage.com", Region: "auto", + Bucket: "backup-bucket", AccessKeyID: "ak", SecretAccessKey: "sk", Prefix: "backups/", + }) + _, err = svc.Update(ctx, ImageStorageSettings{Enabled: true, ReuseBackupS3: true}) + require.NoError(t, err) +} + func TestImageStorageSettingsIncompleteStaysDisabled(t *testing.T) { svc, _, built := newImageStorageFixture(t, config.ImageStorageConfig{}) ctx := context.Background()