anthropics / anthropics/anthropic-sdk-go

bedrock.WithConfig doc comment states AWS_BEARER_TOKEN_BEDROCK takes precedence, but the code prefers cfg.BearerAuthTokenProvider

Đang mở
#415 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
Go
Star
1.2k
Fork
213
Merge trung bình
1 ngày 12 giờ
Pull request đã merge (30 ngày)
11

Mô tả

### Summary

The `WithConfig` doc comment says `AWS_BEARER_TOKEN_BEDROCK` wins over `cfg.BearerAuthTokenProvider`. The code does the opposite: the variable is read only when the provider is nil. One of the two should change — I'd argue the code, since the documented order is also the more useful one.

### Versions

`github.com/anthropics/anthropic-sdk-go v1.55.1`, `bedrock/bedrock.go`.

### The contradiction

Doc comment on `WithConfig`:

> Authentication is determined as follows: if the AWS_BEARER_TOKEN_BEDROCK environment variable is set, it is used for bearer token authentication. Otherwise, if cfg.BearerAuthTokenProvider is set, it is used. If neither is available, cfg.Credentials is used for AWS SigV4 signing and must be set.

The implementation immediately below it:

```go
func WithConfig(cfg aws.Config) option.RequestOption {
var credentialErr error

if cfg.BearerAuthTokenProvider == nil {
if token := os.Getenv("AWS_BEARER_TOKEN_BEDROCK"); token != "" {
cfg.BearerAuthTokenProvider = NewStaticBearerTokenProvider(token)
}
}
...
```

`cfg.BearerAuthTokenProvider` is checked **first**, and the environment variable is consulted only if it is nil — the reverse of the sentence above.

### Why it is not merely cosmetic

The two fields are populated by different parties. `AWS_BEARER_TOKEN_BEDROCK` is set by a person who decided to use a Bedrock API key. `cfg.BearerAuthTokenProvider` is often set by `config.LoadDefaultConfig` on the caller's behalf, from the SSO token cache, without any intent about Bedrock (#414). So the current order lets an incidental value beat a deliberate one: export the variable, load a config on an SSO profile, and the SSO token is what goes on the wire.

I hit this while writing a workaround. Trusting the comment, I cleared the provider only when the variable was unset — which reads as the conservative choice and is in fact the broken one, precisely because the comment describes the opposite precedence. Clearing it unconditionally is what delivers the documented behaviour.

### Suggested fix

Make the code match the comment — check the variable first and let it override:

```go
if token := os.Getenv("AWS_BEARER_TOKEN_BEDROCK"); token != "" {
cfg.BearerAuthTokenProvider = NewStaticBearerTokenProvider(token)
}
```

That is the whole change, and it keeps the explicit opt-in ahead of whatever a config loader happened to fill in. If the current precedence is intentional instead, the doc comment needs the two clauses swapped, and it would help to say why a caller-supplied provider outranks the variable.

Glad to send either patch.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.