Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update environment variable name behavior to allow for blank aliases #83

Merged
merged 7 commits into from
Mar 12, 2024
43 changes: 40 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ To use the action, add a step to your workflow that uses the following syntax.
### Parameters
- `secret-ids`: Secret ARNS, names, and name prefixes.

By default, the step creates each environment variable name from the secret name, transformed to include only uppercase letters, numbers, and underscores, and so that it doesn't begin with a number.
By default, the step creates each environment variable name from the secret name, transformed to include only uppercase letters, numbers, and underscores, and so that it doesn't begin with a number.

To set the environment variable name, enter it before the secret ID, followed by a comma. For example `ENV_VAR_1, secretId` creates an environment variable named **ENV_VAR_1** from the secret `secretId`.
To set the environment variable name, enter it before the secret ID, followed by a comma. For example `ENV_VAR_1, secretId` creates an environment variable named **ENV_VAR_1** from the secret `secretId`. A line with an empty environment variable name but a leading comma (ex: `, secretId`) behaves as if the entry didn't have that leading comma (ex: `secretId`) if the secret is not valid JSON or the `parse-json-secrets` flag is false.

The environment variable name can consist of uppercase letters, numbers, and underscores.

Expand All @@ -57,7 +57,7 @@ To use a prefix, enter at least three characters followed by an asterisk. For ex

Set `parse-json-secrets` to `true` to create environment variables for each key/value pair in the JSON.

Note that if the JSON uses case-sensitive keys such as "name" and "Name", the action will have duplicate name conflicts. In this case, set `parse-json-secrets` to `false` and parse the JSON secret value separately.
Note that if the JSON uses case-sensitive keys such as "name" and "Name", the action will have duplicate name conflicts. In this case, set `parse-json-secrets` to `false` and parse the JSON secret value separately. Additionally, if the secret is JSON and this flag is true: blank aliases are allowed and result in environment variables with no prefix (see Example 4).
### Examples
Expand Down Expand Up @@ -137,6 +137,43 @@ TEST_SECRET_API_KEY: "key"
TEST_SECRET_CONFIG_ACTIVE: "true"
```

**Example 4: Parse JSON in secret with blank alias**
The following example creates environment variables by parsing the JSON in the secret without assigning an alias.
```
- name: Get Secrets by Name and by ARN
uses: aws-actions/aws-secretsmanager-get-secrets@v1
with:
secret-ids: |
, test/blankAliasSecret
, test/blankAliasSecret2
parse-json-secrets: true
```
The secret `test/blankAliasSecret` has the following secret value.
```
{
"api_user": "user",
"api_key": "key",
"config": {
"active": "true"
}
}
```
And secret `test/blankAliasSecret2` has the following secret value.
```
plaintextsecret
```
Environment variables created:
```
API_USER: "user"
API_KEY: "key"
CONFIG_ACTIVE: "true"
TEST_BLANKALIASSECRET2: "plaintextsecret"
```
If the `parse-json-secrets` flag is toggled to false; each secret is treated as a plaintext string (even if it's JSON formatted) and the behavior of `test/blankAliasSecret2` is applied for a blank alias.

## Security

See [CONTRIBUTING](CONTRIBUTING.md#security-issue-notifications) for more information.
Expand Down
96 changes: 91 additions & 5 deletions __tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,23 @@ const ENV_NAME_4 = 'ARN_ALIAS';
const SECRET_4 = "secretString2";
const TEST_ARN_INPUT = ENV_NAME_4 + "," + TEST_ARN_1;

const BLANK_NAME = "test/blank";
const SECRET_FOR_BLANK = '{"username": "integ", "password": "integpw", "config": {"id1": "example1"}}';
const BLANK_ALIAS_INPUT = "," + BLANK_NAME;

const BLANK_NAME_2 = "test/blank2";
const SECRET_FOR_BLANK_2 = "blankNameSecretString";
const BLANK_ALIAS_INPUT_2 = "," + BLANK_NAME_2;

const BLANK_NAME_3 = "test/blank3";
const SECRET_FOR_BLANK_3 = '{"username": "integ", "password": "integpw", "config": {"id2": "example2"}}';
const BLANK_ALIAS_INPUT_3 = "," + BLANK_NAME_3;

// Mock the inputs for Github action
jest.mock('@actions/core', () => {
return {
getMultilineInput: jest.fn((name: string, options?: core.InputOptions) => [TEST_NAME, TEST_INPUT_3, TEST_ARN_INPUT] ),
getBooleanInput: jest.fn((name: string, options?: core.InputOptions) => true),
getMultilineInput: jest.fn(),
getBooleanInput: jest.fn(),
setFailed: jest.fn(),
info: jest.fn(),
debug: jest.fn(),
Expand All @@ -59,6 +71,11 @@ describe('Test main action', () => {
});

test('Retrieves and sets the requested secrets as environment variables, parsing JSON', async () => {
const booleanSpy = jest.spyOn(core, "getBooleanInput").mockReturnValue(true);
const multilineInputSpy = jest.spyOn(core, "getMultilineInput").mockReturnValue(
[TEST_NAME, TEST_INPUT_3, TEST_ARN_INPUT, BLANK_ALIAS_INPUT]
);

// Mock all Secrets Manager calls
smMockClient
.on(GetSecretValueCommand, { SecretId: TEST_NAME_1})
Expand All @@ -84,10 +101,12 @@ describe('Test main action', () => {
Name: TEST_NAME_2
}
]
});
})
.on(GetSecretValueCommand, { SecretId: BLANK_NAME })
.resolves({ Name: BLANK_NAME, SecretString: SECRET_FOR_BLANK });

await run();
expect(core.exportVariable).toHaveBeenCalledTimes(7);
expect(core.exportVariable).toHaveBeenCalledTimes(10);
expect(core.setFailed).not.toHaveBeenCalled();

// JSON secrets should be parsed
Expand All @@ -99,17 +118,81 @@ describe('Test main action', () => {
expect(core.exportVariable).toHaveBeenCalledWith(ENV_NAME_3, SECRET_3);
expect(core.exportVariable).toHaveBeenCalledWith(ENV_NAME_4, SECRET_4);

expect(core.exportVariable).toHaveBeenCalledWith(CLEANUP_NAME, JSON.stringify(['TEST_ONE_USER', 'TEST_ONE_PASSWORD', 'TEST_TWO_USER', 'TEST_TWO_PASSWORD', ENV_NAME_3, ENV_NAME_4]));
// Case when alias is blank, but still comma delimited in workflow and json is parsed
// ex: ,test5/secret
expect(core.exportVariable).toHaveBeenCalledWith("USERNAME", "integ");
expect(core.exportVariable).toHaveBeenCalledWith("PASSWORD", "integpw");
expect(core.exportVariable).toHaveBeenCalledWith("CONFIG_ID1", "example1");

expect(core.exportVariable).toHaveBeenCalledWith(
CLEANUP_NAME,
JSON.stringify([
'TEST_ONE_USER', 'TEST_ONE_PASSWORD',
'TEST_TWO_USER', 'TEST_TWO_PASSWORD',
ENV_NAME_3,
ENV_NAME_4,
"USERNAME", "PASSWORD", "CONFIG_ID1"
])
);

booleanSpy.mockClear();
multilineInputSpy.mockClear();
});

test('Defaults to correct behavior with empty string alias', async () => {
const booleanSpy = jest.spyOn(core, "getBooleanInput").mockReturnValue(false);
const multilineInputSpy = jest.spyOn(core, "getMultilineInput").mockReturnValue(
[BLANK_ALIAS_INPUT_2, BLANK_ALIAS_INPUT_3]
);

smMockClient
.on(GetSecretValueCommand, { SecretId: BLANK_NAME_2 })
.resolves({ Name: BLANK_NAME_2, SecretString: SECRET_FOR_BLANK_2 })
.on(GetSecretValueCommand, { SecretId: BLANK_NAME_3 })
.resolves({ Name: BLANK_NAME_3, SecretString: SECRET_FOR_BLANK_3 });

await run();
expect(core.exportVariable).toHaveBeenCalledTimes(3);
expect(core.setFailed).not.toHaveBeenCalled();

// Case when alias is blank, but still comma delimited in workflow and no json is parsed
// ex: ,test/blank2
expect(core.exportVariable).toHaveBeenCalledWith("TEST_BLANK2", "blankNameSecretString");
expect(core.exportVariable).toHaveBeenCalledWith("TEST_BLANK3", '{"username": "integ", "password": "integpw", "config": {"id2": "example2"}}');

expect(core.exportVariable).toHaveBeenCalledWith(
CLEANUP_NAME,
JSON.stringify([
"TEST_BLANK2",
"TEST_BLANK3"
])
);

booleanSpy.mockClear();
multilineInputSpy.mockClear();
});

test('Fails the action when an error occurs in Secrets Manager', async () => {
const booleanSpy = jest.spyOn(core, "getBooleanInput").mockReturnValue(true);
const multilineInputSpy = jest.spyOn(core, "getMultilineInput").mockReturnValue(
[TEST_NAME, TEST_INPUT_3, TEST_ARN_INPUT]
);

smMockClient.onAnyCommand().resolves({});

await run();
expect(core.setFailed).toHaveBeenCalledTimes(1);

booleanSpy.mockClear();
multilineInputSpy.mockClear();
});

test('Fails the action when multiple secrets exported the same variable name', async () => {
const booleanSpy = jest.spyOn(core, "getBooleanInput").mockReturnValue(true);
const multilineInputSpy = jest.spyOn(core, "getMultilineInput").mockReturnValue(
[TEST_NAME, TEST_INPUT_3, TEST_ARN_INPUT]
);

smMockClient
.on(GetSecretValueCommand, { SecretId: TEST_NAME_1})
.resolves({ Name: TEST_NAME_1, SecretString: SECRET_1 })
Expand Down Expand Up @@ -140,5 +223,8 @@ describe('Test main action', () => {

await run();
expect(core.setFailed).toHaveBeenCalledTimes(1);

booleanSpy.mockClear();
multilineInputSpy.mockClear();
});
});
13 changes: 10 additions & 3 deletions __tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const TEST_NAME_1 = 'test/secret1';
const VALID_ARN_2 = 'arn:aws:secretsmanager:ap-south-1:123456789000:secret:test2-aBcdef';
const TEST_NAME_2 = 'test/secret2';

const VALID_ARN_3 = 'arn:aws:secretsmanager:ap-south-1:123456789000:secret:test3-aBcdef';

const INVALID_ARN = 'aws:secretsmanager:us-east-1:123456789000:secret:test3-aBcdef';

jest.mock('@actions/core');
Expand Down Expand Up @@ -330,9 +332,14 @@ describe('Test secret parsing and handling', () => {
expect(extractAliasAndSecretIdFromInput(`ARN_ALIAS,${VALID_ARN_1}`)).toEqual(['ARN_ALIAS', VALID_ARN_1]);
});

test('Returns blank for alias if none is provided', () => {
expect(extractAliasAndSecretIdFromInput("test/secret")).toEqual(['', 'test/secret']);
expect(extractAliasAndSecretIdFromInput(VALID_ARN_1)).toEqual(['', VALID_ARN_1]);
test('Returns undefined for alias if none is provided', () => {
expect(extractAliasAndSecretIdFromInput("test/secret")).toEqual([undefined, 'test/secret']);
expect(extractAliasAndSecretIdFromInput(VALID_ARN_1)).toEqual([undefined, VALID_ARN_1]);
});

test('Returns empty string for alias if none is provided, but comma delimited', () => {
expect(extractAliasAndSecretIdFromInput(" , test/secret")).toEqual(['', 'test/secret']);
expect(extractAliasAndSecretIdFromInput(" , "+VALID_ARN_3)).toEqual(['', VALID_ARN_3]);
});

test('Throws an error if the provided alias cannot be used as the environment name', () => {
Expand Down
15 changes: 11 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
getSecretValue,
injectSecret,
extractAliasAndSecretIdFromInput,
SecretValueResponse
SecretValueResponse, isJSONString
} from "./utils";
import { CLEANUP_NAME } from "./constants";

Expand All @@ -29,19 +29,26 @@ export async function run(): Promise<void> {
// Get and inject secret values
for (let secretId of secretIds) {
// Optionally let user set an alias, i.e. `ENV_NAME,secret_name`
let secretAlias = '';
let secretAlias: string | undefined = undefined;
[secretAlias, secretId] = extractAliasAndSecretIdFromInput(secretId);

// Retrieves the secret name also, if the value is an ARN
const isArn = isSecretArn(secretId);

try {
const secretValueResponse : SecretValueResponse = await getSecretValue(client, secretId);
if (!secretAlias){
const secretValue = secretValueResponse.secretValue;

// Catch if blank prefix is specified but no json is parsed to avoid blank environment variable
if ((secretAlias === '') && !(parseJsonSecrets && isJSONString(secretValue))) {
secretAlias = undefined;
}

if (secretAlias === undefined) {
secretAlias = isArn ? secretValueResponse.name : secretId;
}

const injectedSecrets = injectSecret(secretAlias, secretValueResponse.secretValue, parseJsonSecrets);
const injectedSecrets = injectSecret(secretAlias, secretValue, parseJsonSecrets);
secretsToCleanup = [...secretsToCleanup, ...injectedSecrets];
} catch (err) {
// Fail action for any error
Expand Down
14 changes: 10 additions & 4 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,14 @@ export function injectSecret(secretName: string, secretValue: string, parseJsonS
for (const k in secretMap) {
const keyValue = typeof secretMap[k] === 'string' ? secretMap[k] as string : JSON.stringify(secretMap[k]);

// Append the current key to the name of the env variable
const newEnvName = `${tempEnvName || transformToValidEnvName(secretName)}_${transformToValidEnvName(k)}`;
// Append the current key to the name of the env variable and check to avoid prepending an underscore
const newEnvName = [
tempEnvName || transformToValidEnvName(secretName),
transformToValidEnvName(k)
]
.filter(elem => elem) // Uses truthy-ness of elem to determine if it remains
.join("_"); // Join the remaining elements with an underscore

secretsToCleanup = [...secretsToCleanup, ...injectSecret(secretName, keyValue, parseJsonSecrets, newEnvName)];
}
} else {
Expand Down Expand Up @@ -205,7 +211,7 @@ export function isSecretArn(secretId: string): boolean {
/*
* Separates a secret alias from the secret name/arn, if one was provided
*/
export function extractAliasAndSecretIdFromInput(input: string): [string, string] {
export function extractAliasAndSecretIdFromInput(input: string): [undefined | string, string] {
const parsedInput = input.split(',');
if (parsedInput.length > 1){
const alias = parsedInput[0].trim();
Expand All @@ -222,7 +228,7 @@ export function extractAliasAndSecretIdFromInput(input: string): [string, string
}

// No alias
return [ '', input.trim() ];
return [ undefined , input.trim() ];
}

/*
Expand Down
Loading