fix: resolve 6 medium issues from full go-review

- MED-1: Document soundboard channelID=0 server-wide permission intent
- MED-2: Document os.Exit(0) in update handler skipping deferred cleanup
- MED-3: Replace os.ReadFile/WriteFile with streaming io.Copy in backup
  restore to avoid loading entire DB into memory
- MED-4: Add GetAllVoiceStates bulk query, eliminating N+1 per-channel
  queries in collectAllVoiceStates
- MED-5: Wrap handlePatchSettings updates in a transaction for atomicity
- MED-6: Add rows.Err() check after scan loop in getReactionsBatch
- MED-9: Replace manual port-stripping in serverHost with net.SplitHostPort
  for correct IPv6 handling
This commit is contained in:
jevb
2026-03-19 04:04:53 +01:00
parent 65a8403a92
commit 13797e7075
8 changed files with 103 additions and 41 deletions
+25 -8
View File
@@ -2,6 +2,7 @@ package admin
import (
"fmt"
"io"
"log/slog"
"net/http"
"os"
@@ -141,14 +142,10 @@ func handleRestoreBackup(database *db.DB) http.Handler {
slog.Warn("pre-restore backup failed", "err", err)
}
// Copy the backup file over the live database.
src, err := os.ReadFile(backupPath)
if err != nil {
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to read backup file")
return
}
if err := os.WriteFile(dbPath, src, 0o644); err != nil {
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to write database file")
// Stream the backup file over the live database to avoid loading
// the entire DB into memory (could be hundreds of MiB).
if err := copyFile(backupPath, dbPath); err != nil {
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to restore database file")
return
}
@@ -162,3 +159,23 @@ func handleRestoreBackup(database *db.DB) http.Handler {
})
})
}
// copyFile streams src to dst without loading the entire file into memory.
func copyFile(src, dst string) error {
in, err := os.Open(src)
if err != nil {
return fmt.Errorf("open source: %w", err)
}
defer in.Close() //nolint:errcheck
out, err := os.Create(dst)
if err != nil {
return fmt.Errorf("create destination: %w", err)
}
defer out.Close() //nolint:errcheck
if _, err := io.Copy(out, in); err != nil {
return fmt.Errorf("copy: %w", err)
}
return out.Close()
}
+20 -1
View File
@@ -41,11 +41,30 @@ func handlePatchSettings(database *db.DB) http.HandlerFunc {
}
actor := actorFromContext(r)
// Apply all settings atomically so a mid-loop failure doesn't leave
// partial updates.
tx, err := database.Begin()
if err != nil {
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to start transaction")
return
}
for key, value := range updates {
if err := database.SetSetting(key, value); err != nil {
if _, txErr := tx.Exec(
`INSERT INTO settings (key, value) VALUES (?, ?)
ON CONFLICT(key) DO UPDATE SET value = excluded.value`,
key, value,
); txErr != nil {
_ = tx.Rollback()
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to update setting: "+key)
return
}
}
if err := tx.Commit(); err != nil {
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to commit settings")
return
}
for key := range updates {
slog.Info("setting changed", "actor_id", actor, "key", key)
_ = database.LogAudit(actor, "setting_change", "setting", 0,
fmt.Sprintf("%s updated", key))
+4 -1
View File
@@ -109,7 +109,10 @@ func handleApplyUpdate(u *updater.Updater, hub HubBroadcaster, _ string) http.Ha
return
}
// Exit current process.
// Exit current process. os.Exit skips deferred cleanup intentionally —
// the process must die to release the file lock on its own binary
// before the new process can replace it on Windows. SQLite WAL mode
// protects DB integrity on unclean shutdown.
slog.Info("update: new process spawned, exiting current process")
os.Exit(0)
}()