Merge pull request #77 from zapier/better-censoring

Better log censoring
zapier · Apr 19, 2018 · 2169b5e · 2169b5e
2 parents 9b0cbda + 51d9895
commit 2169b5e
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 14 deletions.
diff --git a/src/constants.js b/src/constants.js
@@ -20,6 +20,17 @@ const REQUEST_OBJECT_SHORTHAND_OPTIONS = { replace: true };
 const DEFAULT_LOGGING_HTTP_ENDPOINT = 'https://httplogger.zapier.com/input';
 const DEFAULT_LOGGING_HTTP_API_KEY = 'R24hzu86v3jntwtX2DtYECeWAB'; // It's ok, this isn't PROD
 
+const SENSITIVE_KEYS = [
+  'api_key',
+  'apikey',
+  'auth',
+  'passwd',
+  'password',
+  'secret',
+  'signature',
+  'token'
+];
+
 module.exports = {
   IS_TESTING,
   KILL_MIN_LIMIT,
@@ -29,5 +40,6 @@ module.exports = {
   RENDER_ONLY_METHODS,
   REQUEST_OBJECT_SHORTHAND_OPTIONS,
   DEFAULT_LOGGING_HTTP_ENDPOINT,
-  DEFAULT_LOGGING_HTTP_API_KEY
+  DEFAULT_LOGGING_HTTP_API_KEY,
+  SENSITIVE_KEYS
 };
diff --git a/src/tools/create-logger.js b/src/tools/create-logger.js
@@ -60,28 +60,43 @@ const toStdout = (event, msg, data) => {
   }
 };
 
-const makeSensitiveBank = event => {
+const makeSensitiveBank = (event, data) => {
   const bundle = event.bundle || {};
-  const sensitiveData = _.extend(
-    {},
-    bundle.authData || {},
-    _.extend({}, process.env || {})
+  const sensitiveValues = _.values(
+    _.extend({}, bundle.authData || {}, _.extend({}, process.env || {}))
   );
-  return _.values(sensitiveData).reduce((bank, val) => {
-    // 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) {
-      bank[val] = hashing.snipify(val);
+
+  const matcher = (key, value) => {
+    if (typeof value === 'string') {
+      const lowerKey = key.toLowerCase();
+      return _.some(constants.SENSITIVE_KEYS, k => lowerKey.indexOf(k) >= 0);
     }
-    return bank;
-  }, {});
+    return false;
+  };
+
+  dataTools.recurseExtract(data, matcher).map(value => {
+    sensitiveValues.push(value);
+  });
+
+  return _.reduce(
+    sensitiveValues,
+    (bank, val) => {
+      // 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) {
+        bank[val] = hashing.snipify(val);
+      }
+      return bank;
+    },
+    {}
+  );
 };
 
 const sendLog = (options, event, message, data) => {
   data = _.extend({}, data || {}, event.logExtra || {});
   data.log_type = data.log_type || 'console';
 
-  const sensitiveBank = makeSensitiveBank(event);
+  const sensitiveBank = makeSensitiveBank(event, data);
   const safeMessage = truncate(
     cleaner.recurseReplaceBank(message, sensitiveBank)
   );

diff --git a/src/tools/data.js b/src/tools/data.js
@@ -108,6 +108,22 @@ const recurseReplace = (obj, replacer) => {
   return obj;
 };
 
+// Recursively extract values from a nested object based on the matcher function.
+const recurseExtract = (obj, matcher) => {
+  const values = [];
+  Object.keys(obj).map(key => {
+    const value = obj[key];
+    if (matcher(key, value)) {
+      values.push(value);
+    } else if (isPlainObj(value)) {
+      recurseExtract(value, matcher).map(v => {
+        values.push(v);
+      });
+    }
+  });
+  return values;
+};
+
 const _IGNORE = {};
 
 // Flatten a nested object.
@@ -160,6 +176,7 @@ module.exports = {
   jsonCopy,
   deepFreeze,
   recurseReplace,
+  recurseExtract,
   flattenPaths,
   simpleTruncate
 };
diff --git a/test/logger.js b/test/logger.js
@@ -118,4 +118,50 @@ describe('logger', () => {
       })
       .catch(done);
   });
+
+  it('should replace sensitive data inside response', done => {
+    const bundle = {
+      authData: {
+        refresh_token: 'whatever'
+      }
+    };
+    const logger = createlogger({ bundle }, options);
+
+    const data = {
+      response_json: {
+        access_token: 'super_secret',
+        PASSWORD: 'top_secret',
+        name: 'not so secret'
+      },
+      response_content: `{
+        "access_token": "super_secret",
+        "PASSWORD": "top_secret",
+        "name": "not so secret"
+      }`
+    };
+
+    logger('test', data)
+      .then(response => {
+        response.status.should.eql(200);
+        response.content.json.should.eql({
+          token: 'fake-token',
+          message: 'test',
+          data: {
+            response_json: {
+              access_token: ':censored:12:8e4a58294b:',
+              PASSWORD: ':censored:10:b0c55acfea:',
+              name: 'not so secret'
+            },
+            response_content: `{
+        "access_token": ":censored:12:8e4a58294b:",
+        "PASSWORD": ":censored:10:b0c55acfea:",
+        "name": "not so secret"
+      }`,
+            log_type: 'console'
+          }
+        });
+        done();
+      })
+      .catch(done);
+  });
 });
diff --git a/test/misc-tools.js b/test/misc-tools.js
@@ -185,6 +185,30 @@ describe('Tools', () => {
     });
   });
 
+  describe('recurseExtract', () => {
+    it('should extract values that match', () => {
+      const obj = {
+        secret_key: '1234',
+        another: {
+          secret_key: '5678',
+          deep: {
+            secret: 'abcd'
+          }
+        },
+        name: 'dont_care',
+        id: 88
+      };
+      const matcher = (key, value) => {
+        if (typeof value === 'string') {
+          return key.indexOf('secret') >= 0;
+        }
+        return false;
+      };
+      const values = dataTools.recurseExtract(obj, matcher);
+      values.should.eql(['1234', '5678', 'abcd']);
+    });
+  });
+
   describe('recurseCleanFuncs', () => {
     it('should handle objects, arrays and function->str', () => {
       var output = cleaner.recurseCleanFuncs({