Skip to content

fix(G118): treat returned cancel func as called (fixes #1584)#1585

Merged
ccojocar merged 1 commit intosecurego:masterfrom
ccojocar:fix/g118-false-positive-returned-cancel
Mar 6, 2026
Merged

fix(G118): treat returned cancel func as called (fixes #1584)#1585
ccojocar merged 1 commit intosecurego:masterfrom
ccojocar:fix/g118-false-positive-returned-cancel

Conversation

@ccojocar
Copy link
Copy Markdown
Member

@ccojocar ccojocar commented Mar 6, 2026

Summary

Fixes #1584

The isCancelCalled BFS in the G118 (context propagation) analyzer did not handle *ssa.Return, so a cancel function returned from a helper was flagged as "lost" even though responsibility was transferred to the caller.

Root Cause

When a function like initDatabase calls context.WithCancel(ctx) and returns the cancel func to its caller, the SSA representation produces a Return instruction referencing the cancel value. The BFS in isCancelCalled had no case *ssa.Return: branch, so it dead-ended and concluded the cancel was never called.

Changes

  • analyzers/context_propagation.go: Add a *ssa.Return case to the isCancelCalled BFS — when the cancel value appears among the return operands, treat it as called (responsibility transferred to caller).
  • testutils/g118_samples.go: Update the existing "cancel returned to caller" test to expect 0 issues. Add a new test case matching the exact issue bug: false positive on G118 #1584 pattern (cancel returned as func() and invoked by caller through a shutdown chain).

Validation

  • Build: clean
  • All 156 analyzer specs pass
  • Reproduction case from issue bug: false positive on G118 #1584: 0 issues (was 1)
  • Genuinely lost cancel functions: still correctly detected

The isCancelCalled BFS did not handle *ssa.Return, so a cancel
function returned from a helper was flagged as lost even though
responsibility was transferred to the caller.

Add a Return case that recognises the cancel value among the return
operands and treats it as called.

Update the existing 'cancel returned to caller' test to expect 0
issues, and add a new test case matching the exact pattern from
issue securego#1584 (cancel returned as func() and invoked by the caller
through a shutdown chain).
@ccojocar ccojocar merged commit c709ed8 into securego:master Mar 6, 2026
7 checks passed
@ccojocar ccojocar deleted the fix/g118-false-positive-returned-cancel branch March 6, 2026 18:28
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.72%. Comparing base (fa74dd7) to head (daef2cd).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1585   +/-   ##
=======================================
  Coverage   80.71%   80.72%           
=======================================
  Files         104      104           
  Lines        9878     9882    +4     
=======================================
+ Hits         7973     7977    +4     
  Misses       1418     1418           
  Partials      487      487           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: false positive on G118

1 participant