simplify more

* Update to v2.7.1

* update changelog

.github/workflows/actionlint.yaml

@@ -19,3 +19,4 @@ jobs:
           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"

.github/workflows/build.yml

@@ -153,27 +153,6 @@ jobs:
           my-secret/test altSecret | NAMED_ALTSECRET ;
           my-secret/nested/test otherAltSecret ;
 
-    # The ordering of these two Test Vault Action JSON String Format steps matters
-    - name: Test Vault Action JSON String Format (part 1/2)
-      id: import-secrets
-      uses: ./
-      with:
-        url: http://localhost:8200
-        token: testtoken
-        secrets: |
-          secret/data/test-json-string jsonString | JSON_STRING ;
-          secret/data/test-json-data jsonData | JSON_DATA ;
-
-    - name: Test Vault Action JSON String Format (part 2/2)
-      run: |
-        echo "${{ steps.import-secrets.outputs.jsonString }}" > string.json
-        echo "${{ steps.import-secrets.outputs.jsonData }}" > data.json
-        cat string.json
-        cat data.json
-        # we should be able to parse the output as JSON
-        jq -c . < string.json
-        jq -c . < data.json
-
     - name: Test Vault Action (cubbyhole)
       uses: ./
       with:
@@ -201,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
 

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 * Add changes here
 
+## 2.7.1 (July 3, 2023)
+
+Bugs:
+
+* Revert [GH-466](https://github.com/hashicorp/vault-action/pull/466) which caused a regression in secrets stored as JSON strings [GH-471](https://github.com/hashicorp/vault-action/pull/471)
+
 ## 2.7.0 (June 21, 2023)
 
 Bugs:

dist/index.js

@@ -18937,6 +18937,7 @@ module.exports = {
 
 const jsonata = __nccwpck_require__(4245);
 
+
 /**
  * @typedef {Object} SecretRequest
  * @property {string} path
@@ -19003,20 +19004,12 @@ 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 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);
-    }
+    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));
@@ -19025,49 +19018,16 @@ async function selectData(data, selector) {
     }
 
     if (result.startsWith(`"`)) {
-        // 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);
+        result = JSON.parse(result);
     }
     return result;
 }
 
-/**
- * isJSON returns true if str parses as a valid JSON string
- * @param {string} str
- */
-function isJSON(str) {
-    if (typeof str !== "string"){
-        return false;
-    }
-
-    try {
-        JSON.parse(str);
-    } catch (e) {
-        return false;
-    }
-
-    return true;
-}
-
 module.exports = {
     getSecrets,
     selectData
 }
 
-
-
 /***/ }),
 
 /***/ 9491:

docker-compose.yml

@@ -2,7 +2,7 @@
 version: "3.0"
 services:
   vault:
-    image: vault:latest
+    image: hashicorp/vault:latest
     environment:
       VAULT_DEV_ROOT_TOKEN_ID: testtoken
     ports:
@@ -17,7 +17,7 @@ services:
       - 8200:8200
     privileged: true
   vault-tls:
-    image: vault:latest
+    image: hashicorp/vault:latest
     hostname: vault-tls
     environment:
       VAULT_CAPATH: /etc/vault/ca.crt

integrationTests/e2e/e2e.test.js

@@ -10,10 +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"}');
 
-        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);
+        let result = JSON.stringify('{"x":1,"y":"qux"}');
+        result = result.substring(1, result.length - 1);
+        expect(process.env.JSONDATA).toBe(result);
     });
 });

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";
 
+const jsonStringMultiline = '{"x": 1, "y": "q\\nux"}';
 
 (async () => {
     try {
@@ -44,6 +45,7 @@ const vaultToken = `${process.env.VAULT_TOKEN}` === undefined ? `${process.env.V
             },
             json: {
                 data: {
+                    // this is stored in Vault as a string
                     jsonString: '{"x":1,"y":"qux"}',
                 },
             },
@@ -56,11 +58,24 @@ const vaultToken = `${process.env.VAULT_TOKEN}` === undefined ? `${process.env.V
             },
             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: {

src/action.test.js

@@ -220,11 +220,28 @@ describe('exportSecrets', () => {
         expect(core.setOutput).toBeCalledWith('key', '1');
     });
 
-    it('json secret retrieval', async () => {
-        const jsonString = '{"x":1,"y":2}';
-        let result = JSON.stringify(jsonString);
+    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 added 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,
@@ -232,8 +249,27 @@ describe('exportSecrets', () => {
 
         await exportSecrets();
 
-        expect(core.exportVariable).toBeCalledWith('KEY', result);
-        expect(core.setOutput).toBeCalledWith('key', result);
+        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 () => {
@@ -366,7 +402,6 @@ NrRFi9wrf+M7Q==`;
 
         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");
@@ -388,7 +423,7 @@ with blank lines
 
         await exportSecrets();
 
-        expect(core.setSecret).toBeCalledTimes(3); // 1 for each non-empty line + VAULT_TOKEN
+        expect(core.setSecret).toBeCalledTimes(3); // 1 for each non-empty line.
 
         expect(core.setSecret).toBeCalledWith('a multi-line string');
         expect(core.setSecret).toBeCalledWith('with blank lines');

src/secrets.js

@@ -1,5 +1,6 @@
 const jsonata = require("jsonata");
 
+
 /**
  * @typedef {Object} SecretRequest
  * @property {string} path
@@ -72,14 +73,13 @@ async function getSecrets(secretRequests, client) {
 async function selectData(data, selector) {
     const ata = jsonata(selector);
     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);
-    }
+    console.log(selector);
+
+    // If we have a Javascript Object, then this data was stored in Vault as
+    // pure JSON (not a JSON string). We will capture that before we stringify it.
+    const storedAsJSONData = isObject(d);
+    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));
@@ -89,16 +89,18 @@ async function selectData(data, selector) {
 
     if (result.startsWith(`"`)) {
         // 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.
+        // 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}"`
+        console.log(" =>>> PARSING")
+        result = JSON.parse(result);
+        // }
+    } else {
+        console.log('does not start with quote')
+        // 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);
@@ -107,16 +109,24 @@ async function selectData(data, selector) {
 }
 
 /**
- * isJSON returns true if str parses as a valid JSON string
- * @param {string} str
+ * isOjbect returns true if target is a Javascript object
+ * @param {Type} target
  */
-function isJSON(str) {
-    if (typeof str !== "string"){
+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 {
-        JSON.parse(str);
+        JSON.parse(target);
     } catch (e) {
         return false;
     }
@@ -128,4 +138,3 @@ module.exports = {
     getSecrets,
     selectData
 }
-