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

fix(core): hide sensitive data from error messages #87

Closed
wants to merge 3 commits into from
Closed
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
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "zapier-platform-cli",
"version": "8.4.0",
"version": "8.4.1",
"description": "The CLI for apps in the Zapier Developer Platform.",
"repository": "zapier/zapier-platform-cli",
"homepage": "https://zapier.com/",
Expand Down
17 changes: 14 additions & 3 deletions packages/cli/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,36 @@ const ANALYTICS_MODES = {
const PACKAGE_VERSION = require('../package.json').version;
const UPDATE_NOTIFICATION_INTERVAL = 1000 * 60 * 60 * 24 * 7; // one week

// can't just read from argv because they could have lots of extra data, such as
// [ '/Users/david/.nvm/versions/node/v10.13.0/bin/node',
// '/Users/david/projects/zapier/platform/node_modules/.bin/mocha',
// 'src/tests' ]
const argvStr = process.argv.join(' ');
const IS_TESTING =
argvStr.includes('mocha') ||
argvStr.includes('jest') ||
(process.env.NODE_ENV || '').toLowerCase().startsWith('test');

module.exports = {
ANALYTICS_KEY,
ANALYTICS_MODES,
API_PATH,
AUTH_KEY,
AUTH_LOCATION,
AUTH_LOCATION_RAW,
AUTH_LOCATION,
BASE_ENDPOINT,
BLACKLISTED_PATHS,
BUILD_DIR,
BUILD_PATH,
SOURCE_PATH,
BLACKLISTED_PATHS,
CURRENT_APP_FILE,
DEBUG,
DEFINITION_PATH,
ENDPOINT,
IS_TESTING,
LAMBDA_VERSION,
PACKAGE_VERSION,
PLATFORM_PACKAGE,
SOURCE_PATH,
STARTER_REPO,
UPDATE_NOTIFICATION_INTERVAL
};
42 changes: 22 additions & 20 deletions packages/cli/src/utils/analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const { callAPI } = require('./api');
// const { readFile } = require('./files');
const debug = require('debug')('zapier:analytics');
const pkg = require('../../package.json');
const { ANALYTICS_KEY, ANALYTICS_MODES } = require('../constants');
const { ANALYTICS_KEY, ANALYTICS_MODES, IS_TESTING } = require('../constants');
const { readUserConfig, writeUserConfig } = require('./userConfig');

const currentAnalyticsMode = async () => {
Expand All @@ -15,19 +15,22 @@ const setAnalyticsMode = newMode => {
return writeUserConfig({ [ANALYTICS_KEY]: newMode });
};

const shouldSkipAnalytics = mode =>
IS_TESTING ||
process.env.DISABLE_ZAPIER_ANALYTICS ||
mode === ANALYTICS_MODES.disabled;

const recordAnalytics = async (command, isValidCommand, args, flags) => {
const analyticsMode = await currentAnalyticsMode();

const shouldRecordAnalytics =
process.env.DISABLE_ZAPIER_ANALYTICS ||
(process.NODE_ENV !== 'test' && analyticsMode !== ANALYTICS_MODES.disabled);

if (!shouldRecordAnalytics) {
if (shouldSkipAnalytics(analyticsMode)) {
debug('skipping analytics');
return;
}

const shouldRecordAnonymously = analyticsMode === ANALYTICS_MODES.anonymous;

// to make this more testable, we should split this out into its own function
const analyticsBody = {
command,
isValidCommand,
Expand All @@ -41,25 +44,24 @@ const recordAnalytics = async (command, isValidCommand, args, flags) => {
};

debug('sending', analyticsBody);
return shouldRecordAnalytics
? callAPI(
'/analytics',
{
method: 'POST',
body: analyticsBody,
skipDeployKey: shouldRecordAnonymously
},
true,
false
)
.then(({ success }) => debug('success:', success))
.catch(({ errText }) => debug('err:', errText))
: Promise.resolve();
return callAPI(
'/analytics',
{
method: 'POST',
body: analyticsBody,
skipDeployKey: shouldRecordAnonymously
},
true,
false
)
.then(({ success }) => debug('success:', success))
.catch(({ errText }) => debug('err:', errText));
};

module.exports = {
currentAnalyticsMode,
recordAnalytics,
shouldSkipAnalytics,
modes: ANALYTICS_MODES,
setAnalyticsMode
};
4 changes: 2 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "zapier-platform-core",
"version": "8.4.0",
"version": "8.4.1",
"description": "The core SDK for CLI apps in the Zapier Developer Platform.",
"repository": "zapier/zapier-platform-core",
"homepage": "https://zapier.com/",
Expand Down Expand Up @@ -46,7 +46,7 @@
"node-fetch": "1.7.1",
"oauth-sign": "0.9.0",
"semver": "5.6.0",
"zapier-platform-schema": "8.4.0"
"zapier-platform-schema": "8.4.1"
},
"devDependencies": {
"adm-zip": "0.4.13",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
'use strict';

const { stripQueryFromURL } = require('../../tools/http');

const errors = require('../../errors');

const throwForStaleAuth = resp => {
if (resp.status === 401) {
const message = `Got ${resp.status} calling ${resp.request.method} ${
resp.request.url
}, triggering auth refresh.`;
const cleanURL = stripQueryFromURL(resp.request.url);
const message = `Got ${resp.status} calling ${resp.request.method} ${cleanURL}, triggering auth refresh.`;
throw new errors.RefreshAuthError(message);
}

Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/http-middlewares/after/throw-for-status.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
'use strict';

const { stripQueryFromURL } = require('../../tools/http');

const throwForStatus = resp => {
if (resp.status > 300) {
const message = `Got ${resp.status} calling ${resp.request.method} ${
resp.request.url
}, expected 2xx.`;
const cleanURL = stripQueryFromURL(resp.request.url);
const message = `Got ${resp.status} calling ${resp.request.method} ${cleanURL}, expected 2xx.`;
throw new Error(message);
}

Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/tools/http.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { URL } = require('url');

const _ = require('lodash');
const fetch = require('node-fetch');

Expand Down Expand Up @@ -28,7 +30,7 @@ const parseHttpList = s => {
let part = '';

let escape = false;
let quote = false;
let quote = false;

for (let i = 0; i < s.length; i++) {
const cur = s.charAt(i);
Expand Down Expand Up @@ -97,11 +99,18 @@ const parseDictHeader = s => {
const unheader = h =>
h instanceof fetch.Headers && _.isFunction(h.toJSON) ? h.toJSON() : h;

const stripQueryFromURL = url => {
// Strip off querystring for any sensitive data
const u = new URL(url);
return u.origin + u.pathname;
};

module.exports = {
FORM_TYPE,
JSON_TYPE,
JSON_TYPE_UTF8,
getContentType,
parseDictHeader,
stripQueryFromURL,
unheader
};
Loading