From b138504969048cdca4eddd95ba0b9ea051207ddf Mon Sep 17 00:00:00 2001 From: John-Michael Faircloth Date: Thu, 6 Jul 2023 10:51:26 -0500 Subject: [PATCH] fix secrets stored in JSON format (#473) * fix secrets stored in JSON format * add more tests * fix lint and pass token to build * add test cases * add debug * fix ordering of build steps * fix test string format * update test check * fix test string format * final cleanup * remove comment * remove unused var assignment * simplify more * simplify code and add more comments --- .github/workflows/actionlint.yaml | 5 +- .github/workflows/build.yml | 11 +++++ integrationTests/e2e/e2e.test.js | 6 +++ integrationTests/e2e/setup.js | 40 ++++++++++++++++ src/action.test.js | 77 ++++++++++++++++++++++++++++++- src/retries.test.js | 2 +- src/secrets.js | 18 ++++++-- 7 files changed, 153 insertions(+), 6 deletions(-) diff --git a/.github/workflows/actionlint.yaml b/.github/workflows/actionlint.yaml index ee79a646..28b25c4e 100644 --- a/.github/workflows/actionlint.yaml +++ b/.github/workflows/actionlint.yaml @@ -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" diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 778f18c9..d9eb3ad1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -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 }} + e2e-tls: runs-on: ubuntu-latest diff --git a/integrationTests/e2e/e2e.test.js b/integrationTests/e2e/e2e.test.js index 6495d14e..bb9e2056 100644 --- a/integrationTests/e2e/e2e.test.js +++ b/integrationTests/e2e/e2e.test.js @@ -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); }); }); diff --git a/integrationTests/e2e/setup.js b/integrationTests/e2e/setup.js index 96f2295f..33daf379 100644 --- a/integrationTests/e2e/setup.js +++ b/integrationTests/e2e/setup.js @@ -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 @@ -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: { diff --git a/src/action.test.js b/src/action.test.js index 49c33cd3..498ab515 100644 --- a/src/action.test.js +++ b/src/action.test.js @@ -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({ @@ -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 diff --git a/src/retries.test.js b/src/retries.test.js index 132edd52..285a7f25 100644 --- a/src/retries.test.js +++ b/src/retries.test.js @@ -66,4 +66,4 @@ describe('exportSecrets retries', () => { done(); }); }); -}); \ No newline at end of file +}); diff --git a/src/secrets.js b/src/secrets.js index 45b26e0a..34d28679 100644 --- a/src/secrets.js +++ b/src/secrets.js @@ -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)); @@ -81,7 +82,18 @@ 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; } @@ -89,4 +101,4 @@ async function selectData(data, selector) { module.exports = { getSecrets, selectData -} \ No newline at end of file +}