From 58e7c47adf77846879008deadfeeef8a6969fe6c Mon Sep 17 00:00:00 2001 From: Tom Keller <1083460+kellertk@users.noreply.github.com> Date: Thu, 14 May 2026 13:39:34 -0700 Subject: [PATCH] fix: skip credential check on output-env-credentials: false (#1778) Closes #1554. --- README.md | 7 +++++++ package.json | 4 ++-- src/index.ts | 28 ++++++++++++++++------------ test/index.test.ts | 23 +++++++++++++++++++++++ test/mockinputs.test.ts | 8 ++++++++ 5 files changed, 56 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 206ddcc..2e7d2b9 100644 --- a/README.md +++ b/README.md @@ -736,6 +736,13 @@ This example shows that you can reference the fetched credentials as outputs if the `aws-session-token` input in a situation where session tokens are fetched and passed to this action. +If you only want the credentials available as _step outputs_ and not exported to +the environment (for example, on a self-hosted runner where you do not want the +assumed-role credentials to shadow an existing EC2 instance profile), pair +`output-credentials: true` with `output-env-credentials: false`. In that mode, +the action does not run its post-credential SDK-pickup validation step, since +the credentials were never written to the environment. + ### Configure multiple AWS profiles in a single workflow ```yaml diff --git a/package.json b/package.json index 15f39a5..e15799e 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,8 @@ "version": "6.1.1", "scripts": { "build": "tsc", - "lint": "biome check --error-on-warnings ./src && markdownlint -i node_modules -i CHANGELOG.md '**/*.md'", - "lint:fix": "biome check --write ./src && markdownlint -i node_modules -i CHANGELOG.md -f '**/*.md'", + "lint": "biome check --error-on-warnings ./src ./test && markdownlint -i node_modules -i CHANGELOG.md '**/*.md'", + "lint:fix": "biome check --write ./src ./test && markdownlint -i node_modules -i CHANGELOG.md -f '**/*.md'", "package": "esbuild src/index.ts --bundle --platform=node --target=node24 --outfile=dist/index.js && esbuild src/cleanup/index.ts --bundle --platform=node --target=node24 --outfile=dist/cleanup/index.js && npm run license", "test": "npm run lint && vitest run && npm run build", "clean": "del-cli coverage test-reports node_modules", diff --git a/src/index.ts b/src/index.ts index 40d6ae3..2153a32 100644 --- a/src/index.ts +++ b/src/index.ts @@ -210,11 +210,16 @@ export async function run() { // Validate that the SDK can actually pick up credentials. // This validates cases where this action is using existing environment credentials, // and cases where the user intended to provide input credentials but the secrets inputs resolved to empty strings. - await withRetry( - () => credentialsClient.validateCredentials(AccessKeyId, roleChaining, expectedAccountIds), - 'validateCredentials', - ); - sourceAccountId = await withRetry(() => exportAccountId(credentialsClient, maskAccountId), 'exportAccountId'); + // Skip when output-env-credentials is false: input IAM keys were not written to env, so + // the default chain would resolve to ambient runner credentials and the access-key check + // would spuriously fail (see #1554). + if (outputEnvCredentials) { + await withRetry( + () => credentialsClient.validateCredentials(AccessKeyId, roleChaining, expectedAccountIds), + 'validateCredentials', + ); + sourceAccountId = await withRetry(() => exportAccountId(credentialsClient, maskAccountId), 'exportAccountId'); + } } if (customTags && (useGitHubOIDCProvider() || webIdentityTokenFile)) { core.warning( @@ -247,13 +252,12 @@ export async function run() { } while (specialCharacterWorkaround && !verifyKeys(roleCredentials.Credentials)); core.info(`Authenticated as assumedRoleId ${roleCredentials.AssumedRoleUser?.AssumedRoleId}`); exportCredentials(roleCredentials.Credentials, outputCredentials, outputEnvCredentials); - // We need to validate the credentials in 2 of our use-cases - // First: self-hosted runners. If the GITHUB_ACTIONS environment variable - // is set to `true` then we are NOT in a self-hosted runner. - // Second: Customer provided credentials manually (IAM User keys stored in GH Secrets) - // If we are using a profile, don't validate credentials yet (since they most likely won't be in the environment). - // Wait until after creds are written to the profile file to try validation. - if ((!process.env.GITHUB_ACTIONS || AccessKeyId) && !awsProfile) { + // Validate that the SDK can pick up the assumed-role credentials from the environment. + // Skip when output-env-credentials is false: the credentials were never written to env, + // so the default credential provider chain would resolve to ambient runner credentials + // (e.g. an EC2 instance profile) and the access-key-id check would spuriously fail. + // Skip when using a profile: validation runs after the profile file is written below. + if ((!process.env.GITHUB_ACTIONS || AccessKeyId) && !awsProfile && outputEnvCredentials) { await withRetry( () => credentialsClient.validateCredentials( diff --git a/test/index.test.ts b/test/index.test.ts index b84c883..3970951 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -151,6 +151,29 @@ describe('Configure AWS Credentials', {}, () => { }); }); + // Regression test for #1554: IAM keys + role-to-assume on a self-hosted runner + // with ambient credentials (e.g. an EC2 instance profile), and output-env-credentials=false. + // The post-assume-role validation must be skipped, otherwise the SDK loads the runner's + // ambient access key (which doesn't match the assumed role's) and the action fails. + describe('AssumeRole with IAM LTC and output-env-credentials=false', {}, () => { + it('does not validate against ambient credentials', async () => { + vi.mocked(core.getInput).mockImplementation(mocks.getInput(mocks.IAM_ASSUMEROLE_NO_ENV_INPUTS)); + mockedSTSClient.on(AssumeRoleCommand).resolvesOnce(mocks.outputs.STS_CREDENTIALS); + mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY }); + // Simulate the runner's ambient instance-profile credentials. + // biome-ignore lint/suspicious/noExplicitAny: any required to mock private method + vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({ + accessKeyId: 'AMBIENTINSTANCEPROFILEID', + }); + await run(); + expect(core.setFailed).not.toHaveBeenCalled(); + expect(core.exportVariable).not.toHaveBeenCalled(); + expect(core.setOutput).toHaveBeenCalledWith('aws-access-key-id', 'STSAWSACCESSKEYID'); + expect(core.setOutput).toHaveBeenCalledWith('aws-secret-access-key', 'STSAWSSECRETACCESSKEY'); + expect(core.setOutput).toHaveBeenCalledWith('aws-session-token', 'STSAWSSESSIONTOKEN'); + }); + }); + describe('AssumeRole with WebIdentityTokeFile', {}, () => { beforeEach(() => { vi.mocked(core.getInput).mockImplementation(mocks.getInput(mocks.WEBIDENTITY_TOKEN_FILE_INPUTS)); diff --git a/test/mockinputs.test.ts b/test/mockinputs.test.ts index b5233ca..cfc4125 100644 --- a/test/mockinputs.test.ts +++ b/test/mockinputs.test.ts @@ -83,6 +83,14 @@ const inputs = { 'output-env-credentials': 'false', 'output-credentials': 'true', }, + IAM_ASSUMEROLE_NO_ENV_INPUTS: { + 'aws-access-key-id': 'MYAWSACCESSKEYID', + 'aws-secret-access-key': 'MYAWSSECRETACCESSKEY', + 'role-to-assume': 'arn:aws:iam::111111111111:role/MY-ROLE', + 'aws-region': 'fake-region-1', + 'output-env-credentials': 'false', + 'output-credentials': 'true', + }, }; const envs = {