From 3ff74a31423e82619ecffb2040c8d93d8959f4a5 Mon Sep 17 00:00:00 2001 From: JM Faircloth Date: Mon, 3 Jul 2023 15:42:46 -0500 Subject: [PATCH] fix secrets stored in JSON format --- .github/workflows/local-test.yaml | 49 ++++++++++++++++------ .github/workflows/test.yaml | 10 +++++ integrationTests/e2e/setup.js | 40 ++++++++++++++++++ integrationTests/e2e/testdata.json | 4 ++ scripts/parse.js | 50 +++++++++++++++++++++++ src/action.test.js | 32 +++++++++++++++ src/retries.test.js | 2 +- src/secrets.js | 65 ++++++++++++++++++++++++++++-- 8 files changed, 236 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/test.yaml create mode 100644 integrationTests/e2e/testdata.json create mode 100644 scripts/parse.js diff --git a/.github/workflows/local-test.yaml b/.github/workflows/local-test.yaml index 4bb06131..64b52575 100644 --- a/.github/workflows/local-test.yaml +++ b/.github/workflows/local-test.yaml @@ -30,32 +30,57 @@ jobs: - name: NPM Build run: npm run build - - name: Setup Vault - run: node ./integrationTests/e2e/setup.js - env: - VAULT_HOST: localhost - VAULT_PORT: 8200 + # - name: Setup Vault + # run: node ./integrationTests/e2e/setup.js + # env: + # VAULT_HOST: localhost + # VAULT_PORT: 8200 - name: Import Secrets id: import-secrets # use the local changes uses: ./ # run against a specific version of vault-action - # uses: hashicorp/vault-action@v2.1.2 + # uses: hashicorp/vault-action@v2.6.0 + # uses: hashicorp/vault-action@v2.1.1 with: url: http://localhost:8200 method: token token: testtoken + # secret/data/test-json-string jsonString; + # secret/data/test-json-big jsonBig; + # secret/data/test-json-string-big jsonStringBig; secrets: | secret/data/test-json-string jsonString; + secret/data/test-json-string-multiline jsonStringMultiline; + secret/data/test-json-data jsonData; + secret/data/singleline singleline; + secret/data/multiline multiline; - name: Check Secrets run: | - touch secrets.json - echo "${{ steps.import-secrets.outputs.jsonString }}" >> secrets.json + echo "test-json-string" + echo "${{ steps.import-secrets.outputs.jsonString }}" + echo + echo "test-json-string-multiline" + echo "${{ steps.import-secrets.outputs.jsonStringMultiline }}" + echo + echo "test-json-data" + echo "${{ steps.import-secrets.outputs.jsonData }}" + echo + echo "singleline" + echo "${{ steps.import-secrets.outputs.singleline }}" + echo + echo "multiline" + echo "${{ steps.import-secrets.outputs.multiline }}" + echo - - name: Check json file format + - name: test parse run: | - echo - cat secrets.json - jq -c . < secrets.json + node ./scripts/parse.js + + - name: Check jq + run: | + echo "test-json-data" + echo "${{ steps.import-secrets.outputs.jsonData }}" | jq -c . || true + diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 00000000..95d82f63 --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,10 @@ +name: 'test' +inputs: + json: + required: true +outputs: + out: + description: 'The time we greeted you' +runs: + using: 'node16' + main: './scripts/parse.js' 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/integrationTests/e2e/testdata.json b/integrationTests/e2e/testdata.json new file mode 100644 index 00000000..b929e39f --- /dev/null +++ b/integrationTests/e2e/testdata.json @@ -0,0 +1,4 @@ +{ + "x": 1, + "y": "q\nux" +} diff --git a/scripts/parse.js b/scripts/parse.js new file mode 100644 index 00000000..35a6bf57 --- /dev/null +++ b/scripts/parse.js @@ -0,0 +1,50 @@ +// const core = require('@actions/core'); + +try { + let inputs = [ + process.env.JSONSTRING, + process.env.JSONSTRINGMULTILINE, + process.env.JSONDATA, + process.env.SINGLELINE, + process.env.MULTILINE, + ]; + + let names = [ + "test-json-string", + "test-json-string-multiline", + "test-json-data", + "singleline", + "multiline", + ]; + + let i = 0; + inputs.forEach(input => { + console.log(`processing: ${names[i]}`) + i++; + input = (input || '').trim(); + if (!input) { + throw new Error(`Missing service account key JSON (got empty value)`); + } + + // If the string doesn't start with a JSON object character, it is probably + // base64-encoded. + if (!input.startsWith('{')) { + let str = input.replace(/-/g, '+').replace(/_/g, '/'); + while (str.length % 4) str += '='; + input = Buffer.from(str, 'base64').toString('utf8'); + } + + try { + const creds = JSON.parse(input); + console.log('success!') + return creds; + } catch (err) { + console.log('error parsing') + console.log(err) + } + }) + +} catch (error) { + console.log(error) +} + diff --git a/src/action.test.js b/src/action.test.js index 49c33cd3..ca6706c4 100644 --- a/src/action.test.js +++ b/src/action.test.js @@ -220,6 +220,38 @@ describe('exportSecrets', () => { expect(core.setOutput).toBeCalledWith('key', '1'); }); + it('json 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 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({ 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..ea976d98 100644 --- a/src/secrets.js +++ b/src/secrets.js @@ -72,7 +72,21 @@ async function getSecrets(secretRequests, client) { */ async function selectData(data, selector) { const ata = jsonata(selector); - let result = JSON.stringify(await ata.evaluate(data)); + let d = await ata.evaluate(data); + + // If we have a Javascript Object, then this data was stored in Vault as + // pure JSON (not a JSON string) + const storedAsJSONData = isObject(d); + + if (isJSONString(d)) { + // If we already have a JSON string 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 pure 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 +95,57 @@ 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 a JSON string + result = result.substring(1, result.length - 1); + if (!isJSONString(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 (isJSONString(result)) { + if (storedAsJSONData) { + // Support secrets stored in Vault as pure JSON. + // 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); + } else { + // Support secrets stored in Vault as JSON Strings + result = JSON.stringify(result); + result = JSON.parse(result); + } } return result; } +/** + * isOjbect returns true if target is a Javascript object + * @param {Type} target + */ +function isObject(target) { + return typeof target === 'object' && target !== null; +} + +/** + * isJSONString returns true if target parses as a valid JSON string + * @param {Type} target + */ +function isJSONString(target) { + if (typeof target !== "string"){ + return false; + } + + try { + let o = JSON.parse(target); + } catch (e) { + return false; + } + + return true; +} + module.exports = { getSecrets, selectData -} \ No newline at end of file +}