📌 Description
internal/handlers/github_app.go's HandleInstallationCallback kicks off repository syncing in the background so the HTTP redirect isn't blocked on it:
} else {
// Sync repositories in background (don't block redirect)
go h.syncInstallationRepositories(c.Context(), userID, installationID)
}
// Redirect to frontend with success message
...
return c.Redirect(u.String(), fiber.StatusFound)c.Context() on a Fiber *fiber.Ctx returns the underlying *fasthttp.RequestCtx. fasthttp pools and reuses RequestCtx objects across requests for performance -- the framework explicitly documents that a RequestCtx (and anything derived from it) must not be retained or used after the handler that received it returns, because the same object gets recycled and repopulated for a subsequent, unrelated request as soon as this handler function returns and the response is flushed. Here, go h.syncInstallationRepositories(c.Context(), ...) captures that context and the handler immediately continues to c.Redirect(...) and returns -- the goroutine can (and under load, will) start executing while, or after, the underlying RequestCtx has already been handed to fasthttp's pool and potentially reused for a different request. syncInstallationRepositories does add its own bounded timeout (context.WithTimeout(ctx, 60*time.Second)), but that only derives a new context from the already-unsafe one -- it doesn't fix the underlying use-after-handler-return of the fasthttp request context, and any code path that reads values off the original context (deadlines, in the current code just the timeout base) is relying on undefined state.
🧩 Requirements and context
- Background work kicked off from a Fiber handler must not carry the handler's
c.Context() (or anything derived from c.UserContext()/c.Context()) into a goroutine that outlives the handler.
- The background sync should use an independent, freshly-created context (e.g.
context.Background() with its own timeout), not one tied to the completed HTTP request's lifecycle.
- Preserve the "don't block the redirect" behavior and the existing 60-second timeout budget for the sync itself.
🛠️ Suggested execution
- In
HandleInstallationCallback (internal/handlers/github_app.go), change go h.syncInstallationRepositories(c.Context(), userID, installationID) to go h.syncInstallationRepositories(context.Background(), userID, installationID) -- syncInstallationRepositories already wraps whatever it's given in its own context.WithTimeout, so context.Background() as the root is sufficient and safe.
- Audit the rest of
internal/handlers for the same pattern (go func / go h.something(c.Context()...) or go h.something(c.UserContext()...) fired from inside a handler before it returns) -- grep for go .*\bc\.Context\(\) and go .*\bc\.UserContext\(\) across internal/handlers/*.go and fix any other occurrences found the same way, since this is a systemic footgun rather than a one-off.
- Add a regression-style comment at the call site explaining why
context.Background() is used instead of c.Context(), so a future refactor doesn't "fix" it back to the unsafe version.
✅ Acceptance criteria
🔒 Security notes
No new attack surface, but this is a genuine reliability bug: under concurrent request load, a background repository sync can end up reading from (or racing against) a fasthttp.RequestCtx that has already been reset and reassigned to an unrelated in-flight request, which is undefined behavior that can manifest as intermittent, hard-to-reproduce data corruption or panics tied to request volume rather than to any specific input.
📋 Guidelines
- Minimum 95% test coverage
- Clear documentation
- Timeframe: 96 hours
📌 Description
internal/handlers/github_app.go'sHandleInstallationCallbackkicks off repository syncing in the background so the HTTP redirect isn't blocked on it:c.Context()on a Fiber*fiber.Ctxreturns the underlying*fasthttp.RequestCtx. fasthttp pools and reusesRequestCtxobjects across requests for performance -- the framework explicitly documents that aRequestCtx(and anything derived from it) must not be retained or used after the handler that received it returns, because the same object gets recycled and repopulated for a subsequent, unrelated request as soon as this handler function returns and the response is flushed. Here,go h.syncInstallationRepositories(c.Context(), ...)captures that context and the handler immediately continues toc.Redirect(...)and returns -- the goroutine can (and under load, will) start executing while, or after, the underlyingRequestCtxhas already been handed to fasthttp's pool and potentially reused for a different request.syncInstallationRepositoriesdoes add its own bounded timeout (context.WithTimeout(ctx, 60*time.Second)), but that only derives a new context from the already-unsafe one -- it doesn't fix the underlying use-after-handler-return of the fasthttp request context, and any code path that reads values off the original context (deadlines, in the current code just the timeout base) is relying on undefined state.🧩 Requirements and context
c.Context()(or anything derived fromc.UserContext()/c.Context()) into a goroutine that outlives the handler.context.Background()with its own timeout), not one tied to the completed HTTP request's lifecycle.🛠️ Suggested execution
HandleInstallationCallback(internal/handlers/github_app.go), changego h.syncInstallationRepositories(c.Context(), userID, installationID)togo h.syncInstallationRepositories(context.Background(), userID, installationID)--syncInstallationRepositoriesalready wraps whatever it's given in its owncontext.WithTimeout, socontext.Background()as the root is sufficient and safe.internal/handlersfor the same pattern (go func/go h.something(c.Context()...)orgo h.something(c.UserContext()...)fired from inside a handler before it returns) -- grep forgo .*\bc\.Context\(\)andgo .*\bc\.UserContext\(\)acrossinternal/handlers/*.goand fix any other occurrences found the same way, since this is a systemic footgun rather than a one-off.context.Background()is used instead ofc.Context(), so a future refactor doesn't "fix" it back to the unsafe version.✅ Acceptance criteria
syncInstallationRepositoriesis invoked with a context that is safe to use after the originating HTTP handler has returned (not derived fromc.Context()/c.UserContext()).🔒 Security notes
No new attack surface, but this is a genuine reliability bug: under concurrent request load, a background repository sync can end up reading from (or racing against) a
fasthttp.RequestCtxthat has already been reset and reassigned to an unrelated in-flight request, which is undefined behavior that can manifest as intermittent, hard-to-reproduce data corruption or panics tied to request volume rather than to any specific input.📋 Guidelines