From c6821543f2778870516b86db42589ee36174f034 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Thu, 11 Jun 2026 22:16:06 +0100 Subject: [PATCH] Harden the AWS client configuration --- package.json | 3 +- src/state/S3StateStorage.js | 7 +- src/state/utils/get-state-bucket-name.js | 7 +- src/utils/aws/config.js | 27 +- src/utils/aws/credentials.js | 121 +++++++- src/utils/aws/get-credential-provider.js | 47 +++- test/unit/src/state/S3StateStorage.test.js | 18 ++ .../state/utils/get-state-bucket-name.test.js | 41 +++ test/unit/src/utils/aws/config.test.js | 70 ++++- test/unit/src/utils/aws/credentials.test.js | 261 +++++++++++++++++- .../utils/aws/get-credential-provider.test.js | 131 ++++++++- 11 files changed, 680 insertions(+), 53 deletions(-) diff --git a/package.json b/package.json index 9bcd90c..ce3263f 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,8 @@ "dependencies": { "@aws-sdk/client-cloudformation": "^3.1034.0", "@aws-sdk/client-s3": "^3.1034.0", - "@aws-sdk/credential-providers": "^3.975.0", + "@aws-sdk/credential-providers": "^3.1034.0", + "@smithy/core": "^3.23.16", "@smithy/node-http-handler": "^4.6.1", "@dagrejs/graphlib": "^3.0.4", "ajv": "^8.11.0", diff --git a/src/state/S3StateStorage.js b/src/state/S3StateStorage.js index a1cab17..468b304 100644 --- a/src/state/S3StateStorage.js +++ b/src/state/S3StateStorage.js @@ -6,6 +6,7 @@ const streamToString = require('../utils/stream-to-string'); const ServerlessError = require('../serverless-error'); const BaseStateStorage = require('./BaseStateStorage'); const normalizeState = require('./normalize-state'); +const { buildClientConfig } = require('../utils/aws/config'); const getAwsErrorCode = (error) => error && (error.Code || error.code || error.name); @@ -19,8 +20,12 @@ class S3StateStorage extends BaseStateStorage { this.bucketName = config.bucketName; this.stateKey = config.stateKey; + // The fallback routes through buildClientConfig so that proxy, CA, timeout, and + // retry configuration apply; credential semantics are preserved (the SDK default + // chain still applies when no credentials are given) this.s3Client = new S3( - config.clientConfig || { region: this.region, credentials: config.credentials } + config.clientConfig || + buildClientConfig({ region: this.region, credentials: config.credentials }) ); this.writeRequestQueue = pLimit(1); diff --git a/src/state/utils/get-state-bucket-name.js b/src/state/utils/get-state-bucket-name.js index fe220d8..ceda28c 100644 --- a/src/state/utils/get-state-bucket-name.js +++ b/src/state/utils/get-state-bucket-name.js @@ -23,8 +23,7 @@ const getCloudFormationClient = (stateConfiguration = {}, context = {}) => { ); }; -const monitorStackCreation = async (stackName, context, stateConfiguration) => { - const client = getCloudFormationClient(stateConfiguration, context); +const monitorStackCreation = async (client, stackName, context) => { const describeStacksResponse = await client.describeStacks({ StackName: stackName }); const status = describeStacksResponse.Stacks[0].StackStatus; @@ -32,7 +31,7 @@ const monitorStackCreation = async (stackName, context, stateConfiguration) => { // TODO: REMOVE WHEN REPLACED WITH PROGRESS context.logVerbose('Stack deployment in progress'); await sleep(2000); - return await monitorStackCreation(stackName, context, stateConfiguration); + return await monitorStackCreation(client, stackName, context); } if (status === 'CREATE_COMPLETE') { @@ -69,7 +68,7 @@ const ensureRemoteStateBucketStackExists = async (context, stateConfiguration) = ], }); - await monitorStackCreation(COMPOSE_REMOTE_STATE_STACK_NAME, context, stateConfiguration); + await monitorStackCreation(client, COMPOSE_REMOTE_STATE_STACK_NAME, context); context.output.log('S3 bucket for remote state created successfully'); return bucketName; }; diff --git a/src/utils/aws/config.js b/src/utils/aws/config.js index fc984dc..d53bcb7 100644 --- a/src/utils/aws/config.js +++ b/src/utils/aws/config.js @@ -12,6 +12,8 @@ const { NodeHttpHandler } = require('@smithy/node-http-handler'); * @param {Object|Function} options.credentials - AWS credentials or SDK v3 credential provider * @param {number} options.maxAttempts - Maximum retry attempts * @param {string} options.retryMode - Retry mode ('legacy', 'standard', 'adaptive') + * @param {Object} options.requestHandler - Request handler override, used in place of the + * proxy, timeout, and certificate configuration derived from the environment * @returns {Object} AWS SDK v3 client configuration */ function buildClientConfig(options = {}) { @@ -35,10 +37,7 @@ function buildClientConfig(options = {}) { config.requestHandler = requestHandler; } else { // Configure HTTP options (proxy, timeout, certificates) - const httpOptions = buildHttpOptions(); - if (httpOptions) { - config.requestHandler = new NodeHttpHandler(httpOptions); - } + config.requestHandler = new NodeHttpHandler(buildHttpOptions()); } return config; @@ -59,23 +58,22 @@ function getMaxAttempts() { /** * Build HTTP options for AWS SDK v3 clients - * @returns {Object|null} HTTP configuration or null if no special config needed + * @returns {Object} HTTP configuration */ function buildHttpOptions() { const httpOptions = {}; - // Configure timeout + // Socket inactivity timeout, matching the AWS SDK v2 default of 120 seconds. A bare + // requestTimeout would be warn-only in @smithy/node-http-handler 4.4.0+. const timeout = process.env.AWS_CLIENT_TIMEOUT || process.env.aws_client_timeout; - if (timeout) { - httpOptions.requestTimeout = parseInt(timeout, 10); - } + httpOptions.socketTimeout = timeout ? parseInt(timeout, 10) : 120000; // Configure proxy const proxy = getProxyUrl(); - // Configure custom CA certificates + // Configure HTTPS agent options for proxy and custom CA certificates const caCerts = getCACertificates(); - const agentOptions = {}; + const agentOptions = { keepAlive: true }; if (caCerts.length > 0) { Object.assign(agentOptions, { rejectUnauthorized: true, @@ -84,12 +82,15 @@ function buildHttpOptions() { } if (proxy) { - httpOptions.httpsAgent = new HttpsProxyAgent(proxy, agentOptions); + // Assigned for both schemes; https-proxy-agent tunnels plain-http requests too + const proxyAgent = new HttpsProxyAgent(proxy, agentOptions); + httpOptions.httpsAgent = proxyAgent; + httpOptions.httpAgent = proxyAgent; } else if (caCerts.length > 0) { httpOptions.httpsAgent = new https.Agent(agentOptions); } - return httpOptions.httpsAgent || 'requestTimeout' in httpOptions ? httpOptions : null; + return httpOptions; } /** diff --git a/src/utils/aws/credentials.js b/src/utils/aws/credentials.js index 64c8aa1..97c7e65 100644 --- a/src/utils/aws/credentials.js +++ b/src/utils/aws/credentials.js @@ -5,8 +5,19 @@ const os = require('os'); const path = require('path'); const readline = require('readline'); const { fromIni, fromNodeProviderChain } = require('@aws-sdk/credential-providers'); +const { NodeHttpHandler } = require('@smithy/node-http-handler'); +const { buildHttpOptions } = require('./config'); +const ServerlessError = require('../../serverless-error'); +const { log } = require('../serverless-utils/log'); -const defaultConfigProfileSectionRegex = /^profile\s+(?:default|"default"|'default')$/; +// Inner SSO/OIDC/STS clients created during credential resolution inherit the same +// proxy, CA, and timeout configuration as regular clients +let sharedRequestHandler; + +function getCredentialsRequestHandler() { + if (!sharedRequestHandler) sharedRequestHandler = new NodeHttpHandler(buildHttpOptions()); + return sharedRequestHandler; +} function hasEnvironmentCredentials(prefix) { return Boolean( @@ -37,11 +48,24 @@ function fromPrefixedEnv(prefix) { function promptMfaCode(mfaSerial) { const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); - return new Promise((resolve) => { + return new Promise((resolve, reject) => { + let answered = false; rl.question(`Enter MFA code for ${mfaSerial}: `, (answer) => { + answered = true; rl.close(); resolve(answer); }); + // Without this, stdin EOF (e.g. in CI) would leave the promise unsettled forever + rl.on('close', () => { + if (!answered) { + reject( + new ServerlessError( + `MFA code required for ${mfaSerial} but no interactive terminal is available`, + 'MFA_CODE_UNAVAILABLE' + ) + ); + } + }); }); } @@ -62,11 +86,14 @@ function getSharedConfigFilepath() { } function fromProfile(profile) { + maybeWarnIdentityDivergence(profile); return fromIni({ profile, filepath: getSharedCredentialsFilepath(), configFilepath: getSharedConfigFilepath(), mfaCodeProvider: promptMfaCode, + // Region is deliberately omitted: it would override the profile's sso_region + clientConfig: { requestHandler: getCredentialsRequestHandler() }, }); } @@ -90,7 +117,76 @@ function getIniSectionNames(filePath) { } function isDefaultConfigProfileSection(sectionName) { - return sectionName === 'default' || defaultConfigProfileSectionRegex.test(sectionName); + return isConfigProfileSection(sectionName, 'default'); +} + +function isConfigProfileSection(sectionName, profile) { + if (sectionName === profile) return true; + if (!sectionName.startsWith('profile')) return false; + const rest = sectionName.slice('profile'.length); + if (!/^\s/.test(rest)) return false; + const name = rest.trim(); + return name === profile || name === `"${profile}"` || name === `'${profile}'`; +} + +function doesProfileExist(profile) { + if (getIniSectionNames(getSharedCredentialsFilepath()).has(profile)) return true; + + for (const sectionName of getIniSectionNames(getSharedConfigFilepath())) { + if (isConfigProfileSection(sectionName, profile)) return true; + } + + return false; +} + +function doesIniSectionHaveKey(filePath, sectionMatcher, keyName) { + try { + const contents = fs.readFileSync(filePath, 'utf8'); + let inSection = false; + + for (const line of contents.split(/\r?\n/)) { + const trimmedLine = line.split(/(^|\s)[;#]/)[0].trim(); + if (trimmedLine[0] === '[' && trimmedLine[trimmedLine.length - 1] === ']') { + inSection = sectionMatcher(trimmedLine.slice(1, -1)); + } else if (inSection) { + const equalsIndex = trimmedLine.indexOf('='); + if (equalsIndex !== -1 && trimmedLine.slice(0, equalsIndex).trim() === keyName) { + return true; + } + } + } + + return false; + } catch { + return false; + } +} + +const warnedDivergenceProfiles = new Set(); + +function maybeWarnIdentityDivergence(profile) { + if (warnedDivergenceProfiles.has(profile)) return; + warnedDivergenceProfiles.add(profile); + + const hasStaticKeys = doesIniSectionHaveKey( + getSharedCredentialsFilepath(), + (sectionName) => sectionName === profile, + 'aws_access_key_id' + ); + if (!hasStaticKeys) return; + + const hasConfigRoleArn = doesIniSectionHaveKey( + getSharedConfigFilepath(), + (sectionName) => isConfigProfileSection(sectionName, profile), + 'role_arn' + ); + if (!hasConfigRoleArn) return; + + log.warning( + `Profile "${profile}" resolves via the role_arn configured in the AWS config file; ` + + 'the static keys in the credentials file are used only as source credentials for ' + + 'the AssumeRole call, so commands run under the assumed role identity.' + ); } function doesImplicitDefaultProfileExist() { @@ -114,7 +210,11 @@ function fromImplicitDefaultProfileWithFallback() { return await profileProvider(providerOptions); } catch (error) { if (doesImplicitDefaultProfileExist()) throw error; - if (!fallbackProvider) fallbackProvider = fromNodeProviderChain(); + if (!fallbackProvider) { + fallbackProvider = fromNodeProviderChain({ + clientConfig: { requestHandler: getCredentialsRequestHandler() }, + }); + } return fallbackProvider(providerOptions); } }; @@ -132,7 +232,18 @@ function getCredentialProvider({ profile, stage } = {}) { } if (process.env.AWS_PROFILE) return fromProfile(process.env.AWS_PROFILE); if (hasEnvironmentCredentials('AWS')) return fromPrefixedEnv('AWS'); - if (process.env.AWS_DEFAULT_PROFILE) return fromProfile(process.env.AWS_DEFAULT_PROFILE); + if (process.env.AWS_DEFAULT_PROFILE) { + if (doesProfileExist(process.env.AWS_DEFAULT_PROFILE)) { + return fromProfile(process.env.AWS_DEFAULT_PROFILE); + } + log.warning( + `Profile "${process.env.AWS_DEFAULT_PROFILE}" (from AWS_DEFAULT_PROFILE) was not found ` + + 'in the AWS credentials or config files; falling back to the default provider chain.' + ); + return fromNodeProviderChain({ + clientConfig: { requestHandler: getCredentialsRequestHandler() }, + }); + } return fromImplicitDefaultProfileWithFallback(); } diff --git a/src/utils/aws/get-credential-provider.js b/src/utils/aws/get-credential-provider.js index 314a6c5..f870ce3 100644 --- a/src/utils/aws/get-credential-provider.js +++ b/src/utils/aws/get-credential-provider.js @@ -1,5 +1,48 @@ 'use strict'; -const { getCredentialProvider } = require('./credentials'); +const { + memoizeIdentityProvider, + isIdentityExpired, + doesIdentityRequireRefresh, +} = require('@smithy/core'); +const { getCredentialProvider, hasEnvironmentCredentials } = require('./credentials'); -module.exports = ({ profile, stage } = {}) => getCredentialProvider({ profile, stage }); +const memoizedProviders = new Map(); + +// The key covers every input the resolution chain reads, so environment changes +// produce a fresh provider instead of a stale cache hit +function getCacheKey({ profile, stage } = {}) { + const stageUpper = stage ? stage.toUpperCase() : null; + + return JSON.stringify({ + profile: profile || null, + stage: stage || null, + awsProfile: process.env.AWS_PROFILE || null, + awsDefaultProfile: process.env.AWS_DEFAULT_PROFILE || null, + stageProfile: stageUpper ? process.env[`AWS_${stageUpper}_PROFILE`] || null : null, + hasStageEnvironmentCredentials: stageUpper + ? hasEnvironmentCredentials(`AWS_${stageUpper}`) + : false, + hasEnvironmentCredentials: hasEnvironmentCredentials('AWS'), + sharedCredentialsFile: process.env.AWS_SHARED_CREDENTIALS_FILE || null, + sharedConfigFile: process.env.AWS_CONFIG_FILE || null, + }); +} + +module.exports = ({ profile, stage } = {}) => { + const cacheKey = getCacheKey({ profile, stage }); + + if (!memoizedProviders.has(cacheKey)) { + // Memoize process-wide; the memoized flag stops each client adding its own memoizer + // and re-resolving (repeated MFA prompts, AssumeRole, credential_process) + const memoized = memoizeIdentityProvider( + getCredentialProvider({ profile, stage }), + isIdentityExpired, + doesIdentityRequireRefresh + ); + memoized.memoized = true; + memoizedProviders.set(cacheKey, memoized); + } + + return memoizedProviders.get(cacheKey); +}; diff --git a/test/unit/src/state/S3StateStorage.test.js b/test/unit/src/state/S3StateStorage.test.js index 30012cc..ce2fb12 100644 --- a/test/unit/src/state/S3StateStorage.test.js +++ b/test/unit/src/state/S3StateStorage.test.js @@ -174,6 +174,24 @@ describe('test/unit/src/state/S3StateStorage.test.js', () => { retryMode: 'standard', }); }); + + it('builds transport-aware client config when no client config is given', () => { + const S3 = sinon.stub().returns({}); + const S3StateStorageWithStubbedClient = proxyquire + .noCallThru() + .load('../../../../src/state/S3StateStorage', { + '@aws-sdk/client-s3': { S3 }, + }); + + new S3StateStorageWithStubbedClient({ bucketName, stateKey, region: 'eu-central-1' }); + + expect(S3).to.have.been.calledOnce; + const config = S3.firstCall.args[0]; + expect(config.region).to.equal('eu-central-1'); + expect(config.requestHandler).to.exist; + expect(config.maxAttempts).to.be.a('number'); + expect(config).to.not.have.property('credentials'); + }); }); it('serializes concurrent state writes', async () => { diff --git a/test/unit/src/state/utils/get-state-bucket-name.test.js b/test/unit/src/state/utils/get-state-bucket-name.test.js index 9c04322..afa6b63 100644 --- a/test/unit/src/state/utils/get-state-bucket-name.test.js +++ b/test/unit/src/state/utils/get-state-bucket-name.test.js @@ -126,6 +126,47 @@ describe('test/unit/src/state/utils/get-state-bucket-name.test.js', () => { getStateBucketName(configuration, context) ).to.be.eventually.rejected.and.have.property('code', 'CANNOT_DEPLOY_S3_REMOTE_STATE_STACK'); }); + + it('reuses one CloudFormation client across stack creation polls', async () => { + const constructed = []; + const describeStacks = sinon.stub(); + describeStacks.onCall(0).resolves({ Stacks: [{ StackStatus: 'CREATE_IN_PROGRESS' }] }); + describeStacks.onCall(1).resolves({ Stacks: [{ StackStatus: 'CREATE_COMPLETE' }] }); + const stackDoesNotExistError = Object.assign(new Error('Stack "test" does not exist'), { + Code: 'ValidationError', + }); + class FakeCloudFormation { + constructor(config) { + constructed.push(config); + } + + describeStacks(input) { + return describeStacks(input); + } + + describeStackResource() { + return Promise.reject(stackDoesNotExistError); + } + + createStack() { + return Promise.resolve({}); + } + } + const getStateBucketNameWithStubs = proxyquire( + '../../../../../src/state/utils/get-state-bucket-name', + { + '@aws-sdk/client-cloudformation': { CloudFormation: FakeCloudFormation }, + '../../utils': { sleep: sinon.stub().resolves() }, + } + ); + + expect(await getStateBucketNameWithStubs({ backend: 's3' }, context)).to.match( + /^serverless-compose-state-[a-f0-9]{24}$/ + ); + expect(describeStacks).to.have.been.calledTwice; + // One client for the failed stack lookup, one shared by creation and all polls + expect(constructed).to.have.length(2); + }); }); describe('CloudFormation client config', () => { diff --git a/test/unit/src/utils/aws/config.test.js b/test/unit/src/utils/aws/config.test.js index a46d3a5..64e4c10 100644 --- a/test/unit/src/utils/aws/config.test.js +++ b/test/unit/src/utils/aws/config.test.js @@ -1,9 +1,12 @@ 'use strict'; const chai = require('chai'); +const net = require('net'); const proxyquire = require('proxyquire'); +const { NodeHttpHandler } = require('@smithy/node-http-handler'); const { withClearedEnv } = require('../../../../lib/env'); +const { buildHttpOptions } = require('../../../../../src/utils/aws/config'); const { expect } = chai; @@ -108,7 +111,7 @@ describe('test/unit/src/utils/aws/config.test.js', () => { const config = buildClientConfig(); - expect(config.requestHandler.options).to.deep.equal({ requestTimeout: 1234 }); + expect(config.requestHandler.options).to.deep.equal({ socketTimeout: 1234 }); }); }); @@ -119,7 +122,70 @@ describe('test/unit/src/utils/aws/config.test.js', () => { const config = buildClientConfig(); - expect(config.requestHandler.options).to.deep.equal({ requestTimeout: 0 }); + expect(config.requestHandler.options).to.deep.equal({ socketTimeout: 0 }); + }); + }); + + it('defaults to a 120 second socket timeout and always constructs a request handler', async () => { + await withEnv(async () => { + const { buildClientConfig } = loadConfig(); + + const config = buildClientConfig(); + + expect(config.requestHandler.options).to.deep.equal({ socketTimeout: 120000 }); + }); + }); + + it('assigns the proxy agent for both http and https requests', async () => { + await withEnv(async () => { + process.env.HTTPS_PROXY = 'https://proxy.example.com:1234'; + const { buildClientConfig } = loadConfig(); + + const config = buildClientConfig(); + + expect(config.requestHandler.options.httpAgent).to.equal( + config.requestHandler.options.httpsAgent + ); + expect(config.requestHandler.options.httpsAgent.proxy).to.equal( + 'https://proxy.example.com:1234' + ); + expect(config.requestHandler.options.httpsAgent.options).to.include({ keepAlive: true }); + }); + }); + + it('enforces the socket inactivity timeout with the real transport', async () => { + await withEnv(async () => { + process.env.AWS_CLIENT_TIMEOUT = '500'; + const sockets = new Set(); + const server = net.createServer((socket) => { + // Accept the connection and never respond + sockets.add(socket); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const handler = new NodeHttpHandler(buildHttpOptions()); + + try { + const error = await handler + .handle({ + protocol: 'http:', + hostname: '127.0.0.1', + port: server.address().port, + method: 'GET', + path: '/', + headers: {}, + }) + .then( + () => null, + (handleError) => handleError + ); + + expect(error).to.exist; + expect(error.name).to.equal('TimeoutError'); + } finally { + handler.destroy(); + for (const socket of sockets) socket.destroy(); + await new Promise((resolve) => server.close(resolve)); + } }); }); diff --git a/test/unit/src/utils/aws/credentials.test.js b/test/unit/src/utils/aws/credentials.test.js index d47b797..ee7a1e9 100644 --- a/test/unit/src/utils/aws/credentials.test.js +++ b/test/unit/src/utils/aws/credentials.test.js @@ -42,7 +42,25 @@ describe('test/unit/src/utils/aws/credentials.test.js', () => { ); } - function loadCredentials({ files = {}, fromIni, fromNodeProviderChain } = {}) { + class FakeNodeHttpHandler { + constructor(options) { + this.options = options; + FakeNodeHttpHandler.instances.push(this); + } + } + FakeNodeHttpHandler.instances = []; + + beforeEach(() => { + FakeNodeHttpHandler.instances = []; + }); + + function loadCredentials({ + files = {}, + fromIni, + fromNodeProviderChain, + readline, + logWarning, + } = {}) { const readFileSync = sinon.stub().callsFake((filePath) => { if (Object.prototype.hasOwnProperty.call(files, filePath)) { const result = files[filePath]; @@ -52,14 +70,43 @@ describe('test/unit/src/utils/aws/credentials.test.js', () => { throw createMissingFileError(); }); - return proxyquire('../../../../../src/utils/aws/credentials', { + const stubs = { '@aws-sdk/credential-providers': { fromIni, fromNodeProviderChain, }, + '@smithy/node-http-handler': { NodeHttpHandler: FakeNodeHttpHandler }, + '../serverless-utils/log': { log: { warning: logWarning || sinon.stub() } }, 'fs': { readFileSync }, 'os': { homedir: () => homeDir }, - }); + }; + if (readline) stubs.readline = readline; + + return proxyquire('../../../../../src/utils/aws/credentials', stubs); + } + + function createFakeReadline({ answer } = {}) { + return { + createInterface: () => { + const handlers = {}; + return { + question: (prompt, callback) => { + if (answer !== undefined) { + callback(answer); + } else { + // Simulate stdin EOF: the question callback never fires, the interface closes + setImmediate(() => handlers.close && handlers.close()); + } + }, + close: () => { + if (handlers.close) handlers.close(); + }, + on: (event, handler) => { + handlers[event] = handler; + }, + }; + }, + }; } it('does not mutate AWS_PROFILE when AWS_DEFAULT_PROFILE is set', async () => { @@ -71,7 +118,13 @@ describe('test/unit/src/utils/aws/credentials.test.js', () => { }); const fromIni = sinon.stub().returns(profileProvider); const fromNodeProviderChain = sinon.stub(); - const { getCredentialProvider } = loadCredentials({ fromIni, fromNodeProviderChain }); + const { getCredentialProvider } = loadCredentials({ + files: { + [credentialsFilePath]: ['[custom-default]', 'aws_access_key_id = accessKeyId'].join('\n'), + }, + fromIni, + fromNodeProviderChain, + }); getCredentialProvider(); @@ -291,24 +344,30 @@ describe('test/unit/src/utils/aws/credentials.test.js', () => { }); }); - it('does not fallback when AWS_DEFAULT_PROFILE is explicitly set but absent', async () => { + it('falls back to the default provider chain when AWS_DEFAULT_PROFILE is absent', async () => { await withEnv(async () => { process.env.AWS_DEFAULT_PROFILE = 'missing-default'; - const fallbackProvider = sinon.stub().resolves({ + const fallbackCredentials = { accessKeyId: 'fallbackAccessKeyId', secretAccessKey: 'fallbackSecretAccessKey', - }); + }; + const fallbackProvider = sinon.stub().resolves(fallbackCredentials); const fromIni = sinon .stub() .returns(sinon.stub().rejects(createUnresolvedProfileError('missing-default'))); const fromNodeProviderChain = sinon.stub().returns(fallbackProvider); - const { getCredentialProvider } = loadCredentials({ fromIni, fromNodeProviderChain }); + const logWarning = sinon.spy(); + const { getCredentialProvider } = loadCredentials({ + fromIni, + fromNodeProviderChain, + logWarning, + }); - await expect(getCredentialProvider()()).to.be.rejectedWith( - 'Could not resolve credentials using profile' - ); - expect(fromNodeProviderChain).to.not.have.been.called; - expect(fallbackProvider).to.not.have.been.called; + await expect(getCredentialProvider()()).to.eventually.deep.equal(fallbackCredentials); + expect(fromIni).to.not.have.been.called; + expect(fromNodeProviderChain).to.have.been.calledOnce; + expect(logWarning).to.have.been.calledOnce; + expect(logWarning.firstCall.args[0]).to.include('missing-default'); }); }); @@ -508,4 +567,180 @@ describe('test/unit/src/utils/aws/credentials.test.js', () => { expect(fallbackProvider).to.not.have.been.called; }); }); + + it('passes a request handler to profile providers without overriding region', async () => { + await withEnv(async () => { + process.env.AWS_PROFILE = 'dev'; + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + loadCredentials({ fromIni, fromNodeProviderChain }).getCredentialProvider(); + + expect(fromIni).to.have.been.calledOnce; + const initOptions = fromIni.firstCall.args[0]; + expect(initOptions.profile).to.equal('dev'); + expect(initOptions.clientConfig.requestHandler).to.be.an.instanceOf(FakeNodeHttpHandler); + expect(initOptions.clientConfig).to.not.have.property('region'); + }); + }); + + it('passes a request handler to the default provider chain fallback', async () => { + await withEnv(async () => { + const fallbackCredentials = { + accessKeyId: 'fallbackAccessKeyId', + secretAccessKey: 'fallbackSecretAccessKey', + }; + const fromIni = sinon + .stub() + .returns(sinon.stub().rejects(createUnresolvedProfileError('default'))); + const fromNodeProviderChain = sinon + .stub() + .returns(sinon.stub().resolves(fallbackCredentials)); + const { getCredentialProvider } = loadCredentials({ fromIni, fromNodeProviderChain }); + + await expect(getCredentialProvider()()).to.eventually.deep.equal(fallbackCredentials); + expect(fromNodeProviderChain).to.have.been.calledOnce; + const initOptions = fromNodeProviderChain.firstCall.args[0]; + expect(initOptions.clientConfig.requestHandler).to.be.an.instanceOf(FakeNodeHttpHandler); + expect(initOptions.clientConfig).to.not.have.property('region'); + }); + }); + + it('shares a single request handler across all credential providers', async () => { + await withEnv(async () => { + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + const { getCredentialProvider } = loadCredentials({ fromIni, fromNodeProviderChain }); + + getCredentialProvider({ profile: 'first' }); + getCredentialProvider({ profile: 'second' }); + + expect(fromIni).to.have.been.calledTwice; + expect(FakeNodeHttpHandler.instances).to.have.length(1); + expect(fromIni.firstCall.args[0].clientConfig.requestHandler).to.equal( + fromIni.secondCall.args[0].clientConfig.requestHandler + ); + }); + }); + + it('rejects the MFA prompt when input closes without an answer', async () => { + await withEnv(async () => { + process.env.AWS_PROFILE = 'mfa-profile'; + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + loadCredentials({ + fromIni, + fromNodeProviderChain, + readline: createFakeReadline(), + }).getCredentialProvider(); + + const { mfaCodeProvider } = fromIni.firstCall.args[0]; + const error = await mfaCodeProvider('arn:aws:iam::123456789012:mfa/user').then( + () => null, + (promptError) => promptError + ); + + expect(error).to.exist; + expect(error.code).to.equal('MFA_CODE_UNAVAILABLE'); + expect(error.message).to.include('arn:aws:iam::123456789012:mfa/user'); + }); + }); + + it('resolves the MFA prompt with the provided answer', async () => { + await withEnv(async () => { + process.env.AWS_PROFILE = 'mfa-profile'; + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + loadCredentials({ + fromIni, + fromNodeProviderChain, + readline: createFakeReadline({ answer: '123456' }), + }).getCredentialProvider(); + + const { mfaCodeProvider } = fromIni.firstCall.args[0]; + + await expect(mfaCodeProvider('arn:aws:iam::123456789012:mfa/user')).to.eventually.equal( + '123456' + ); + }); + }); + + it('warns once when a profile has static keys shadowed by a config file role_arn', async () => { + await withEnv(async () => { + process.env.AWS_PROFILE = 'divergent'; + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + const logWarning = sinon.spy(); + const { getCredentialProvider } = loadCredentials({ + files: { + [credentialsFilePath]: [ + '[divergent]', + 'aws_access_key_id = accessKeyId', + 'aws_secret_access_key = secretAccessKey', + ].join('\n'), + [configFilePath]: [ + '[profile divergent]', + 'role_arn = arn:aws:iam::123456789012:role/deploy', + 'source_profile = divergent', + ].join('\n'), + }, + fromIni, + fromNodeProviderChain, + logWarning, + }); + + getCredentialProvider(); + getCredentialProvider(); + + expect(logWarning).to.have.been.calledOnce; + expect(logWarning.firstCall.args[0]).to.include('divergent'); + expect(logWarning.firstCall.args[0]).to.include('role_arn'); + }); + }); + + it('does not warn when a profile has no config file role_arn', async () => { + await withEnv(async () => { + process.env.AWS_PROFILE = 'plain'; + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + const logWarning = sinon.spy(); + loadCredentials({ + files: { + [credentialsFilePath]: [ + '[plain]', + 'aws_access_key_id = accessKeyId', + 'aws_secret_access_key = secretAccessKey', + ].join('\n'), + [configFilePath]: ['[profile plain]', 'region = us-east-1'].join('\n'), + }, + fromIni, + fromNodeProviderChain, + logWarning, + }).getCredentialProvider(); + + expect(logWarning).to.not.have.been.called; + }); + }); + + it('does not warn when a config file role_arn profile has no static keys', async () => { + await withEnv(async () => { + process.env.AWS_PROFILE = 'role-only'; + const fromIni = sinon.stub().returns(sinon.stub().resolves({})); + const fromNodeProviderChain = sinon.stub(); + const logWarning = sinon.spy(); + loadCredentials({ + files: { + [configFilePath]: [ + '[profile role-only]', + 'role_arn = arn:aws:iam::123456789012:role/deploy', + 'credential_source = Environment', + ].join('\n'), + }, + fromIni, + fromNodeProviderChain, + logWarning, + }).getCredentialProvider(); + + expect(logWarning).to.not.have.been.called; + }); + }); }); diff --git a/test/unit/src/utils/aws/get-credential-provider.test.js b/test/unit/src/utils/aws/get-credential-provider.test.js index f8abe5a..15a712a 100644 --- a/test/unit/src/utils/aws/get-credential-provider.test.js +++ b/test/unit/src/utils/aws/get-credential-provider.test.js @@ -4,24 +4,131 @@ const chai = require('chai'); const proxyquire = require('proxyquire'); const sinon = require('sinon'); +const { withClearedEnv } = require('../../../../lib/env'); + const expect = chai.expect; describe('test/unit/src/utils/aws/get-credential-provider.test.js', () => { - it('forwards profile and stage to the aligned credential resolver', () => { - const credentialProvider = sinon.stub().returns('provider'); + const envKeys = [ + 'AWS_PROFILE', + 'AWS_DEFAULT_PROFILE', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY', + 'AWS_SHARED_CREDENTIALS_FILE', + 'AWS_CONFIG_FILE', + ]; + + const withEnv = (callback) => withClearedEnv(envKeys, callback); + + function loadGetCredentialProvider({ getCredentialProvider }) { + return proxyquire.noCallThru().load('../../../../../src/utils/aws/get-credential-provider', { + './credentials': { + getCredentialProvider, + hasEnvironmentCredentials: () => false, + }, + }); + } + + it('forwards profile and stage to the credential resolver', async () => { + await withEnv(async () => { + const baseProvider = sinon.stub().resolves({ accessKeyId: 'key', secretAccessKey: 's' }); + const credentialProvider = sinon.stub().returns(baseProvider); + const getCredentialProvider = loadGetCredentialProvider({ + getCredentialProvider: credentialProvider, + }); + + getCredentialProvider({ profile: 'custom-profile', stage: 'prod' }); + + expect(credentialProvider).to.have.been.calledOnceWithExactly({ + profile: 'custom-profile', + stage: 'prod', + }); + }); + }); + + it('memoizes credential resolution process-wide', async () => { + await withEnv(async () => { + const resolvedCredentials = { accessKeyId: 'accessKeyId', secretAccessKey: 'secret' }; + const baseProvider = sinon.stub().resolves(resolvedCredentials); + const getCredentialProvider = loadGetCredentialProvider({ + getCredentialProvider: sinon.stub().returns(baseProvider), + }); + + const provider = getCredentialProvider({ profile: 'custom', stage: 'dev' }); + + expect(provider.memoized).to.equal(true); + await expect(provider()).to.eventually.deep.equal(resolvedCredentials); + await expect(provider()).to.eventually.deep.equal(resolvedCredentials); + // A later lookup with the same inputs must not trigger another resolution either + await expect( + getCredentialProvider({ profile: 'custom', stage: 'dev' })() + ).to.eventually.deep.equal(resolvedCredentials); + expect(baseProvider).to.have.been.calledOnce; + }); + }); + + it('keeps providers separate per profile and stage', async () => { + await withEnv(async () => { + const credentialProvider = sinon + .stub() + .callsFake(({ profile }) => + sinon.stub().resolves({ accessKeyId: profile, secretAccessKey: 'secret' }) + ); + const getCredentialProvider = loadGetCredentialProvider({ + getCredentialProvider: credentialProvider, + }); + + const firstProvider = getCredentialProvider({ profile: 'first' }); + const secondProvider = getCredentialProvider({ profile: 'second' }); + + expect(firstProvider).to.not.equal(secondProvider); + await expect(firstProvider()).to.eventually.deep.equal({ + accessKeyId: 'first', + secretAccessKey: 'secret', + }); + await expect(secondProvider()).to.eventually.deep.equal({ + accessKeyId: 'second', + secretAccessKey: 'secret', + }); + }); + }); + + it('resolves a fresh provider when the credential environment changes', async () => { + await withEnv(async () => { + const credentialProvider = sinon + .stub() + .returns(sinon.stub().resolves({ accessKeyId: 'key', secretAccessKey: 'secret' })); + const getCredentialProvider = loadGetCredentialProvider({ + getCredentialProvider: credentialProvider, + }); + + const beforeProvider = getCredentialProvider(); + process.env.AWS_PROFILE = 'changed'; + const afterProvider = getCredentialProvider(); - const getCredentialProvider = proxyquire - .noCallThru() - .load('../../../../../src/utils/aws/get-credential-provider', { - './credentials': { getCredentialProvider: credentialProvider }, + expect(beforeProvider).to.not.equal(afterProvider); + expect(credentialProvider).to.have.been.calledTwice; + }); + }); + + it('refreshes expiring credentials instead of serving them stale', async () => { + await withEnv(async () => { + const expiringCredentials = { + accessKeyId: 'accessKeyId', + secretAccessKey: 'secret', + expiration: new Date(Date.now() + 60 * 1000), + }; + const baseProvider = sinon.stub().resolves(expiringCredentials); + const getCredentialProvider = loadGetCredentialProvider({ + getCredentialProvider: sinon.stub().returns(baseProvider), }); - expect(getCredentialProvider({ profile: 'custom-profile', stage: 'prod' })).to.equal( - 'provider' - ); - expect(credentialProvider).to.have.been.calledOnceWithExactly({ - profile: 'custom-profile', - stage: 'prod', + const provider = getCredentialProvider(); + await provider(); + await provider(); + + // Within the SDK's 5-minute expiry window every call re-resolves + expect(baseProvider.callCount).to.be.greaterThan(1); }); }); });