From 37f9f78ec374e525cfefee3fb29a709577beed52 Mon Sep 17 00:00:00 2001 From: Tom Keller Date: Mon, 25 May 2026 12:14:19 -0700 Subject: [PATCH] Revert "fix: skip credential check on output-env-credentials: false (#1778)" This reverts commit 58e7c47adf77846879008deadfeeef8a6969fe6c. --- README.md | 7 ------- package.json | 4 ++-- src/index.ts | 28 ++++++++++++---------------- test/index.test.ts | 23 ----------------------- test/mockinputs.test.ts | 8 -------- 5 files changed, 14 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 2e7d2b9..206ddcc 100644 --- a/README.md +++ b/README.md @@ -736,13 +736,6 @@ 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 e15799e..15f39a5 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 ./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'", + "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'", "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 2153a32..40d6ae3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -210,16 +210,11 @@ 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. - // 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'); - } + await withRetry( + () => credentialsClient.validateCredentials(AccessKeyId, roleChaining, expectedAccountIds), + 'validateCredentials', + ); + sourceAccountId = await withRetry(() => exportAccountId(credentialsClient, maskAccountId), 'exportAccountId'); } if (customTags && (useGitHubOIDCProvider() || webIdentityTokenFile)) { core.warning( @@ -252,12 +247,13 @@ export async function run() { } while (specialCharacterWorkaround && !verifyKeys(roleCredentials.Credentials)); core.info(`Authenticated as assumedRoleId ${roleCredentials.AssumedRoleUser?.AssumedRoleId}`); exportCredentials(roleCredentials.Credentials, outputCredentials, outputEnvCredentials); - // 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) { + // 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) { await withRetry( () => credentialsClient.validateCredentials( diff --git a/test/index.test.ts b/test/index.test.ts index 3970951..b84c883 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -151,29 +151,6 @@ 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 cfc4125..b5233ca 100644 --- a/test/mockinputs.test.ts +++ b/test/mockinputs.test.ts @@ -83,14 +83,6 @@ 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 = {