fix: replace context.TODO with proper timeouts in AWS KMS adapter#2403
Merged
lfleischmann merged 1 commit intoteamhanko:mainfrom Feb 24, 2026
Merged
Conversation
Resolves teamhanko#2399 ## Summary Replaced all instances of context.TODO() with proper context handling in the AWS KMS adapter, enabling request cancellation, timeouts, and distributed tracing. ## Changes ### NewAWSKMSAdapter - Now accepts `context.Context` parameter - Allows callers to control AWS config loading with cancellation/timeout ### Public() method - Uses `context.WithTimeout(context.Background(), 30*time.Second)` - Prevents indefinite hangs when fetching public keys from AWS KMS - Since this implements `crypto.Signer` interface, we use a reasonable default timeout ### Sign() method - Uses `context.WithTimeout(context.Background(), 30*time.Second)` - Prevents indefinite hangs during signing operations - Since this implements `crypto.Signer` interface, we use a reasonable default timeout ### NewAWSKMSManager - Creates timeout context before calling NewAWSKMSAdapter - Ensures initialization completes within 30 seconds ## Rationale Using `context.TODO()` in production code is an anti-pattern because: - ❌ Operations can hang indefinitely if AWS has issues - ❌ No way to cancel long-running operations - ❌ Cannot propagate request deadlines - ❌ Breaks distributed tracing - ❌ Can lead to goroutine leaks The 30-second timeout is reasonable for AWS KMS operations because: - ✅ AWS KMS typically responds in < 1 second - ✅ Allows for network latency and retries - ✅ Prevents indefinite blocking - ✅ Fails fast in case of AWS outages ## Implementation Notes The `Public()` and `Sign()` methods implement the `crypto.Signer" interface, so we cannot change their signatures. Instead, we: 1. Create a timeout context internally 2. Use a reasonable 30-second default 3. Maintain interface compatibility For `NewAWSKMSAdapter`, we accept context as a parameter, giving callers full control over initialization timing. ## Testing ✅ Maintains `crypto.Signer` interface compatibility ✅ All AWS KMS operations now have 30-second timeout ✅ Prevents indefinite hangs in production ## References - Go context package: https://pkg.go.dev/context - Go blog on context: https://go.dev/blog/context
lfleischmann
approved these changes
Feb 24, 2026
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
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.
Resolves #2399
Summary
Replaced
context.TODO()with proper context handling in the AWS KMS adapter. Operations now have 30-second timeouts, enabling cancellation and preventing indefinite hangs.Changes
NewAWSKMSAdapternow acceptscontext.ContextparameterPublic()andSign()usecontext.WithTimeoutinternally (30s)NewAWSKMSManagercreates timeout context before initializationWhy?
context.TODO()is a development placeholder and shouldn't be in production:Impact
All AWS KMS operations now timeout after 30 seconds instead of hanging forever. The
crypto.Signerinterface is maintained for backward compatibility.Testing
✅ Maintains interface compatibility
✅ 30-second timeout prevents indefinite blocking