fix(graphql): return gin context error uniformly across all resolvers#638
Merged
Conversation
All 50 GraphQL resolver files previously used `gc, _ :=
utils.GinContextFromContext(ctx)`, silently discarding the error and
relying on MetaFromGin's nil safety. This is fine at runtime but masks
programming errors and leaks an inconsistent contract: resolvers that
call ApplyToGin(gc, side) would panic on a nil gc, while read-only
resolvers would silently produce empty request metadata.
Apply the pattern already established in signup.go uniformly: check the
error from GinContextFromContext and return it immediately. The caller
gets the actual error ("could not retrieve gin.Context") instead of a
downstream auth failure or a nil-pointer panic.
For all 51 GraphQL resolvers the previous error branch was:
return nil, err
Now it is:
g.Log.Debug().Err(err).Msg("failed to get gin context")
metrics.RecordSecurityEvent(metrics.SecurityEventGinContextMissing, "graphql")
return nil, err
Changes:
- internal/metrics/metrics.go: add SecurityEventGinContextMissing constant
- internal/graphql/*.go (51 files): add structured debug log and security
metric to the gin context error branch; add metrics import
- internal/integration_tests/gin_context_missing_test.go: 9 subtests
covering resolvers with/without ApplyToGin and with/without auth,
each called with context.Background() (no embedded gin.Context)
Not affected:
- HTTP handlers receive gin.Context directly as a function parameter
- gRPC handlers use transport.MetaFromGRPC which degrades gracefully
when no gRPC metadata is present (already tested in
internal/grpcsrv/transport/grpc_metadata_test.go TestMetaFromGRPC_NoMetadata)
## GORM stdout → stderr (fixes smoke mcp_stdio) GORM's default logger writes to os.Stdout. The MCP server speaks JSON-RPC over stdio, so any stray stdout line (slow-query warnings, AutoMigrate errors) corrupts the stream and produces "unexpected end of JSON input" on the client. PR #632 scoped logger.Discard to clearLegacyColumnUniqueness but the global gorm.Config had no Logger set, leaving AutoMigrate and all other GORM operations on the default stdout logger. Fix: wire a stderr-backed GORM logger into ormConfig so every GORM operation routes to stderr rather than stdout. Slow-query warnings and errors remain visible; stdout stays clean for stdio transports. ## context.WithoutCancel in fire-and-forget goroutines 18 goroutines across 9 service files were capturing the request ctx and using it for EventsProvider.RegisterEvent, StorageProvider.AddSession, etc. If the request context is cancelled (client disconnect, upstream timeout), those goroutines would return early without firing webhooks or persisting sessions. Fix: inject ctx := context.WithoutCancel(ctx) as the first statement in every goroutine that uses the request ctx. This preserves any context values (trace IDs, logger) while detaching from the cancellation signal, so side effects complete independently of the request lifetime. Goroutines that do not use ctx (MemoryStoreProvider calls, EmailProvider calls whose signature takes no context) are left unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gc, _ := utils.GinContextFromContext(ctx), silently discarding the error from missing gin contextsignup.gouniformly: return the actual error immediately instead of ignoring itApplyToGin(gc, side)were already at risk of a nil-pointer panic if gin context was absent; this removes that risk entirelyMetaFromGinnow return a clear error instead of silently producing empty request metadataBehaviour change
Before: missing gin context →
MetaFromGin(nil)→ emptyRequestMetadata→ downstream auth failure with a generic "unauthorized" message, or nil-pointer panic in resolvers callingApplyToGin.After: missing gin context → resolver returns the actual
"could not retrieve gin.Context"error immediately.Test plan
go build ./...passes (verified locally)go clean --testcache && TEST_DBS=sqlite go test -p 1 ./internal/integration_tests/...