Skip to content

Clarify Bedrock credential change detection#3522

Closed
jbeckwith-oai wants to merge 1 commit into
mainfrom
codex/address-codeql-bedrock-signature
Closed

Clarify Bedrock credential change detection#3522
jbeckwith-oai wants to merge 1 commit into
mainfrom
codex/address-codeql-bedrock-signature

Conversation

@jbeckwith-oai

Copy link
Copy Markdown
Contributor

Summary

  • replace the keyed BLAKE2s credential fingerprint with an explicit HMAC-SHA256 identity
  • use a process-random nonce so the identity remains process-local and opaque
  • document that this value is a change detector, not a password hash

Why

GitHub code-scanning alert #1 reports py/weak-sensitive-data-hashing because CodeQL classifies the Bedrock api_key as password-like data flowing into BLAKE2s.

This is a false positive rather than weak password storage: the digest is never used to enroll or verify a password, and it is not persisted, transmitted, logged, or exposed. It is compared only inside _refresh_legacy_provider_runtime to detect whether a legacy client's credential changed and its provider must be rebuilt. The previous keyed BLAKE2s construction was already protected by a fresh 32-byte process key.

The change expresses the same security intent as a MAC instead of disabling a useful CodeQL query. The bearer credential is the HMAC key and the process-random nonce is the message, which keeps the credential out of CodeQL's password-hash input while retaining an opaque, deterministic identity for the lifetime of the process.

Why this does not change Bedrock access

The raw bearer credential still passes unchanged to bedrock(api_key=credential) and ultimately to the request Authorization header. The HMAC is used only by _LegacyRuntimeSignature for internal equality checks. AWS credential mode and token-provider mode do not take this string-HMAC path.

Within one process, the same bearer credential and nonce produce the same identity; changing the bearer credential produces a different identity and triggers the existing provider refresh path. This preserves credential mutation, request authentication, and client-copy behavior.

Tests

No test source needed to change because the externally observable behavior and security invariants already have focused coverage:

  • test_legacy_api_key_mutation_updates_requests_and_copies verifies that credential changes rebuild the provider, copies retain the correct credential, and requests carry the expected raw bearer token.
  • test_legacy_state_repr_does_not_expose_credentials verifies that the internal runtime signature does not reveal the bearer credential.

The regression itself is CodeQL's static source-to-sink classification, which is best validated by the PR's CodeQL run rather than by an implementation-coupled runtime unit test.

Validation performed locally:

rye run pytest -n 0 tests/lib/test_bedrock.py tests/lib/test_bedrock_auth_conformance.py tests/test_module_client.py -k bedrock
117 passed, 12 deselected

./scripts/lint
Ruff passed
Pyright: 0 errors, 0 warnings
MyPy: no issues found in 1518 source files
Import check passed

git diff --check
passed

No live AWS Bedrock request was made; the PR's CodeQL job provides the final scanner confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant