Skip to content

execcommandwithoutcontext precision: false positive on nil-guarded exec.Command fallback — autofix injects a nil context that pa [Content truncated due to length] #38281

Description

@github-actions

Summary

The newly-landed execcommandwithoutcontext analyzer (24th custom linter) flags exec.Command calls that sit in the nil branch of an explicit ctx-nil guard, and its suggested autofix rewrites them to exec.CommandContext(ctx, ...). On that exact control-flow path ctx is provably nil, and the Go standard library's exec.CommandContext panics when given a nil context (panic("nil Context")). The diagnostic is a false positive and the autofix would introduce a runtime panic.

  • Linter: pkg/linters/execcommandwithoutcontext (registered cmd/linters/main.go:52)
  • Concrete false positive: pkg/workflow/github_cli.go:32
  • Status: not yet CI-enforced, so it is not breaking builds today — but it blocks enforcement and the autofix is unsafe.

Root cause

run() reports any exec.Command whose enclosing FuncDecl/FuncLit has a named context.Context parameter, without inspecting the control flow guarding the call:

// pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go
ctxParamName, hasCtx := contextParamName(pass, funcType)
if !hasCtx {
    continue
}
pass.Report(... "use exec.CommandContext(%s, ...)" ...) // no guard analysis

The analyzer correctly uses type identity for the package match (ObjectOf -> *types.PkgName -> Imported().Path() == "os/exec") and types.Identical for the ctx param, so this is not a syntactic-match issue — it is a missing control-flow guard check.

Concrete false positive

// pkg/workflow/github_cli.go:23
// When ctx is nil, it uses exec.Command; when ctx is provided, it uses exec.CommandContext.
func setupGHCommand(ctx context.Context, args ...string) *exec.Cmd {
    var cmd *exec.Cmd
    if ctx != nil {
        cmd = exec.CommandContext(ctx, "gh", args...)
    } else {
        cmd = exec.Command("gh", args...) // line 32 — FLAGGED, but ctx is nil here by construction
    }
    ...
}

This is the intentional dual-path shared implementation behind the public ExecGH (nil context) and ExecGHContext(ctx) wrappers — ExecGH even carries //nolint:staticcheck for exactly this reason. Applying the linter's suggested fix yields exec.CommandContext(ctx, "gh", args...) on the ctx == nil path, which panics at runtime.

Impact

  • False positive on a legitimate, documented idiom (nil-context fallback APIs).
  • Unsafe autofix: the suggested SuggestedFix would convert a working program into one that panics.
  • Blocks enforcement: the linter cannot be added to CI (cgo.yml) while it mis-flags this site, and it currently has no internal/nolint support to suppress the warning (see companion enforce-readiness issue).

Recommendation

In run(), skip the exec.Command call when it is control-flow-guarded against the nil-context path. Two viable detection strategies (prefer the first, fall back to the second):

  1. Guard-aware skip: walk the enclosing *ast.IfStmt chain; if the call is in the else of an if ctx != nil (or the then of an if ctx == nil) comparing the ctx parameter against nil, do not report.
  2. Dual-path heuristic: if the same enclosing function already contains an exec.CommandContext(ctx, ...) call using that same ctx identifier, treat the function as an intentional dual-path implementation and skip.

Add testdata cases mirroring setupGHCommand (a // want-free if ctx != nil { CommandContext } else { Command } function) so the guarded fallback is covered.

Validation checklist

  • execcommandwithoutcontext no longer reports pkg/workflow/github_cli.go:32.
  • New testdata case: nil-guarded fallback (if ctx != nil {...} else { exec.Command(...) }) produces no diagnostic.
  • Existing // want cases (BadRunCommand, method receiver, closure, func-lit) still fire.
  • go test ./pkg/linters/execcommandwithoutcontext/... passes.

Effort: Small — single analyzer file (run() guard check) + testdata. Mirrors prior single-file precision fixes (sg29a1 #37492, sg30a2 #37741, sg31a2 #38029) that landed within ~1 day.

References: §27254549539

Generated by 🤖 Sergo - Serena Go Expert · 294.4 AIC · ⌖ 14.5 AIC · ⊞ 6.3K ·

  • expires on Jun 16, 2026, 9:18 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions