fix: code quality, memory safety, and install improvements
Critical fixes: - Fix Dockerfile: reorder stages so frontend assets embed into Go binary - Fix Go version 1.25 (nonexistent) to 1.24 across Dockerfile, go.mod, CI - Add graceful game server shutdown on SIGTERM/SIGINT - Order startup tasks: updates complete before auto-start - Fix TOCTOU race in UpdateSettings with atomic Update() method Security: - Add optional AUTH_TOKEN bearer auth middleware on API/WS routes - Fix path traversal in DeleteMod using filepath.Rel instead of HasPrefix - Add input validation for IPPort, ServerParameters, ScheduledUpdate Memory safety: - Cap RPT buffer allocation to 64KB to prevent OOM on large logs - Cap GetLog file read to 10MB - Fix context cancel leak in SteamCmdManager.run() - Remove data-raced cancel field in steamcmd.go - Atomic file writes (write-temp-then-rename) across all managers Reliability: - Log save errors in ProcessManager.Stop() - Atomic file writes prevent corruption on crash Tests: - Add mod_manager_test.go (12 tests: ListWorkshopMods, ListLocalMods, BuildUsageMap, RemoveMod, dirSize) - Add scheduler_test.go (6 tests: Start/Stop, Refresh with empty, invalid, valid, and replaced cron expressions) - Add TestRestart to server_process_test.go CI/Docs: - Add -race flag to go test in CI and Makefile - Add npm lint step to CI - Add Go/npm module caching to CI - Update README: prerequisites, AUTH_TOKEN/GIN_MODE/SERVERS_DIR docs, fix manual quickstart to use make build
This commit is contained in:
@@ -59,11 +59,11 @@ func (cm *ConfigManager) Create(name, content string) error {
|
||||
if _, err := os.Stat(path); err == nil {
|
||||
return fmt.Errorf("config already exists")
|
||||
}
|
||||
return os.WriteFile(path, []byte(content), 0644)
|
||||
return writeFileAtomic(path, []byte(content))
|
||||
}
|
||||
|
||||
func (cm *ConfigManager) Update(name, content string) error {
|
||||
return os.WriteFile(cm.path(name), []byte(content), 0644)
|
||||
return writeFileAtomic(cm.path(name), []byte(content))
|
||||
}
|
||||
|
||||
func (cm *ConfigManager) Delete(name string) error {
|
||||
|
||||
@@ -0,0 +1,209 @@
|
||||
package services
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"arma3-web-server/internal/models"
|
||||
)
|
||||
|
||||
func TestListWorkshopMods_Empty(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
mods := ListWorkshopMods(dir)
|
||||
if len(mods) != 0 {
|
||||
t.Fatalf("expected 0 mods, got %d", len(mods))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListWorkshopMods_WithMods(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
workshopDir := filepath.Join(dir, "steamapps", "workshop", "content", "107410")
|
||||
if err := os.MkdirAll(filepath.Join(workshopDir, "123456"), 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(workshopDir, "789012"), 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
mods := ListWorkshopMods(dir)
|
||||
if len(mods) != 2 {
|
||||
t.Fatalf("expected 2 mods, got %d", len(mods))
|
||||
}
|
||||
|
||||
ids := map[string]bool{}
|
||||
for _, m := range mods {
|
||||
ids[m.ID] = true
|
||||
if m.Source != "workshop" {
|
||||
t.Errorf("expected source 'workshop', got %q", m.Source)
|
||||
}
|
||||
}
|
||||
if !ids["123456"] || !ids["789012"] {
|
||||
t.Errorf("expected both mod IDs, got %v", ids)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListWorkshopMods_SkipsFiles(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
workshopDir := filepath.Join(dir, "steamapps", "workshop", "content", "107410")
|
||||
if err := os.MkdirAll(workshopDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(workshopDir, "notadir.txt"), []byte("hi"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(workshopDir, "111111"), 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
mods := ListWorkshopMods(dir)
|
||||
if len(mods) != 1 {
|
||||
t.Fatalf("expected 1 mod, got %d", len(mods))
|
||||
}
|
||||
if mods[0].ID != "111111" {
|
||||
t.Errorf("expected ID '111111', got %q", mods[0].ID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListLocalMods_Empty(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
mods := ListLocalMods(dir)
|
||||
if len(mods) != 0 {
|
||||
t.Fatalf("expected 0 mods, got %d", len(mods))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListLocalMods_WithMods(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.MkdirAll(filepath.Join(dir, "@ACE3"), 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(dir, "@TFAR"), 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
mods := ListLocalMods(dir)
|
||||
if len(mods) != 2 {
|
||||
t.Fatalf("expected 2 mods, got %d", len(mods))
|
||||
}
|
||||
|
||||
for _, m := range mods {
|
||||
if m.Source != "local" {
|
||||
t.Errorf("expected source 'local', got %q", m.Source)
|
||||
}
|
||||
if len(m.Name) > 0 && m.Name[0] != '@' {
|
||||
t.Errorf("expected name to start with '@', got %q", m.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestListLocalMods_SkipsFiles(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "readme.txt"), []byte("hi"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Join(dir, "@Mod"), 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
mods := ListLocalMods(dir)
|
||||
if len(mods) != 1 {
|
||||
t.Fatalf("expected 1 mod, got %d", len(mods))
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildUsageMap(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
mm := NewModlistManager(dataDir)
|
||||
|
||||
ml, err := mm.Create("Test List")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
ml, err = mm.Update(ml.ID, ml.Name, []models.ModEntry{
|
||||
{ID: "111111", Name: "ACE3", Enabled: true},
|
||||
{ID: "222222", Name: "", Enabled: true},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_ = ml
|
||||
|
||||
usedWorkshop, usedLocal := BuildUsageMap(mm)
|
||||
|
||||
if lists, ok := usedWorkshop["111111"]; !ok || len(lists) == 0 {
|
||||
t.Errorf("expected workshop mod 111111 to be in use")
|
||||
}
|
||||
if _, ok := usedWorkshop["222222"]; !ok {
|
||||
t.Errorf("expected workshop mod 222222 to be in use")
|
||||
}
|
||||
if _, ok := usedLocal["ACE3"]; !ok {
|
||||
t.Errorf("expected local mod ACE3 to be in use")
|
||||
}
|
||||
if _, ok := usedLocal["@ACE3"]; !ok {
|
||||
t.Errorf("expected local mod @ACE3 to be in use")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildUsageMap_Empty(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
mm := NewModlistManager(dataDir)
|
||||
|
||||
usedWorkshop, usedLocal := BuildUsageMap(mm)
|
||||
if len(usedWorkshop) != 0 || len(usedLocal) != 0 {
|
||||
t.Errorf("expected empty maps, got workshop=%v local=%v", usedWorkshop, usedLocal)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemoveMod(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
modDir := filepath.Join(dir, "@TestMod")
|
||||
if err := os.MkdirAll(modDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(modDir, "config.cpp"), []byte("x"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if err := RemoveMod(modDir); err != nil {
|
||||
t.Fatalf("RemoveMod failed: %v", err)
|
||||
}
|
||||
if _, err := os.Stat(modDir); !os.IsNotExist(err) {
|
||||
t.Errorf("expected directory to be removed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRemoveMod_Nonexistent(t *testing.T) {
|
||||
err := RemoveMod("/nonexistent/path/that/does/not/exist")
|
||||
if err != nil {
|
||||
t.Errorf("RemoveMod on nonexistent path should not error, got: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDirSize(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "a.txt"), []byte("hello"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sub := filepath.Join(dir, "sub")
|
||||
if err := os.MkdirAll(sub, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(sub, "b.txt"), []byte("world!"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
size := dirSize(dir)
|
||||
if size != 11 {
|
||||
t.Errorf("expected size 11, got %d", size)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDirSize_Empty(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
size := dirSize(dir)
|
||||
if size != 0 {
|
||||
t.Errorf("expected size 0, got %d", size)
|
||||
}
|
||||
}
|
||||
@@ -103,7 +103,7 @@ func (mm *ModlistManager) Create(name string) (*models.Modlist, error) {
|
||||
if err := os.MkdirAll(mm.dir, 0755); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := os.WriteFile(mm.path(m.ID), data, 0644); err != nil {
|
||||
if err := writeFileAtomic(mm.path(m.ID), data); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return m, nil
|
||||
@@ -126,7 +126,7 @@ func (mm *ModlistManager) Update(id, name string, mods []models.ModEntry) (*mode
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := os.WriteFile(mm.path(id), data, 0644); err != nil {
|
||||
if err := writeFileAtomic(mm.path(id), data); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return m, nil
|
||||
@@ -160,7 +160,7 @@ func (mm *ModlistManager) Duplicate(id, newName string) (*models.Modlist, error)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := os.WriteFile(mm.path(dup.ID), data, 0644); err != nil {
|
||||
if err := writeFileAtomic(mm.path(dup.ID), data); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return dup, nil
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
package services
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"arma3-web-server/internal/models"
|
||||
)
|
||||
|
||||
func TestNewScheduler(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
settings := NewSettingsManager(dataDir)
|
||||
modlists := NewModlistManager(dataDir)
|
||||
streamer := NewLogStreamer()
|
||||
steamcmd := NewSteamCmdManager(t.TempDir(), streamer)
|
||||
|
||||
s := NewScheduler(settings, modlists, steamcmd)
|
||||
if s == nil {
|
||||
t.Fatal("expected non-nil scheduler")
|
||||
}
|
||||
if s.cron == nil {
|
||||
t.Fatal("expected non-nil cron")
|
||||
}
|
||||
}
|
||||
|
||||
func TestScheduler_StartStop(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
settings := NewSettingsManager(dataDir)
|
||||
modlists := NewModlistManager(dataDir)
|
||||
streamer := NewLogStreamer()
|
||||
steamcmd := NewSteamCmdManager(t.TempDir(), streamer)
|
||||
|
||||
s := NewScheduler(settings, modlists, steamcmd)
|
||||
s.Start()
|
||||
time.Sleep(50 * time.Millisecond)
|
||||
s.Stop()
|
||||
}
|
||||
|
||||
func TestScheduler_Refresh_Empty(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
settings := NewSettingsManager(dataDir)
|
||||
modlists := NewModlistManager(dataDir)
|
||||
streamer := NewLogStreamer()
|
||||
steamcmd := NewSteamCmdManager(t.TempDir(), streamer)
|
||||
|
||||
s := NewScheduler(settings, modlists, steamcmd)
|
||||
s.Start()
|
||||
defer s.Stop()
|
||||
|
||||
s.Refresh()
|
||||
if s.entryID != 0 {
|
||||
t.Errorf("expected entryID 0 with empty settings, got %d", s.entryID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScheduler_Refresh_InvalidCron(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
settings := NewSettingsManager(dataDir)
|
||||
modlists := NewModlistManager(dataDir)
|
||||
streamer := NewLogStreamer()
|
||||
steamcmd := NewSteamCmdManager(t.TempDir(), streamer)
|
||||
|
||||
_, err := settings.Update(func(s *models.ServerSettings) {
|
||||
s.ScheduledUpdate = "not-a-cron"
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
sched := NewScheduler(settings, modlists, steamcmd)
|
||||
sched.Start()
|
||||
defer sched.Stop()
|
||||
|
||||
sched.Refresh()
|
||||
if sched.entryID != 0 {
|
||||
t.Errorf("expected entryID 0 with invalid cron, got %d", sched.entryID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScheduler_Refresh_ValidCron(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
settings := NewSettingsManager(dataDir)
|
||||
modlists := NewModlistManager(dataDir)
|
||||
streamer := NewLogStreamer()
|
||||
steamcmd := NewSteamCmdManager(t.TempDir(), streamer)
|
||||
|
||||
_, err := settings.Update(func(s *models.ServerSettings) {
|
||||
s.ScheduledUpdate = "0 4 * * *"
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
sched := NewScheduler(settings, modlists, steamcmd)
|
||||
sched.Start()
|
||||
defer sched.Stop()
|
||||
|
||||
sched.Refresh()
|
||||
if sched.entryID == 0 {
|
||||
t.Errorf("expected non-zero entryID with valid cron")
|
||||
}
|
||||
}
|
||||
|
||||
func TestScheduler_Refresh_Replace(t *testing.T) {
|
||||
dataDir := t.TempDir()
|
||||
settings := NewSettingsManager(dataDir)
|
||||
modlists := NewModlistManager(dataDir)
|
||||
streamer := NewLogStreamer()
|
||||
steamcmd := NewSteamCmdManager(t.TempDir(), streamer)
|
||||
|
||||
sched := NewScheduler(settings, modlists, steamcmd)
|
||||
sched.Start()
|
||||
defer sched.Stop()
|
||||
|
||||
_, err := settings.Update(func(s *models.ServerSettings) {
|
||||
s.ScheduledUpdate = "0 4 * * *"
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sched.Refresh()
|
||||
id1 := sched.entryID
|
||||
|
||||
_, err = settings.Update(func(s *models.ServerSettings) {
|
||||
s.ScheduledUpdate = "0 5 * * *"
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sched.Refresh()
|
||||
id2 := sched.entryID
|
||||
|
||||
if id1 == id2 {
|
||||
t.Errorf("expected different entryIDs after refresh, both got %d", id1)
|
||||
}
|
||||
}
|
||||
@@ -3,6 +3,7 @@ package services
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log"
|
||||
"net"
|
||||
"os"
|
||||
"os/exec"
|
||||
@@ -184,7 +185,9 @@ func (pm *ProcessManager) Stop() error {
|
||||
s, err := pm.settings.Load()
|
||||
if err == nil {
|
||||
s.WasRunning = false
|
||||
pm.settings.Save(s)
|
||||
if err := pm.settings.Save(s); err != nil {
|
||||
log.Printf("stop: save was_running: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -307,7 +310,7 @@ func (pm *ProcessManager) WriteUserconfigFiles(s *models.ServerSettings) error {
|
||||
}
|
||||
for name, content := range files {
|
||||
path := filepath.Join(userconfigDir, name)
|
||||
if err := os.WriteFile(path, []byte(content), 0644); err != nil {
|
||||
if err := writeFileAtomic(path, []byte(content)); err != nil {
|
||||
return fmt.Errorf("write %s: %w", name, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -983,3 +983,29 @@ func TestStart_StubWritesRPTLog(t *testing.T) {
|
||||
pm.Stop()
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
}
|
||||
|
||||
func TestRestart(t *testing.T) {
|
||||
pm, _ := setupStubPM(t, "-t 10")
|
||||
|
||||
if err := pm.Start(); err != nil {
|
||||
t.Fatalf("Start() error = %v", err)
|
||||
}
|
||||
if !pm.IsRunning() {
|
||||
t.Fatal("IsRunning() should be true after Start()")
|
||||
}
|
||||
|
||||
if err := pm.Restart(); err != nil {
|
||||
t.Fatalf("Restart() error = %v", err)
|
||||
}
|
||||
if !pm.IsRunning() {
|
||||
t.Fatal("IsRunning() should be true after Restart()")
|
||||
}
|
||||
|
||||
if err := pm.Stop(); err != nil {
|
||||
t.Fatalf("Stop() error = %v", err)
|
||||
}
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
if pm.IsRunning() {
|
||||
t.Fatal("IsRunning() should be false after Stop()")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,29 @@ import (
|
||||
"arma3-web-server/internal/models"
|
||||
)
|
||||
|
||||
func writeFileAtomic(path string, data []byte) error {
|
||||
dir := filepath.Dir(path)
|
||||
tmp, err := os.CreateTemp(dir, ".tmp-*")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
tmpPath := tmp.Name()
|
||||
if _, err := tmp.Write(data); err != nil {
|
||||
tmp.Close()
|
||||
os.Remove(tmpPath)
|
||||
return err
|
||||
}
|
||||
if err := tmp.Close(); err != nil {
|
||||
os.Remove(tmpPath)
|
||||
return err
|
||||
}
|
||||
if err := os.Rename(tmpPath, path); err != nil {
|
||||
os.Remove(tmpPath)
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type SettingsManager struct {
|
||||
path string
|
||||
mu sync.Mutex
|
||||
@@ -49,7 +72,45 @@ func (sm *SettingsManager) Save(s *models.ServerSettings) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return os.WriteFile(sm.path, data, 0644)
|
||||
return writeFileAtomic(sm.path, data)
|
||||
}
|
||||
|
||||
func (sm *SettingsManager) Update(fn func(*models.ServerSettings)) (*models.ServerSettings, error) {
|
||||
sm.mu.Lock()
|
||||
defer sm.mu.Unlock()
|
||||
|
||||
data, err := os.ReadFile(sm.path)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
s := sm.defaults()
|
||||
fn(s)
|
||||
s.UpdatedAt = time.Now().UTC()
|
||||
out, err := json.MarshalIndent(s, "", " ")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := writeFileAtomic(sm.path, out); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return s, nil
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var s models.ServerSettings
|
||||
if err := json.Unmarshal(data, &s); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
fn(&s)
|
||||
s.UpdatedAt = time.Now().UTC()
|
||||
out, err := json.MarshalIndent(s, "", " ")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := writeFileAtomic(sm.path, out); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &s, nil
|
||||
}
|
||||
|
||||
func (sm *SettingsManager) defaults() *models.ServerSettings {
|
||||
|
||||
@@ -25,7 +25,6 @@ type SteamCmdManager struct {
|
||||
serverfileDir string
|
||||
streamer *LogStreamer
|
||||
running atomic.Bool
|
||||
cancel context.CancelFunc
|
||||
}
|
||||
|
||||
func NewSteamCmdManager(serverfileDir string, streamer *LogStreamer) *SteamCmdManager {
|
||||
@@ -115,15 +114,13 @@ func (s *SteamCmdManager) run(label string, args []string) error {
|
||||
return fmt.Errorf("start steamcmd: %w", err)
|
||||
}
|
||||
|
||||
s.cancel = cancel
|
||||
|
||||
go s.streamer.Stream(ctx, "steamcmd", stdout, "")
|
||||
go s.streamer.Stream(ctx, "steamcmd", stderr, "")
|
||||
|
||||
go func() {
|
||||
err := cmd.Wait()
|
||||
cancel()
|
||||
s.running.Store(false)
|
||||
s.cancel = nil
|
||||
if err == nil {
|
||||
s.streamer.Broadcast("steamcmd", "[STEAMCMD] SUCCESS: "+label+" finished")
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user