fix secrets stored in JSON format

hashicorp · Jul 3, 2023 · 3ff74a3 · 3ff74a3
1 parent e926631
commit 3ff74a3
Show file tree

Hide file tree

Showing 8 changed files with 236 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,4 @@
+{
+  "x": 1,
+  "y": "q\nux"
+}
diff --git a/scripts/parse.js 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
@@ -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
@@ -66,4 +66,4 @@ describe('exportSecrets retries', () => {
             done();
         });
     });
-});
+});
diff --git 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
-}
+}