Full codebase audit: 21 security/robustness fixes, 328 tests across 4 packages

Security & economy:
- Credit()/Debit() reject non-positive amounts (closes infinite-money exploit)
- Lottery ticket purchase uses in-transaction count check (closes TOCTOU race)
- Arena bail channel ownership prevents double-close panic
- Entry.ID validated before exec.Command in fetch-esteemed
- Internal errors no longer leaked to users (forex, esteemed)

Robustness:
- safeGo() panic recovery added to 17 goroutine launch sites across 14 plugins
- db.Close() added and called on shutdown
- Miniflux mutex pattern fixed (snapshot-under-lock)
- Silently ignored DB/JSON errors now logged (lottery_db)

Performance:
- Added indexes: idx_arena_runs_user(user_id, status), idx_euro_bal_user(user_id)

UX:
- Empty !buy args shows usage instead of "No item matching ''"
- "Failed to load character" now suggests !adventure

Test coverage:
- internal/util/parser_test.go (19 tests: XP, levels, progress bar, commands, parsing)
- internal/crypto/crypto_test.go (12 tests: encrypt/decrypt, HMAC, ParseKey)
- internal/plugin/helpers_test.go (30 tests: formatNumber, calc, lottery, chat perks)
- internal/plugin/audit_fixes_test.go (25 tests: safeGo, bail channels, error leaks, overflow)
- internal/db/db_test.go (4 tests: Init, Close, schema indexes)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
prosolis
2026-04-08 17:04:40 -07:00
parent 68b2f8b7a5
commit e459b6e78d
34 changed files with 2443 additions and 486 deletions

View File

@@ -56,6 +56,18 @@ func Get() *sql.DB {
return globalDB
}
// Close closes the database connection. Call on shutdown.
func Close() {
mu.Lock()
defer mu.Unlock()
if globalDB != nil {
if err := globalDB.Close(); err != nil {
slog.Error("db: close failed", "err", err)
}
globalDB = nil
}
}
func runMigrations(d *sql.DB) error {
if _, err := d.Exec(schema); err != nil {
return err
@@ -86,6 +98,8 @@ func runMigrations(d *sql.DB) error {
`ALTER TABLE adventure_characters ADD COLUMN combat_actions_used INTEGER NOT NULL DEFAULT 0`,
`ALTER TABLE adventure_characters ADD COLUMN harvest_actions_used INTEGER NOT NULL DEFAULT 0`,
`ALTER TABLE adventure_characters ADD COLUMN last_pardon_used DATETIME`,
`ALTER TABLE arena_runs ADD COLUMN tier_earnings INTEGER NOT NULL DEFAULT 0`,
`ALTER TABLE arena_runs ADD COLUMN xp_accumulated INTEGER NOT NULL DEFAULT 0`,
}
for _, stmt := range columnMigrations {
if _, err := d.Exec(stmt); err != nil {
@@ -698,6 +712,8 @@ CREATE TABLE IF NOT EXISTS euro_balances (
updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
);
CREATE INDEX IF NOT EXISTS idx_euro_bal_user ON euro_balances(user_id);
CREATE TABLE IF NOT EXISTS euro_transactions (
id INTEGER PRIMARY KEY AUTOINCREMENT,
user_id TEXT NOT NULL,
@@ -1030,6 +1046,8 @@ CREATE TABLE IF NOT EXISTS arena_runs (
ended_at INTEGER
);
CREATE INDEX IF NOT EXISTS idx_arena_runs_user ON arena_runs(user_id, status);
CREATE TABLE IF NOT EXISTS arena_history (
id INTEGER PRIMARY KEY AUTOINCREMENT,
user_id TEXT NOT NULL,

112
internal/db/db_test.go Normal file
View File

@@ -0,0 +1,112 @@
package db
import (
"os"
"path/filepath"
"testing"
)
func TestInitAndClose(t *testing.T) {
dir := t.TempDir()
if err := Init(dir); err != nil {
t.Fatalf("Init: %v", err)
}
// DB should be usable
d := Get()
if d == nil {
t.Fatal("Get() returned nil after Init")
}
// Verify DB file was created
dbPath := filepath.Join(dir, "gogobee.db")
if _, err := os.Stat(dbPath); err != nil {
t.Fatalf("DB file not created: %v", err)
}
// Close should not panic
Close()
// After Close, globalDB should be nil
if globalDB != nil {
t.Error("globalDB should be nil after Close")
}
// Double Close should not panic
Close()
}
func TestInit_Idempotent(t *testing.T) {
dir := t.TempDir()
if err := Init(dir); err != nil {
t.Fatalf("first Init: %v", err)
}
// Second Init should be a no-op (globalDB already set)
if err := Init(dir); err != nil {
t.Fatalf("second Init: %v", err)
}
Close()
}
func TestInit_SchemaContainsIndexes(t *testing.T) {
// Verify the schema string contains our audit-added indexes
if !containsSubstring(schema, "idx_arena_runs_user") {
t.Error("schema missing idx_arena_runs_user index")
}
if !containsSubstring(schema, "idx_euro_bal_user") {
t.Error("schema missing idx_euro_bal_user index")
}
}
func TestInit_SchemaRunsCleanly(t *testing.T) {
dir := t.TempDir()
if err := Init(dir); err != nil {
t.Fatalf("Init failed: %v", err)
}
defer Close()
// Verify we can query a table that was created by the schema
d := Get()
var count int
err := d.QueryRow("SELECT COUNT(*) FROM users").Scan(&count)
if err != nil {
t.Fatalf("query users table: %v", err)
}
// Verify indexes exist
rows, err := d.Query("SELECT name FROM sqlite_master WHERE type='index' AND name LIKE 'idx_%'")
if err != nil {
t.Fatalf("query indexes: %v", err)
}
defer rows.Close()
indexes := make(map[string]bool)
for rows.Next() {
var name string
rows.Scan(&name)
indexes[name] = true
}
for _, idx := range []string{"idx_arena_runs_user", "idx_euro_bal_user", "idx_euro_tx_user"} {
if !indexes[idx] {
t.Errorf("missing index: %s", idx)
}
}
}
func containsSubstring(s, sub string) bool {
return len(s) >= len(sub) && searchSubstring(s, sub)
}
func searchSubstring(s, sub string) bool {
for i := 0; i <= len(s)-len(sub); i++ {
if s[i:i+len(sub)] == sub {
return true
}
}
return false
}