Skip to content
This repository has been archived by the owner on Jul 12, 2019. It is now read-only.

Be more defensive when creating a buffer #155

Merged
merged 5 commits into from
Jun 4, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
wip testing
  • Loading branch information
xavdid committed May 31, 2019
commit b7b3df830c0de60e6b036e628fc0dcfe9e90e4af
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
"preversion": "git pull && npm test",
"version": "node bin/bump-dependencies.js && npm install && git add package.json package-lock.json",
"postversion": "git push && git push --tags",
"test": "mocha -t 10000 --recursive test",
"test": "mocha -t 10000 --inspect-brk --recursive test",
"test:w": "mocha -t 10000 --recursive test --watch",
"posttest": "npm run lint",
"/posttest": "npm run lint",
"plain-test": "mocha -t 5000 --recursive test",
"integration-test": "mocha -t 10000 integration-test",
"local-integration-test": "mocha -t 10000 integration-test --local",
Expand Down
23 changes: 15 additions & 8 deletions src/tools/cleaner.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,34 @@ const recurseCleanFuncs = (obj, path) => {

// Recurse a nested object replace all instances of keys->vals in the bank.
const recurseReplaceBank = (obj, bank = {}) => {
console.log('build blank');
const replacer = out => {
if (typeof out === 'number') {
out = String(out);
if (out === 314159265) {
console.log('replacing');
}
if (typeof out !== 'string') {
if (!['string', 'number'].includes(typeof out)) {
return out;
}

if (out === 314159265) {
debugger; // eslint-disable-line no-debugger
console.log('stayed');
}
let str = String(out);

Object.keys(bank).forEach(key => {
// Escape characters (ex. {{foo}} => \\{\\{foo\\}\\} )
const escapedKey = key.replace(/[-[\]/{}()\\*+?.^$|]/g, '\\$&');
const matchesKey = new RegExp(escapedKey, 'g');

if (!matchesKey.test(out)) {
if (!matchesKey.test(str)) {
return;
}

const matchesCurlies = /({{.*?}})/;
const valueParts = out.split(matchesCurlies).filter(Boolean);
const valueParts = str.split(matchesCurlies).filter(Boolean);
const replacementValue = bank[key];
const isPartOfString = !matchesCurlies.test(out) || valueParts.length > 1;
const isPartOfString = !matchesCurlies.test(str) || valueParts.length > 1;
const shouldThrowTypeError =
isPartOfString &&
(Array.isArray(replacementValue) || _.isPlainObject(replacementValue));
Expand All @@ -76,12 +83,12 @@ const recurseReplaceBank = (obj, bank = {}) => {
);
}

out = isPartOfString
str = isPartOfString
? valueParts.join('').replace(matchesKey, replacementValue)
: replacementValue;
});

return out;
return str;
};
return recurseReplace(obj, replacer);
};
Expand Down
21 changes: 10 additions & 11 deletions src/tools/create-logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ const makeSensitiveBank = (event, data) => {
return false;
};

dataTools.recurseExtract(data, matcher).map(value => {
dataTools.recurseExtract(data, matcher).forEach(value => {
sensitiveValues.push(value);
});

Expand All @@ -108,19 +108,18 @@ const makeSensitiveBank = (event, data) => {
// keeps short values from spamming censor strings in logs, < 6 chars is not a proper secret
// see https://github.com/zapier/zapier-platform-core/issues/4#issuecomment-277855071
if (val && String(val).length > 5) {
val = String(val);
const censored = hashing.snipify(val);
bank[val] = censored;
bank[encodeURIComponent(val)] = censored;
try {
bank[Buffer.from(val).toString('base64')] = censored;
} catch (e) {
if (e.name !== 'TypeError') {
throw e;
}
// ignore; Buffer is semi-selective about what types it takes
// not sure why we get non-strings here, but we do occasionally.
}
// try {
bank[Buffer.from(String(val)).toString('base64')] = censored;
// } catch (e) {
// if (e.name !== 'TypeError') {
// throw e;
// }
// // ignore; Buffer is semi-selective about what types it takes
// // not sure why we get non-strings here, but we do occasionally.
// }
}
return bank;
},
Expand Down
11 changes: 6 additions & 5 deletions src/tools/hashing.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ const hashify = (algo, s, encoding, input_encoding) => {

// Clean up sensitive values in a hashed manner so they don't get logged.
const snipify = s => {
if (typeof s !== 'string') {
if (!['string', 'number'].includes(typeof s)) {
return null;
}
const length = s.length;
s += process.env.SECRET_SALT || 'doesntmatterreally';
const result = hashify('sha256', s);
return `:censored:${length}:${result.substr(0, 10)}:`;
const str = String(s);
const length = str.length;
const salted = str + (process.env.SECRET_SALT || 'doesntmatterreally');
const hashed = hashify('sha256', salted);
return `:censored:${length}:${hashed.substr(0, 10)}:`;
};

const md5 = s =>
Expand Down
37 changes: 37 additions & 0 deletions test/create-request-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,43 @@ describe('request client', () => {
});
});

it.only('should keep valid data types that are hard-coded', () => {
// This may seem like an usual case to be in, and for most apps it is.
// However, converted apps that rely on legacy-scripting-runner can have
// request bodies that are pure data, no {{}}, so we need to be sure to preserve those to
const event = {
bundle: {
inputData: {
number: 123,
bool: true,
float: 123.456,
arr: [1, 2, 3],
nested: { very: 'cool' }
}
}
};
const bodyInput = createInput({}, event, testLogger);
const request = createAppRequestClient(bodyInput);
return request({
url: 'https://httpbin.org/post',
method: 'POST',
body: {
number: 123,
bool: true,
float: 123.456,
arr: [1, 2, 3]
}
}).then(response => {
const { json } = response.json;

should(json.empty).eql(undefined);
json.number.should.eql(123);
json.bool.should.eql(true);
json.float.should.eql(123.456);
json.arr.should.eql([1, 2, 3]);
});
});

it('should remove keys from body for empty values if configured to', () => {
const event = {
bundle: {
Expand Down
2 changes: 1 addition & 1 deletion test/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ describe('logger', () => {
});
});

it('should replace sensitive data that is not a string', () => {
it.only('should replace sensitive data that is not a string', () => {
const bundle = {
authData: {
numerical_token: 314159265
Expand Down