Description
The AWS KMS adapter in backend/crypto/jwk/aws_kms/adapter.go uses context.TODO() in multiple places, which is a placeholder meant for development and should not be used in production code.
Locations
- Line 37 -
NewAWSKMSAdapter:
awsCfg, err := awsConfig.LoadDefaultConfig(
context.TODO(), // ❌ Should use proper context
awsConfig.WithRegion(cfg.Region),
)
- Line 70 -
Public() method:
result, err := k.KmsClient.GetPublicKey(context.TODO(), &kms.GetPublicKeyInput{ // ❌
KeyId: &k.KeyId,
})
- Line 107 -
Sign() method:
result, err := k.KmsClient.Sign(context.TODO(), &kms.SignInput{ // ❌
KeyId: &k.KeyId,
Message: digest,
MessageType: types.MessageTypeDigest,
SigningAlgorithm: signingAlgorithm,
})Problems
Using context.TODO() means:
- No cancellation: Can't cancel long-running AWS KMS operations
- No timeouts: Operations can hang indefinitely
- No tracing: Can't track requests through distributed systems
- No deadlines: Can't enforce SLAs or request budgets
- Violates Go best practices:
context.TODO is explicitly meant as a placeholder during development
Impact
- AWS KMS calls can hang indefinitely if AWS has issues
- No way to implement request timeouts from the caller
- Makes debugging and tracing difficult in production
- Can lead to goroutine leaks if operations never complete
Recommended Fix
Pass context through the call chain:
// Update NewAWSKMSAdapter to accept context
func NewAWSKMSAdapter(ctx context.Context, cfg config.KeyManagement) (*AWSKMSAdapter, error) {
awsCfg, err := awsConfig.LoadDefaultConfig(
ctx, // ✅ Use provided context
awsConfig.WithRegion(cfg.Region),
)
// ...
}
// Update Public() to accept context
func (k *AWSKMSAdapter) Public(ctx context.Context) crypto.PublicKey {
// ...
result, err := k.KmsClient.GetPublicKey(ctx, &kms.GetPublicKeyInput{
KeyId: &k.KeyId,
})
// ...
}
// Update Sign() to accept context (in addition to existing params)
func (k *AWSKMSAdapter) Sign(ctx context.Context, rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) {
// ...
result, err := k.KmsClient.Sign(ctx, &kms.SignInput{
KeyId: &k.KeyId,
Message: digest,
MessageType: types.MessageTypeDigest,
SigningAlgorithm: signingAlgorithm,
})
// ...
}Then callers can use:
// With timeout
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
adapter, err := NewAWSKMSAdapter(ctx, cfg)
// With cancellation
ctx, cancel := context.WithCancel(req.Context())
defer cancel()
signature, err := adapter.Sign(ctx, rand, digest, opts)
References
- Go context package docs
- Go blog: Context
context.TODO godoc: "Code should use context.TODO when it's unclear which Context to use or it is not yet available (because the surrounding function has not yet been extended to accept a Context parameter)."
Description
The AWS KMS adapter in
backend/crypto/jwk/aws_kms/adapter.gousescontext.TODO()in multiple places, which is a placeholder meant for development and should not be used in production code.Locations
NewAWSKMSAdapter:Public()method:Sign()method:Problems
Using
context.TODO()means:context.TODOis explicitly meant as a placeholder during developmentImpact
Recommended Fix
Pass context through the call chain:
Then callers can use:
References
context.TODOgodoc: "Code should use context.TODO when it's unclear which Context to use or it is not yet available (because the surrounding function has not yet been extended to accept a Context parameter)."