Skip to content

fix: G118: Goroutine uses context.Background/TODO (gosec)#6666

Open
thaJeztah wants to merge 2 commits into
moby:masterfrom
thaJeztah:fix_gosec_G118
Open

fix: G118: Goroutine uses context.Background/TODO (gosec)#6666
thaJeztah wants to merge 2 commits into
moby:masterfrom
thaJeztah:fix_gosec_G118

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Apr 7, 2026

Copy link
Copy Markdown
Member 
cache/manager.go:238:3: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
        go link.Release(context.TODO())
        ^
cache/refs.go:1519:2: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
    go func() {
    ^
session/testutil/testutil.go:22:3: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
        go func() {
        ^
solver/edge.go:975:4: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
            go results[i].Release(context.TODO())
            ^
util/leaseutil/manager.go:79:2: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
    go l.Discard()
    ^

tonistiigi
tonistiigi reviewed Apr 8, 2026
View reviewed changes
Comment thread cache/blobs.go Outdated
defer func() {
if err != nil {
l.Discard()
_ = l.Discard(ctx)

@tonistiigi tonistiigi Apr 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defers should not use original context

Comment thread util/leaseutil/manager.go
func (l *LeaseRef) Discard() error {
return l.lm.Delete(context.Background(), l.l)
func (l *LeaseRef) Discard(ctx context.Context) error {
return l.lm.Delete(context.WithoutCancel(ctx), l.l)

@tonistiigi tonistiigi Apr 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This WithoutCancel should not be added in Discard

@thaJeztah

thaJeztah commented Jun 22, 2026

Copy link
Copy Markdown
Member Author 
#27 [golangci-lint 1/1] RUN --mount=target=/go/src/github.com/moby/buildkit     --mount=target=/root/.cache,type=cache,id=lint-cache-default-freebsd/amd64   xx-go --wrap &&   golangci-lint run --build-tags "" &&   touch /golangci-lint.done
#27 117.6 util/pull/pullprogress/progress.go:36:57: directive `//nolint:gosec // Progress tracking is tied to the reader lifetime and canceled by Close.` is unused for linter "gosec" (nolintlint)
#27 117.6 	go trackProgress(progressCtx, desc, p.Manager, doneCh) //nolint:gosec // Progress tracking is tied to the reader lifetime and canceled by Close.
#27 117.6 	                                                       ^
#27 117.6 util/pull/pullprogress/progress.go:70:57: directive `//nolint:gosec // Progress tracking is tied to the reader lifetime and canceled by Close.` is unused for linter "gosec" (nolintlint)
#27 117.6 	go trackProgress(progressCtx, desc, f.Manager, doneCh) //nolint:gosec // Progress tracking is tied to the reader lifetime and canceled by Close.
#27 117.6 	                                                       ^
#27 117.6 2 issues:
#27 117.6 * nolintlint: 2

@thaJeztah thaJeztah force-pushed the fix_gosec_G118 branch 2 times, most recently from bcaa960 to 35daa44 Compare June 23, 2026 07:07
@thaJeztah thaJeztah marked this pull request as ready for review June 30, 2026 14:52
tonistiigi
tonistiigi reviewed Jul 7, 2026
View reviewed changes
Comment thread client/build_test.go Outdated
proxy.Director = func(req *http.Request) {
director(req)
req.Host = target.Host
proxy.Rewrite = func(pr *httputil.ProxyRequest) {

@tonistiigi tonistiigi Jul 7, 2026 •
edited by thaJeztah
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iiuc NewSingleHostReverseProxy can't be used anymore and struct should be used directly.

Otherwise both Rewrite and Director would be set and https://github.com/golang/go/blob/go1.26.4/src/net/http/httputil/reverseproxy.go#L464-L467 is reached.

I don't know why this doesn't fail the test though. @crazy-max Looks like there is potentially a secondary issue with the test.

@thaJeztah thaJeztah Jul 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; didn't realise NewSingleHostReverseProxy still set Director https://github.com/golang/go/blob/go1.26.4/src/net/http/httputil/reverseproxy.go#L333-L338

@thaJeztah
fix: G118: Goroutine uses context.Background/TODO (gosec)
9212136 
    cache/manager.go:238:3: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
            go link.Release(context.TODO())
            ^
    cache/refs.go:1519:2: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
        go func() {
        ^
    session/testutil/testutil.go:22:3: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
            go func() {
            ^
    solver/edge.go:975:4: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
                go results[i].Release(context.TODO())
                ^
    util/leaseutil/manager.go:79:2: G118: Goroutine uses context.Background/TODO while request-scoped context is available (gosec)
        go l.Discard()
        ^

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
client: remove use of deprecated proxy.Director (staticcheck)
4119ec8 
    client/build_test.go:2363:14: SA1019: proxy.Director has been deprecated since Go 1.26 and an alternative has been available since Go 1.20: Use Rewrite instead. (staticcheck)

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonistiigi tonistiigi tonistiigi left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@thaJeztah thaJeztah

Labels

area/client area/storage area/testing area/util

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thaJeztah @tonistiigi