From 189f3f01d7bfcb5f010bbeda97f094549c6664ba Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Tue, 27 Nov 2018 15:53:39 +0200 Subject: [PATCH 1/9] feat: add tracking for command options --- lib/bootstrap.ts | 1 + lib/common/services/commands-service.ts | 4 +- lib/constants.ts | 3 +- lib/declarations.d.ts | 4 + lib/helpers/options-track-helper.ts | 128 ++++++++++++++++++++++++ 5 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 lib/helpers/options-track-helper.ts diff --git a/lib/bootstrap.ts b/lib/bootstrap.ts index 3b12c03486..32f5942be1 100644 --- a/lib/bootstrap.ts +++ b/lib/bootstrap.ts @@ -128,6 +128,7 @@ $injector.require("bundleValidatorHelper", "./helpers/bundle-validator-helper"); $injector.require("androidBundleValidatorHelper", "./helpers/android-bundle-validator-helper"); $injector.require("liveSyncCommandHelper", "./helpers/livesync-command-helper"); $injector.require("deployCommandHelper", "./helpers/deploy-command-helper"); +$injector.require("optionsTrackHelper", "./helpers/options-track-helper"); $injector.requirePublicClass("localBuildService", "./services/local-build-service"); $injector.requirePublicClass("liveSyncService", "./services/livesync/livesync-service"); diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index 3a84a7c23e..0c4fdc0c2d 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -30,7 +30,8 @@ export class CommandsService implements ICommandsService { private $resources: IResourceLoader, private $staticConfig: Config.IStaticConfig, private $helpService: IHelpService, - private $extensibilityService: IExtensibilityService) { + private $extensibilityService: IExtensibilityService, + private $optionsTrackHelper: IOptionsTrackHelper) { } public allCommands(opts: { includeDevCommands: boolean }): string[] { @@ -63,6 +64,7 @@ export class CommandsService implements ICommandsService { } await analyticsService.trackInGoogleAnalytics(googleAnalyticsPageData); + await this.$optionsTrackHelper.trackOptions(this.$options); } const shouldExecuteHooks = !this.$staticConfig.disableCommandHooks && (command.enableHooks === undefined || command.enableHooks === true); diff --git a/lib/constants.ts b/lib/constants.ts index 8b55dd1238..60c2814ede 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -149,7 +149,8 @@ export const enum TrackActionNames { LiveSync = "LiveSync", RunSetupScript = "Run Setup Script", CheckLocalBuildSetup = "Check Local Build Setup", - CheckEnvironmentRequirements = "Check Environment Requirements" + CheckEnvironmentRequirements = "Check Environment Requirements", + Options = "Options" } export const AnalyticsEventLabelDelimiter = "__"; diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index eafdaab489..eaf88884c3 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -910,6 +910,10 @@ interface IAndroidBundleValidatorHelper { validateRuntimeVersion(projectData: IProjectData): void } +interface IOptionsTrackHelper { + trackOptions(options: IOptions): Promise +} + interface INativeScriptCloudExtensionService { /** * Installs nativescript-cloud extension diff --git a/lib/helpers/options-track-helper.ts b/lib/helpers/options-track-helper.ts new file mode 100644 index 0000000000..aff60ecabb --- /dev/null +++ b/lib/helpers/options-track-helper.ts @@ -0,0 +1,128 @@ +import * as path from "path"; +import { TrackActionNames } from "../constants"; + +export class OptionsTrackHelper { + public static SINGLE_OBJECT_SIZE_LIMIT = 400; + public static SINGLE_PROPERTY_SIZE_LIMIT = 300; + public static PASSWORD_DETECTION_STRING = "password"; + public static PASSOWRD_REPLACE_VALUE = "privateData"; + public static PATH_REPLACE_VALUE = "_localpath"; + public static SIZE_EXEEDED_REPLACE_VALUE = "popertySizeExceeded"; + + + constructor( + private $analyticsService: IAnalyticsService) { + } + + public async trackOptions(options: IOptions) { + const trackObjects = this.getTrackObjects(options); + const promises: Promise[] = []; + _.forEach(trackObjects, trackObject => { + const json = JSON.stringify(trackObject); + + const trackPromise = this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: TrackActionNames.Options, + additionalData: json + }); + + promises.push(trackPromise); + }); + + await Promise.all(promises); + } + + private getTrackObjects(options: IOptions): Object[] { + const optionsArgvCopy = _.cloneDeep(options.argv); + const data = this.sanitizeTrackObject(optionsArgvCopy, options); + + return this.splitTrackObjects(data); + } + + private sanitizeTrackObject(data: IDictionary, options?: IOptions): IDictionary { + const shorthands = options ? options.shorthands: []; + const optionsDefinitions = options ? options.options: {}; + + _.forEach(data, (value, key) => { + if(this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { + delete data[key]; + } else { + if(key.toLowerCase().indexOf(OptionsTrackHelper.PASSWORD_DETECTION_STRING) >= 0) { + value = OptionsTrackHelper.PASSOWRD_REPLACE_VALUE; + } else if(typeof value === "string" && value !== path.basename(value)) { + value = OptionsTrackHelper.PATH_REPLACE_VALUE; + } else if(_.isObject(value) && !_.isArray(value)) { + value = this.sanitizeTrackObject(value); + } + data[key] = value; + } + }); + + return data; + } + + private shouldSkipProperty(key: string, value: any, shorthands: string[] = [], options: IDictionary = {}): Boolean { + if(shorthands.indexOf(key) >= 0){ + return true; + } + + if(key.indexOf("-") >= 0) { + return true; + } + + if(key === "_") { + return true; + } + + const optionDef = options[key]; + if(optionDef && optionDef.type === OptionType.Boolean) { + if(optionDef.default !== true && value === false || optionDef.default === true && value === true) { + return true; + } + } + + if(_.isUndefined(value)) { + return true; + } + } + + private splitTrackObjects(trackData: Object): Object[] { + const json = JSON.stringify(trackData); + const firstObject: IDictionary = {}; + const secondObject: IDictionary = {}; + const bigFields: Object[] = []; + + if(json.length > OptionsTrackHelper.SINGLE_OBJECT_SIZE_LIMIT){ + const keys = _.keys(trackData); + + if(keys.length === 1) { + return [trackData]; + } + + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + let value = trackData[key]; + const valueLength = JSON.stringify(value).length; + + if(valueLength > OptionsTrackHelper.SINGLE_OBJECT_SIZE_LIMIT) { + value = "SIZE_EXEEDED_REPLACE_VALUE"; + } + + if(valueLength > OptionsTrackHelper.SINGLE_PROPERTY_SIZE_LIMIT) { + bigFields.push({ + [key]: value + }); + } else if(i < keys.length/2) { + firstObject[key] = value; + } else { + secondObject[key] = value; + } + } + + return bigFields.concat(this.splitTrackObjects(firstObject), this.splitTrackObjects(secondObject)); + } else { + return [trackData]; + } + } +} + +$injector.register("optionsTrackHelper", OptionsTrackHelper); From 97684f2b40d388306e78abd78caa3d2df05adf6b Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Wed, 28 Nov 2018 17:06:22 +0200 Subject: [PATCH 2/9] chroe: remove split object logic as we can't get reliable data. --- lib/helpers/options-track-helper.ts | 93 +++++++---------------------- 1 file changed, 21 insertions(+), 72 deletions(-) diff --git a/lib/helpers/options-track-helper.ts b/lib/helpers/options-track-helper.ts index aff60ecabb..d8ebdb1ecb 100644 --- a/lib/helpers/options-track-helper.ts +++ b/lib/helpers/options-track-helper.ts @@ -2,55 +2,43 @@ import * as path from "path"; import { TrackActionNames } from "../constants"; export class OptionsTrackHelper { - public static SINGLE_OBJECT_SIZE_LIMIT = 400; - public static SINGLE_PROPERTY_SIZE_LIMIT = 300; public static PASSWORD_DETECTION_STRING = "password"; - public static PASSOWRD_REPLACE_VALUE = "privateData"; + public static PASSOWRD_REPLACE_VALUE = "private"; public static PATH_REPLACE_VALUE = "_localpath"; - public static SIZE_EXEEDED_REPLACE_VALUE = "popertySizeExceeded"; - + public static SIZE_EXEEDED_REPLACE_VALUE = "sizeExceeded"; constructor( private $analyticsService: IAnalyticsService) { } public async trackOptions(options: IOptions) { - const trackObjects = this.getTrackObjects(options); - const promises: Promise[] = []; - _.forEach(trackObjects, trackObject => { - const json = JSON.stringify(trackObject); - - const trackPromise = this.$analyticsService.trackEventActionInGoogleAnalytics({ - action: TrackActionNames.Options, - additionalData: json - }); + const trackObject = this.getTrackObject(options); - promises.push(trackPromise); + await this.$analyticsService.trackEventActionInGoogleAnalytics({ + action: TrackActionNames.Options, + additionalData: JSON.stringify(trackObject) }); - - await Promise.all(promises); } - private getTrackObjects(options: IOptions): Object[] { + private getTrackObject(options: IOptions): IDictionary { const optionsArgvCopy = _.cloneDeep(options.argv); - const data = this.sanitizeTrackObject(optionsArgvCopy, options); - return this.splitTrackObjects(data); + return this.sanitizeTrackObject(optionsArgvCopy, options); } private sanitizeTrackObject(data: IDictionary, options?: IOptions): IDictionary { - const shorthands = options ? options.shorthands: []; - const optionsDefinitions = options ? options.options: {}; + const shorthands = options ? options.shorthands : []; + const optionsDefinitions = options ? options.options : {}; _.forEach(data, (value, key) => { - if(this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { + if (this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { delete data[key]; } else { - if(key.toLowerCase().indexOf(OptionsTrackHelper.PASSWORD_DETECTION_STRING) >= 0) { + if (key.toLowerCase().indexOf(OptionsTrackHelper.PASSWORD_DETECTION_STRING) >= 0) { value = OptionsTrackHelper.PASSOWRD_REPLACE_VALUE; - } else if(typeof value === "string" && value !== path.basename(value)) { + } else if (_.isString(value) && value !== path.basename(value)) { value = OptionsTrackHelper.PATH_REPLACE_VALUE; - } else if(_.isObject(value) && !_.isArray(value)) { + } else if (_.isObject(value) && !_.isArray(value)) { value = this.sanitizeTrackObject(value); } data[key] = value; @@ -61,68 +49,29 @@ export class OptionsTrackHelper { } private shouldSkipProperty(key: string, value: any, shorthands: string[] = [], options: IDictionary = {}): Boolean { - if(shorthands.indexOf(key) >= 0){ + if (shorthands.indexOf(key) >= 0) { return true; } - if(key.indexOf("-") >= 0) { + if (key.indexOf("-") >= 0) { return true; } - if(key === "_") { + if (key === "_") { return true; } - + const optionDef = options[key]; - if(optionDef && optionDef.type === OptionType.Boolean) { - if(optionDef.default !== true && value === false || optionDef.default === true && value === true) { + if (optionDef && optionDef.type === OptionType.Boolean) { + if (optionDef.default !== true && value === false || optionDef.default === true && value === true) { return true; } } - if(_.isUndefined(value)) { + if (_.isUndefined(value)) { return true; } } - - private splitTrackObjects(trackData: Object): Object[] { - const json = JSON.stringify(trackData); - const firstObject: IDictionary = {}; - const secondObject: IDictionary = {}; - const bigFields: Object[] = []; - - if(json.length > OptionsTrackHelper.SINGLE_OBJECT_SIZE_LIMIT){ - const keys = _.keys(trackData); - - if(keys.length === 1) { - return [trackData]; - } - - for (let i = 0; i < keys.length; i++) { - const key = keys[i]; - let value = trackData[key]; - const valueLength = JSON.stringify(value).length; - - if(valueLength > OptionsTrackHelper.SINGLE_OBJECT_SIZE_LIMIT) { - value = "SIZE_EXEEDED_REPLACE_VALUE"; - } - - if(valueLength > OptionsTrackHelper.SINGLE_PROPERTY_SIZE_LIMIT) { - bigFields.push({ - [key]: value - }); - } else if(i < keys.length/2) { - firstObject[key] = value; - } else { - secondObject[key] = value; - } - } - - return bigFields.concat(this.splitTrackObjects(firstObject), this.splitTrackObjects(secondObject)); - } else { - return [trackData]; - } - } } $injector.register("optionsTrackHelper", OptionsTrackHelper); From ee05e039dc07d77922c1244e7b9b63fb5e63ee36 Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Thu, 29 Nov 2018 19:02:38 +0200 Subject: [PATCH 3/9] chore: fix comments --- lib/bootstrap.ts | 2 +- lib/common/services/commands-service.ts | 4 ++-- lib/declarations.d.ts | 2 +- lib/helpers/options-track-helper.ts | 10 +++++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/bootstrap.ts b/lib/bootstrap.ts index 32f5942be1..4b5d722efa 100644 --- a/lib/bootstrap.ts +++ b/lib/bootstrap.ts @@ -128,7 +128,7 @@ $injector.require("bundleValidatorHelper", "./helpers/bundle-validator-helper"); $injector.require("androidBundleValidatorHelper", "./helpers/android-bundle-validator-helper"); $injector.require("liveSyncCommandHelper", "./helpers/livesync-command-helper"); $injector.require("deployCommandHelper", "./helpers/deploy-command-helper"); -$injector.require("optionsTrackHelper", "./helpers/options-track-helper"); +$injector.require("optionsTracker", "./helpers/options-track-helper"); $injector.requirePublicClass("localBuildService", "./services/local-build-service"); $injector.requirePublicClass("liveSyncService", "./services/livesync/livesync-service"); diff --git a/lib/common/services/commands-service.ts b/lib/common/services/commands-service.ts index 0c4fdc0c2d..921c0cd7d4 100644 --- a/lib/common/services/commands-service.ts +++ b/lib/common/services/commands-service.ts @@ -31,7 +31,7 @@ export class CommandsService implements ICommandsService { private $staticConfig: Config.IStaticConfig, private $helpService: IHelpService, private $extensibilityService: IExtensibilityService, - private $optionsTrackHelper: IOptionsTrackHelper) { + private $optionsTracker: IOptionsTracker) { } public allCommands(opts: { includeDevCommands: boolean }): string[] { @@ -64,7 +64,7 @@ export class CommandsService implements ICommandsService { } await analyticsService.trackInGoogleAnalytics(googleAnalyticsPageData); - await this.$optionsTrackHelper.trackOptions(this.$options); + await this.$optionsTracker.trackOptions(this.$options); } const shouldExecuteHooks = !this.$staticConfig.disableCommandHooks && (command.enableHooks === undefined || command.enableHooks === true); diff --git a/lib/declarations.d.ts b/lib/declarations.d.ts index eaf88884c3..a9a02077b6 100644 --- a/lib/declarations.d.ts +++ b/lib/declarations.d.ts @@ -910,7 +910,7 @@ interface IAndroidBundleValidatorHelper { validateRuntimeVersion(projectData: IProjectData): void } -interface IOptionsTrackHelper { +interface IOptionsTracker { trackOptions(options: IOptions): Promise } diff --git a/lib/helpers/options-track-helper.ts b/lib/helpers/options-track-helper.ts index d8ebdb1ecb..e8c0453ade 100644 --- a/lib/helpers/options-track-helper.ts +++ b/lib/helpers/options-track-helper.ts @@ -1,7 +1,7 @@ import * as path from "path"; import { TrackActionNames } from "../constants"; -export class OptionsTrackHelper { +export class OptionsTracker { public static PASSWORD_DETECTION_STRING = "password"; public static PASSOWRD_REPLACE_VALUE = "private"; public static PATH_REPLACE_VALUE = "_localpath"; @@ -34,10 +34,10 @@ export class OptionsTrackHelper { if (this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { delete data[key]; } else { - if (key.toLowerCase().indexOf(OptionsTrackHelper.PASSWORD_DETECTION_STRING) >= 0) { - value = OptionsTrackHelper.PASSOWRD_REPLACE_VALUE; + if (key.toLowerCase().indexOf(OptionsTracker.PASSWORD_DETECTION_STRING) >= 0) { + value = OptionsTracker.PASSOWRD_REPLACE_VALUE; } else if (_.isString(value) && value !== path.basename(value)) { - value = OptionsTrackHelper.PATH_REPLACE_VALUE; + value = OptionsTracker.PATH_REPLACE_VALUE; } else if (_.isObject(value) && !_.isArray(value)) { value = this.sanitizeTrackObject(value); } @@ -74,4 +74,4 @@ export class OptionsTrackHelper { } } -$injector.register("optionsTrackHelper", OptionsTrackHelper); +$injector.register("optionsTracker", OptionsTracker); From 8abd9e359bf7c13e90ca28f188bb377a390e354e Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Thu, 29 Nov 2018 19:15:27 +0200 Subject: [PATCH 4/9] chore: fix tests --- test/platform-commands.ts | 3 +++ test/plugins-service.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/test/platform-commands.ts b/test/platform-commands.ts index f4da8918df..c63686abaa 100644 --- a/test/platform-commands.ts +++ b/test/platform-commands.ts @@ -179,6 +179,9 @@ function createTestInjector() { testInjector.register("pacoteService", { extractPackage: async (packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise => undefined }); + testInjector.register("optionsTracker", { + trackOptions: () => Promise.resolve(null) + }); testInjector.register("usbLiveSyncService", ({})); return testInjector; diff --git a/test/plugins-service.ts b/test/plugins-service.ts index 2a6650afa8..39fcaf4f23 100644 --- a/test/plugins-service.ts +++ b/test/plugins-service.ts @@ -73,6 +73,9 @@ function createTestInjector() { testInjector.register("logger", stubs.LoggerStub); testInjector.register("staticConfig", StaticConfig); testInjector.register("hooksService", stubs.HooksServiceStub); + testInjector.register("optionsTracker", { + trackOptions: () => Promise.resolve(null) + }); testInjector.register("commandsService", CommandsService); testInjector.register("commandsServiceProvider", { registerDynamicSubCommands: () => { /* intentionally empty body */ } From a521f365ca176a486aaffc8a138125dc51bdd7a2 Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Fri, 30 Nov 2018 16:59:12 +0200 Subject: [PATCH 5/9] feat: add private flag to some options to skip tracking for them --- lib/common/declarations.d.ts | 4 ++++ lib/helpers/options-track-helper.ts | 9 +++++--- lib/options.ts | 36 +++++++++++++++-------------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/lib/common/declarations.d.ts b/lib/common/declarations.d.ts index bd7bb95b7f..e8c75572f6 100644 --- a/lib/common/declarations.d.ts +++ b/lib/common/declarations.d.ts @@ -1269,6 +1269,10 @@ interface IDashedOption { * Type of the option. It can be string, boolean, Array, etc. */ type: string; + /** + * Should skip in tracking + */ + private?: boolean; /** * Shorthand option passed on the command line with `-` sign, for example `-v` */ diff --git a/lib/helpers/options-track-helper.ts b/lib/helpers/options-track-helper.ts index e8c0453ade..9d9c109645 100644 --- a/lib/helpers/options-track-helper.ts +++ b/lib/helpers/options-track-helper.ts @@ -3,7 +3,7 @@ import { TrackActionNames } from "../constants"; export class OptionsTracker { public static PASSWORD_DETECTION_STRING = "password"; - public static PASSOWRD_REPLACE_VALUE = "private"; + public static PRIVATE_REPLACE_VALUE = "private"; public static PATH_REPLACE_VALUE = "_localpath"; public static SIZE_EXEEDED_REPLACE_VALUE = "sizeExceeded"; @@ -34,13 +34,16 @@ export class OptionsTracker { if (this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { delete data[key]; } else { - if (key.toLowerCase().indexOf(OptionsTracker.PASSWORD_DETECTION_STRING) >= 0) { - value = OptionsTracker.PASSOWRD_REPLACE_VALUE; + if (options && optionsDefinitions[key] && optionsDefinitions[key].private) { + value = OptionsTracker.PRIVATE_REPLACE_VALUE; + } else if (key.toLowerCase().indexOf(OptionsTracker.PASSWORD_DETECTION_STRING) >= 0) { + value = OptionsTracker.PRIVATE_REPLACE_VALUE; } else if (_.isString(value) && value !== path.basename(value)) { value = OptionsTracker.PATH_REPLACE_VALUE; } else if (_.isObject(value) && !_.isArray(value)) { value = this.sanitizeTrackObject(value); } + data[key] = value; } }); diff --git a/lib/options.ts b/lib/options.ts index 821430db09..2c4691242f 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -12,9 +12,9 @@ export class Options { verbose: { type: OptionType.Boolean, alias: "v" }, version: { type: OptionType.Boolean }, help: { type: OptionType.Boolean, alias: "h" }, - profileDir: { type: OptionType.String }, + profileDir: { type: OptionType.String, private: true }, analyticsClient: { type: OptionType.String }, - path: { type: OptionType.String, alias: "p" }, + path: { type: OptionType.String, alias: "p", private: true }, // This will parse all non-hyphenated values as strings. _: { type: OptionType.String } }; @@ -47,14 +47,14 @@ export class Options { framework: { type: OptionType.String }, frameworkVersion: { type: OptionType.String }, forDevice: { type: OptionType.Boolean }, - provision: { type: OptionType.Object }, + provision: { type: OptionType.Object, private: true }, client: { type: OptionType.Boolean, default: true }, env: { type: OptionType.Object }, production: { type: OptionType.Boolean }, debugTransport: { type: OptionType.Boolean }, keyStorePath: { type: OptionType.String }, keyStorePassword: { type: OptionType.String, }, - keyStoreAlias: { type: OptionType.String }, + keyStoreAlias: { type: OptionType.String, private: true }, keyStoreAliasPassword: { type: OptionType.String }, ignoreScripts: { type: OptionType.Boolean }, disableNpmInstall: { type: OptionType.Boolean }, @@ -75,46 +75,48 @@ export class Options { androidTypings: { type: OptionType.Boolean }, bundle: { type: OptionType.String }, all: { type: OptionType.Boolean }, - teamId: { type: OptionType.Object }, + teamId: { type: OptionType.Object, private: true }, syncAllFiles: { type: OptionType.Boolean, default: false }, chrome: { type: OptionType.Boolean }, inspector: { type: OptionType.Boolean }, clean: { type: OptionType.Boolean }, watch: { type: OptionType.Boolean, default: true }, background: { type: OptionType.String }, - username: { type: OptionType.String }, + username: { type: OptionType.String, private: true }, pluginName: { type: OptionType.String }, hmr: { type: OptionType.Boolean }, collection: { type: OptionType.String, alias: "c" }, json: { type: OptionType.Boolean }, - avd: { type: OptionType.String }, + avd: { type: OptionType.String, private: true }, + // check not used config: { type: OptionType.Array }, insecure: { type: OptionType.Boolean, alias: "k" }, debug: { type: OptionType.Boolean, alias: "d" }, timeout: { type: OptionType.String }, - device: { type: OptionType.String }, - availableDevices: { type: OptionType.Boolean }, - appid: { type: OptionType.String }, - geny: { type: OptionType.String }, + device: { type: OptionType.String, private: true }, + availableDevices: { type: OptionType.Boolean, private: true }, + appid: { type: OptionType.String, private: true }, + geny: { type: OptionType.String, private: true }, debugBrk: { type: OptionType.Boolean }, debugPort: { type: OptionType.Number }, start: { type: OptionType.Boolean }, stop: { type: OptionType.Boolean }, - ddi: { type: OptionType.String }, // the path to developer disk image + ddi: { type: OptionType.String, private: true }, // the path to developer disk image justlaunch: { type: OptionType.Boolean }, - file: { type: OptionType.String }, + file: { type: OptionType.String, private: true }, force: { type: OptionType.Boolean, alias: "f" }, + // remove legacy companion: { type: OptionType.Boolean }, emulator: { type: OptionType.Boolean }, sdk: { type: OptionType.String }, - template: { type: OptionType.String }, - certificate: { type: OptionType.String }, + template: { type: OptionType.String, private: true }, + certificate: { type: OptionType.String, private: true }, certificatePassword: { type: OptionType.String }, release: { type: OptionType.Boolean, alias: "r" }, - var: { type: OptionType.Object }, + var: { type: OptionType.Object, private: true }, default: { type: OptionType.Boolean }, count: { type: OptionType.Number }, - analyticsLogFile: { type: OptionType.String }, + analyticsLogFile: { type: OptionType.String, private: true }, hooks: { type: OptionType.Boolean, default: true }, link: { type: OptionType.Boolean, default: false }, aab: { type: OptionType.Boolean } From d69244b7a2869006d96a2a2b55e69f21306285b8 Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Fri, 30 Nov 2018 19:10:38 +0200 Subject: [PATCH 6/9] chore: fix comments --- lib/common/declarations.d.ts | 4 ++-- lib/helpers/options-track-helper.ts | 2 +- lib/options.ts | 34 ++++++++++++++--------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/common/declarations.d.ts b/lib/common/declarations.d.ts index e8c75572f6..2dd3de6a7e 100644 --- a/lib/common/declarations.d.ts +++ b/lib/common/declarations.d.ts @@ -1270,9 +1270,9 @@ interface IDashedOption { */ type: string; /** - * Should skip in tracking + * Option has sensitive value */ - private?: boolean; + hasSensitiveValue?: boolean; /** * Shorthand option passed on the command line with `-` sign, for example `-v` */ diff --git a/lib/helpers/options-track-helper.ts b/lib/helpers/options-track-helper.ts index 9d9c109645..93dc0a51c2 100644 --- a/lib/helpers/options-track-helper.ts +++ b/lib/helpers/options-track-helper.ts @@ -34,7 +34,7 @@ export class OptionsTracker { if (this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { delete data[key]; } else { - if (options && optionsDefinitions[key] && optionsDefinitions[key].private) { + if (options && optionsDefinitions[key] && optionsDefinitions[key].hasSensitiveValue) { value = OptionsTracker.PRIVATE_REPLACE_VALUE; } else if (key.toLowerCase().indexOf(OptionsTracker.PASSWORD_DETECTION_STRING) >= 0) { value = OptionsTracker.PRIVATE_REPLACE_VALUE; diff --git a/lib/options.ts b/lib/options.ts index 2c4691242f..f0e825002e 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -12,9 +12,9 @@ export class Options { verbose: { type: OptionType.Boolean, alias: "v" }, version: { type: OptionType.Boolean }, help: { type: OptionType.Boolean, alias: "h" }, - profileDir: { type: OptionType.String, private: true }, + profileDir: { type: OptionType.String, hasSensitiveValue: true }, analyticsClient: { type: OptionType.String }, - path: { type: OptionType.String, alias: "p", private: true }, + path: { type: OptionType.String, alias: "p", hasSensitiveValue: true }, // This will parse all non-hyphenated values as strings. _: { type: OptionType.String } }; @@ -47,14 +47,14 @@ export class Options { framework: { type: OptionType.String }, frameworkVersion: { type: OptionType.String }, forDevice: { type: OptionType.Boolean }, - provision: { type: OptionType.Object, private: true }, + provision: { type: OptionType.Object, hasSensitiveValue: true }, client: { type: OptionType.Boolean, default: true }, env: { type: OptionType.Object }, production: { type: OptionType.Boolean }, debugTransport: { type: OptionType.Boolean }, keyStorePath: { type: OptionType.String }, keyStorePassword: { type: OptionType.String, }, - keyStoreAlias: { type: OptionType.String, private: true }, + keyStoreAlias: { type: OptionType.String, hasSensitiveValue: true }, keyStoreAliasPassword: { type: OptionType.String }, ignoreScripts: { type: OptionType.Boolean }, disableNpmInstall: { type: OptionType.Boolean }, @@ -75,48 +75,48 @@ export class Options { androidTypings: { type: OptionType.Boolean }, bundle: { type: OptionType.String }, all: { type: OptionType.Boolean }, - teamId: { type: OptionType.Object, private: true }, + teamId: { type: OptionType.Object, hasSensitiveValue: true }, syncAllFiles: { type: OptionType.Boolean, default: false }, chrome: { type: OptionType.Boolean }, inspector: { type: OptionType.Boolean }, clean: { type: OptionType.Boolean }, watch: { type: OptionType.Boolean, default: true }, background: { type: OptionType.String }, - username: { type: OptionType.String, private: true }, + username: { type: OptionType.String, hasSensitiveValue: true }, pluginName: { type: OptionType.String }, hmr: { type: OptionType.Boolean }, collection: { type: OptionType.String, alias: "c" }, json: { type: OptionType.Boolean }, - avd: { type: OptionType.String, private: true }, + avd: { type: OptionType.String, hasSensitiveValue: true }, // check not used config: { type: OptionType.Array }, insecure: { type: OptionType.Boolean, alias: "k" }, debug: { type: OptionType.Boolean, alias: "d" }, timeout: { type: OptionType.String }, - device: { type: OptionType.String, private: true }, - availableDevices: { type: OptionType.Boolean, private: true }, - appid: { type: OptionType.String, private: true }, - geny: { type: OptionType.String, private: true }, + device: { type: OptionType.String, hasSensitiveValue: true }, + availableDevices: { type: OptionType.Boolean, hasSensitiveValue: true }, + appid: { type: OptionType.String, hasSensitiveValue: true }, + geny: { type: OptionType.String, hasSensitiveValue: true }, debugBrk: { type: OptionType.Boolean }, debugPort: { type: OptionType.Number }, start: { type: OptionType.Boolean }, stop: { type: OptionType.Boolean }, - ddi: { type: OptionType.String, private: true }, // the path to developer disk image + ddi: { type: OptionType.String, hasSensitiveValue: true }, // the path to developer disk image justlaunch: { type: OptionType.Boolean }, - file: { type: OptionType.String, private: true }, + file: { type: OptionType.String, hasSensitiveValue: true }, force: { type: OptionType.Boolean, alias: "f" }, // remove legacy companion: { type: OptionType.Boolean }, emulator: { type: OptionType.Boolean }, sdk: { type: OptionType.String }, - template: { type: OptionType.String, private: true }, - certificate: { type: OptionType.String, private: true }, + template: { type: OptionType.String, hasSensitiveValue: true }, + certificate: { type: OptionType.String, hasSensitiveValue: true }, certificatePassword: { type: OptionType.String }, release: { type: OptionType.Boolean, alias: "r" }, - var: { type: OptionType.Object, private: true }, + var: { type: OptionType.Object, hasSensitiveValue: true }, default: { type: OptionType.Boolean }, count: { type: OptionType.Number }, - analyticsLogFile: { type: OptionType.String, private: true }, + analyticsLogFile: { type: OptionType.String, hasSensitiveValue: true }, hooks: { type: OptionType.Boolean, default: true }, link: { type: OptionType.Boolean, default: false }, aab: { type: OptionType.Boolean } From 225c06ce62890b6e05812577193ab9bfbd6dc190 Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Mon, 3 Dec 2018 18:49:24 +0200 Subject: [PATCH 7/9] chore: make hasSensitiveValue mandatory field --- lib/common/declarations.d.ts | 2 +- lib/helpers/options-track-helper.ts | 2 +- lib/options.ts | 136 ++++++++++++++-------------- 3 files changed, 70 insertions(+), 70 deletions(-) diff --git a/lib/common/declarations.d.ts b/lib/common/declarations.d.ts index 2dd3de6a7e..31e16b3c83 100644 --- a/lib/common/declarations.d.ts +++ b/lib/common/declarations.d.ts @@ -1272,7 +1272,7 @@ interface IDashedOption { /** * Option has sensitive value */ - hasSensitiveValue?: boolean; + hasSensitiveValue: boolean; /** * Shorthand option passed on the command line with `-` sign, for example `-v` */ diff --git a/lib/helpers/options-track-helper.ts b/lib/helpers/options-track-helper.ts index 93dc0a51c2..bb192fcdb7 100644 --- a/lib/helpers/options-track-helper.ts +++ b/lib/helpers/options-track-helper.ts @@ -34,7 +34,7 @@ export class OptionsTracker { if (this.shouldSkipProperty(key, value, shorthands, optionsDefinitions)) { delete data[key]; } else { - if (options && optionsDefinitions[key] && optionsDefinitions[key].hasSensitiveValue) { + if (options && optionsDefinitions[key] && optionsDefinitions[key].hasSensitiveValue !== false) { value = OptionsTracker.PRIVATE_REPLACE_VALUE; } else if (key.toLowerCase().indexOf(OptionsTracker.PASSWORD_DETECTION_STRING) >= 0) { value = OptionsTracker.PRIVATE_REPLACE_VALUE; diff --git a/lib/options.ts b/lib/options.ts index f0e825002e..33861d1784 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -8,15 +8,15 @@ export class Options { private optionsWhiteList = ["ui", "recursive", "reporter", "require", "timeout", "_", "$0"]; // These options shouldn't be validated public argv: IYargArgv; private globalOptions: IDictionary = { - log: { type: OptionType.String }, - verbose: { type: OptionType.Boolean, alias: "v" }, - version: { type: OptionType.Boolean }, - help: { type: OptionType.Boolean, alias: "h" }, + log: { type: OptionType.String, hasSensitiveValue: false }, + verbose: { type: OptionType.Boolean, alias: "v", hasSensitiveValue: false }, + version: { type: OptionType.Boolean, hasSensitiveValue: false }, + help: { type: OptionType.Boolean, alias: "h", hasSensitiveValue: false }, profileDir: { type: OptionType.String, hasSensitiveValue: true }, - analyticsClient: { type: OptionType.String }, + analyticsClient: { type: OptionType.String, hasSensitiveValue: false }, path: { type: OptionType.String, alias: "p", hasSensitiveValue: true }, // This will parse all non-hyphenated values as strings. - _: { type: OptionType.String } + _: { type: OptionType.String, hasSensitiveValue: false } }; public options: IDictionary; @@ -41,85 +41,85 @@ export class Options { private get commonOptions(): IDictionary { return { - ipa: { type: OptionType.String }, - frameworkPath: { type: OptionType.String }, - frameworkName: { type: OptionType.String }, - framework: { type: OptionType.String }, - frameworkVersion: { type: OptionType.String }, - forDevice: { type: OptionType.Boolean }, + ipa: { type: OptionType.String, hasSensitiveValue: false }, + frameworkPath: { type: OptionType.String, hasSensitiveValue: false }, + frameworkName: { type: OptionType.String, hasSensitiveValue: false }, + framework: { type: OptionType.String, hasSensitiveValue: false }, + frameworkVersion: { type: OptionType.String, hasSensitiveValue: false }, + forDevice: { type: OptionType.Boolean, hasSensitiveValue: false }, provision: { type: OptionType.Object, hasSensitiveValue: true }, - client: { type: OptionType.Boolean, default: true }, - env: { type: OptionType.Object }, - production: { type: OptionType.Boolean }, - debugTransport: { type: OptionType.Boolean }, - keyStorePath: { type: OptionType.String }, - keyStorePassword: { type: OptionType.String, }, + client: { type: OptionType.Boolean, default: true, hasSensitiveValue: false }, + env: { type: OptionType.Object, hasSensitiveValue: false }, + production: { type: OptionType.Boolean, hasSensitiveValue: false }, + debugTransport: { type: OptionType.Boolean, hasSensitiveValue: false }, + keyStorePath: { type: OptionType.String, hasSensitiveValue: false }, + keyStorePassword: { type: OptionType.String, hasSensitiveValue: true }, keyStoreAlias: { type: OptionType.String, hasSensitiveValue: true }, - keyStoreAliasPassword: { type: OptionType.String }, - ignoreScripts: { type: OptionType.Boolean }, - disableNpmInstall: { type: OptionType.Boolean }, - compileSdk: { type: OptionType.Number }, - port: { type: OptionType.Number }, - copyTo: { type: OptionType.String }, - platformTemplate: { type: OptionType.String }, - js: { type: OptionType.Boolean }, - javascript: { type: OptionType.Boolean }, - ng: { type: OptionType.Boolean }, - angular: { type: OptionType.Boolean }, - vue: { type: OptionType.Boolean }, - vuejs: { type: OptionType.Boolean }, - tsc: { type: OptionType.Boolean }, - ts: { type: OptionType.Boolean }, - typescript: { type: OptionType.Boolean }, - yarn: { type: OptionType.Boolean }, - androidTypings: { type: OptionType.Boolean }, - bundle: { type: OptionType.String }, - all: { type: OptionType.Boolean }, + keyStoreAliasPassword: { type: OptionType.String, hasSensitiveValue: true }, + ignoreScripts: { type: OptionType.Boolean, hasSensitiveValue: false }, + disableNpmInstall: { type: OptionType.Boolean, hasSensitiveValue: false }, + compileSdk: { type: OptionType.Number, hasSensitiveValue: false }, + port: { type: OptionType.Number, hasSensitiveValue: false }, + copyTo: { type: OptionType.String, hasSensitiveValue: false }, + platformTemplate: { type: OptionType.String, hasSensitiveValue: false }, + js: { type: OptionType.Boolean, hasSensitiveValue: false }, + javascript: { type: OptionType.Boolean, hasSensitiveValue: false }, + ng: { type: OptionType.Boolean, hasSensitiveValue: false }, + angular: { type: OptionType.Boolean, hasSensitiveValue: false }, + vue: { type: OptionType.Boolean, hasSensitiveValue: false }, + vuejs: { type: OptionType.Boolean, hasSensitiveValue: false }, + tsc: { type: OptionType.Boolean, hasSensitiveValue: false }, + ts: { type: OptionType.Boolean, hasSensitiveValue: false }, + typescript: { type: OptionType.Boolean, hasSensitiveValue: false }, + yarn: { type: OptionType.Boolean, hasSensitiveValue: false }, + androidTypings: { type: OptionType.Boolean, hasSensitiveValue: false }, + bundle: { type: OptionType.String, hasSensitiveValue: false }, + all: { type: OptionType.Boolean, hasSensitiveValue: false }, teamId: { type: OptionType.Object, hasSensitiveValue: true }, - syncAllFiles: { type: OptionType.Boolean, default: false }, - chrome: { type: OptionType.Boolean }, - inspector: { type: OptionType.Boolean }, - clean: { type: OptionType.Boolean }, - watch: { type: OptionType.Boolean, default: true }, - background: { type: OptionType.String }, + syncAllFiles: { type: OptionType.Boolean, default: false, hasSensitiveValue: false }, + chrome: { type: OptionType.Boolean, hasSensitiveValue: false }, + inspector: { type: OptionType.Boolean, hasSensitiveValue: false }, + clean: { type: OptionType.Boolean, hasSensitiveValue: false }, + watch: { type: OptionType.Boolean, default: true, hasSensitiveValue: false }, + background: { type: OptionType.String, hasSensitiveValue: false }, username: { type: OptionType.String, hasSensitiveValue: true }, - pluginName: { type: OptionType.String }, - hmr: { type: OptionType.Boolean }, - collection: { type: OptionType.String, alias: "c" }, - json: { type: OptionType.Boolean }, + pluginName: { type: OptionType.String, hasSensitiveValue: false }, + hmr: { type: OptionType.Boolean, hasSensitiveValue: false }, + collection: { type: OptionType.String, alias: "c", hasSensitiveValue: false }, + json: { type: OptionType.Boolean, hasSensitiveValue: false }, avd: { type: OptionType.String, hasSensitiveValue: true }, // check not used - config: { type: OptionType.Array }, - insecure: { type: OptionType.Boolean, alias: "k" }, - debug: { type: OptionType.Boolean, alias: "d" }, - timeout: { type: OptionType.String }, + config: { type: OptionType.Array, hasSensitiveValue: false }, + insecure: { type: OptionType.Boolean, alias: "k", hasSensitiveValue: false }, + debug: { type: OptionType.Boolean, alias: "d", hasSensitiveValue: false }, + timeout: { type: OptionType.String, hasSensitiveValue: false }, device: { type: OptionType.String, hasSensitiveValue: true }, availableDevices: { type: OptionType.Boolean, hasSensitiveValue: true }, appid: { type: OptionType.String, hasSensitiveValue: true }, geny: { type: OptionType.String, hasSensitiveValue: true }, - debugBrk: { type: OptionType.Boolean }, - debugPort: { type: OptionType.Number }, - start: { type: OptionType.Boolean }, - stop: { type: OptionType.Boolean }, + debugBrk: { type: OptionType.Boolean, hasSensitiveValue: false }, + debugPort: { type: OptionType.Number, hasSensitiveValue: false }, + start: { type: OptionType.Boolean, hasSensitiveValue: false }, + stop: { type: OptionType.Boolean, hasSensitiveValue: false }, ddi: { type: OptionType.String, hasSensitiveValue: true }, // the path to developer disk image - justlaunch: { type: OptionType.Boolean }, + justlaunch: { type: OptionType.Boolean, hasSensitiveValue: false }, file: { type: OptionType.String, hasSensitiveValue: true }, - force: { type: OptionType.Boolean, alias: "f" }, + force: { type: OptionType.Boolean, alias: "f", hasSensitiveValue: false }, // remove legacy - companion: { type: OptionType.Boolean }, - emulator: { type: OptionType.Boolean }, - sdk: { type: OptionType.String }, + companion: { type: OptionType.Boolean, hasSensitiveValue: false }, + emulator: { type: OptionType.Boolean, hasSensitiveValue: false }, + sdk: { type: OptionType.String, hasSensitiveValue: false }, template: { type: OptionType.String, hasSensitiveValue: true }, certificate: { type: OptionType.String, hasSensitiveValue: true }, - certificatePassword: { type: OptionType.String }, - release: { type: OptionType.Boolean, alias: "r" }, + certificatePassword: { type: OptionType.String, hasSensitiveValue: true }, + release: { type: OptionType.Boolean, alias: "r", hasSensitiveValue: false }, var: { type: OptionType.Object, hasSensitiveValue: true }, - default: { type: OptionType.Boolean }, - count: { type: OptionType.Number }, + default: { type: OptionType.Boolean, hasSensitiveValue: false }, + count: { type: OptionType.Number, hasSensitiveValue: false }, analyticsLogFile: { type: OptionType.String, hasSensitiveValue: true }, - hooks: { type: OptionType.Boolean, default: true }, - link: { type: OptionType.Boolean, default: false }, - aab: { type: OptionType.Boolean } + hooks: { type: OptionType.Boolean, default: true, hasSensitiveValue: false }, + link: { type: OptionType.Boolean, default: false, hasSensitiveValue: false }, + aab: { type: OptionType.Boolean, hasSensitiveValue: false } }; } From 14901e2b4131fd4ee573e0bc8de7646bee7a44ad Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Tue, 4 Dec 2018 11:30:46 +0200 Subject: [PATCH 8/9] chore: fix tests --- test/options.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/options.ts b/test/options.ts index eb58c86f59..cfa225e2f7 100644 --- a/test/options.ts +++ b/test/options.ts @@ -166,7 +166,7 @@ describe("options", () => { process.argv.push("--test1"); process.argv.push("value"); const options = createOptions(testInjector); - options.validateOptions({ test1: { type: OptionType.String } }); + options.validateOptions({ test1: { type: OptionType.String, hasSensitiveValue: false } }); process.argv.pop(); process.argv.pop(); assert.isFalse(isExecutionStopped); @@ -175,7 +175,7 @@ describe("options", () => { it("does not break execution when valid commandSpecificOptions are passed and user specifies globally valid option", () => { const options = createOptions(testInjector); process.argv.push("--version"); - options.validateOptions({ test1: { type: OptionType.String } }); + options.validateOptions({ test1: { type: OptionType.String, hasSensitiveValue: false } }); process.argv.pop(); assert.isFalse(isExecutionStopped); }); @@ -253,7 +253,7 @@ describe("options", () => { it("does not break execution when dashed option with two dashes is passed", () => { process.argv.push("--special-dashed-v"); const options = createOptions(testInjector); - options.validateOptions({ specialDashedV: { type: OptionType.Boolean } }); + options.validateOptions({ specialDashedV: { type: OptionType.Boolean, hasSensitiveValue: false } }); process.argv.pop(); assert.isFalse(isExecutionStopped, "Dashed options should be validated in specific way. Make sure validation allows yargs specific behavior:" + "Dashed options (special-dashed-v) are added to yargs.argv in two ways: special-dashed-v and specialDashedV"); From 26435cfb52895dad4d5e738ff8955b629810b8eb Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Tue, 4 Dec 2018 15:34:35 +0200 Subject: [PATCH 9/9] chore: mark some more options as sensitive --- lib/options.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/options.ts b/lib/options.ts index 33861d1784..e984e9ef09 100644 --- a/lib/options.ts +++ b/lib/options.ts @@ -41,8 +41,8 @@ export class Options { private get commonOptions(): IDictionary { return { - ipa: { type: OptionType.String, hasSensitiveValue: false }, - frameworkPath: { type: OptionType.String, hasSensitiveValue: false }, + ipa: { type: OptionType.String, hasSensitiveValue: true }, + frameworkPath: { type: OptionType.String, hasSensitiveValue: true }, frameworkName: { type: OptionType.String, hasSensitiveValue: false }, framework: { type: OptionType.String, hasSensitiveValue: false }, frameworkVersion: { type: OptionType.String, hasSensitiveValue: false }, @@ -52,7 +52,7 @@ export class Options { env: { type: OptionType.Object, hasSensitiveValue: false }, production: { type: OptionType.Boolean, hasSensitiveValue: false }, debugTransport: { type: OptionType.Boolean, hasSensitiveValue: false }, - keyStorePath: { type: OptionType.String, hasSensitiveValue: false }, + keyStorePath: { type: OptionType.String, hasSensitiveValue: true }, keyStorePassword: { type: OptionType.String, hasSensitiveValue: true }, keyStoreAlias: { type: OptionType.String, hasSensitiveValue: true }, keyStoreAliasPassword: { type: OptionType.String, hasSensitiveValue: true }, @@ -60,8 +60,8 @@ export class Options { disableNpmInstall: { type: OptionType.Boolean, hasSensitiveValue: false }, compileSdk: { type: OptionType.Number, hasSensitiveValue: false }, port: { type: OptionType.Number, hasSensitiveValue: false }, - copyTo: { type: OptionType.String, hasSensitiveValue: false }, - platformTemplate: { type: OptionType.String, hasSensitiveValue: false }, + copyTo: { type: OptionType.String, hasSensitiveValue: true }, + platformTemplate: { type: OptionType.String, hasSensitiveValue: true }, js: { type: OptionType.Boolean, hasSensitiveValue: false }, javascript: { type: OptionType.Boolean, hasSensitiveValue: false }, ng: { type: OptionType.Boolean, hasSensitiveValue: false }, @@ -94,7 +94,7 @@ export class Options { debug: { type: OptionType.Boolean, alias: "d", hasSensitiveValue: false }, timeout: { type: OptionType.String, hasSensitiveValue: false }, device: { type: OptionType.String, hasSensitiveValue: true }, - availableDevices: { type: OptionType.Boolean, hasSensitiveValue: true }, + availableDevices: { type: OptionType.Boolean, hasSensitiveValue: false }, appid: { type: OptionType.String, hasSensitiveValue: true }, geny: { type: OptionType.String, hasSensitiveValue: true }, debugBrk: { type: OptionType.Boolean, hasSensitiveValue: false },