Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "fix secrets stored in json format (#466)" #471

Merged
merged 2 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}

}