fix(data): address medium-severity audit findings

Three fixes from the code audit:

1. parseCents now rejects negative values -- all money fields represent
   costs, fees, or budgets where negatives don't make sense. Inputs like
   "-$5.00", "$-100", and "--$5" are all rejected with ErrInvalidMoney.

2. requireParentAlive now distinguishes soft-deleted parents from
   truly missing parents. Callers show "X is deleted -- restore it
   first" vs "X no longer exists" as appropriate. Introduced
   ErrParentDeleted / ErrParentNotFound sentinel errors.

3. SQLite Open() now sets journal_mode=WAL, synchronous=NORMAL, and
   busy_timeout=5000 for better durability and concurrency behavior.

closes #186

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: 3 people

internal/app/forms.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -1512,10 +1513,17 @@ func optionalDate(label string) func(string) error {
 	}
 }
 
+func moneyError(label string, err error) error {
+	if errors.Is(err, data.ErrNegativeMoney) {
+		return fmt.Errorf("%s must be a positive amount", label)
+	}
+	return fmt.Errorf("%s should look like 1250.00", label)
+}
+
 func optionalMoney(label string) func(string) error {
 	return func(input string) error {
 		if _, err := data.ParseOptionalCents(input); err != nil {
-			return fmt.Errorf("%s should look like 1250.00", label)
+			return moneyError(label, err)
 		}
 		return nil
 	}
@@ -1524,7 +1532,7 @@ func optionalMoney(label string) func(string) error {
 func requiredMoney(label string) func(string) error {
 	return func(input string) error {
 		if _, err := data.ParseRequiredCents(input); err != nil {
-			return fmt.Errorf("%s should look like 1250.00", label)
+			return moneyError(label, err)
 		}
 		return nil
 	}

internal/data/store.go

@@ -33,8 +33,19 @@ func Open(path string) (*Store, error) {
 	if err != nil {
 		return nil, fmt.Errorf("open db: %w", err)
 	}
-	if err := db.Exec("PRAGMA foreign_keys = ON").Error; err != nil {
-		return nil, fmt.Errorf("enable foreign keys: %w", err)
+	pragmas := []struct {
+		sql  string
+		desc string
+	}{
+		{"PRAGMA foreign_keys = ON", "enable foreign keys"},
+		{"PRAGMA journal_mode = WAL", "enable WAL mode"},
+		{"PRAGMA synchronous = NORMAL", "set synchronous mode"},
+		{"PRAGMA busy_timeout = 5000", "set busy timeout"},
+	}
+	for _, p := range pragmas {
+		if err := db.Exec(p.sql).Error; err != nil {
+			return nil, fmt.Errorf("%s: %w", p.desc, err)
+		}
 	}
 	return &Store{db: db}, nil
 }
@@ -710,11 +721,11 @@ func (s *Store) RestoreServiceLog(id uint) error {
 		return err
 	}
 	if err := s.requireParentAlive(&MaintenanceItem{}, entry.MaintenanceItemID); err != nil {
-		return fmt.Errorf("maintenance item is deleted -- restore it first")
+		return parentRestoreError("maintenance item", err)
 	}
 	if entry.VendorID != nil {
 		if err := s.requireParentAlive(&Vendor{}, *entry.VendorID); err != nil {
-			return fmt.Errorf("vendor is deleted -- restore it first")
+			return parentRestoreError("vendor", err)
 		}
 	}
 	return s.restoreEntity(&ServiceLogEntry{}, DeletionEntityServiceLog, id)
@@ -784,7 +795,7 @@ func (s *Store) RestoreProject(id uint) error {
 	}
 	if project.PreferredVendorID != nil {
 		if err := s.requireParentAlive(&Vendor{}, *project.PreferredVendorID); err != nil {
-			return fmt.Errorf("preferred vendor is deleted -- restore it first")
+			return parentRestoreError("preferred vendor", err)
 		}
 	}
 	return s.restoreEntity(&Project{}, DeletionEntityProject, id)
@@ -796,10 +807,10 @@ func (s *Store) RestoreQuote(id uint) error {
 		return err
 	}
 	if err := s.requireParentAlive(&Project{}, quote.ProjectID); err != nil {
-		return fmt.Errorf("project is deleted -- restore it first")
+		return parentRestoreError("project", err)
 	}
 	if err := s.requireParentAlive(&Vendor{}, quote.VendorID); err != nil {
-		return fmt.Errorf("vendor is deleted -- restore it first")
+		return parentRestoreError("vendor", err)
 	}
 	return s.restoreEntity(&Quote{}, DeletionEntityQuote, id)
 }
@@ -811,7 +822,7 @@ func (s *Store) RestoreMaintenance(id uint) error {
 	}
 	if item.ApplianceID != nil {
 		if err := s.requireParentAlive(&Appliance{}, *item.ApplianceID); err != nil {
-			return fmt.Errorf("appliance is deleted -- restore it first or clear the link")
+			return parentRestoreError("appliance", err)
 		}
 	}
 	return s.restoreEntity(&MaintenanceItem{}, DeletionEntityMaintenance, id)
@@ -821,11 +832,39 @@ func (s *Store) RestoreAppliance(id uint) error {
 	return s.restoreEntity(&Appliance{}, DeletionEntityAppliance, id)
 }
 
-// requireParentAlive returns an error if the given parent record is
-// soft-deleted (or doesn't exist). Uses the default GORM scope which
-// excludes soft-deleted rows.
+var (
+	// ErrParentDeleted indicates the parent record exists but is soft-deleted.
+	ErrParentDeleted = errors.New("parent record is deleted")
+	// ErrParentNotFound indicates the parent record doesn't exist at all.
+	ErrParentNotFound = errors.New("parent record not found")
+)
+
+// requireParentAlive returns ErrParentDeleted if the parent record is
+// soft-deleted, or ErrParentNotFound if it doesn't exist at all. Returns nil
+// when the parent is alive.
 func (s *Store) requireParentAlive(model any, id uint) error {
-	return s.db.First(model, id).Error
+	err := s.db.First(model, id).Error
+	if err == nil {
+		return nil
+	}
+	if !errors.Is(err, gorm.ErrRecordNotFound) {
+		return err
+	}
+	// Distinguish soft-deleted from truly missing.
+	if err := s.db.Unscoped().First(model, id).Error; err != nil {
+		return ErrParentNotFound
+	}
+	return ErrParentDeleted
+}
+
+// parentRestoreError returns a user-facing error message for a failed parent
+// alive check, distinguishing soft-deleted parents (restorable) from missing
+// parents (permanently gone).
+func parentRestoreError(entity string, err error) error {
+	if errors.Is(err, ErrParentNotFound) {
+		return fmt.Errorf("%s no longer exists", entity)
+	}
+	return fmt.Errorf("%s is deleted -- restore it first", entity)
 }
 
 // countDependents counts non-deleted rows in model where fkColumn equals id.

internal/data/validation.go

@@ -17,16 +17,17 @@ import (
 const DateLayout = "2006-01-02"
 
 var (
-	ErrInvalidMoney = errors.New("invalid money value")
-	ErrInvalidDate  = errors.New("invalid date value")
-	ErrInvalidInt   = errors.New("invalid integer value")
-	ErrInvalidFloat = errors.New("invalid decimal value")
+	ErrInvalidMoney  = errors.New("invalid money value")
+	ErrNegativeMoney = errors.New("negative money value")
+	ErrInvalidDate   = errors.New("invalid date value")
+	ErrInvalidInt    = errors.New("invalid integer value")
+	ErrInvalidFloat  = errors.New("invalid decimal value")
 )
 
 func ParseRequiredCents(input string) (int64, error) {
 	cents, err := parseCents(strings.TrimSpace(input))
 	if err != nil {
-		return 0, ErrInvalidMoney
+		return 0, err
 	}
 	return cents, nil
 }
@@ -38,7 +39,7 @@ func ParseOptionalCents(input string) (*int64, error) {
 	}
 	cents, err := parseCents(trimmed)
 	if err != nil {
-		return nil, ErrInvalidMoney
+		return nil, err
 	}
 	return &cents, nil
 }
@@ -182,6 +183,10 @@ func ComputeNextDue(last *time.Time, intervalMonths int) *time.Time {
 
 func parseCents(input string) (int64, error) {
 	clean := strings.ReplaceAll(input, ",", "")
+	// Reject negative values -- all money fields are costs/fees/budgets.
+	if strings.HasPrefix(clean, "-") {
+		return 0, ErrNegativeMoney
+	}
 	clean = strings.TrimPrefix(clean, "$")
 	if clean == "" {
 		return 0, ErrInvalidMoney

internal/data/validation_test.go

@@ -91,6 +91,30 @@ func TestFormatCentsNegative(t *testing.T) {
 	assert.Equal(t, "-$5.00", FormatCents(-500))
 }
 
+func TestParseCentsRejectsNegative(t *testing.T) {
+	// Leading-negative inputs return ErrNegativeMoney specifically.
+	for _, input := range []string{"-$5.00", "-5.00", "-$1,234.56"} {
+		_, err := ParseRequiredCents(input)
+		assert.ErrorIs(t, err, ErrNegativeMoney, "input=%q", input)
+	}
+	// Malformed negatives (sign in wrong position, bare dash) return ErrInvalidMoney.
+	for _, input := range []string{"$-100", "--$5", "-", "-$"} {
+		_, err := ParseRequiredCents(input)
+		assert.Error(t, err, "input=%q should be rejected", input)
+	}
+}
+
+func TestParseCentsFormatRoundtrip(t *testing.T) {
+	// FormatCents output should parse back to the same value.
+	values := []int64{0, 1, 99, 100, 123456}
+	for _, cents := range values {
+		formatted := FormatCents(cents)
+		parsed, err := ParseRequiredCents(formatted)
+		require.NoError(t, err, "roundtrip failed for %d (formatted=%q)", cents, formatted)
+		assert.Equal(t, cents, parsed, "roundtrip mismatch for %d (formatted=%q)", cents, formatted)
+	}
+}
+
 func TestFormatCentsZero(t *testing.T) {
 	assert.Equal(t, "$0.00", FormatCents(0))
 }