fix: skip credential check on output-env-credentials: false (#1778)
Closes #1554.
This commit is contained in:
parent
f35a7d7d7e
commit
58e7c47adf
5 changed files with 56 additions and 14 deletions
|
|
@ -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
|
the `aws-session-token` input in a situation where session tokens are fetched
|
||||||
and passed to this action.
|
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
|
### Configure multiple AWS profiles in a single workflow
|
||||||
|
|
||||||
```yaml
|
```yaml
|
||||||
|
|
|
||||||
|
|
@ -4,8 +4,8 @@
|
||||||
"version": "6.1.1",
|
"version": "6.1.1",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"build": "tsc",
|
"build": "tsc",
|
||||||
"lint": "biome check --error-on-warnings ./src && markdownlint -i node_modules -i CHANGELOG.md '**/*.md'",
|
"lint": "biome check --error-on-warnings ./src ./test && markdownlint -i node_modules -i CHANGELOG.md '**/*.md'",
|
||||||
"lint:fix": "biome check --write ./src && markdownlint -i node_modules -i CHANGELOG.md -f '**/*.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",
|
"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",
|
"test": "npm run lint && vitest run && npm run build",
|
||||||
"clean": "del-cli coverage test-reports node_modules",
|
"clean": "del-cli coverage test-reports node_modules",
|
||||||
|
|
|
||||||
28
src/index.ts
28
src/index.ts
|
|
@ -210,11 +210,16 @@ export async function run() {
|
||||||
// Validate that the SDK can actually pick up credentials.
|
// Validate that the SDK can actually pick up credentials.
|
||||||
// This validates cases where this action is using existing environment 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.
|
// and cases where the user intended to provide input credentials but the secrets inputs resolved to empty strings.
|
||||||
await withRetry(
|
// Skip when output-env-credentials is false: input IAM keys were not written to env, so
|
||||||
() => credentialsClient.validateCredentials(AccessKeyId, roleChaining, expectedAccountIds),
|
// the default chain would resolve to ambient runner credentials and the access-key check
|
||||||
'validateCredentials',
|
// would spuriously fail (see #1554).
|
||||||
);
|
if (outputEnvCredentials) {
|
||||||
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)) {
|
if (customTags && (useGitHubOIDCProvider() || webIdentityTokenFile)) {
|
||||||
core.warning(
|
core.warning(
|
||||||
|
|
@ -247,13 +252,12 @@ export async function run() {
|
||||||
} while (specialCharacterWorkaround && !verifyKeys(roleCredentials.Credentials));
|
} while (specialCharacterWorkaround && !verifyKeys(roleCredentials.Credentials));
|
||||||
core.info(`Authenticated as assumedRoleId ${roleCredentials.AssumedRoleUser?.AssumedRoleId}`);
|
core.info(`Authenticated as assumedRoleId ${roleCredentials.AssumedRoleUser?.AssumedRoleId}`);
|
||||||
exportCredentials(roleCredentials.Credentials, outputCredentials, outputEnvCredentials);
|
exportCredentials(roleCredentials.Credentials, outputCredentials, outputEnvCredentials);
|
||||||
// We need to validate the credentials in 2 of our use-cases
|
// Validate that the SDK can pick up the assumed-role credentials from the environment.
|
||||||
// First: self-hosted runners. If the GITHUB_ACTIONS environment variable
|
// Skip when output-env-credentials is false: the credentials were never written to env,
|
||||||
// is set to `true` then we are NOT in a self-hosted runner.
|
// so the default credential provider chain would resolve to ambient runner credentials
|
||||||
// Second: Customer provided credentials manually (IAM User keys stored in GH Secrets)
|
// (e.g. an EC2 instance profile) and the access-key-id check would spuriously fail.
|
||||||
// If we are using a profile, don't validate credentials yet (since they most likely won't be in the environment).
|
// Skip when using a profile: validation runs after the profile file is written below.
|
||||||
// Wait until after creds are written to the profile file to try validation.
|
if ((!process.env.GITHUB_ACTIONS || AccessKeyId) && !awsProfile && outputEnvCredentials) {
|
||||||
if ((!process.env.GITHUB_ACTIONS || AccessKeyId) && !awsProfile) {
|
|
||||||
await withRetry(
|
await withRetry(
|
||||||
() =>
|
() =>
|
||||||
credentialsClient.validateCredentials(
|
credentialsClient.validateCredentials(
|
||||||
|
|
|
||||||
|
|
@ -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', {}, () => {
|
describe('AssumeRole with WebIdentityTokeFile', {}, () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.mocked(core.getInput).mockImplementation(mocks.getInput(mocks.WEBIDENTITY_TOKEN_FILE_INPUTS));
|
vi.mocked(core.getInput).mockImplementation(mocks.getInput(mocks.WEBIDENTITY_TOKEN_FILE_INPUTS));
|
||||||
|
|
|
||||||
|
|
@ -83,6 +83,14 @@ const inputs = {
|
||||||
'output-env-credentials': 'false',
|
'output-env-credentials': 'false',
|
||||||
'output-credentials': 'true',
|
'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 = {
|
const envs = {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue