Repository navigation
fix(core): freeze credentials before signing SageMaker bearer token - #6360
Merged
nargokul merged 1 commit intoSep 29, 2026
Merged
Conversation
_generate_token passed botocore RefreshableCredentials straight into SigV4QueryAuth.add_auth(), which reads access_key, token, and secret_key as three separate, non-atomic property accesses. A credential refresh landing mid-signature mixes key material from two versions, producing a presigned URL whose signature fails verification with InvalidSignature (intermittent under concurrency; ~58% of tokens in a 96-thread repro). Snapshot the credentials with get_frozen_credentials() before signing so all three reads come from a single credential version. The read is taken under botocore's refresh lock. Static credentials also expose the method and return a coherent snapshot, so the change is behavior-preserving for non-refreshable credentials; a hasattr guard leaves any custom credential object without the method unchanged. Adds regression tests: a deterministic rotate-on-read case (100% corrupt without the fix), a concurrency case, and a signer-snapshot assertion.
HollowTube
had a problem deploying
to
manual-approval
September 29, 2026 17:13 — with 
GitHub Actions
Failure
GitHub Actions
Failure
HollowTube
had a problem deploying
to
manual-approval
September 29, 2026 17:13 — with
GitHub Actions
Failure
GitHub Actions
Failure
nargokul
approved these changes
Sep 29, 2026
This branch had an error being deployed
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.
Issue
sagemaker.core.token_generator._generate_tokenintermittently produces bearer tokens that are rejected withInvalidSignature. It is a race between botocore's SigV4 signer and credential refresh.Root cause
_generate_tokenpasses botocoreRefreshableCredentialsstraight intoSigV4QueryAuth(...).add_auth().add_authreads the credential fields as three separate, non-atomic accesses —access_key(→X-Amz-Credential), thentoken(→X-Amz-Security-Token), thensecret_key(→ the signing key). If a credential refresh lands between those reads, the presigned URL carries one version's access-key/token but a signature keyed on another version's secret, so verification fails. botocore's ownRefreshableCredentials.access_keydocstring warns to useget_frozen_credentials()for exactly this reason.Fix
Snapshot the credentials with
get_frozen_credentials()once, before constructing the signer. The snapshot is taken under botocore's refresh lock, so all three reads come from a single credential version.Behavior-preserving: static (non-refreshable)
Credentialsalso exposeget_frozen_credentials()and return a coherentReadOnlyCredentials, so their output is unchanged; thehasattrguard leaves any custom credential object without the method untouched.Reproduction / validation (offline, no AWS)
A rotate-on-read
RefreshableCredentials(expiry pinned inside botocore's advisory refresh window so every field access rotates) makes the race deterministic. On botocore 1.43.104:Tests
Adds
TestGenerateTokenFreezesCredentialstotests/unit/test_token_generator.py: a deterministic rotate-on-read regression (primary, CI-stable), a concurrency case, and an assertion that the signer is constructed with a single immutableReadOnlyCredentialssnapshot. Corruption is detected offline by decoding the presigned URL and recomputing the SigV4 signature with the secret paired to the embedded access key. Existing static-credential tests remain green (16 passed).