Skip to content

Commit

Permalink
Revert "fix secrets stored in json format (#466)" (#471)
Browse files Browse the repository at this point in the history
* Revert "fix secrets stored in json format (#466)"

This reverts commit b9f4d16.

* fix build: use new Verified Publisher image hashicorp/vault
  • Loading branch information
fairclothjm committed Jul 3, 2023
1 parent 357cb9c commit 5213b69
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 144 deletions.
4 changes: 1 addition & 3 deletions .github/workflows/actionlint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,4 @@ 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"
-ignore "property \"jsonstring\" is not defined in object type"
args: '-ignore "property \"othersecret\" is not defined in object type"'
21 changes: 0 additions & 21 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
version: "3.0"
services:
vault:
image: vault:latest
image: hashicorp/vault:latest
environment:
VAULT_DEV_ROOT_TOKEN_ID: testtoken
ports:
Expand All @@ -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
Expand Down
5 changes: 0 additions & 5 deletions integrationTests/e2e/e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,5 @@ 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);
});
});
25 changes: 0 additions & 25 deletions integrationTests/e2e/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ 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
Expand Down Expand Up @@ -37,30 +36,6 @@ 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/secret/data/test-json-data`, {
method: 'POST',
headers: {
'X-Vault-Token': vaultToken,
},
json: {
data: {
jsonData: {"x":1,"y":"qux"},
},
},
});

await got(`http://${vaultUrl}/v1/sys/mounts/my-secret`, {
method: 'POST',
headers: {
Expand Down
44 changes: 2 additions & 42 deletions src/action.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,22 +220,6 @@ 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({
Expand Down Expand Up @@ -350,31 +334,7 @@ describe('exportSecrets', () => {
expect(core.setOutput).toBeCalledWith('key', 'secret');
})

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 () => {
it('multi-line secret gets masked for each line', async () => {
const multiLineString = `a multi-line string
with blank lines
Expand All @@ -388,7 +348,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');
Expand Down
2 changes: 1 addition & 1 deletion src/retries.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,4 @@ describe('exportSecrets retries', () => {
done();
});
});
});
});
51 changes: 6 additions & 45 deletions src/secrets.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const jsonata = require("jsonata");


/**
* @typedef {Object} SecretRequest
* @property {string} path
Expand Down Expand Up @@ -66,20 +67,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));
Expand All @@ -88,44 +81,12 @@ 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
}

}

0 comments on commit 5213b69

Please sign in to comment.