goroutinemissingrecover: recover-defer placed after risky code is accepted as safe, but never protects earlier statements

[github-actions]

Summary

goroutinemissingrecover treats a goroutine as "safe" whenever its function-literal body contains any top-level defer that resolves to a direct recover() call — regardless of where that defer sits relative to other statements. Per the Go spec, a defer only registers its call for the panic-unwind sequence once control flow actually reaches and executes the defer statement. If a panic-prone statement runs before the recover-guarding defer in source order, the panic occurs before the defer is registered, so recover() never runs and the goroutine still crashes the process — exactly the failure mode this linter exists to catch. The linter silently passes this case as "safe".

Location

pkg/linters/goroutinemissingrecover/goroutinemissingrecover.go:97-125 (hasTopLevelRecoverDefer)

func hasTopLevelRecoverDefer(body *ast.BlockStmt, typesInfo *types.Info, funcBodies map[*types.Func]*ast.BlockStmt) bool {
	if body == nil {
		return false
	}
	for _, stmt := range body.List {
		deferStmt, ok := stmt.(*ast.DeferStmt)
		if !ok {
			continue
		}
		...

The loop scans the entire statement list for any *ast.DeferStmt with a resolvable recover, with no check that the defer precedes the risky code.

Evidence

Minimal repro (not flagged, but crashes the process):

go func() {
	var m map[string]int
	m["x"] = 1 // panics: assignment to entry in nil map — runs BEFORE the defer below is registered
	defer func() {
		if r := recover(); r != nil {
			log.Printf("recovered: %v", r)
		}
	}()
}()

The existing test fixture (testdata/src/a/a.go) only covers the case where defer is the first statement (safeGoroutine, lines 4-14) or where the risky call precedes an unrelated defer (unrelatedDeferGoroutine, lines 30-38, which is correctly flagged only because the defer body itself lacks recover()). No fixture exercises "recover-defer present, but after other statements" — the actual gap here.

Impact

This is a false negative in the core safety property the linter is meant to enforce: a goroutine can be misclassified as "protected" when it is not, giving developers false confidence that a panic won't take down the process. Since goroutinemissingrecover output is presumably used to gate manual remediation (add a recover guard), a passing/clean result here is actively misleading in this ordering case.

Recommendation

Require the recover-guarding defer to be the first statement in the function-literal body (the idiomatic Go pattern for this guard, and the only position that unconditionally protects the rest of the body), or at minimum stop scanning body.List and only accept a defer found before the first statement capable of panicking. The simplest, lowest-risk fix matching idiomatic usage: change hasTopLevelRecoverDefer to only inspect body.List[0] rather than ranging over the whole list, and update the doc comment accordingly. Add a golden test case with a panic-prone statement preceding a recover-defer to lock in the fix.

Validation checklist

• Add testdata case: risky statement before a valid recover-defer → should now be flagged
• Existing safeGoroutine fixture (defer-first) still passes with no diagnostic
• nestedClosureRecoverGoroutine/other existing fixtures unaffected
• Update doc comment on hasTopLevelRecoverDefer to state the ordering requirement

Effort

Small — single-function change plus one new testdata case and golden file.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• api.anthropic.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.anthropic.com"

See Network Configuration for more information.

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 308.5 AIC · ⌖ 5.78 AIC · ⊞ 6.8K · ◷

• expires on Sep 9, 2026, 9:19 PM UTC-08:00