fix(G118): eliminate false positive when cancel is called via struct field in a closure (#1596)

* fix(G118): eliminate false positive when cancel stored in struct field post-construction

When a cancel function is assigned to a struct field after construction
(e.g. s.cancel = cancel), the SSA FieldAddr for the store is a distinct
value from any FieldAddr created later for defer s.cancel() or inside a
closure. The existing isCancelCalledViaStructField only matched receiver
methods and missed these patterns.

Add isFieldCalledInAnyFunc which scans all SSA functions (including
closures) for a FieldAddr with matching struct pointer type and field
index, then checks whether the loaded value is called. As a side effect,
this also resolves the known false positive for nested struct field access.

fixes: 1595

* update rules documentation

Author: ravisastryk

RULES.md

@@ -16,6 +16,7 @@
   - [G104](#g104)
   - [G111](#g111)
   - [G117](#g117)
+  - [G118](#g118)
   - [G301, G302, G306, G307](#g301-g302-g306-g307)
 
 ## Rules List
@@ -38,7 +39,7 @@
 - G115 — Type conversion which leads to integer overflow (**SSA**)
 - G116 — Detect Trojan Source attacks using bidirectional Unicode characters (**AST**)
 - [G117](#g117) — Potential exposure of secrets via JSON/YAML/XML/TOML marshaling (**AST**)
-- G118 — Context propagation failure leading to goroutine/resource leaks (**SSA**)
+- [G118](#g118) — Context propagation failure leading to goroutine/resource leaks (**SSA**)
 - G119 — Unsafe redirect policy may propagate sensitive headers (**SSA**)
 - G120 — Unbounded form parsing in HTTP handlers can cause memory exhaustion (**SSA**)
 - G121 — Unsafe CrossOriginProtection bypass patterns (**SSA**)
@@ -170,6 +171,89 @@ This replaces the default pattern.
 }
 ```
 
+### G118
+
+`G118` detects three classes of context-propagation failure using SSA-level analysis:
+
+**1. Lost cancel function (CWE-400)**
+
+Reports when a `context.WithCancel`, `context.WithTimeout`, or `context.WithDeadline` call
+returns a cancel function that is never called, potentially leaking resources.
+
+```go
+// Flagged: cancel never called
+func work(ctx context.Context) {
+    child, _ := context.WithTimeout(ctx, time.Second)
+    _ = child
+}
+
+// Safe: cancel deferred
+func work(ctx context.Context) {
+    child, cancel := context.WithTimeout(ctx, time.Second)
+    defer cancel()
+    _ = child
+}
+```
+
+The following patterns are all recognised as *safe* (cancel is considered called):
+
+| Pattern | Description |
+|---|---|
+| `defer cancel()` | Direct deferred call |
+| `defer func() { cancel() }()` | Cancel in a deferred closure |
+| `cancelCopy := cancel; defer cancelCopy()` | Alias via variable |
+| `return ctx, cancel` | Cancel returned to caller (responsibility transferred) |
+| `s.cancelFn = cancel` + method `s.cancelFn()` | Stored in struct field, called via receiver method |
+| `s.cancel = cancel; defer s.cancel()` | Stored in struct field, deferred in same function |
+| `s.cancel = cancel; defer func() { s.cancel() }()` | Stored in struct field, called in closure |
+| Struct containing field is returned | Caller inherits cancel responsibility |
+
+**2. Goroutine uses `context.Background`/`TODO` when request context is available (CWE-400)**
+
+Reports when a goroutine spawned inside an HTTP handler or a function accepting a
+`context.Context` / `*http.Request` uses `context.Background()` or `context.TODO()`
+instead of the request-scoped context.
+
+```go
+// Flagged
+func handler(w http.ResponseWriter, r *http.Request) {
+    go func() {
+        ctx := context.Background() // ignores request context
+        doWork(ctx)
+    }()
+}
+```
+
+**3. Long-running loop without `ctx.Done()` guard (CWE-400)**
+
+Reports an infinite loop that performs blocking I/O (e.g. `http.Get`, `db.Query`,
+`time.Sleep`, interface methods such as `Read`/`Write`) but never checks `ctx.Done()`,
+making the loop impossible to cancel.
+
+```go
+// Flagged
+func poll(ctx context.Context) {
+    for {
+        http.Get("https://example.com") // blocks, no cancellation path
+        time.Sleep(time.Second)
+    }
+}
+
+// Safe
+func poll(ctx context.Context) {
+    for {
+        select {
+        case <-ctx.Done():
+            return
+        case <-time.After(time.Second):
+            http.Get("https://example.com")
+        }
+    }
+}
+```
+
+Loops with an external exit path (e.g. a `break` or bounded `for i < n`) are not flagged.
+
 ### G301, G302, G306, G307
 
 File and directory permission rules can be configured with stricter maximum permissions:

analyzers/context_propagation.go

@@ -710,6 +710,14 @@ func isCancelCalled(cancelValue ssa.Value, allFuncs []*ssa.Function) bool {
 					if isStructFieldReturnedFromFunc(fa) {
 						return true
 					}
+					// Check if any function (including closures capturing the
+					// struct) loads and calls the same field. This handles
+					// post-construction storage such as:
+					//   s.cancel = cancel; defer s.cancel()
+					//   s.cancel = cancel; defer func() { s.cancel() }()
+					if isFieldCalledInAnyFunc(fa, allFuncs) {
+						return true
+					}
 				}
 				queue = append(queue, r.Addr)
 			case *ssa.UnOp:
@@ -783,6 +791,39 @@ func isStructFieldReturnedFromFunc(fa *ssa.FieldAddr) bool {
 	return false
 }
 
+// isFieldCalledInAnyFunc checks whether a cancel function stored into a struct
+// field is subsequently called in any function (including closures) that
+// accesses the same field by struct pointer type and field index. This covers
+// post-construction storage patterns not handled by isCancelCalledViaStructField:
+//
+//	s.cancel = cancel; defer s.cancel()
+//	s.cancel = cancel; defer func() { s.cancel() }()
+func isFieldCalledInAnyFunc(fa *ssa.FieldAddr, allFuncs []*ssa.Function) bool {
+	structPtrType := fa.X.Type()
+	fieldIdx := fa.Field
+
+	for _, fn := range allFuncs {
+		if fn == nil {
+			continue
+		}
+		for _, block := range fn.Blocks {
+			for _, instr := range block.Instrs {
+				otherFA, ok := instr.(*ssa.FieldAddr)
+				if !ok || otherFA.Field != fieldIdx {
+					continue
+				}
+				if !types.Identical(otherFA.X.Type(), structPtrType) {
+					continue
+				}
+				if isFieldValueCalled(otherFA) {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
 // isCancelCalledViaStructField checks whether a cancel function stored into a
 // struct field (e.g., job.cancelFn = cancel) is subsequently called in any other
 // method of the same receiver type (e.g., job.Close() calls job.cancelFn()).

testutils/g118_samples.go

@@ -852,7 +852,7 @@ func multiPhiEdges(ctx context.Context, a, b, c bool) {
 }
 `}, 0, gosec.NewConfig()},
 
-	// Note: nested field access not tracked by current implementation
+	// Safe: nested field cancel called via outer method on Inner type (fixed by isFieldCalledInAnyFunc)
 	{[]string{`
 package main
 
@@ -876,7 +876,7 @@ func (o *Outer) Teardown() {
 		o.inner.cancel()
 	}
 }
-`}, 1, gosec.NewConfig()},
+`}, 0, gosec.NewConfig()},
 
 	// Vulnerable: loop with interface method Do (tests analyzeBlockFeatures invoke)
 	{[]string{`
@@ -1591,5 +1591,41 @@ func main() {
 	foo := NewFoo()
 	foo.Cancel()
 }
+`}, 0, gosec.NewConfig()},
+
+	// Safe: cancel stored in struct field post-construction, called via defer in same function (issue #1595)
+	{[]string{`
+package main
+
+import "context"
+
+type State struct {
+	done context.CancelFunc
+}
+
+func manage(ctx context.Context) {
+	s := &State{}
+	_, s.done = context.WithCancel(ctx)
+	defer s.done()
+}
+`}, 0, gosec.NewConfig()},
+
+	// Safe: cancel stored in struct field post-construction, called via deferred closure (issue #1595)
+	{[]string{`
+package main
+
+import "context"
+
+type Runner struct {
+	stop context.CancelFunc
+}
+
+func launch(ctx context.Context) {
+	r := &Runner{}
+	_, r.stop = context.WithCancel(ctx)
+	defer func() {
+		r.stop()
+	}()
+}
 `}, 0, gosec.NewConfig()},
 }