[codex] Add AWS Bedrock credential authentication#687
Conversation
jliccini
left a comment
There was a problem hiding this comment.
Bedrock endpoint and configuration-safety review.
jliccini
left a comment
There was a problem hiding this comment.
Remaining Go API and lifecycle review notes.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Requesting updates before merge. The request-finalizer ordering, per-attempt signing, body replay, credential caching, environment isolation, and test coverage are all thoughtfully implemented, but two validated issues need to be fixed: bearer credentials can cross an origin boundary through Go's default redirect handling, and fully explicit AWS configuration can still be broken by unrelated ambient AWS config files.
Please also address—or make an explicit documented contract decision on—the two Go developer-experience follow-ups below: SkipAuth rejects the standard explicit API-key option, and permanent provider policy errors are retried with backoff.
I reproduced the two blocking issues with focused tests. Targeted race tests and go vet passed, and the affected packages also passed after merging current main into this head.
There was a problem hiding this comment.
Pull request overview
Adds first-class Amazon Bedrock support by introducing a dedicated bedrock.NewClient constructor that configures the existing OpenAI Go client to use Bedrock’s OpenAI-compatible API with SigV4 (or bearer token) authentication and AWS SDK v2 credential resolution.
Changes:
- Introduces the
bedrockpackage, including SigV4 signing (bedrock-mantle) and AWS credential/region/base-URL resolution with safety checks. - Adds an internal request-finalization hook (
WithRequestFinalizer) plus a deterministic “no retry” error marker to support provider auth applied closest to the wire and to avoid retrying policy/setup failures. - Updates docs/examples and improves debug header redaction to include
X-Amz-Security-Token.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents Bedrock usage, routing, auth modes, and links to a full example. |
| option/middleware.go | Redacts x-amz-security-token in debug logging. |
| option/middleware_test.go | Adds regression test ensuring AWS session tokens are redacted without mutating originals. |
| internal/requestconfig/requestconfig.go | Adds request finalizers + deterministic no-retry errors; updates retry decision to consider deterministic errors. |
| internal/requestconfig/requestconfig_test.go | Tests finalizer composition/order relative to other options. |
| client.go | Allows provider-owned clients to skip inheriting ambient OPENAI_* environment defaults. |
| go.mod / go.sum | Adds AWS SDK v2 dependencies for Bedrock support. |
| examples/go.mod / examples/go.sum | Updates examples module deps for AWS SDK v2. |
| examples/bedrock/main.go | Adds a runnable Responses API example targeting Bedrock. |
| bedrock/bedrock.go | Defines Bedrock Config and NewClient constructor returning the standard OpenAI client. |
| bedrock/auth.go | Implements auth selection, AWS config loading, SigV4/bearer middleware, redirect suppression, replayable-body enforcement, and safe error wrapping. |
| bedrock/auth_test.go | Adds tests for credential/config resolution, signing fixtures, retries re-signing, middleware ordering, redirect suppression, and env isolation. |
| bedrock/auth_additional_test.go | Adds additional tests for failure modes, safety checks, and helper behaviors. |
| bedrock/example_test.go | Adds Go doc examples (currently problematic due to panics on missing env/config). |
| bedrock/testdata/sigv4.json | Adds deterministic cross-SDK SigV4 signing fixture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed c42880b. The four requested issues are addressed: bearer and SigV4 clients now share redirect protection, complete explicit AWS configuration no longer loads unrelated ambient config, SkipAuth supports the standard explicit gateway credential options, and deterministic provider-policy failures bypass retries while transient provider failures remain retryable. I also checked the self-contained executable examples. Full tests, repository lint/build, go vet, targeted race tests, and current GitHub checks are green. Approving.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the current head (479c078) comprehensively. I found no substantive issues. I validated the Bedrock auth flows, request retry/body lifecycle changes, configuration isolation, error paths, generated/manual API boundaries, docs, and examples. Local checks passed: full go test ./... against an isolated Steady mock, targeted race tests, go vet ./..., ./scripts/lint, gofmt/diff cleanliness, and go mod tidy -diff plus tests across root/examples/consumer/tools. CI is green, including CodeQL, breaking-change detection, and govulncheck.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the exact merge head 04dce82. No substantive issues found. The Bedrock authentication, request-finalizer, retry, and request-body lifecycle implementation is byte-for-byte unchanged from the previously approved head. I inspected the merge resolution and full current PR diff: the dependency conflicts correctly preserve the AWS additions while incorporating main’s Azure/security updates, and generated/manual boundaries remain intact. Local checks passed: full go test ./... against an isolated Steady server, targeted race tests, go vet ./..., ./scripts/lint, examples and external-consumer tests, gofmt/diff cleanliness, and go mod tidy -diff across all four modules. Current CI, CodeQL, govulncheck, breaking-change detection, Go support policy, and OkTest 237/237 are green.
Linear: SDK-79
Summary
This adds first-class Amazon Bedrock support through a dedicated
bedrock.NewClientconstructor that returns the standard OpenAI Go client.The change:
bedrock-mantleon every request attemptAuthentication and routing
Authentication is selected in this order:
AWS_BEARER_TOKEN_BEDROCKRegion resolution uses
AWSRegion, thenAWS_REGION,AWS_DEFAULT_REGION, and the AWS SDK region chain. The default endpoint ishttps://bedrock-mantle.{region}.api.aws/openai/v1, withBaseURLandAWS_BEDROCK_BASE_URLoverrides.The
/openai/v1prefix is intentional and matches the approved Node and Python provider implementations. AWS confirmed that it is the OpenAI compatibility contract; the generic/v1route is a different API surface and is not interchangeable.The provider does not inherit ambient
OPENAI_*credentials, routing, or custom headers. It rejects custom authorization and cross-origin credential forwarding, disables automatic redirects for signed requests, requires replayable SigV4 request bodies, and redacts AWS session tokens from debug logs. SigV4 requires an*http.Clientso redirect suppression is enforceable; callers can still customize its transport.Request lifecycle
A small internal request-finalization seam runs after all client- and method-level options have been applied. Bedrock uses it to place authentication closest to the wire, so user middleware mutations are covered by the signature and retries receive fresh timestamps and refreshed credentials.
Response streaming remains supported. Unsupported Bedrock features remain server-owned and surface through the standard OpenAI client error path.
Testing
Coverage includes:
~/.aws/credentialsand~/.aws/configresolutionValidation run:
./scripts/test./scripts/lintgo vet ./...go test -cover ./bedrock(97.4% statement coverage)