felhom-controller/controller/internal/settings/r578_callback_lock_gate_test.go

package settings

import (
	"go/ast"
	"go/parser"
	"go/token"
	"os"
	"path/filepath"
	"sort"
	"strconv"
	"strings"
	"testing"
)

// R-578 — the settings-callback deadlock, as a gate over EVERY package of the controller.
//
// `UpdateOffboxStatus` (and every other *Settings method that takes a func and the settings lock)
// runs its callback while holding the settings WRITE lock. sync.RWMutex is not reentrant: anything
// inside that callback that takes the settings lock again — a getter, a setter, or a package helper
// that ends in one (`m.note` → `m.boxLang` → `GetLanguage`) — DEADLOCKS while holding the lock, and
// wedges everything else on the box that touches settings.json. Release C of localisation slice 2
// shipped exactly that; the only symptom was a 25-minute test timeout. The backup-only predecessor,
// TestNoteHelpersAreNotCalledUnderTheSettingsLock, knew three helper names in one package.
//
// This test derives everything from source instead of a list:
//   - lockers:   *Settings methods that take s.mu (directly or through another *Settings method);
//   - takers:    lockers that have a func-typed parameter (the callback runners);
//   - tainted:   per package, every func/method whose body calls an exported locker or another
//     tainted name of the same package (fixpoint) — so a new helper is covered the day it is written;
//   - violation: a call to a locker or a tainted name inside a func literal passed to a taker, OR a
//     taker given something that is not a literal (the gate could not see inside it).
//
// The matching is by NAME (no type checker — this must stay fast inside `go test ./...`). A false
// positive is possible if an unrelated type has a method named like a settings locker and is called
// inside a callback; rename it or hoist the call — both are cheap, a hang is not.
//
// RED-PROOF (R-578): put `lang := m.note("x")` inside the final UpdateOffboxStatus callback in
// internal/backup/offbox.go → this test names internal/backup/offbox.go:<line> and the call chain.
func TestR578NothingTakesTheSettingsLockInsideASettingsCallback(t *testing.T) {
	root := filepath.Join("..", "..") // controller/
	pkgs := parseControllerPackages(t, root)

	own, ok := pkgs[filepath.Join(root, "internal", "settings")]
	if !ok {
		t.Fatal("internal/settings was not parsed — the walk no longer finds this package")
	}
	lockers, takers := settingsLockers(own)
	if len(lockers) < 20 || len(takers) == 0 {
		t.Fatalf("found %d settings lockers and %d callback runners — the pattern no longer matches the code", len(lockers), len(takers))
	}
	exported := map[string]bool{}
	for name := range lockers {
		if ast.IsExported(name) {
			exported[name] = true
		}
	}

	var bad []string
	callbacks := 0
	backupTaint := map[string]bool{}
	for dir, files := range pkgs {
		tainted := taintedFuncs(files, exported)
		if dir == filepath.Join(root, "internal", "backup") {
			backupTaint = tainted
		}
		for _, pf := range files {
			ast.Inspect(pf.file, func(n ast.Node) bool {
				call, ok := n.(*ast.CallExpr)
				if !ok {
					return true
				}
				sel, ok := call.Fun.(*ast.SelectorExpr)
				if !ok {
					return true
				}
				idx, isTaker := takers[sel.Sel.Name]
				if !isTaker {
					return true
				}
				for _, i := range idx {
					if i >= len(call.Args) {
						continue
					}
					arg := call.Args[i]
					if lit, ok := arg.(*ast.FuncLit); ok {
						callbacks++
						for _, hit := range lockingCalls(lit.Body, exported, tainted) {
							bad = append(bad, pf.pos(hit.pos)+"  "+hit.name+"() inside "+sel.Sel.Name+"(func…)")
						}
						continue
					}
					bad = append(bad, pf.pos(arg.Pos())+"  "+sel.Sel.Name+" is passed a non-literal callback — the gate cannot see inside it; pass a func literal")
				}
				return true
			})
		}
	}

	// Positive controls: the instrument is looking at something, and the transitive taint reaches
	// the helpers that actually caused the 2026-09-18 deadlock.
	if callbacks < 10 {
		t.Fatalf("found only %d settings callbacks across the controller — the walk no longer matches the code", callbacks)
	}
	for _, h := range []string{"boxLang", "note", "noteErr"} {
		if !backupTaint[h] {
			t.Errorf("internal/backup %s() is not seen as taking the settings lock — the taint analysis lost the chain that deadlocked release C", h)
		}
	}
	if len(bad) > 0 {
		sort.Strings(bad)
		t.Errorf("a settings callback (which holds the settings WRITE lock) calls something that takes "+
			"the settings lock again — sync.RWMutex is not reentrant, so this DEADLOCKS and wedges "+
			"settings.json for the whole box. Hoist the call before the callback (%d):\n  %s",
			len(bad), strings.Join(bad, "\n  "))
	}
	t.Logf("%d packages, %d settings lockers, %d callback runners, %d callbacks examined", len(pkgs), len(lockers), len(takers), callbacks)
}

// TestR578GateConvictsAHelperChain is the decoy: a synthetic package whose callback reaches the
// settings lock only through two local helpers. If the taint fixpoint stops following calls, this
// fails before the real gate quietly goes blind.
func TestR578GateConvictsAHelperChain(t *testing.T) {
	src := `package decoy
type M struct{ settings *S }
func (m *M) lang() string { return m.settings.GetLanguage() }
func (m *M) msg() string  { return "x" + m.lang() }
func (m *M) clean() string { return "y" }
func (m *M) run() {
	_ = m.settings.UpdateOffboxStatus(func(o *T) { o.A = m.msg(); o.B = m.clean() })
}`
	fset := token.NewFileSet()
	f, err := parser.ParseFile(fset, "decoy.go", src, 0)
	if err != nil {
		t.Fatal(err)
	}
	files := []parsedFile{{fset: fset, file: f, path: "decoy.go"}}
	exported := map[string]bool{"GetLanguage": true}
	tainted := taintedFuncs(files, exported)
	if !tainted["msg"] || !tainted["lang"] || tainted["clean"] {
		t.Fatalf("taint = %v; want lang+msg tainted, clean not", tainted)
	}
	var lit *ast.FuncLit
	ast.Inspect(f, func(n ast.Node) bool {
		if l, ok := n.(*ast.FuncLit); ok {
			lit = l
		}
		return true
	})
	hits := lockingCalls(lit.Body, exported, tainted)
	if len(hits) != 1 || hits[0].name != "msg" {
		t.Fatalf("hits = %+v; want exactly msg()", hits)
	}
}

type parsedFile struct {
	fset *token.FileSet
	file *ast.File
	path string
}

func (p parsedFile) pos(pos token.Pos) string {
	ps := p.fset.Position(pos)
	return filepath.ToSlash(p.path) + ":" + strconv.Itoa(ps.Line)
}

// parseControllerPackages parses every non-test .go file under controller/, grouped by directory.
func parseControllerPackages(t *testing.T, root string) map[string][]parsedFile {
	t.Helper()
	fset := token.NewFileSet()
	out := map[string][]parsedFile{}
	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
		if err != nil {
			return err
		}
		if info.IsDir() {
			base := info.Name()
			if path != root && (strings.HasPrefix(base, ".") || base == "testdata" || base == "vendor") {
				return filepath.SkipDir
			}
			return nil
		}
		if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
			return nil
		}
		f, perr := parser.ParseFile(fset, path, nil, 0)
		if perr != nil {
			return perr
		}
		dir := filepath.Dir(path)
		rel, _ := filepath.Rel(root, path)
		out[dir] = append(out[dir], parsedFile{fset: fset, file: f, path: rel})
		return nil
	})
	if err != nil {
		t.Fatal(err)
	}
	return out
}

// settingsLockers returns the *Settings methods that take s.mu (directly, or by calling another
// *Settings method that does), and the subset that also take a func-typed parameter (with the
// argument positions of those parameters).
func settingsLockers(files []parsedFile) (lockers map[string]bool, takers map[string][]int) {
	type method struct {
		recv string
		decl *ast.FuncDecl
	}
	var methods []method
	for _, pf := range files {
		for _, d := range pf.file.Decls {
			fd, ok := d.(*ast.FuncDecl)
			if !ok || fd.Recv == nil || len(fd.Recv.List) != 1 || fd.Body == nil {
				continue
			}
			star, ok := fd.Recv.List[0].Type.(*ast.StarExpr)
			if !ok {
				continue
			}
			if id, ok := star.X.(*ast.Ident); !ok || id.Name != "Settings" {
				continue
			}
			recv := "_"
			if len(fd.Recv.List[0].Names) == 1 {
				recv = fd.Recv.List[0].Names[0].Name
			}
			methods = append(methods, method{recv, fd})
		}
	}
	lockers = map[string]bool{}
	for changed := true; changed; {
		changed = false
		for _, m := range methods {
			if lockers[m.decl.Name.Name] {
				continue
			}
			found := false
			ast.Inspect(m.decl.Body, func(n ast.Node) bool {
				call, ok := n.(*ast.CallExpr)
				if !ok || found {
					return !found
				}
				sel, ok := call.Fun.(*ast.SelectorExpr)
				if !ok {
					return true
				}
				// s.mu.Lock() / s.mu.RLock()
				if (sel.Sel.Name == "Lock" || sel.Sel.Name == "RLock") && isRecvField(sel.X, m.recv, "mu") {
					found = true
				}
				// s.OtherLockingMethod()
				if id, ok := sel.X.(*ast.Ident); ok && id.Name == m.recv && lockers[sel.Sel.Name] {
					found = true
				}
				return !found
			})
			if found {
				lockers[m.decl.Name.Name] = true
				changed = true
			}
		}
	}
	// takers: locker -> the argument positions that are func-typed.
	takers = map[string][]int{}
	for _, m := range methods {
		if !lockers[m.decl.Name.Name] {
			continue
		}
		pos := 0
		for _, p := range m.decl.Type.Params.List {
			n := len(p.Names)
			if n == 0 {
				n = 1
			}
			if _, ok := p.Type.(*ast.FuncType); ok {
				for k := 0; k < n; k++ {
					takers[m.decl.Name.Name] = append(takers[m.decl.Name.Name], pos+k)
				}
			}
			pos += n
		}
	}
	return lockers, takers
}

func isRecvField(x ast.Expr, recv, field string) bool {
	sel, ok := x.(*ast.SelectorExpr)
	if !ok || sel.Sel.Name != field {
		return false
	}
	id, ok := sel.X.(*ast.Ident)
	return ok && id.Name == recv
}

// calledName is the name a call expression invokes: `f(…)` → f, `x.y.f(…)` → f.
func calledName(call *ast.CallExpr) string {
	switch fn := call.Fun.(type) {
	case *ast.Ident:
		return fn.Name
	case *ast.SelectorExpr:
		return fn.Sel.Name
	}
	return ""
}

// taintedFuncs returns, for one package, the names of funcs/methods whose body (transitively, within
// the package) calls an exported settings locker.
func taintedFuncs(files []parsedFile, exported map[string]bool) map[string]bool {
	bodies := map[string][]*ast.BlockStmt{}
	for _, pf := range files {
		for _, d := range pf.file.Decls {
			if fd, ok := d.(*ast.FuncDecl); ok && fd.Body != nil {
				bodies[fd.Name.Name] = append(bodies[fd.Name.Name], fd.Body)
			}
		}
	}
	tainted := map[string]bool{}
	for changed := true; changed; {
		changed = false
		for name, bs := range bodies {
			if tainted[name] {
				continue
			}
			for _, b := range bs {
				if len(lockingCalls(b, exported, tainted)) > 0 {
					tainted[name] = true
					changed = true
					break
				}
			}
		}
	}
	return tainted
}

type lockHit struct {
	name string
	pos  token.Pos
}

func lockingCalls(body ast.Node, exported, tainted map[string]bool) []lockHit {
	var hits []lockHit
	ast.Inspect(body, func(n ast.Node) bool {
		call, ok := n.(*ast.CallExpr)
		if !ok {
			return true
		}
		name := calledName(call)
		_, isSel := call.Fun.(*ast.SelectorExpr)
		if (isSel && exported[name]) || tainted[name] {
			hits = append(hits, lockHit{name, call.Pos()})
		}
		return true
	})
	return hits
}