HH-441: harden durable storage and recovery (#92)

* HH-441: harden durable storage and recovery

* HH-441: clear recovery review blockers

* HH-441: enforce offsite backup failure domain

---------

Co-authored-by: Rogee <rogee@ipao.vip>
This commit is contained in:
Rogee
2026-08-22 02:16:02 +08:00
committed by GitHub
co-authored by rogee
parent 77c91662d2
commit 6d6d80dd86
20 changed files with 966 additions and 202 deletions
+14
View File
@@ -73,6 +73,9 @@ func Bootstrap(env string) (*App, error) {
if err != nil {
return nil, fmt.Errorf("config load failed: %w", err)
}
if err := validateStartupMigrations(env, cfg); err != nil {
return nil, err
}
// Step 2: Validate configuration
if err := config.Validate(cfg); err != nil {
@@ -788,6 +791,10 @@ func Bootstrap(env string) (*App, error) {
WithWidgetAuth(inboxRepo, contactInboxRepo).
WithConversationRepo(conversationRepo).
WithAccessDB(db)
service.RegisterUploadCleanupJobs(workerPool, uploadService)
if _, err := service.EnqueueUploadCleanup(context.Background(), workerPool, time.Now()); err != nil {
applogger.L().Warnf("failed to enqueue upload cleanup: %v", err)
}
uploadHandler := v1.NewUploadHandler(uploadService).WithAccessAuth(jwtService)
// Step 9: Wire handlers (HTTP presentation layer)
@@ -1000,6 +1007,13 @@ func Bootstrap(env string) (*App, error) {
}, nil
}
func validateStartupMigrations(env string, cfg *config.Config) error {
if env == "production" && cfg.Database.RunMigrations {
return fmt.Errorf("database.run_migrations must be false in production; run the one-shot migration job before web/worker startup")
}
return nil
}
// hubTypingAdapter implements service.TypingIndicator by broadcasting
// typing_on/typing_off events directly through the WS hub's SendToAccount method.
// This avoids the Redis dependency required by the full ws.TypingTracker.
@@ -0,0 +1,14 @@
package app
import (
"testing"
"github.com/gochat/gochat/internal/config"
"github.com/stretchr/testify/require"
)
func TestValidateStartupMigrations(t *testing.T) {
cfg := &config.Config{Database: config.DatabaseConfig{RunMigrations: true}}
require.ErrorContains(t, validateStartupMigrations("production", cfg), "one-shot migration job")
require.NoError(t, validateStartupMigrations("development", cfg))
}
@@ -0,0 +1,134 @@
package database
import (
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/driver/postgres"
"gorm.io/gorm"
"gorm.io/gorm/logger"
)
func TestHighRiskMigrationsPreserveRollbackData(t *testing.T) {
if os.Getenv("GOCHAT_TEST_DB") == "sqlite" {
t.Skip("requires PostgreSQL migration semantics")
}
dsn := os.Getenv("GOCHAT_TEST_DB_URL")
if dsn == "" {
dsn = "host=localhost port=5432 user=postgres password=postgres dbname=gochat_test sslmode=disable"
}
db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
require.NoError(t, err)
sqlDB, err := db.DB()
require.NoError(t, err)
sqlDB.SetMaxOpenConns(1)
t.Cleanup(func() { _ = sqlDB.Close() })
schema := "high_risk_migrations_" + time.Now().Format("20060102150405000000000")
require.NoError(t, db.Exec("CREATE SCHEMA "+schema).Error)
t.Cleanup(func() { _ = db.Exec("DROP SCHEMA " + schema + " CASCADE").Error })
require.NoError(t, db.Exec("SET search_path TO "+schema).Error)
require.NoError(t, db.Exec(`
CREATE TABLE reporting_events_rollups (
id SERIAL PRIMARY KEY, account_id INTEGER NOT NULL, dimension VARCHAR(50) NOT NULL,
dimension_value VARCHAR(255) NOT NULL, metric_name VARCHAR(50) NOT NULL,
value DOUBLE PRECISION NOT NULL, value_in_business_hours DOUBLE PRECISION,
period VARCHAR(50) NOT NULL, created_at TIMESTAMPTZ DEFAULT NOW(),
updated_at TIMESTAMPTZ DEFAULT NOW(), deleted_at TIMESTAMPTZ,
UNIQUE(account_id, dimension, dimension_value, metric_name, period)
);
INSERT INTO reporting_events_rollups
(account_id, dimension, dimension_value, metric_name, value, value_in_business_hours, period)
VALUES (1, 'inbox', '42', 'conversations_count', 7, 3, '2026-08-21');
CREATE TABLE custom_attribute_definitions (attribute_model TEXT, attribute_name TEXT);
CREATE TABLE conversations (id BIGINT PRIMARY KEY, custom_attributes JSONB, updated_at TIMESTAMPTZ);
INSERT INTO custom_attribute_definitions VALUES ('conversation_attribute', 'swt_source_url');
INSERT INTO conversations VALUES (1, '{"swt_source_url":"https://example.test"}', NOW());
`).Error)
up48, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000048_fix_reporting_events_rollups_schema.up.sql"))
require.NoError(t, err)
require.NoError(t, db.Exec(string(up48)).Error)
var rollup struct {
DimensionID int64
Count int64
SumValue float64
}
require.NoError(t, db.Table("reporting_events_rollups").Select("dimension_id, count, sum_value").Scan(&rollup).Error)
assert.Equal(t, int64(42), rollup.DimensionID)
assert.Equal(t, int64(7), rollup.Count)
assert.Equal(t, float64(7), rollup.SumValue)
up76, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000076_replace_shangwutong_source_attributes_with_messages.up.sql"))
require.NoError(t, err)
require.NoError(t, db.Exec(string(up76)).Error)
var definitions, conversations int64
require.NoError(t, db.Table("custom_attribute_definitions").Count(&definitions).Error)
require.NoError(t, db.Table("conversations").Where("custom_attributes->>'swt_source_url' IS NOT NULL").Count(&conversations).Error)
assert.Equal(t, int64(1), definitions)
assert.Equal(t, int64(1), conversations)
down48, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000048_fix_reporting_events_rollups_schema.down.sql"))
require.NoError(t, err)
require.ErrorContains(t, db.Exec(string(down48)).Error, "irreversible")
}
func TestMigration48RejectsUnsafeLegacyData(t *testing.T) {
if os.Getenv("GOCHAT_TEST_DB") == "sqlite" {
t.Skip("requires PostgreSQL migration semantics")
}
dsn := os.Getenv("GOCHAT_TEST_DB_URL")
if dsn == "" {
dsn = "host=localhost port=5432 user=postgres password=postgres dbname=gochat_test sslmode=disable"
}
db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)})
require.NoError(t, err)
sqlDB, err := db.DB()
require.NoError(t, err)
sqlDB.SetMaxOpenConns(1)
t.Cleanup(func() { _ = sqlDB.Close() })
up48, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000048_fix_reporting_events_rollups_schema.up.sql"))
require.NoError(t, err)
tests := []struct {
name, values, want string
rows int64
}{
{"invalid_period", "(1, 'inbox', '42', 'conversations_count', 1, 'not-a-date')", "cannot convert to date", 1},
{"invalid_dimension", "(1, 'inbox', 'vip', 'conversations_count', 1, '2026-08-21')", "cannot convert to bigint", 1},
{"converted_key_collision", "(1, 'inbox', '042', 'conversations_count', 1, '2026-08-21'), (1, 'inbox', '42', 'conversations_count', 2, '2026-08-21')", "unique-key collision", 2},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
schema := "migration_48_rejection_" + tt.name
t.Cleanup(func() { _ = db.Exec("DROP SCHEMA IF EXISTS " + schema + " CASCADE").Error })
require.NoError(t, db.Exec("DROP SCHEMA IF EXISTS "+schema+" CASCADE").Error)
require.NoError(t, db.Exec("CREATE SCHEMA "+schema).Error)
require.NoError(t, db.Exec("SET search_path TO "+schema).Error)
require.NoError(t, db.Exec(`
CREATE TABLE reporting_events_rollups (
id SERIAL PRIMARY KEY, account_id INTEGER NOT NULL, dimension VARCHAR(50) NOT NULL,
dimension_value VARCHAR(255) NOT NULL, metric_name VARCHAR(50) NOT NULL,
value DOUBLE PRECISION NOT NULL, value_in_business_hours DOUBLE PRECISION,
period VARCHAR(50) NOT NULL, created_at TIMESTAMPTZ DEFAULT NOW(),
updated_at TIMESTAMPTZ DEFAULT NOW(), deleted_at TIMESTAMPTZ,
UNIQUE(account_id, dimension, dimension_value, metric_name, period)
);
INSERT INTO reporting_events_rollups
(account_id, dimension, dimension_value, metric_name, value, period)
VALUES `+tt.values).Error)
require.ErrorContains(t, db.Exec(string(up48)).Error, tt.want)
require.NoError(t, db.Exec("ROLLBACK").Error)
var rows int64
require.NoError(t, db.Table(schema+".reporting_events_rollups").Count(&rows).Error)
assert.Equal(t, tt.rows, rows)
require.NoError(t, db.Exec("SET search_path TO public").Error)
require.NoError(t, db.Exec("DROP SCHEMA "+schema+" CASCADE").Error)
})
}
}
@@ -64,6 +64,11 @@ func (r *DirectUploadRepo) UpdateStatus(ctx context.Context, id uint, status mod
return r.db.WithContext(ctx).Model(&model.DirectUpload{}).Where("id = ?", id).Update("status", status).Error
}
// Delete permanently removes a staged upload after its object is gone.
func (r *DirectUploadRepo) Delete(ctx context.Context, id uint) error {
return r.db.WithContext(ctx).Unscoped().Delete(&model.DirectUpload{}, id).Error
}
// FindExpired retrieves all direct uploads that have passed their expiry timestamp
// and are still in pending status.
func (r *DirectUploadRepo) FindExpired(ctx context.Context, before time.Time) ([]model.DirectUpload, error) {
@@ -83,4 +88,4 @@ func (r *DirectUploadRepo) BatchDeleteExpired(ctx context.Context, before time.T
Where("status = ? AND expires_at < ?", model.DirectUploadStatusPending, before).
Delete(&model.DirectUpload{})
return result.RowsAffected, result.Error
}
}
+136 -5
View File
@@ -26,9 +26,15 @@ import (
"github.com/gochat/gochat/internal/model"
"github.com/gochat/gochat/internal/repository"
"github.com/gochat/gochat/internal/security"
"github.com/gochat/gochat/internal/worker"
applogger "github.com/gochat/gochat/pkg/logger"
)
const (
TaskTypeCleanupExpiredUploads = "storage:cleanup_expired_uploads"
uploadCleanupInterval = 24 * time.Hour
)
// UploadService handles file uploads for both account-level and widget direct uploads.
type UploadService struct {
directUploadRepo *repository.DirectUploadRepo
@@ -846,14 +852,139 @@ func randomStorageKey() string {
return hex.EncodeToString(buf)
}
// CleanupExpiredUploads removes expired direct upload records and their files.
// CleanupExpiredUploads removes each object before its record. Failed object
// deletions leave the row intact so the durable job can retry safely.
func (s *UploadService) CleanupExpiredUploads(ctx context.Context) (int64, error) {
count, err := s.directUploadRepo.BatchDeleteExpired(ctx, time.Now())
now := time.Now()
uploads, err := s.directUploadRepo.FindExpired(ctx, now)
if err != nil {
return 0, fmt.Errorf("failed to cleanup expired uploads: %w", err)
return 0, fmt.Errorf("find expired uploads: %w", err)
}
applogger.L().Infof("Cleaned up %d expired direct uploads", count)
return count, nil
var count int64
var cleanupErr error
for i := range uploads {
if err := s.removeUploadFile(uploads[i].FileURL); err != nil {
cleanupErr = errors.Join(cleanupErr, fmt.Errorf("remove upload %s: %w", uploads[i].UploadUUID, err))
continue
}
if err := s.directUploadRepo.Delete(ctx, uploads[i].ID); err != nil {
cleanupErr = errors.Join(cleanupErr, fmt.Errorf("delete upload %s: %w", uploads[i].UploadUUID, err))
continue
}
count++
}
orphans, err := s.cleanupOrphanedUploadFiles(ctx, now.Add(-uploadCleanupInterval))
count += orphans
cleanupErr = errors.Join(cleanupErr, err)
applogger.L().Infof("Cleaned up %d expired or orphaned uploads", count)
return count, cleanupErr
}
// cleanupOrphanedUploadFiles reconciles only roots owned by UploadService.
// The grace period keeps an in-flight file-to-row write from racing cleanup.
func (s *UploadService) cleanupOrphanedUploadFiles(ctx context.Context, cutoff time.Time) (int64, error) {
if s.accessDB == nil {
return 0, nil
}
references := map[string]struct{}{}
var urls []string
for _, column := range []string{"file_url", "thumb_url"} {
urls = nil
if err := s.accessDB.WithContext(ctx).Model(&model.DirectUpload{}).
Where(column+" LIKE ?", "/uploads/%").Pluck(column, &urls).Error; err != nil {
return 0, fmt.Errorf("load direct upload references: %w", err)
}
for _, url := range urls {
references[url] = struct{}{}
}
}
if s.accessDB.Migrator().HasTable(&model.User{}) {
urls = nil
if err := s.accessDB.WithContext(ctx).Model(&model.User{}).
Where("avatar_url LIKE ?", "/uploads/account/%").Pluck("avatar_url", &urls).Error; err != nil {
return 0, fmt.Errorf("load avatar references: %w", err)
}
for _, url := range urls {
references[url] = struct{}{}
}
}
localPath := "./uploads"
if s.cfg != nil && s.cfg.Storage.LocalPath != "" {
localPath = s.cfg.Storage.LocalPath
}
var removed int64
var reconcileErr error
for _, subdir := range []string{"account", "widget_direct"} {
root := filepath.Join(localPath, subdir)
err := filepath.Walk(root, func(path string, info os.FileInfo, walkErr error) error {
if walkErr != nil {
if errors.Is(walkErr, os.ErrNotExist) {
return nil
}
return walkErr
}
if !info.Mode().IsRegular() || info.ModTime().After(cutoff) {
return nil
}
relative, err := filepath.Rel(localPath, path)
if err != nil {
return err
}
fileURL := "/uploads/" + filepath.ToSlash(relative)
if _, ok := references[fileURL]; ok {
return nil
}
if err := os.Remove(path); err != nil {
if errors.Is(err, os.ErrNotExist) {
return nil
}
return err
}
removed++
return nil
})
if err != nil && !errors.Is(err, os.ErrNotExist) {
reconcileErr = errors.Join(reconcileErr, fmt.Errorf("reconcile %s uploads: %w", subdir, err))
}
}
return removed, reconcileErr
}
func (s *UploadService) removeUploadFile(fileURL string) error {
relative := strings.TrimPrefix(fileURL, "/uploads/")
if relative == fileURL || relative == "" || filepath.IsAbs(relative) || strings.HasPrefix(filepath.Clean(relative), "..") {
return fmt.Errorf("invalid upload path %q", fileURL)
}
localPath := s.cfg.Storage.LocalPath
if localPath == "" {
localPath = "./uploads"
}
err := os.Remove(filepath.Join(localPath, relative))
if errors.Is(err, os.ErrNotExist) {
return nil
}
return err
}
// RegisterUploadCleanupJobs runs orphan cleanup daily through the existing
// durable, idempotent worker queue.
func RegisterUploadCleanupJobs(wp *worker.WorkerPool, svc *UploadService) {
wp.Register(TaskTypeCleanupExpiredUploads, func(ctx context.Context, _ *model.BackgroundJob) error {
_, cleanupErr := svc.CleanupExpiredUploads(ctx)
_, enqueueErr := EnqueueUploadCleanup(ctx, wp, time.Now().Add(uploadCleanupInterval))
return errors.Join(cleanupErr, enqueueErr)
})
}
func EnqueueUploadCleanup(ctx context.Context, wp *worker.WorkerPool, at time.Time) (*model.BackgroundJob, error) {
bucket := at.UTC().Truncate(uploadCleanupInterval).Unix()
return wp.Enqueue(ctx, TaskTypeCleanupExpiredUploads, nil,
worker.WithQueue("low"),
worker.WithScheduledAt(at),
worker.WithMaxAttempts(10),
worker.WithIdempotencyKey(fmt.Sprintf("storage:cleanup_expired_uploads:%d", bucket)),
)
}
// --- MIME detection helpers (reuse patterns from widget_theme_service.go) ---
@@ -5,12 +5,14 @@ import (
"encoding/json"
"mime/multipart"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"
"time"
"github.com/gin-gonic/gin"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/driver/sqlite"
@@ -145,7 +147,8 @@ func setupUploadServiceTest(t *testing.T) (*gorm.DB, *UploadService, string) {
directUploadRepo := repository.NewDirectUploadRepo(db)
uploadService := NewUploadService(directUploadRepo, cfg).
WithWidgetAuth(repository.NewInboxRepo(db), repository.NewContactInboxRepo(db))
WithWidgetAuth(repository.NewInboxRepo(db), repository.NewContactInboxRepo(db)).
WithAccessDB(db)
return db, uploadService, tmpDir
}
@@ -195,6 +198,25 @@ func TestUploadService_AccountUpload_Success(t *testing.T) {
assert.GreaterOrEqual(t, len(files), 1, "file should be saved on disk")
}
func TestUploadSharedStorageSurvivesReplicaRecreation(t *testing.T) {
_, svc, storagePath := setupUploadServiceTest(t)
content := []byte("shared attachment")
upload, err := svc.AccountUpload(context.Background(), 1, AccountUploadRequest{
FileHeader: createTestFileHeader(t, "shared.txt", content),
})
require.NoError(t, err)
for range 2 {
replica := gin.New()
replica.StaticFS("/uploads", gin.Dir(storagePath, false))
response := httptest.NewRecorder()
request := httptest.NewRequest(http.MethodGet, upload.FileURL, nil)
replica.ServeHTTP(response, request)
assert.Equal(t, http.StatusOK, response.Code)
assert.Equal(t, content, response.Body.Bytes())
}
}
func TestUploadService_AccountUpload_NoAccountID(t *testing.T) {
_, svc, _ := setupUploadServiceTest(t)
@@ -459,7 +481,7 @@ func TestUploadService_PrivateAttachmentAccessIsTenantAndSessionScoped(t *testin
}
func TestUploadService_CleanupExpiredUploads(t *testing.T) {
db, svc, _ := setupUploadServiceTest(t)
db, svc, tmpDir := setupUploadServiceTest(t)
// Insert a pending-but-expired upload record (pending status with past expires_at)
// BatchDeleteExpired deletes pending uploads where expires_at < now
@@ -476,6 +498,9 @@ func TestUploadService_CleanupExpiredUploads(t *testing.T) {
ExpiresAt: time.Now().Add(-1 * time.Hour), // already past expiry
}
require.NoError(t, db.Create(expiredUpload).Error)
expiredPath := filepath.Join(tmpDir, "account", "1", "old_file.png")
require.NoError(t, os.MkdirAll(filepath.Dir(expiredPath), 0o755))
require.NoError(t, os.WriteFile(expiredPath, []byte("expired"), 0o600))
// Insert a non-expired upload (pending status with future expires_at)
activeUpload := &model.DirectUpload{
@@ -491,13 +516,55 @@ func TestUploadService_CleanupExpiredUploads(t *testing.T) {
ExpiresAt: time.Now().Add(24 * time.Hour),
}
require.NoError(t, db.Create(activeUpload).Error)
failedUpload := &model.DirectUpload{
UploadUUID: "failed-uuid-789", AccountID: 1, Status: model.DirectUploadStatusPending,
Source: model.DirectUploadSourceAccount, OriginalName: "invalid.png", FileType: "image",
MimeType: "image/png", FileSize: 100, FileURL: "https://example.test/invalid.png",
ExpiresAt: time.Now().Add(-time.Hour),
}
require.NoError(t, db.Create(failedUpload).Error)
count, err := svc.CleanupExpiredUploads(context.Background())
require.NoError(t, err)
require.ErrorContains(t, err, "invalid upload path")
assert.Equal(t, int64(1), count, "should clean up 1 expired upload")
assert.NoFileExists(t, expiredPath)
// Verify active upload still exists
var remaining model.DirectUpload
require.NoError(t, db.Where("upload_uuid = ?", "active-uuid-456").First(&remaining).Error)
assert.Equal(t, model.DirectUploadStatusPending, remaining.Status)
remaining = model.DirectUpload{}
require.NoError(t, db.Where("upload_uuid = ?", "failed-uuid-789").First(&remaining).Error)
}
func TestUploadService_ReconcilesCrashWindowOrphans(t *testing.T) {
db, svc, storagePath := setupUploadServiceTest(t)
dir := filepath.Join(storagePath, "account", "1")
require.NoError(t, os.MkdirAll(dir, 0o755))
orphan := filepath.Join(dir, "crash-window.png")
referenced := filepath.Join(dir, "referenced.png")
fresh := filepath.Join(dir, "in-flight.png")
for _, path := range []string{orphan, referenced, fresh} {
require.NoError(t, os.WriteFile(path, testPNG, 0o600))
}
old := time.Now().Add(-uploadCleanupInterval - time.Hour)
require.NoError(t, os.Chtimes(orphan, old, old))
require.NoError(t, os.Chtimes(referenced, old, old))
require.NoError(t, db.Create(&model.DirectUpload{
UploadUUID: "referenced", AccountID: 1, Status: model.DirectUploadStatusPending,
Source: model.DirectUploadSourceAccount, OriginalName: "referenced.png", FileType: "image",
MimeType: "image/png", FileSize: int64(len(testPNG)), FileURL: "/uploads/account/1/referenced.png",
ExpiresAt: time.Now().Add(time.Hour),
}).Error)
count, err := svc.CleanupExpiredUploads(context.Background())
require.NoError(t, err)
assert.Equal(t, int64(1), count)
assert.NoFileExists(t, orphan)
assert.FileExists(t, referenced)
assert.FileExists(t, fresh)
count, err = svc.CleanupExpiredUploads(context.Background())
require.NoError(t, err)
assert.Zero(t, count)
}