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
hashicorp · Jul 6, 2023 · b138504 · b138504
1 parent e926631
commit b138504
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -66,4 +66,4 @@ describe('exportSecrets retries', () => {
             done();
         });
     });
-});
+});
diff --git 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,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
-}
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -66,4 +66,4 @@ describe('exportSecrets retries', () => { @@
                 done();
             });
         });
-    });
+    });