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

fix secrets stored in JSON format #473

Merged
merged 14 commits into from
Jul 6, 2023
5 changes: 4 additions & 1 deletion .github/workflows/actionlint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ jobs:
# in our e2e tests.
# This error occurs because vault-action's outputs are dynamic but
# actionlint expects action.yml to define them.
args: '-ignore "property \"othersecret\" is not defined in object type"'
args: >
-ignore "property \"othersecret\" is not defined in object type"
-ignore "property \"jsonstring\" is not defined in object type"
-ignore "property \"jsonstringmultiline\" is not defined in object type"
11 changes: 11 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,22 @@ jobs:
secrets: |
secret/data/subsequent-test secret | SUBSEQUENT_TEST_SECRET;

- name: Test JSON Secrets
uses: ./
with:
url: http://localhost:8200
token: testtoken
secrets: |
secret/data/test-json-data jsonData;
secret/data/test-json-string jsonString;
secret/data/test-json-string-multiline jsonStringMultiline;

- name: Verify Vault Action Outputs
run: npm run test:integration:e2e
env:
OTHER_SECRET_OUTPUT: ${{ steps.kv-secrets.outputs.otherSecret }}


Copy link
Member

@robmonte robmonte Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an accidental second newline added?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops submitted this too late 😓

e2e-tls:
runs-on: ubuntu-latest

Expand Down
6 changes: 6 additions & 0 deletions integrationTests/e2e/e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,11 @@ describe('e2e', () => {
expect(process.env.FOO).toBe("bar");
expect(process.env.NAMED_CUBBYSECRET).toBe("zap");
expect(process.env.SUBSEQUENT_TEST_SECRET).toBe("SUBSEQUENT_TEST_SECRET");
expect(process.env.JSONSTRING).toBe('{"x":1,"y":"qux"}');
expect(process.env.JSONSTRINGMULTILINE).toBe('{"x": 1, "y": "q\\nux"}');

let result = JSON.stringify('{"x":1,"y":"qux"}');
result = result.substring(1, result.length - 1);
expect(process.env.JSONDATA).toBe(result);
});
});
40 changes: 40 additions & 0 deletions integrationTests/e2e/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ const got = require('got');
const vaultUrl = `${process.env.VAULT_HOST}:${process.env.VAULT_PORT}`;
const vaultToken = `${process.env.VAULT_TOKEN}` === undefined ? `${process.env.VAULT_TOKEN}` : "testtoken";

const jsonStringMultiline = '{"x": 1, "y": "q\\nux"}';

(async () => {
try {
// Verify Connection
Expand Down Expand Up @@ -36,6 +38,44 @@ const vaultToken = `${process.env.VAULT_TOKEN}` === undefined ? `${process.env.V
}
});

await got(`http://${vaultUrl}/v1/secret/data/test-json-string`, {
method: 'POST',
headers: {
'X-Vault-Token': vaultToken,
},
json: {
data: {
// this is stored in Vault as a string
jsonString: '{"x":1,"y":"qux"}',
},
},
});

await got(`http://${vaultUrl}/v1/secret/data/test-json-data`, {
method: 'POST',
headers: {
'X-Vault-Token': vaultToken,
},
json: {
data: {
// this is stored in Vault as a map
jsonData: {"x":1,"y":"qux"},
},
},
});

await got(`http://${vaultUrl}/v1/secret/data/test-json-string-multiline`, {
method: 'POST',
headers: {
'X-Vault-Token': vaultToken,
},
json: {
data: {
jsonStringMultiline,
},
},
});

await got(`http://${vaultUrl}/v1/sys/mounts/my-secret`, {
method: 'POST',
headers: {
Expand Down
77 changes: 76 additions & 1 deletion src/action.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,58 @@ describe('exportSecrets', () => {
expect(core.setOutput).toBeCalledWith('key', '1');
});

it('JSON data secret retrieval', async () => {
const jsonData = {"x":1,"y":2};

// for secrets stored in Vault as pure JSON, we call stringify twice
// and remove the surrounding quotes
let result = JSON.stringify(JSON.stringify(jsonData));
result = result.substring(1, result.length - 1);

mockInput('test key');
mockVaultData({
key: jsonData,
});

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('KEY', result);
expect(core.setOutput).toBeCalledWith('key', result);
});

it('JSON string secret retrieval', async () => {
const jsonString = '{"x":1,"y":2}';

mockInput('test key');
mockVaultData({
key: jsonString,
});

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('KEY', jsonString);
expect(core.setOutput).toBeCalledWith('key', jsonString);
});

it('multi-line JSON string secret retrieval', async () => {
const jsonString = `
{
"x":1,
"y":"bar"
}
`;

mockInput('test key');
mockVaultData({
key: jsonString,
});

await exportSecrets();

expect(core.exportVariable).toBeCalledWith('KEY', jsonString);
expect(core.setOutput).toBeCalledWith('key', jsonString);
});

it('intl secret retrieval', async () => {
mockInput('测试 测试');
mockVaultData({
Expand Down Expand Up @@ -334,7 +386,30 @@ describe('exportSecrets', () => {
expect(core.setOutput).toBeCalledWith('key', 'secret');
})

it('multi-line secret gets masked for each line', async () => {
it('multi-line secret', async () => {
const multiLineString = `ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSU
GPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3
Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XA
NrRFi9wrf+M7Q==`;

mockInput('test key');
mockVaultData({
key: multiLineString
});
mockExportToken("false")

await exportSecrets();

expect(core.setSecret).toBeCalledTimes(5); // 1 for each non-empty line + VAULT_TOKEN

expect(core.setSecret).toBeCalledWith("ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAklOUpkDHrfHY17SbrmTIpNLTGK9Tjom/BWDSU");
expect(core.setSecret).toBeCalledWith("GPl+nafzlHDTYW7hdI4yZ5ew18JH4JW9jbhUFrviQzM7xlELEVf4h9lFX5QVkbPppSwg0cda3");
expect(core.setSecret).toBeCalledWith("Pbv7kOdJ/MTyBlWXFCR+HAo3FXRitBqxiX1nKhXpHAZsMciLq8V6RjsNAQwdsdMFvSlVK/7XA");
expect(core.setSecret).toBeCalledWith("NrRFi9wrf+M7Q==");
expect(core.setOutput).toBeCalledWith('key', multiLineString);
})

it('multi-line secret gets masked for each non-empty line', async () => {
const multiLineString = `a multi-line string

with blank lines
Expand Down
2 changes: 1 addition & 1 deletion src/retries.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,4 @@ describe('exportSecrets retries', () => {
done();
});
});
});
});
18 changes: 15 additions & 3 deletions src/secrets.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,13 @@ async function getSecrets(secretRequests, client) {

/**
* Uses a Jsonata selector retrieve a bit of data from the result
* @param {object} data
* @param {string} selector
* @param {object} data
* @param {string} selector
*/
async function selectData(data, selector) {
const ata = jsonata(selector);
let result = JSON.stringify(await ata.evaluate(data));

// Compat for custom engines
if (!result && ((ata.ast().type === "path" && ata.ast()['steps'].length === 1) || ata.ast().type === "string") && selector !== 'data' && 'data' in data) {
result = JSON.stringify(await jsonata(`data.${selector}`).evaluate(data));
Expand All @@ -81,12 +82,23 @@ async function selectData(data, selector) {
}

if (result.startsWith(`"`)) {
// Support multi-line secrets like JSON strings and ssh keys, see https://github.com/hashicorp/vault-action/pull/173
// Deserialize the value so that newlines and special characters are
// not escaped in our return value.
result = JSON.parse(result);
} else {
// Support secrets stored in Vault as pure JSON, see https://github.com/hashicorp/vault-action/issues/194
// Serialize the value so that any special characters in the data are
// properly escaped.
result = JSON.stringify(result);
// strip the surrounding quotes added by stringify because the data did
// not have them in the first place
result = result.substring(1, result.length - 1);
}
return result;
}

module.exports = {
getSecrets,
selectData
}
}