From a74e8100d34642f79a8d3e245048ad5f9d8820ca Mon Sep 17 00:00:00 2001 From: smail Date: Mon, 21 Sep 2026 00:47:38 +0800 Subject: [PATCH] =?UTF-8?q?=E5=8F=82=E6=95=B0=E5=8C=96=E6=95=B0=E6=8D=AE?= =?UTF-8?q?=E5=BA=93=E5=A4=8D=E5=88=B6=E5=B9=B6=E8=AE=B0=E5=BD=95=E8=B5=84?= =?UTF-8?q?=E6=BA=90=E6=B8=85=E7=90=86=E5=A4=B1=E8=B4=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/config/resource_cleanup.go | 19 +++++++ backend/config/resource_cleanup_test.go | 54 +++++++++++++++++++ backend/config/ssl_flat.go | 13 ++--- backend/config/ssl_retention.go | 6 ++- backend/config/ssl_storage.go | 17 +++--- backend/config/state_database.go | 7 ++- backend/config/state_store.go | 5 +- backend/res_init/init.go | 16 +++--- .../services/dbmigration/sqlite_executor.go | 3 +- .../dbmigration/sqlite_executor_test.go | 27 ++++++++++ 10 files changed, 129 insertions(+), 38 deletions(-) create mode 100644 backend/config/resource_cleanup.go create mode 100644 backend/config/resource_cleanup_test.go diff --git a/backend/config/resource_cleanup.go b/backend/config/resource_cleanup.go new file mode 100644 index 0000000..3c45e79 --- /dev/null +++ b/backend/config/resource_cleanup.go @@ -0,0 +1,19 @@ +package config + +import ( + "io" + + log "github.com/sirupsen/logrus" +) + +// 无返回值的租约释放接口记录关闭失败，不将路径或驱动错误中的敏感信息写入日志。 +func releaseConfigLease(unlock func(), lease io.Closer) { + unlock() + closeConfigResource(lease) +} + +func closeConfigResource(resource io.Closer) { + if err := resource.Close(); err != nil { + log.WithField("action", "释放配置资源").Error("关闭配置资源失败，请检查文件句柄和数据库连接状态") + } +} diff --git a/backend/config/resource_cleanup_test.go b/backend/config/resource_cleanup_test.go new file mode 100644 index 0000000..9d2dd31 --- /dev/null +++ b/backend/config/resource_cleanup_test.go @@ -0,0 +1,54 @@ +package config + +import ( + "errors" + "fmt" + "strings" + "testing" + + log "github.com/sirupsen/logrus" + logtest "github.com/sirupsen/logrus/hooks/test" +) + +type cleanupTestResource struct { + close func() error +} + +func (r cleanupTestResource) Close() error { return r.close() } + +func TestReleaseConfigLeaseUnlocksBeforeClosing(t *testing.T) { + var steps []string + resource := cleanupTestResource{close: func() error { + steps = append(steps, "close") + return nil + }} + releaseConfigLease(func() { steps = append(steps, "unlock") }, resource) + if strings.Join(steps, ",") != "unlock,close" { + t.Fatalf("资源释放顺序不正确: %v", steps) + } +} + +func TestCloseConfigResourceReportsFailureWithoutSensitiveDetails(t *testing.T) { + previous := log.StandardLogger().ReplaceHooks(make(log.LevelHooks)) + t.Cleanup(func() { log.StandardLogger().ReplaceHooks(previous) }) + hook := logtest.NewGlobal() + const sensitive = "sensitive-cleanup-value" + closeCount := 0 + resource := cleanupTestResource{close: func() error { + closeCount++ + return errors.New(sensitive) + }} + closeConfigResource(resource) + entry := hook.LastEntry() + if closeCount != 1 || entry == nil || entry.Level != log.ErrorLevel { + t.Fatal("关闭失败没有记录为错误") + } + if strings.Contains(entry.Message+fmt.Sprint(entry.Data), sensitive) { + t.Fatal("关闭错误日志泄露了资源内部信息") + } + hook.Reset() + closeConfigResource(cleanupTestResource{close: func() error { return nil }}) + if hook.LastEntry() != nil { + t.Fatal("成功关闭资源不应记录错误") + } +} diff --git a/backend/config/ssl_flat.go b/backend/config/ssl_flat.go index 15e74fc..34c2e78 100644 --- a/backend/config/ssl_flat.go +++ b/backend/config/ssl_flat.go @@ -2,6 +2,7 @@ package config import ( "crypto/tls" + "errors" "os" "path/filepath" "strings" @@ -99,10 +100,9 @@ func stageFlatSSLAtRoot(root, mode string, privateKey, certificate, issuer []byt } unlock, err := lockStateFile(lease) if err != nil { - lease.Close() - return ManagedSSLReference{}, err + return ManagedSSLReference{}, errors.Join(err, lease.Close()) } - managedSSLStorage.staged[publicPath] = func() { unlock(); lease.Close() } + managedSSLStorage.staged[publicPath] = func() { releaseConfigLease(unlock, lease) } complete = true return ManagedSSLReference{Mode: mode, CertificateRef: "ssl/" + base + ".crt", PrivateKeyRef: "ssl/" + base + ".key", CertificatePath: publicPath, PrivateKeyPath: privatePath}, nil } @@ -204,7 +204,9 @@ func pruneFlatSSLLocked(root string, protected []SSLConfig) error { } unlock, err := lockStateFile(lease) if err != nil { - lease.Close() + if closeErr := lease.Close(); closeErr != nil { + return errors.Join(err, closeErr) + } continue } err = storage.Remove(filepath.Base(privatePath)) @@ -212,8 +214,7 @@ func pruneFlatSSLLocked(root string, protected []SSLConfig) error { err = storage.Remove(filepath.Base(publicPath)) } unlock() - lease.Close() - if err != nil { + if err := errors.Join(err, lease.Close()); err != nil { return err } } diff --git a/backend/config/ssl_retention.go b/backend/config/ssl_retention.go index 2901c8c..e96251e 100644 --- a/backend/config/ssl_retention.go +++ b/backend/config/ssl_retention.go @@ -218,9 +218,11 @@ func pruneManagedSSLLocked(root string, protected []SSLConfig) error { unlock, err := lockStateFile(lease) if err != nil { keep[path] = true - lease.Close() + if closeErr := lease.Close(); closeErr != nil { + return errors.Join(err, closeErr) + } } else { - leases = append(leases, func() { unlock(); lease.Close() }) + leases = append(leases, func() { releaseConfigLease(unlock, lease) }) } } versions = append(versions, retainedSSLVersion{directory: path, modified: info.ModTime()}) diff --git a/backend/config/ssl_storage.go b/backend/config/ssl_storage.go index a4d8314..7528c57 100644 --- a/backend/config/ssl_storage.go +++ b/backend/config/ssl_storage.go @@ -164,16 +164,13 @@ func importManagedSSLAtRoot(root string, mode string, privateKeyPEM, certificate return atomicfile.WriteRoot(candidateRoot, name, data, permission) } if err := write("private.key", privateKeyPEM, 0600); err != nil { - candidateRoot.Close() - return ManagedSSLReference{}, err + return ManagedSSLReference{}, errors.Join(err, candidateRoot.Close()) } if err := write("public.crt", certificatePEM, 0600); err != nil { - candidateRoot.Close() - return ManagedSSLReference{}, err + return ManagedSSLReference{}, errors.Join(err, candidateRoot.Close()) } if err := write("issuer.crt", issuerPEM, 0600); err != nil { - candidateRoot.Close() - return ManagedSSLReference{}, err + return ManagedSSLReference{}, errors.Join(err, candidateRoot.Close()) } if err := candidateRoot.Close(); err != nil { return ManagedSSLReference{}, err @@ -557,10 +554,9 @@ func stageManagedSSLAtRoot(root string, mode string, privateKeyPEM, certificateP } unlock, err := lockStateFile(lease) if err != nil { - lease.Close() - return ManagedSSLReference{}, err + return ManagedSSLReference{}, errors.Join(err, lease.Close()) } - managedSSLStorage.staged[directory] = func() { unlock(); lease.Close() } + managedSSLStorage.staged[directory] = func() { releaseConfigLease(unlock, lease) } return ManagedSSLReference{Mode: mode, CertificateRef: filepath.ToSlash(publicRef), PrivateKeyRef: filepath.ToSlash(privateRef), CertificatePath: publicPath, PrivateKeyPath: privatePath}, nil } @@ -656,8 +652,7 @@ func removeManagedSSLDirectory(directory string) error { } for _, entry := range entries { if err := storage.Remove(entry.Name()); err != nil { - storage.Close() - return err + return errors.Join(err, storage.Close()) } } if err := storage.Close(); err != nil { diff --git a/backend/config/state_database.go b/backend/config/state_database.go index 2a734c7..ad5653f 100644 --- a/backend/config/state_database.go +++ b/backend/config/state_database.go @@ -163,19 +163,18 @@ func (s *TransactionStore) openState() (*stateDatabase, error) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() if err := database.PingContext(ctx); err != nil { - database.Close() + closeConfigResource(database) return nil, ErrStateDatabaseUnavailable } if !s.legacyState { if _, err := database.Exec(configstate.Schema(driver)); err != nil { - database.Close() + closeConfigResource(database) return nil, ErrStateDatabaseUnavailable } } if filename != "" { if err := os.Chmod(filename, 0600); err != nil { - database.Close() - return nil, err + return nil, errors.Join(err, database.Close()) } } return database, nil diff --git a/backend/config/state_store.go b/backend/config/state_store.go index 74bbe00..9745e53 100644 --- a/backend/config/state_store.go +++ b/backend/config/state_store.go @@ -37,10 +37,9 @@ func (s *TransactionStore) lockState() (func(), error) { } unlock, err := lockStateFile(file) if err != nil { - file.Close() - return nil, errors.New("配置状态正在被其他进程使用，请稍后重试") + return nil, errors.Join(errors.New("配置状态正在被其他进程使用，请稍后重试"), file.Close()) } - return func() { unlock(); file.Close() }, nil + return func() { releaseConfigLease(unlock, file) }, nil } func (s *TransactionStore) saveState(kind, id string, record stateRecord) error { diff --git a/backend/res_init/init.go b/backend/res_init/init.go index 0b0b383..2cb42b0 100644 --- a/backend/res_init/init.go +++ b/backend/res_init/init.go @@ -2,6 +2,7 @@ package res_init import ( "context" + "errors" "os" "path/filepath" "strings" @@ -206,8 +207,7 @@ func Init(serverVersion string) error { return err } if err := startAdminResetControl(); err != nil { - stopServices() - return err + return errors.Join(err, stopServices()) } serviceErrors := make(chan error, 9) adminResetErrors := adminResetServeErrors @@ -221,13 +221,11 @@ func Init(serverVersion string) error { hooks.Init(serverVersion) outboxRuntime, err = outbox.Start(db.Instance) if err != nil { - stopServices() - return fmt.Errorf("start persistent outbox: %w", err) + return errors.Join(fmt.Errorf("启动持久化发件队列失败: %w", err), stopServices()) } httpRun, httpsRun, err := http_server.PrepareWebServers() if err != nil { - stopServices() - return fmt.Errorf("prepare web services: %w", err) + return errors.Join(fmt.Errorf("准备网页服务失败: %w", err), stopServices()) } mailSecurity := config.CurrentMailSecurity() // SMTP 25 用于服务器间投递，始终保留并支持机会性 STARTTLS。 @@ -244,8 +242,7 @@ func Init(serverVersion string) error { // 等待 HTTP 服务启动后再执行需要 HTTP 回调的 ACME 操作。 if err := waitHTTPReady(serviceErrors); err != nil { - stopServices() - return err + return errors.Join(err, stopServices()) } // 更新 SSL 配置。 @@ -269,8 +266,7 @@ func Init(serverVersion string) error { select { case err := <-serviceErrors: log.WithError(err).WithFields(log.Fields{"action": "运行邮箱服务", "result": "failure"}).Error("邮箱服务运行失败") - stopServices() - return err + return errors.Join(err, stopServices()) case <-signal.RestartChan: log.WithFields(log.Fields{"action": "重启服务", "result": "started"}).Info("已收到服务重启请求") if err := stopServices(); err != nil { diff --git a/backend/services/dbmigration/sqlite_executor.go b/backend/services/dbmigration/sqlite_executor.go index 7bb755e..26ebb84 100644 --- a/backend/services/dbmigration/sqlite_executor.go +++ b/backend/services/dbmigration/sqlite_executor.go @@ -288,8 +288,7 @@ func vacuumSQLite(ctx context.Context, source, target string) error { if err := database.PingContext(ctx); err != nil { return fmt.Errorf("ping sqlite source: %w", err) } - query := "VACUUM INTO '" + strings.ReplaceAll(target, "'", "''") + "'" - if _, err := database.ExecContext(ctx, query); err != nil { + if _, err := database.ExecContext(ctx, "VACUUM INTO ?", target); err != nil { return fmt.Errorf("copy sqlite database: %w", err) } return nil diff --git a/backend/services/dbmigration/sqlite_executor_test.go b/backend/services/dbmigration/sqlite_executor_test.go index 7997808..57faa91 100644 --- a/backend/services/dbmigration/sqlite_executor_test.go +++ b/backend/services/dbmigration/sqlite_executor_test.go @@ -78,6 +78,33 @@ func TestSQLiteExecutorCopiesDatabaseAndSwitchesConfiguration(t *testing.T) { } } +func TestVacuumSQLiteBindsDestinationAsData(t *testing.T) { + for _, name := range []string{"copy with spaces.db", "copy'quote.db", "copy'; DROP TABLE messages; --.db"} { + t.Run(name, func(t *testing.T) { + _, root := newSQLiteMigrationFixture(t, nil) + source := filepath.Join(root, "data", "smail.db") + target := filepath.Join(root, "data", name) + if err := vacuumSQLite(context.Background(), source, target); err != nil { + t.Fatal(err) + } + for _, path := range []string{source, target} { + database, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + var body string + queryErr := database.QueryRow("SELECT body FROM messages WHERE id = 1").Scan(&body) + if err := errors.Join(queryErr, database.Close()); err != nil { + t.Fatal(err) + } + if body != "测试邮件" { + t.Fatal("源库或复制库的邮件内容被意外修改") + } + } + }) + } +} + func TestSQLiteExecutorSupportsNestedReferences(t *testing.T) { var switched config.DatabaseConfig service, root := newSQLiteMigrationFixture(t, func(_ context.Context, candidate config.DatabaseConfig) error {