fix(bedrock): fix unreachable credential validation in WithConfig#290
Open
MaxwellCalkin wants to merge 1 commit intoanthropics:mainfrom
Open
fix(bedrock): fix unreachable credential validation in WithConfig#290MaxwellCalkin wants to merge 1 commit intoanthropics:mainfrom
MaxwellCalkin wants to merge 1 commit intoanthropics:mainfrom
Conversation
The credential validation check in WithConfig was unreachable dead code.
The if/else-if structure was:
if cfg.BearerAuthTokenProvider == nil {
// try env var
} else if cfg.BearerAuthTokenProvider == nil && cfg.Credentials == nil {
// set error
}
The else-if branch only executes when BearerAuthTokenProvider is NOT nil
(since the if-branch handles the nil case), but then immediately checks
if it IS nil — a condition that can never be true. This means users who
call WithConfig without any credentials (no bearer token provider, no
AWS_BEARER_TOKEN_BEDROCK env var, and no AWS credentials) would not
receive the expected error. Instead, the request would proceed and fail
later with an opaque nil pointer error during SigV4 signing.
The fix moves the credential check into the existing if-block as a
nested else-if, so it executes when BearerAuthTokenProvider is nil AND
the env var fallback is also not available.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
bedrock.WithConfigis unreachable dead code due to a logic error in the if/else-if chainelse ifbranch checkscfg.BearerAuthTokenProvider == nil, but it can only execute when the outerif cfg.BearerAuthTokenProvider == nilwas false — meaningBearerAuthTokenProvideris NOT nil, making the nested nil check always falseWithConfigwithout any credentials (no bearer token, noAWS_BEARER_TOKEN_BEDROCKenv var, nocfg.Credentials) silently proceed instead of getting the intended"expected AWS credentials to be set"error, eventually hitting a nil pointer panic during SigV4 signingBefore (broken):
After (fixed):
Test plan
bedrock.WithConfig(aws.Config{})(no credentials set, no env var) now returns the"expected AWS credentials to be set"errorAWS_BEARER_TOKEN_BEDROCKenv var still works correctlycfg.Credentialsstill works correctlycfg.BearerAuthTokenProviderdirectly still works correctly🤖 Generated with Claude Code