execcommandwithoutcontext: enclosing-scope walk crosses non-go/defer FuncLit boundaries — false positives + misleading fix (same [Content truncated due to length]

[github-actions]

Summary

execcommandwithoutcontext (now CI-enforced, #42416) walks enclosing functions innermost-first to find a context.Context parameter, but — unlike its twin timesleepnocontext — it does not stop at a callback FuncLit boundary. It unconditionally continues past a ctx-less function literal and attributes the outer FuncDecl's ctx to an exec.Command call inside an inner callback closure. This is the exact over-attribution false-positive class that was fixed for timesleepnocontext in #42901.

Location

pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go:54-62

for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) {
    funcType := astutil.EnclosingFuncType(encl.Node())
    if funcType == nil {
        continue
    }
    ctxParamName, hasCtx := astutil.ContextParamName(pass, funcType)
    if !hasCtx {
        continue          // <-- always continues outward, crossing the FuncLit boundary
    }
    ... pass.Report( use exec.CommandContext(ctxParamName, ...) ) ...
    break
}

Evidence — the twin already fixed this

pkg/linters/timesleepnocontext/timesleepnocontext.go:54-66 uses an identical enclosing-walk, but discriminates the boundary:

if !hasCtx {
    if _, isFuncLit := funcNode.(*ast.FuncLit); isFuncLit && !isGoOrDeferClosure(encl) {
        break        // regular callback FuncLit -> stop, do NOT borrow outer ctx
    }
    continue         // go/defer closure -> keep walking (it captures + shares the ctx)
}

ctxbackground (fixed via #41164) is conservative in the other direction — it breaks on the first ctx-less enclosing func. execcommandwithoutcontext is the only ctx-family linter left that crosses arbitrary FuncLit boundaries with no discrimination.

False-positive scenario

func serve(ctx context.Context) {
    http.HandleFunc("/run", func(w http.ResponseWriter, r *http.Request) {
        // callback runs per-request, potentially long after serve()'s ctx is cancelled
        _ = exec.Command("ls")   // FLAGGED: "use exec.CommandContext(ctx, ...)"
    })
}

Walk from exec.Command: innermost enclosing is func(w, r) (no ctx param) -> continue -> next enclosing is serve(ctx) (has ctx) -> report + suggest exec.CommandContext(ctx, ...). But ctx is the server-setup scope, not the request scope; the correct context is r.Context(). The suggested fix is actively wrong, and in a CI-enforced linter this blocks legitimate code. The same misattribution hits any non-go/defer callback (sort.Slice, errgroup.Go registration, event handlers) lexically nested in a ctx-receiving function.

Why this is an oversight, not intent

The testdata (testdata/src/execcommandwithoutcontext/execcommandwithoutcontext.go) covers OuterCtxInnerClosure — a go func(){ exec.Command }() that should be flagged (goroutine inherits ctx) — but has no case for a regular callback FuncLit nested in a ctx-receiving function. The distinction timesleepnocontext makes is simply absent here.

Impact

• False positives on exec.Command inside detached/callback closures whose own scope does not share the enclosing ctx.
• Misleading autofix: rewrites to exec.CommandContext(outerCtx, ...), wiring in a context whose lifetime does not match the callback.
• Elevated severity because the linter is CI-enforced (errortypeassertion (new 38th linter) + execcommandwithoutcontext enforce-readiness: zero production violations, add both to the [Content truncated due to length] #42416) — a false positive fails the build.

Recommended fix

Port the proven timesleepnocontext discrimination:

1. Add the isGoOrDeferClosure(inspector.Cursor) bool helper (copy from timesleepnocontext.go:79-122, requires importing golang.org/x/tools/go/ast/inspector).
2. Replace the if !hasCtx { continue } (line 60-62) with:

if !hasCtx {
    if _, isFuncLit := encl.Node().(*ast.FuncLit); isFuncLit && !isGoOrDeferClosure(encl) {
        break
    }
    continue
}

Validation checklist

• Add testdata: exec.Command inside a non-go/defer callback FuncLit (e.g. http.HandleFunc, sort.Slice) nested in a ctx-receiving func -> no diagnostic.
• Keep existing OuterCtxInnerClosure (go func(){...}()) still flagged.
• Add a defer func(){ exec.Command }() case inside a ctx func -> still flagged.
• go test ./pkg/linters/execcommandwithoutcontext/... green (analysistest + golden).
• Confirm zero production false positives remain after the fix (linter is CI-enforced).

Effort: Small — ~40 lines, direct port of an already-reviewed helper; single file + testdata.

Related

• timesleepnocontext: enclosing-scope walk crosses arbitrary FuncLit boundaries — false positives + misleading fix for request/cal [Content truncated due to length] #42901 (timesleepnocontext) — same over-attribution class, already fixed (the template).
• ctxbackground: enclosing-scope walk only inspects *ast.FuncDecl, skipping FuncLit boundaries — false negative for closures with [Content truncated due to length] #41164 (ctxbackground) — sibling FuncLit-boundary fix.
• errortypeassertion (new 38th linter) + execcommandwithoutcontext enforce-readiness: zero production violations, add both to the [Content truncated due to length] #42416 — added execcommandwithoutcontext to CI enforcement.

Generated by 🤖 Sergo - Serena Go Expert · 217.8 AIC · ⌖ 14.1 AIC · ⊞ 5.9K · ◷

• expires on Jul 12, 2026, 9:13 PM UTC-08:00