From 00e0fcabda5057bbc58fc2b032b5d7126d29bf5b Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Fri, 16 Jun 2023 19:47:44 -0500 Subject: [PATCH] fix secrets in json format --- .github/workflows/build.yml | 17 +++++++++++ .github/workflows/local-test.yaml | 47 +++++++++++++++++++++++++------ Makefile | 3 ++ integrationTests/e2e/e2e.test.js | 5 ++++ integrationTests/e2e/setup.js | 13 +++++++++ src/action.test.js | 44 +++++++++++++++++++++++++++-- src/retries.test.js | 2 +- src/secrets.js | 47 +++++++++++++++++++++++++++---- 8 files changed, 161 insertions(+), 17 deletions(-) create mode 100644 Makefile diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 778f18c..ad006fd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -153,6 +153,23 @@ jobs: my-secret/test altSecret | NAMED_ALTSECRET ; my-secret/nested/test otherAltSecret ; + - name: Test Vault Action (JSON string format) + id: import-secrets + uses: ./ + with: + url: http://localhost:8200 + token: testtoken + secrets: | + secret/data/test-json-string jsonString ; + secret/data/test-json-string jsonString | JSON_STRING ; + + - name: Test Vault Action (verify JSON string format) + run: | + echo "${{ steps.import-secrets.outputs.jsonString }}" > secrets.json + cat secrets.json + # we should be able to parse the output as JSON + cat secrets.json | jq -c + - name: Test Vault Action (cubbyhole) uses: ./ with: diff --git a/.github/workflows/local-test.yaml b/.github/workflows/local-test.yaml index fb3bce3..4d857a2 100644 --- a/.github/workflows/local-test.yaml +++ b/.github/workflows/local-test.yaml @@ -18,11 +18,42 @@ jobs: name: local-test runs-on: ubuntu-latest steps: - - name: Import Secrets - uses: hashicorp/vault-action@YOUR_BRANCH_NAME - with: - url: http://localhost:8200 - method: token - token: testtoken - secrets: | - secret/data/test secret | SAMPLE_SECRET; + - uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2 + + - uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v3.6.0 + with: + node-version: '16.14.0' + + - name: NPM Install + run: npm ci + + - name: NPM Build + run: npm run build + + - name: Setup Vault + run: node ./integrationTests/e2e/setup.js + env: + VAULT_HOST: localhost + VAULT_PORT: 8200 + + - name: Import Secrets + id: import-secrets + uses: ./ + # uses: hashicorp/vault-action@v2.1.2 + with: + url: http://localhost:8200 + method: token + token: testtoken + secrets: | + secret/data/test-json-string jsonString; + + - name: Check Secrets + run: | + touch secrets.json + echo "${{ steps.import-secrets.outputs.jsonString }}" >> secrets.json + + - name: Check json file + run: | + echo + cat secrets.json + cat secrets.json | jq -c diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..b27a661 --- /dev/null +++ b/Makefile @@ -0,0 +1,3 @@ +.PHONY: local-test +local-test: + docker compose down && docker-compose up -d vault && act workflow_dispatch -j local-test diff --git a/integrationTests/e2e/e2e.test.js b/integrationTests/e2e/e2e.test.js index 6495d14..3e43490 100644 --- a/integrationTests/e2e/e2e.test.js +++ b/integrationTests/e2e/e2e.test.js @@ -10,5 +10,10 @@ 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"); + + const jsonString = '{"x":1,"y":"qux"}'; + let jsonResult = JSON.stringify(jsonString); + jsonResult = jsonResult.substring(1, jsonResult.length - 1); + expect(process.env.JSON_STRING).toBe(jsonResult); }); }); diff --git a/integrationTests/e2e/setup.js b/integrationTests/e2e/setup.js index 96f2295..6ef2cf8 100644 --- a/integrationTests/e2e/setup.js +++ b/integrationTests/e2e/setup.js @@ -3,6 +3,7 @@ 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"; + (async () => { try { // Verify Connection @@ -36,6 +37,18 @@ 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: { + jsonString: '{"x":1,"y":"qux"}', + }, + }, + }); + 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 49c33cd..f630400 100644 --- a/src/action.test.js +++ b/src/action.test.js @@ -220,6 +220,22 @@ describe('exportSecrets', () => { expect(core.setOutput).toBeCalledWith('key', '1'); }); + it('json secret retrieval', async () => { + const jsonString = '{"x":1,"y":2}'; + let result = JSON.stringify(jsonString); + result = result.substring(1, result.length - 1); + + mockInput('test key'); + mockVaultData({ + key: jsonString, + }); + + await exportSecrets(); + + expect(core.exportVariable).toBeCalledWith('KEY', result); + expect(core.setOutput).toBeCalledWith('key', result); + }); + it('intl secret retrieval', async () => { mockInput('测试 测试'); mockVaultData({ @@ -334,7 +350,31 @@ 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("EXAMPLE"); // called for 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 @@ -348,7 +388,7 @@ with blank lines await exportSecrets(); - expect(core.setSecret).toBeCalledTimes(3); // 1 for each non-empty line. + expect(core.setSecret).toBeCalledTimes(3); // 1 for each non-empty line + VAULT_TOKEN expect(core.setSecret).toBeCalledWith('a multi-line string'); expect(core.setSecret).toBeCalledWith('with blank lines'); diff --git a/src/retries.test.js b/src/retries.test.js index 132edd5..285a7f2 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 45b26e0..229d787 100644 --- a/src/secrets.js +++ b/src/secrets.js @@ -1,6 +1,5 @@ const jsonata = require("jsonata"); - /** * @typedef {Object} SecretRequest * @property {string} path @@ -67,12 +66,20 @@ 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)); + let d = await ata.evaluate(data); + if (isJSON(d)) { + // If we already have JSON we will not "stringify" it yet so that we + // don't end up calling JSON.parse. This would break the secrets that + // are stored as json. See: https://github.com/hashicorp/vault-action/issues/194 + result = d; + } else { + result = JSON.stringify(d); + } // 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,12 +88,40 @@ async function selectData(data, selector) { } if (result.startsWith(`"`)) { - result = JSON.parse(result); + // we need to strip the beginning and ending quotes otherwise it will + // always successfully parse as JSON + result = result.substring(1, result.length - 1); + if (!isJSON(result)) { + // add the quotes back so we can parse it into a Javascript object + // to allow support for multi-line secrets. See https://github.com/hashicorp/vault-action/issues/160 + result = `"${result}"` + result = JSON.parse(result); + } + } else if (isJSON(result)) { + // This is required to support secrets in JSON format. + // See https://github.com/hashicorp/vault-action/issues/194 and https://github.com/hashicorp/vault-action/pull/173 + result = JSON.stringify(result); + result = result.substring(1, result.length - 1); } return result; } +function isJSON(str) { + if (typeof str !== "string"){ + return false; + } + + try { + JSON.parse(str); + } catch (e) { + return false; + } + + return isNaN(str); +} + module.exports = { getSecrets, selectData -} \ No newline at end of file +} +