Harden the AWS client configuration#42
Merged
Merged
Conversation
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.
The AWS client configuration utilities in this repository are copies of the files that were recently hardened in osls, and they carried the same defects. Since compose polls CloudFormation while creating the remote state stack and reads and writes state in S3, these affected every command that touches remote state.
The documented
AWS_CLIENT_TIMEOUTenvironment variable was mapped torequestTimeout, which@smithy/node-http-handlertreats as warn-only since 4.4.0 across our entire declared range, so it never aborted anything. Worse, when no proxy, certificate, or timeout environment variables were set, no request handler was attached at all, leaving clients with no timeout whatsoever — a hung connection during state stack polling would hang compose forever. The variable now maps tosocketTimeoutwith inactivity semantics, the default is 120 seconds, and a handler is always constructed. The proxy agent is applied to both the https and plain-http schemes, and thekeepAliveagent option that was lost when the file was copied from osls is restored.Credential resolution had its own transport gap. The
fromIniandfromNodeProviderChainproviders were constructed without aclientConfig, so the inner SSO and OIDC clients they spin up made direct connections that ignored the proxy, custom certificate authorities, and timeouts. A single sharedNodeHttpHandleris now passed throughclientConfig.requestHandleron every construction path, with the region deliberately left out because it would override the profile'ssso_region. The MFA prompt now rejects withMFA_CODE_UNAVAILABLEwhen stdin reaches end of file instead of hanging forever in CI, and anAWS_DEFAULT_PROFILEthat names a profile absent from both shared files now logs a warning and falls back to the SDK default provider chain instead of failing hard; this deliberately reverses semantics that a previous test had pinned and matches the behavior osls ships. A one-time warning is also logged when a profile's static keys in the credentials file are shadowed by arole_arnin the config file, since commands then run under the assumed role identity.Credential resolution results are now memoized process-wide in
get-credential-provider.js, keyed by every input the resolution chain reads so environment changes still produce a fresh provider. Previously nothing cached at all: every client construction re-ran the full resolution, andmonitorStackCreationconstructed a new CloudFormation client on every two-second poll, which for a profile withmfa_serialorcredential_processmeant a prompt or process spawn per poll. The provider is wrapped once withmemoizeIdentityProviderfrom@smithy/coreand marked with thememoizedflag so clients skip their own per-client wrapping; this was verified against the aggregated client classes this repository uses, where one shared provider now resolves exactly once across multiple clients. The polling loop also reuses one client for the whole stack creation instead of constructing one per poll.The
S3StateStorageconstructor previously fell back to a raw{ region, credentials }config when no client config was given, bypassing all proxy, certificate, timeout, and retry configuration; the fallback now routes throughbuildClientConfigwhile preserving the credential semantics.@aws-sdk/credential-providersis raised to^3.1034.0to match the two client packages, and@smithy/coreis added as a direct dependency at^3.23.16, the exact range the SDK floor itself declares.The new tests include a real-socket timeout test against a listener that never responds, end-of-file and answered MFA prompt cases, request handler inheritance assertions on every credential construction path, a process-wide memoization suite covering cache hits, per-profile separation, environment-change misses, and expiring-credential refresh, and a stack creation poll test asserting a single client construction.